public inbox for netfilter-devel@vger.kernel.org
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: Florian Westphal <fw@strlen.de>
Cc: Xiang Mei <xmei5@asu.edu>,
	netfilter-devel@vger.kernel.org, Phil Sutter <phil@nwl.cc>,
	coreteam@netfilter.org, Weiming Shi <bestswngs@gmail.com>
Subject: Re: [PATCH nf] netfilter: nfnetlink_osf: fix divide-by-zero in OSF_WSS_MODULO
Date: Tue, 14 Apr 2026 13:01:41 +0200	[thread overview]
Message-ID: <ad4elUEYrkQ18iX8@chamomile> (raw)
In-Reply-To: <adqx_IBgoyAMIJ5I@strlen.de>

On Sat, Apr 11, 2026 at 10:41:32PM +0200, Florian Westphal wrote:
> Xiang Mei <xmei5@asu.edu> wrote:
> > The OSF_WSS_MODULO branch in nf_osf_match_one() performs:
> > 
> >   ctx->window % f->wss.val
> > 
> > without guarding against f->wss.val == 0.  A user with CAP_NET_ADMIN
> > can add an OSF fingerprint with wss.wc = OSF_WSS_MODULO and wss.val = 0
> > via nfnetlink.  When a matching TCP SYN packet arrives, the kernel
> > executes a division by zero and panics.
> > 
> > The OSF_WSS_PLAIN case already treats val == 0 as a wildcard (match
> > everything).  Apply the same semantics to OSF_WSS_MODULO: if val is 0,
> > any window value matches rather than dividing by zero.
> > 
> > Crash:
> >  Oops: divide error: 0000 [#1] SMP KASAN NOPTI
> >  RIP: 0010:nf_osf_match_one (net/netfilter/nfnetlink_osf.c:98)
> >  Call Trace:
> >  <IRQ>
> >   nf_osf_match (net/netfilter/nfnetlink_osf.c:220 (discriminator 6))
> >   xt_osf_match_packet (net/netfilter/xt_osf.c:32)
> >   ipt_do_table (net/ipv4/netfilter/ip_tables.c:348)
> >   nf_hook_slow (net/netfilter/core.c:622 (discriminator 1))
> >   ip_local_deliver (net/ipv4/ip_input.c:265)
> >   ip_rcv (include/linux/skbuff.h:1162)
> >   __netif_receive_skb_one_core (net/core/dev.c:6181)
> >   process_backlog (.include/linux/skbuff.h:2502 net/core/dev.c:6642)
> >   __napi_poll (net/core/dev.c:7710)
> >   net_rx_action (net/core/dev.c:7945)
> >   handle_softirqs (kernel/softirq.c:622)
> > 
> > Fixes: 31a9c29210e2 ("netfilter: nf_osf: add struct nf_osf_hdr_ctx")
> 
> Hmm, why?  AFAICS the bug was there from start:
> 
> 11eeef41d5f63 case OSF_WSS_MODULO:
> 11eeef41d5f63    if ((window % f->wss.val) == 0)
> 11eeef41d5f63        fmatch = FMATCH_OK;
> 
> So:
> Fixes: 11eeef41d5f6 ("netfilter: passive OS fingerprint xtables match")
> 
> > diff --git a/net/netfilter/nfnetlink_osf.c b/net/netfilter/nfnetlink_osf.c
> > index 45d9ad231..193436aa9 100644
> > --- a/net/netfilter/nfnetlink_osf.c
> > +++ b/net/netfilter/nfnetlink_osf.c
> > @@ -150,7 +150,7 @@ static bool nf_osf_match_one(const struct sk_buff *skb,
> >  				fmatch = FMATCH_OK;
> >  			break;
> >  		case OSF_WSS_MODULO:
> > -			if ((ctx->window % f->wss.val) == 0)
> > +			if (f->wss.val == 0 || (ctx->window % f->wss.val) == 0)
> 
> Could you send a v2 that rejects this from control plane instead?
> Nobody is using a 0 value, else we'd have gotted such crash reports by
> now.

No news from this, I think this should be fine:

@@ -329,6 +332,15 @@ static int nfnl_osf_add_callback(struct sk_buff *skb,
                if (f->opt[i].kind == OSFOPT_MSS && f->opt[i].length < 4)
                        return -EINVAL;
 
+               switch (f->wss.wc) {
+               case OSF_WSS_MODULO:
+                       if (f->wss.val == 0)
+                               return -EINVAL;
+                       break;
+               default:
+                       break;
+               }
+
                tot_opt_len += f->opt[i].length;
                if (tot_opt_len > MAX_IPOPTLEN)
                        return -EINVAL;

If no concerns, I will post a patch.

  reply	other threads:[~2026-04-14 11:01 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-10 20:48 [PATCH nf] netfilter: nfnetlink_osf: fix divide-by-zero in OSF_WSS_MODULO Xiang Mei
2026-04-11  7:56 ` Fernando Fernandez Mancera
2026-04-11 20:41 ` Florian Westphal
2026-04-14 11:01   ` Pablo Neira Ayuso [this message]
2026-04-14 11:11     ` Florian Westphal
2026-04-14 22:00       ` Xiang Mei
  -- strict thread matches above, loose matches on Subject: below --
2026-04-14 11:17 Pablo Neira Ayuso
2026-04-14 21:58 ` Xiang Mei

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ad4elUEYrkQ18iX8@chamomile \
    --to=pablo@netfilter.org \
    --cc=bestswngs@gmail.com \
    --cc=coreteam@netfilter.org \
    --cc=fw@strlen.de \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=phil@nwl.cc \
    --cc=xmei5@asu.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox