Re: [PATCH net v5 2/2] packet: use consistent hard_header_len in TX_RING send path

Jakub Kicinski wrote:
> On Thu, 30 Jul 2026 05:22:21 -0400 Willem de Bruijn wrote:
> > Qihang wrote:
> > > tpacket_snd() reads dev->hard_header_len independently for skb
> > > allocation and header construction in tpacket_fill_skb(). Concurrent
> > > netdevice reconfiguration can therefore make the reserved headroom
> > > smaller than the amount later pushed, or make copylen - hard_header_len
> > > negative.
> > > 
> > > Snapshot hard_header_len once before processing ring frames and use it
> > > for the frame limit, headroom allocation, copy length, and skb
> > > construction. Pass the snapshot to tpacket_fill_skb().
> > > 
> > > The separate SOCK_DGRAM consistency problem between hard_header_len and
> > > header_ops->create is not addressed here.
> > > 
> > > Fixes: 69e3c75f4d54 ("net: TX_RING and packet mmap")
> > > Cc: [email protected]
> > > Signed-off-by: Qihang <[email protected]>  

I missed this before, but Qihang is that your full name?

https://www.kernel.org/doc/html/latest/process/submitting-patches.html#sign-your-work-the-developer-s-certificate-of-origin

> > 
> > Reviewed-by: Willem de Bruijn <[email protected]>
> 
> Willem, does the *shiko feedback look like false positives?
> Both instances seem to provide different feedback, if I'm reading my own
> tea leaves right Claude discarded the Gemini feedback about a panic
> as a hallucination..
> 
> https://sashiko.dev/#/patchset/[email protected]
> 
> https://netdev-ai.bots.linux.dev/sashiko/#/patchset/[email protected]

If I read Gemini feedback correctly, all reports are about the current
solution falling short of fixing the instance of this race condition
with dev_hard_header(), which reads dev->hard_header_len directly.
The patch indeed mentions this limitation.

As well as the same issue with dev_validate_header(), which the commit
does not list as limitation. But that one is only reached with variable
length L2 protocols.

Side note, in net-next I can probably remove header_ops.validate now
that ax25 is gone, and with that all of dev_validate_header. No other
variable length L2 protocol ever implemented that callback.

But that won't help older kernels, of course. I think it's fine to
remove that CAP_SYS_RAWIO branch in the fix, which never had much
value anyway. It was there to allow testing purposely bad input or so.