* [PATCH 1/2 nf] netfilter: nfnetlink_osf: fix out-of-bounds read on option matching
@ 2026-04-17 16:20 Fernando Fernandez Mancera
2026-04-17 16:20 ` [PATCH 2/2 nf] netfilter: nfnetlink_osf: fix potential NULL dereference in ttl check Fernando Fernandez Mancera
2026-04-18 7:57 ` [PATCH 1/2 nf] netfilter: nfnetlink_osf: fix out-of-bounds read on option matching Pablo Neira Ayuso
0 siblings, 2 replies; 4+ messages in thread
From: Fernando Fernandez Mancera @ 2026-04-17 16:20 UTC (permalink / raw)
To: netfilter-devel
Cc: netdev, coreteam, pablo, fw, phil, Fernando Fernandez Mancera
In nf_osf_match(), the nf_osf_hdr_ctx structure is initialized once
and passed by reference to nf_osf_match_one() for each fingerprint
checked. During TCP option parsing, nf_osf_match_one() advances the
shared ctx->optp pointer.
If a fingerprint perfectly matches, the function returns early without
restoring ctx->optp to its initial state. If the user has configured
NF_OSF_LOGLEVEL_ALL, the loop continues to the next fingerprint.
However, because ctx->optp was not restored, the next call to
nf_osf_match_one() starts parsing from the end of the options buffer.
This causes subsequent matches to read garbage data and fail
immediately, making it impossible to log more than one match or logging
incorrect matches.
Instead of using a shared ctx->optp pointer, pass the context as a
constant pointer and use a local pointer (optp) for TCP option
traversal. This makes nf_osf_match_one() strictly stateless from the
caller's perspective, ensuring every fingerprint check starts at the
correct option offset.
Fixes: 1a6a0951fc00 ("netfilter: nfnetlink_osf: add missing fmatch check")
Suggested-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
net/netfilter/nfnetlink_osf.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/net/netfilter/nfnetlink_osf.c b/net/netfilter/nfnetlink_osf.c
index 45d9ad231a92..f58267986453 100644
--- a/net/netfilter/nfnetlink_osf.c
+++ b/net/netfilter/nfnetlink_osf.c
@@ -64,9 +64,9 @@ struct nf_osf_hdr_ctx {
static bool nf_osf_match_one(const struct sk_buff *skb,
const struct nf_osf_user_finger *f,
int ttl_check,
- struct nf_osf_hdr_ctx *ctx)
+ const struct nf_osf_hdr_ctx *ctx)
{
- const __u8 *optpinit = ctx->optp;
+ const __u8 *optp = ctx->optp;
unsigned int check_WSS = 0;
int fmatch = FMATCH_WRONG;
int foptsize, optnum;
@@ -95,17 +95,17 @@ static bool nf_osf_match_one(const struct sk_buff *skb,
check_WSS = f->wss.wc;
for (optnum = 0; optnum < f->opt_num; ++optnum) {
- if (f->opt[optnum].kind == *ctx->optp) {
+ if (f->opt[optnum].kind == *optp) {
__u32 len = f->opt[optnum].length;
- const __u8 *optend = ctx->optp + len;
+ const __u8 *optend = optp + len;
fmatch = FMATCH_OK;
- switch (*ctx->optp) {
+ switch (*optp) {
case OSFOPT_MSS:
- mss = ctx->optp[3];
+ mss = optp[3];
mss <<= 8;
- mss |= ctx->optp[2];
+ mss |= optp[2];
mss = ntohs((__force __be16)mss);
break;
@@ -113,7 +113,7 @@ static bool nf_osf_match_one(const struct sk_buff *skb,
break;
}
- ctx->optp = optend;
+ optp = optend;
} else
fmatch = FMATCH_OPT_WRONG;
@@ -156,9 +156,6 @@ static bool nf_osf_match_one(const struct sk_buff *skb,
}
}
- if (fmatch != FMATCH_OK)
- ctx->optp = optpinit;
-
return fmatch == FMATCH_OK;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/2 nf] netfilter: nfnetlink_osf: fix potential NULL dereference in ttl check
2026-04-17 16:20 [PATCH 1/2 nf] netfilter: nfnetlink_osf: fix out-of-bounds read on option matching Fernando Fernandez Mancera
@ 2026-04-17 16:20 ` Fernando Fernandez Mancera
2026-04-18 7:53 ` Pablo Neira Ayuso
2026-04-18 7:57 ` [PATCH 1/2 nf] netfilter: nfnetlink_osf: fix out-of-bounds read on option matching Pablo Neira Ayuso
1 sibling, 1 reply; 4+ messages in thread
From: Fernando Fernandez Mancera @ 2026-04-17 16:20 UTC (permalink / raw)
To: netfilter-devel
Cc: netdev, coreteam, pablo, fw, phil, Fernando Fernandez Mancera,
Kito Xu (veritas501)
The nf_osf_ttl() function accessed skb->dev to perform a local interface
address lookup without verifying that the device pointer was valid.
Additionally, the implementation utilized an in_dev_for_each_ifa_rcu
loop to match the packet source address against local interface
addresses. It assumed that packets from the same subnet should not see a
decrement on the initial TTL. A packet might appear it is from the same
subnet but it actually isn't especially in modern environments with
containers and virtual switching.
Remove the device dereference and interface loop. Replace the logic with
a switch statement that evaluates the TTL according to the ttl_check.
Fixes: 11eeef41d5f6 ("netfilter: passive OS fingerprint xtables match")
Reported-by: Kito Xu (veritas501) <hxzene@gmail.com>
Closes: https://lore.kernel.org/netfilter-devel/20260414074556.2512750-1-hxzene@gmail.com/
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
Note: if some help is needed during the backport I can assist.
---
net/netfilter/nfnetlink_osf.c | 22 +++++++---------------
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/net/netfilter/nfnetlink_osf.c b/net/netfilter/nfnetlink_osf.c
index f58267986453..f0d1e596e146 100644
--- a/net/netfilter/nfnetlink_osf.c
+++ b/net/netfilter/nfnetlink_osf.c
@@ -31,26 +31,18 @@ EXPORT_SYMBOL_GPL(nf_osf_fingers);
static inline int nf_osf_ttl(const struct sk_buff *skb,
int ttl_check, unsigned char f_ttl)
{
- struct in_device *in_dev = __in_dev_get_rcu(skb->dev);
const struct iphdr *ip = ip_hdr(skb);
- const struct in_ifaddr *ifa;
- int ret = 0;
- if (ttl_check == NF_OSF_TTL_TRUE)
+ switch (ttl_check) {
+ case NF_OSF_TTL_TRUE:
return ip->ttl == f_ttl;
- if (ttl_check == NF_OSF_TTL_NOCHECK)
- return 1;
- else if (ip->ttl <= f_ttl)
+ break;
+ case NF_OSF_TTL_NOCHECK:
return 1;
-
- in_dev_for_each_ifa_rcu(ifa, in_dev) {
- if (inet_ifa_match(ip->saddr, ifa)) {
- ret = (ip->ttl == f_ttl);
- break;
- }
+ case NF_OSF_TTL_LESS:
+ default:
+ return ip->ttl <= f_ttl;
}
-
- return ret;
}
struct nf_osf_hdr_ctx {
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 2/2 nf] netfilter: nfnetlink_osf: fix potential NULL dereference in ttl check
2026-04-17 16:20 ` [PATCH 2/2 nf] netfilter: nfnetlink_osf: fix potential NULL dereference in ttl check Fernando Fernandez Mancera
@ 2026-04-18 7:53 ` Pablo Neira Ayuso
0 siblings, 0 replies; 4+ messages in thread
From: Pablo Neira Ayuso @ 2026-04-18 7:53 UTC (permalink / raw)
To: Fernando Fernandez Mancera
Cc: netfilter-devel, netdev, coreteam, fw, phil, Kito Xu (veritas501)
On Fri, Apr 17, 2026 at 06:20:57PM +0200, Fernando Fernandez Mancera wrote:
> The nf_osf_ttl() function accessed skb->dev to perform a local interface
> address lookup without verifying that the device pointer was valid.
>
> Additionally, the implementation utilized an in_dev_for_each_ifa_rcu
> loop to match the packet source address against local interface
> addresses. It assumed that packets from the same subnet should not see a
> decrement on the initial TTL. A packet might appear it is from the same
> subnet but it actually isn't especially in modern environments with
> containers and virtual switching.
>
> Remove the device dereference and interface loop. Replace the logic with
> a switch statement that evaluates the TTL according to the ttl_check.
>
> Fixes: 11eeef41d5f6 ("netfilter: passive OS fingerprint xtables match")
> Reported-by: Kito Xu (veritas501) <hxzene@gmail.com>
> Closes: https://lore.kernel.org/netfilter-devel/20260414074556.2512750-1-hxzene@gmail.com/
> Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
> Note: if some help is needed during the backport I can assist.
> ---
> net/netfilter/nfnetlink_osf.c | 22 +++++++---------------
> 1 file changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/net/netfilter/nfnetlink_osf.c b/net/netfilter/nfnetlink_osf.c
> index f58267986453..f0d1e596e146 100644
> --- a/net/netfilter/nfnetlink_osf.c
> +++ b/net/netfilter/nfnetlink_osf.c
> @@ -31,26 +31,18 @@ EXPORT_SYMBOL_GPL(nf_osf_fingers);
> static inline int nf_osf_ttl(const struct sk_buff *skb,
> int ttl_check, unsigned char f_ttl)
> {
> - struct in_device *in_dev = __in_dev_get_rcu(skb->dev);
> const struct iphdr *ip = ip_hdr(skb);
> - const struct in_ifaddr *ifa;
> - int ret = 0;
>
> - if (ttl_check == NF_OSF_TTL_TRUE)
> + switch (ttl_check) {
> + case NF_OSF_TTL_TRUE:
> return ip->ttl == f_ttl;
> - if (ttl_check == NF_OSF_TTL_NOCHECK)
> - return 1;
> - else if (ip->ttl <= f_ttl)
> + break;
> + case NF_OSF_TTL_NOCHECK:
> return 1;
> -
> - in_dev_for_each_ifa_rcu(ifa, in_dev) {
> - if (inet_ifa_match(ip->saddr, ifa)) {
> - ret = (ip->ttl == f_ttl);
> - break;
> - }
> + case NF_OSF_TTL_LESS:
> + default:
> + return ip->ttl <= f_ttl;
> }
> -
> - return ret;
> }
>
> struct nf_osf_hdr_ctx {
> --
> 2.53.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2 nf] netfilter: nfnetlink_osf: fix out-of-bounds read on option matching
2026-04-17 16:20 [PATCH 1/2 nf] netfilter: nfnetlink_osf: fix out-of-bounds read on option matching Fernando Fernandez Mancera
2026-04-17 16:20 ` [PATCH 2/2 nf] netfilter: nfnetlink_osf: fix potential NULL dereference in ttl check Fernando Fernandez Mancera
@ 2026-04-18 7:57 ` Pablo Neira Ayuso
1 sibling, 0 replies; 4+ messages in thread
From: Pablo Neira Ayuso @ 2026-04-18 7:57 UTC (permalink / raw)
To: Fernando Fernandez Mancera; +Cc: netfilter-devel, netdev, coreteam, fw, phil
On Fri, Apr 17, 2026 at 06:20:56PM +0200, Fernando Fernandez Mancera wrote:
> In nf_osf_match(), the nf_osf_hdr_ctx structure is initialized once
> and passed by reference to nf_osf_match_one() for each fingerprint
> checked. During TCP option parsing, nf_osf_match_one() advances the
> shared ctx->optp pointer.
>
> If a fingerprint perfectly matches, the function returns early without
> restoring ctx->optp to its initial state. If the user has configured
> NF_OSF_LOGLEVEL_ALL, the loop continues to the next fingerprint.
> However, because ctx->optp was not restored, the next call to
> nf_osf_match_one() starts parsing from the end of the options buffer.
> This causes subsequent matches to read garbage data and fail
> immediately, making it impossible to log more than one match or logging
> incorrect matches.
>
> Instead of using a shared ctx->optp pointer, pass the context as a
> constant pointer and use a local pointer (optp) for TCP option
> traversal. This makes nf_osf_match_one() strictly stateless from the
> caller's perspective, ensuring every fingerprint check starts at the
> correct option offset.
>
> Fixes: 1a6a0951fc00 ("netfilter: nfnetlink_osf: add missing fmatch check")
> Suggested-by: Florian Westphal <fw@strlen.de>
> Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-04-18 7:57 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-17 16:20 [PATCH 1/2 nf] netfilter: nfnetlink_osf: fix out-of-bounds read on option matching Fernando Fernandez Mancera
2026-04-17 16:20 ` [PATCH 2/2 nf] netfilter: nfnetlink_osf: fix potential NULL dereference in ttl check Fernando Fernandez Mancera
2026-04-18 7:53 ` Pablo Neira Ayuso
2026-04-18 7:57 ` [PATCH 1/2 nf] netfilter: nfnetlink_osf: fix out-of-bounds read on option matching Pablo Neira Ayuso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox