Re: [PATCH v3 net 6/7] net: enetc: linearize PTP event packets with one-step TX timestamping

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net/enetc: data corruption due to skb_linearize instead of skb_linearize_cow
--

commit 4e574e48c7ebf46bd9831e2efa9f6f3ac1500c3d
Author: vladimir.oltean@nxp.com <vladimir.oltean@nxp.com>

net: enetc: linearize PTP event packets with one-step TX timestamping

This commit addresses parsing issues with fragmented PTP headers in
enetc_ptp_parse() by linearizing packets with one-step timestamping requests.
The commit message states that it uses skb_linearize_cow() to also solve a
secondary issue where enetc_update_ptp_sync_msg() modifies the packet data
in-place later in the TX path, which requires the SKB to be exclusively owned.

Link: https://sashiko.dev/#/patchset/20260401172246.1075883-1-vladimir.oltean%40nxp.com

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
> index 8a9ba168eab1e..62cdcaab3f3fb 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> @@ -1099,6 +1099,11 @@ netdev_tx_t enetc_xmit(struct sk_buff *skb, struct net_device *ndev)
>  
>  	/* Fall back to two-step timestamp if not one-step Sync packet */
>  	if (enetc_cb->flag & ENETC_F_TX_ONESTEP_SYNC_TSTAMP) {
> +		if (unlikely(skb_linearize(skb))) {

[Severity: High]
Does this code use skb_linearize() instead of the intended
skb_linearize_cow()?

The commit message and v2->v3 changelog explicitly mention using
skb_linearize_cow() to ensure the packet is exclusively owned and writable.
However, the code uses skb_linearize(), which only unshares the buffer if
it is nonlinear. A linear cloned SKB (e.g., from AF_PACKET, tc mirred, or
bridging) will bypass the copy.

Since enetc_update_ptp_sync_msg() modifies the originTimestamp field directly
in the SKB buffer later in the TX path:

    *(__be16 *)(data + tstamp_off) = new_sec_h;
    *(__be32 *)(data + tstamp_off + 2) = new_sec_l;
    *(__be32 *)(data + tstamp_off + 6) = new_nsec;

Could this corrupt the buffer for other users if the SKB was linear and
cloned?

> +			dev_kfree_skb_any(skb);
> +			return NETDEV_TX_OK;
> +		}
> +
>  		if (enetc_ptp_parse(skb, &udp, &msgtype, &twostep,
>  				    &offset1, &offset2) ||
>  		    msgtype != PTP_MSGTYPE_SYNC || twostep != 0) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915222735.1016937-1-vladimir.oltean@nxp.com?part=6