Re: [PATCH net v6 1/3] net: remove CAP_SYS_RAWIO zero-padding in dev_validate_header

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

Qihang wrote:
> From: Qihang Tang <q.h.hack.winter@gmail.com>
> 
> dev_validate_header() reads dev->hard_header_len directly when
> zero-padding short link layer headers for CAP_SYS_RAWIO holders:
> 
>     if (capable(CAP_SYS_RAWIO)) {
>         memset(ll_header + len, 0, dev->hard_header_len - len);
>         return true;
>     }
> 
> Packet send paths call dev_validate_header() on skbs whose headroom was
> allocated from an earlier hard_header_len read. If the device is
> reconfigured so that dev->hard_header_len increases before validation,
> the memset writes past the reserved buffer, an out-of-bounds write.
> 
> This out-of-bounds write is masked in some SOCK_RAW paths today because
> the same concurrent increase can first make skb_push() exceed the
> reserved headroom and trigger skb_under_panic(). Remove the zero-padding
> branch before making those hard_header_len reads consistent, so the
> snapshot fixes do not turn a loud panic into a silent overwrite.
> 
> This path is only reached for variable length L2 protocols, where
> len < hard_header_len but len >= min_header_len. No remaining in-tree
> variable length L2 protocol implements header_ops->validate, and the
> CAP_SYS_RAWIO bypass that zero-pads and accepts short headers has no
> real value beyond allowing testing of intentionally malformed input.
> 
> Drop the CAP_SYS_RAWIO branch. The remaining reads of
> dev->hard_header_len in dev_validate_header() are comparisons only and
> have no memory safety impact.
> 
> Suggested-by: Willem de Bruijn <willemb@google.com>
> Fixes: 2793a23aacbd ("net: validate variable length ll headers")
> Cc: stable@vger.kernel.org
> Signed-off-by: Qihang Tang <q.h.hack.winter@gmail.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

For future patches: don't send new revisions as replies to existing threads.