Re: [PATCH net-next v4 5/5] net: dsa: mv88e6xxx: embedded PTP timestamp support

[Luke Howard <lukeh@padl.com>]

> [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