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

[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