* [PATCH] netfilter: ipset: harden payload calculation in call_ad()
@ 2026-03-13 18:01 David Baum
2026-04-10 1:05 ` Pablo Neira Ayuso
0 siblings, 1 reply; 3+ messages in thread
From: David Baum @ 2026-03-13 18:01 UTC (permalink / raw)
To: pablo, fw
Cc: phil, davem, edumazet, kuba, pabeni, horms, netfilter-devel,
coreteam, netdev, David Baum
call_ad() computes the netlink error payload size with
min(SIZE_MAX, sizeof(*errmsg) + nlmsg_len(nlh)), but min(SIZE_MAX, x)
is always x, so the guard is a no-op.
Replace it with an explicit negative-length check and
check_add_overflow() so the addition is validated before being passed
to nlmsg_new().
Signed-off-by: David Baum <davidbaum461@gmail.com>
---
net/netfilter/ipset/ip_set_core.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index a2fe711cb5e3..11d3854d9b11 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -10,6 +10,7 @@
#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/ip.h>
+#include <linux/overflow.h>
#include <linux/skbuff.h>
#include <linux/spinlock.h>
#include <linux/rculist.h>
@@ -1763,13 +1764,18 @@ call_ad(struct net *net, struct sock *ctnl, struct sk_buff *skb,
struct nlmsghdr *rep, *nlh = nlmsg_hdr(skb);
struct sk_buff *skb2;
struct nlmsgerr *errmsg;
- size_t payload = min(SIZE_MAX,
- sizeof(*errmsg) + nlmsg_len(nlh));
+ int nlmsg_payload_len = nlmsg_len(nlh);
+ size_t payload;
int min_len = nlmsg_total_size(sizeof(struct nfgenmsg));
struct nlattr *cda[IPSET_ATTR_CMD_MAX + 1];
struct nlattr *cmdattr;
u32 *errline;
+ if (nlmsg_payload_len < 0 ||
+ check_add_overflow(sizeof(*errmsg),
+ (size_t)nlmsg_payload_len, &payload))
+ return -ENOMEM;
+
skb2 = nlmsg_new(payload, GFP_KERNEL);
if (!skb2)
return -ENOMEM;
--
2.50.1 (Apple Git-155)
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] netfilter: ipset: harden payload calculation in call_ad()
2026-03-13 18:01 [PATCH] netfilter: ipset: harden payload calculation in call_ad() David Baum
@ 2026-04-10 1:05 ` Pablo Neira Ayuso
2026-04-10 1:19 ` Pablo Neira Ayuso
0 siblings, 1 reply; 3+ messages in thread
From: Pablo Neira Ayuso @ 2026-04-10 1:05 UTC (permalink / raw)
To: David Baum
Cc: fw, phil, davem, edumazet, kuba, pabeni, horms, netfilter-devel,
coreteam, netdev
On Fri, Mar 13, 2026 at 01:01:32PM -0500, David Baum wrote:
> call_ad() computes the netlink error payload size with
> min(SIZE_MAX, sizeof(*errmsg) + nlmsg_len(nlh)), but min(SIZE_MAX, x)
> is always x, so the guard is a no-op.
>
> Replace it with an explicit negative-length check and
> check_add_overflow() so the addition is validated before being passed
> to nlmsg_new().
>
> Signed-off-by: David Baum <davidbaum461@gmail.com>
> ---
> net/netfilter/ipset/ip_set_core.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
> index a2fe711cb5e3..11d3854d9b11 100644
> --- a/net/netfilter/ipset/ip_set_core.c
> +++ b/net/netfilter/ipset/ip_set_core.c
> @@ -10,6 +10,7 @@
> #include <linux/module.h>
> #include <linux/moduleparam.h>
> #include <linux/ip.h>
> +#include <linux/overflow.h>
> #include <linux/skbuff.h>
> #include <linux/spinlock.h>
> #include <linux/rculist.h>
> @@ -1763,13 +1764,18 @@ call_ad(struct net *net, struct sock *ctnl, struct sk_buff *skb,
> struct nlmsghdr *rep, *nlh = nlmsg_hdr(skb);
> struct sk_buff *skb2;
> struct nlmsgerr *errmsg;
> - size_t payload = min(SIZE_MAX,
> - sizeof(*errmsg) + nlmsg_len(nlh));
> + int nlmsg_payload_len = nlmsg_len(nlh);
> + size_t payload;
> int min_len = nlmsg_total_size(sizeof(struct nfgenmsg));
> struct nlattr *cda[IPSET_ATTR_CMD_MAX + 1];
> struct nlattr *cmdattr;
> u32 *errline;
>
> + if (nlmsg_payload_len < 0 ||
Hm, nlh was already sanitized by nfnetlink?
> + check_add_overflow(sizeof(*errmsg),
> + (size_t)nlmsg_payload_len, &payload))
Maybe cap this to int:
int payload = sizeof(struct nlmsgerr);
...
if (check_add_overflow(payload, nlmsg_payload_len, &payload))
> + return -ENOMEM;
> +
> skb2 = nlmsg_new(payload, GFP_KERNEL);
> if (!skb2)
> return -ENOMEM;
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] netfilter: ipset: harden payload calculation in call_ad()
2026-04-10 1:05 ` Pablo Neira Ayuso
@ 2026-04-10 1:19 ` Pablo Neira Ayuso
0 siblings, 0 replies; 3+ messages in thread
From: Pablo Neira Ayuso @ 2026-04-10 1:19 UTC (permalink / raw)
To: David Baum
Cc: fw, phil, davem, edumazet, kuba, pabeni, horms, netfilter-devel,
coreteam, netdev
On Fri, Apr 10, 2026 at 03:05:18AM +0200, Pablo Neira Ayuso wrote:
> On Fri, Mar 13, 2026 at 01:01:32PM -0500, David Baum wrote:
> > call_ad() computes the netlink error payload size with
> > min(SIZE_MAX, sizeof(*errmsg) + nlmsg_len(nlh)), but min(SIZE_MAX, x)
> > is always x, so the guard is a no-op.
> >
> > Replace it with an explicit negative-length check and
> > check_add_overflow() so the addition is validated before being passed
> > to nlmsg_new().
> >
> > Signed-off-by: David Baum <davidbaum461@gmail.com>
> > ---
> > net/netfilter/ipset/ip_set_core.c | 10 ++++++++--
> > 1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
> > index a2fe711cb5e3..11d3854d9b11 100644
> > --- a/net/netfilter/ipset/ip_set_core.c
> > +++ b/net/netfilter/ipset/ip_set_core.c
> > @@ -10,6 +10,7 @@
> > #include <linux/module.h>
> > #include <linux/moduleparam.h>
> > #include <linux/ip.h>
> > +#include <linux/overflow.h>
> > #include <linux/skbuff.h>
> > #include <linux/spinlock.h>
> > #include <linux/rculist.h>
> > @@ -1763,13 +1764,18 @@ call_ad(struct net *net, struct sock *ctnl, struct sk_buff *skb,
> > struct nlmsghdr *rep, *nlh = nlmsg_hdr(skb);
> > struct sk_buff *skb2;
> > struct nlmsgerr *errmsg;
> > - size_t payload = min(SIZE_MAX,
> > - sizeof(*errmsg) + nlmsg_len(nlh));
> > + int nlmsg_payload_len = nlmsg_len(nlh);
> > + size_t payload;
> > int min_len = nlmsg_total_size(sizeof(struct nfgenmsg));
> > struct nlattr *cda[IPSET_ATTR_CMD_MAX + 1];
> > struct nlattr *cmdattr;
> > u32 *errline;
> >
> > + if (nlmsg_payload_len < 0 ||
>
> Hm, nlh was already sanitized by nfnetlink?
>
> > + check_add_overflow(sizeof(*errmsg),
> > + (size_t)nlmsg_payload_len, &payload))
>
> Maybe cap this to int:
>
> int payload = sizeof(struct nlmsgerr);
> ...
>
> if (check_add_overflow(payload, nlmsg_payload_len, &payload))
Wait, then payload and nlmsg_payload_len should be __u32, not int.
>
> > + return -ENOMEM;
> > +
> > skb2 = nlmsg_new(payload, GFP_KERNEL);
> > if (!skb2)
> > return -ENOMEM;
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-04-10 1:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-13 18:01 [PATCH] netfilter: ipset: harden payload calculation in call_ad() David Baum
2026-04-10 1:05 ` Pablo Neira Ayuso
2026-04-10 1:19 ` 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