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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.