[PATCH net] calipso: do not charge request socket options to the listener

[PATCH net] calipso: do not charge request socket options to the listener

[Eric Dumazet]

calipso_req_setattr() and calipso_req_delattr() allocate ireq->ipv6_opt
with ipv6_renew_options(), which charges the listener sk_omem_alloc
through sock_kmalloc().

tcp_v6_reqsk_destructor() frees ireq->ipv6_opt without uncharging it,
so each request socket leaks its options size from the listener
budget. Once sysctl_optmem_max is reached, sock_kmalloc() fails,
calipso_req_setattr() returns -ENOBUFS, and tcp_v6_route_req() drops
all subsequent SYN packets. __sk_destruct() also reports an optmem
leakage when the listener is destroyed.

Uncharging in tcp_v6_reqsk_destructor() would not be correct: after a
migration, req->rsk_listener is no longer the socket that was charged.

These options belong to the request socket, there is no reason to
charge them to the listener. Change ipv6_renew_options() to use
kmalloc() when its sk argument is NULL, and use this from
calipso_req_setattr() and calipso_req_delattr(),
like cipso_v4_req_setattr() which uses kzalloc().

Note:

After this patch, calipso_req_setattr() and calipso_req_delattr()
no longer need the listener. Commit 10876da918fa ("calipso: Fix
null-ptr-deref in calipso_req_{set,del}attr().") could be reverted
in net-next (also removing the then unused sk variables), so that
CALIPSO works with SYN cookies.

Fixes: e1adea927080 ("calipso: Allow request sockets to be relabelled by the lsm.")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/netdev/179378292255.488856.6917577451813010963@kernel.org/
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
Cc: Kuniyuki Iwashima <kuniyu@google.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: linux-security-module@vger.kernel.org
---
 net/ipv6/calipso.c | 12 ++++--------
 net/ipv6/exthdrs.c |  5 +++--
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/net/ipv6/calipso.c b/net/ipv6/calipso.c
index e399a42e30839a1ceab2dfcd2b29beaca13f2dc1..4cd0ae9313b322204f03b010021ab6ae005f05fb 100644
--- a/net/ipv6/calipso.c
+++ b/net/ipv6/calipso.c
@@ -1219,7 +1219,7 @@ static int calipso_req_setattr(struct request_sock *req,
 	if (IS_ERR(new))
 		return PTR_ERR(new);
 
-	txopts = ipv6_renew_options(sk, req_inet->ipv6_opt, IPV6_HOPOPTS, new);
+	txopts = ipv6_renew_options(NULL, req_inet->ipv6_opt, IPV6_HOPOPTS, new);
 
 	kfree(new);
 
@@ -1227,10 +1227,8 @@ static int calipso_req_setattr(struct request_sock *req,
 		return PTR_ERR(txopts);
 
 	txopts = xchg(&req_inet->ipv6_opt, txopts);
-	if (txopts) {
-		atomic_sub(txopts->tot_len, &sk->sk_omem_alloc);
+	if (txopts)
 		txopt_put(txopts);
-	}
 
 	return 0;
 }
@@ -1260,14 +1258,12 @@ static void calipso_req_delattr(struct request_sock *req)
 	if (calipso_opt_del(req_inet->ipv6_opt->hopopt, &new))
 		return; /* Nothing to do */
 
-	txopts = ipv6_renew_options(sk, req_inet->ipv6_opt, IPV6_HOPOPTS, new);
+	txopts = ipv6_renew_options(NULL, req_inet->ipv6_opt, IPV6_HOPOPTS, new);
 
 	if (!IS_ERR(txopts)) {
 		txopts = xchg(&req_inet->ipv6_opt, txopts);
-		if (txopts) {
-			atomic_sub(txopts->tot_len, &sk->sk_omem_alloc);
+		if (txopts)
 			txopt_put(txopts);
-		}
 	}
 	kfree(new);
 }
diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
index 96c8402a27a549a5001b42e9402c41acd2a4f49a..ccc549de3c8a07e605f5a27a7c8cb251d83b5f4e 100644
--- a/net/ipv6/exthdrs.c
+++ b/net/ipv6/exthdrs.c
@@ -1281,7 +1281,7 @@ static void ipv6_renew_option(int renewtype,
  * not copied into the new set of options.
  *
  * The new set of options is allocated from the socket option memory
- * buffer of @sk.
+ * buffer of @sk, or with kmalloc() if @sk is NULL.
  */
 struct ipv6_txoptions *
 ipv6_renew_options(struct sock *sk, struct ipv6_txoptions *opt,
@@ -1309,7 +1309,8 @@ ipv6_renew_options(struct sock *sk, struct ipv6_txoptions *opt,
 		return NULL;
 
 	tot_len += sizeof(*opt2);
-	opt2 = sock_kmalloc(sk, tot_len, GFP_ATOMIC);
+	opt2 = sk ? sock_kmalloc(sk, tot_len, GFP_ATOMIC) :
+		    kmalloc(tot_len, GFP_ATOMIC);
 	if (!opt2)
 		return ERR_PTR(-ENOBUFS);
 
-- 
2.53.0

Re: [PATCH net] calipso: do not charge request socket options to the listener

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] calipso: do not charge request socket options to the listener

[Kuniyuki Iwashima]

On Wed, 04 Nov 2026 16:03:18 +0100, Eric Dumazet wrote:
> calipso_req_setattr() and calipso_req_delattr() allocate ireq->ipv6_opt
> with ipv6_renew_options(), which charges the listener sk_omem_alloc
> through sock_kmalloc().
> 
> tcp_v6_reqsk_destructor() frees ireq->ipv6_opt without uncharging it,
> so each request socket leaks its options size from the listener
> budget. Once sysctl_optmem_max is reached, sock_kmalloc() fails,
> calipso_req_setattr() returns -ENOBUFS, and tcp_v6_route_req() drops
> all subsequent SYN packets. __sk_destruct() also reports an optmem
> leakage when the listener is destroyed.
> 
> Uncharging in tcp_v6_reqsk_destructor() would not be correct: after a
> migration, req->rsk_listener is no longer the socket that was charged.
> 
> These options belong to the request socket, there is no reason to
> charge them to the listener. Change ipv6_renew_options() to use
> kmalloc() when its sk argument is NULL, and use this from
> calipso_req_setattr() and calipso_req_delattr(),
> like cipso_v4_req_setattr() which uses kzalloc().
> 
> Note:
> 
> After this patch, calipso_req_setattr() and calipso_req_delattr()
> no longer need the listener. Commit 10876da918fa ("calipso: Fix
> null-ptr-deref in calipso_req_{set,del}attr().") could be reverted
> in net-next (also removing the then unused sk variables), so that
> CALIPSO works with SYN cookies.
> 
> Fixes: e1adea927080 ("calipso: Allow request sockets to be relabelled by the lsm.")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/netdev/179378292255.488856.6917577451813010963@kernel.org/
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>

Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com>

Re: [PATCH net] calipso: do not charge request socket options to the listener

[Paul Moore]

On Wed, Nov 4, 2026 at 10:03 AM Eric Dumazet <edumazet@kernel.org> wrote:
>
> calipso_req_setattr() and calipso_req_delattr() allocate ireq->ipv6_opt
> with ipv6_renew_options(), which charges the listener sk_omem_alloc
> through sock_kmalloc().
>
> tcp_v6_reqsk_destructor() frees ireq->ipv6_opt without uncharging it,
> so each request socket leaks its options size from the listener
> budget. Once sysctl_optmem_max is reached, sock_kmalloc() fails,
> calipso_req_setattr() returns -ENOBUFS, and tcp_v6_route_req() drops
> all subsequent SYN packets. __sk_destruct() also reports an optmem
> leakage when the listener is destroyed.
>
> Uncharging in tcp_v6_reqsk_destructor() would not be correct: after a
> migration, req->rsk_listener is no longer the socket that was charged.
>
> These options belong to the request socket, there is no reason to
> charge them to the listener. Change ipv6_renew_options() to use
> kmalloc() when its sk argument is NULL, and use this from
> calipso_req_setattr() and calipso_req_delattr(),
> like cipso_v4_req_setattr() which uses kzalloc().
>
> Note:
>
> After this patch, calipso_req_setattr() and calipso_req_delattr()
> no longer need the listener. Commit 10876da918fa ("calipso: Fix
> null-ptr-deref in calipso_req_{set,del}attr().") could be reverted
> in net-next (also removing the then unused sk variables), so that
> CALIPSO works with SYN cookies.

That would be great, are you planning to submit a follow-up patch for
that once this lands?  Or is that left for others?  Either answer is
fine, just trying to get ahead of any duplicated effort.

Otherwise, this looks good to me, thanks Eric.

Acked-by: Paul Moore <paul@paul-moore.com>

> Fixes: e1adea927080 ("calipso: Allow request sockets to be relabelled by the lsm.")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/netdev/179378292255.488856.6917577451813010963@kernel.org/
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>
> Cc: Kuniyuki Iwashima <kuniyu@google.com>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: linux-security-module@vger.kernel.org
> ---
>  net/ipv6/calipso.c | 12 ++++--------
>  net/ipv6/exthdrs.c |  5 +++--
>  2 files changed, 7 insertions(+), 10 deletions(-)

-- 
paul-moore.com

Re: [PATCH net] calipso: do not charge request socket options to the listener

[Eric Dumazet]

Le ven. 6 nov. 2026 à 01:10, Paul Moore <paul@paul-moore.com> a écrit :
>
> On Wed, Nov 4, 2026 at 10:03 AM Eric Dumazet <edumazet@kernel.org> wrote:
> >
> > calipso_req_setattr() and calipso_req_delattr() allocate ireq->ipv6_opt
> > with ipv6_renew_options(), which charges the listener sk_omem_alloc
> > through sock_kmalloc().
> >
> > tcp_v6_reqsk_destructor() frees ireq->ipv6_opt without uncharging it,
> > so each request socket leaks its options size from the listener
> > budget. Once sysctl_optmem_max is reached, sock_kmalloc() fails,
> > calipso_req_setattr() returns -ENOBUFS, and tcp_v6_route_req() drops
> > all subsequent SYN packets. __sk_destruct() also reports an optmem
> > leakage when the listener is destroyed.
> >
> > Uncharging in tcp_v6_reqsk_destructor() would not be correct: after a
> > migration, req->rsk_listener is no longer the socket that was charged.
> >
> > These options belong to the request socket, there is no reason to
> > charge them to the listener. Change ipv6_renew_options() to use
> > kmalloc() when its sk argument is NULL, and use this from
> > calipso_req_setattr() and calipso_req_delattr(),
> > like cipso_v4_req_setattr() which uses kzalloc().
> >
> > Note:
> >
> > After this patch, calipso_req_setattr() and calipso_req_delattr()
> > no longer need the listener. Commit 10876da918fa ("calipso: Fix
> > null-ptr-deref in calipso_req_{set,del}attr().") could be reverted
> > in net-next (also removing the then unused sk variables), so that
> > CALIPSO works with SYN cookies.
>
> That would be great, are you planning to submit a follow-up patch for
> that once this lands?  Or is that left for others?  Either answer is
> fine, just trying to get ahead of any duplicated effort.
>

I will provide the follow-up patch, thanks !

> Otherwise, this looks good to me, thanks Eric.
>
> Acked-by: Paul Moore <paul@paul-moore.com>
>