Re: [PATCH nf] netfilter: nf_log: validate MAC header was set before dumping it

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Tue, Jun 09, 2026 at 03:01:14PM -0700, Xiang Mei wrote:
> On Tue, Jun 09, 2026 at 10:54:41PM +0200, Pablo Neira Ayuso wrote:
> > On Sun, Jun 07, 2026 at 05:11:24PM -0700, Xiang Mei wrote:
> > > The fallback path of dump_mac_header() guards the MAC header access
> > > only with "skb->mac_header != skb->network_header", without checking
> > > skb_mac_header_was_set().  When the MAC header is unset, mac_header is
> > > 0xffff, so the test passes and skb_mac_header(skb) returns
> > > skb->head + 0xffff, ~64 KiB past the buffer; the loop then reads
> > > dev->hard_header_len bytes out of bounds into the kernel log.
> > > 
> > > This is reachable via the netdev logger: nf_log_unknown_packet() calls
> > > dump_mac_header() unconditionally, and an skb sent through AF_PACKET
> > > with PACKET_QDISC_BYPASS reaches the egress hook with mac_header still
> > > unset (__dev_queue_xmit(), which would reset it, is bypassed).
> > > 
> > > Add the skb_mac_header_was_set() check the ARPHRD_ETHER path already
> > > uses.  Only skbs with an unset MAC header are affected; valid ones are
> > > dumped as before.
> > > 
> > >  BUG: KASAN: slab-out-of-bounds in dump_mac_header (net/netfilter/nf_log_syslog.c:831)
> > >  Read of size 1 at addr ffff88800ea49d3f by task exploit/148
> > >  Call Trace:
> > >   kasan_report (mm/kasan/report.c:595)
> > >   dump_mac_header (net/netfilter/nf_log_syslog.c:831)
> > >   nf_log_netdev_packet (net/netfilter/nf_log_syslog.c:938 net/netfilter/nf_log_syslog.c:963)
> > >   nf_log_packet (net/netfilter/nf_log.c:260)
> > >   nft_log_eval (net/netfilter/nft_log.c:60)
> > >   nft_do_chain (net/netfilter/nf_tables_core.c:285)
> > >   nft_do_chain_netdev (net/netfilter/nft_chain_filter.c:307)
> > >   nf_hook_slow (net/netfilter/core.c:619)
> > >   nf_hook_direct_egress (net/packet/af_packet.c:257)
> > >   packet_xmit (net/packet/af_packet.c:280)
> > >   packet_sendmsg (net/packet/af_packet.c:3114)
> > >   __sys_sendto (net/socket.c:2265)
> > > 
> > > Fixes: 7eb9282cd0ef ("netfilter: ipt_LOG/ip6t_LOG: add option to print decoded MAC header")
> > > Reported-by: Weiming Shi <bestswngs@gmail.com>
> > > Assisted-by: Claude:claude-opus-4-8
> > > Signed-off-by: Xiang Mei <xmei5@asu.edu>
> > > ---
> > >  net/netfilter/nf_log_syslog.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/net/netfilter/nf_log_syslog.c b/net/netfilter/nf_log_syslog.c
> > > index 7a8952b049d1..ed5283fb6b67 100644
> > > --- a/net/netfilter/nf_log_syslog.c
> > > +++ b/net/netfilter/nf_log_syslog.c
> > > @@ -815,7 +815,7 @@ static void dump_mac_header(struct nf_log_buf *m,
> > >  
> > >  fallback:
> > >  	nf_log_buf_add(m, "MAC=");
> > > -	if (dev->hard_header_len &&
> > > +	if (dev->hard_header_len && skb_mac_header_was_set(skb) &&
> > >  	    skb->mac_header != skb->network_header) {
> > 
> > Maybe this instead?
> > 
> > +           skb_mac_header_was_set(skb) &&
> > +           skb_mac_header_len(skb) != 0) {
> 
> Thanks for the quick reply to this patch.
> 
> The skb_mac_header_len is a combination of
> 1) `skb_mac_header_was_set(skb)` and
> 2) `skb->network_header - skb->mac_header`

No, 1) is only true if DEBUG_NET_WARN_ON_ONCE() is enabled.

> However, we have skb_mac_header_was_set added in the
> original patch, and we have `skb->network_header - skb->mac_header`

I think this is the last spot which opencodes skb_mac_header_len() in
the netfilter tree.

> at the start of the fallback code block:
> 
> ```
> fallback:
> 	nf_log_buf_add(m, "MAC=");
> 	if (dev->hard_header_len &&
> 	    skb->mac_header != skb->network_header) {
> 	    ...
> ```
> 
> So I think the original patch should be enough.

I think this patch could be streamlined like this:

ebb966d3bdfe ("netfilter: fix regression in looped (broad|multi)cast's MAC handling")