Re: [PATCH net-next v4 5/5] net: dsa: mv88e6xxx: embedded PTP timestamp support
Luke Howard <[email protected]>
| Newsgroups | dev.linux.lists.sashiko-reviews,org.kernel.vger.netdev |
|---|---|
| Message-ID | <[email protected]> |
> [Severity: Low] > Is it possible for chip->tag_protocol to be modified concurrently by > mv88e6xxx_change_tag_protocol() in process context? > > If this runs in the fast path rx softirq without READ_ONCE() and > WRITE_ONCE() annotations, could this cause a data race? Should not be an issue in practice because the tag protocol cannot be changed whilst the port is up. > [Severity: High] > If ptp_ops->global_enable() or ptp_ops->port_enable() fails and returns an > error here, does this code leak the increment to chip->enable_count? > > If the count remains incremented on failure, could this leave the hardware > state permanently out of sync and break the ability to enable or disable PTP? Fixed in next revision. > [Severity: High] > This is a pre-existing issue, but can this dereference of hdr->sequence_id > result in an out-of-bounds read? > > If an skb containing a PTP packet is received fragmented such that the PTP > header sits in the paged data area, ptp_parse_header() returns a pointer > extending past the linear region. Same issue as you pointed out in 1/5 and 2/5. It is pre-existing and affects any caller of ptp_parse_header(). Perhaps ptp_parse_header() needs to linearize the SKB itself. Luke