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

Willem de Bruijn <[email protected]>
Newsgroups org.kernel.vger.stable,org.kernel.vger.netdev
Message-ID <[email protected]>
Qihang wrote:
> From: Qihang Tang <[email protected]>
> 
> 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 <[email protected]>
> Fixes: 2793a23aacbd ("net: validate variable length ll headers")
> Cc: [email protected]
> Signed-off-by: Qihang Tang <[email protected]>

Reviewed-by: Willem de Bruijn <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.