From: Ido Schimmel <idosch@nvidia.com>
To: ZhilingZouzhilinz@nebusec.ai
Cc: pabeni@redhat.com, netdev@vger.kernel.org, dsahern@kernel.org,
davem@davemloft.net, edumazet@google.com, horms@kernel.org,
vega@nebusec.ai, zhilinz@nebusec.ai
Subject: Re: [PATCH net v3 1/1] ipv6: flowlabel: cap duplicate leases per socket
Date: Mon, 24 Aug 2026 18:22:14 +0300 [thread overview]
Message-ID: <20260824152214.GA1112115@shredder> (raw)
In-Reply-To: <69d00f6ba09b1d1afd9281370cf7ae5e2ce806b6.1787387550.git.zhilinz@nebusec.ai>
On Sat, Aug 22, 2026 at 04:41:42PM +0800, ZhilingZouzhilinz@nebusec.ai wrote:
> From: Zhiling Zou <zhilinz@nebusec.ai>
>
> ipv6_flowlabel_get() allocates an ipv6_fl_socklist entry for every
> successful GET. The recheck path for a compatible existing flowlabel
> links another lease without applying any lease admission check. Repeated
> GET requests for one shareable label can therefore grow a socket's lease
> list without bound.
>
> Count the socket's existing leases during the lookup and reject a new
> unprivileged lease once the count reaches FL_MAX_PER_SOCK. This matches
> mem_check()'s per-socket accounting and also covers leases acquired from
> distinct globally interned labels. The IPv6 sockopt lock serializes the
> count with fl_link() and PUT, so no global ip6_fl_lock is needed.
>
> Check CAP_NET_ADMIN only when the socket reaches the limit. This avoids a
> capability audit on successful unprivileged GET requests below the cap,
> while privileged callers remain unrestricted.
>
> Do the admission check before updating linger and expires. A rejected GET
> does not refresh the shared label, matching the existing socket-list
> allocation failure path.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Reported-by: Vega <vega@nebusec.ai>
> Signed-off-by: Zhiling Zou <zhilinz@nebusec.ai>
> ---
> changes in v3:
> - Count the socket's full lease list instead of only matching leases.
> - Defer the CAP_NET_ADMIN check until the lease count reaches the limit.
> - v2 Link: https://lore.kernel.org/all/528bc30d301bcf32aca37cda1933b617d2d295f4.1786447968.git.zhilinz@nebusec.ai/
>
> changes in v2:
> - Count only leases of the flowlabel being reused.
> - Fold the count into the existing socket-list lookup.
> - Keep new-label admission under the existing mem_check() policy.
> - Stop after the first matching lease for CAP_NET_ADMIN callers.
> - Explain why a rejected GET does not refresh linger or expires.
> - Correct the reuse-path explanation and trim the Fixes hash.
> - v1 Link: https://lore.kernel.org/all/cf4fdc79ae4dc46bd4eb7eeb57e5de2091c13cd3.1785746178.git.zhilinz@nebusec.ai/
>
> net/ipv6/ip6_flowlabel.c | 18 ++++++++++++++----
> 1 file changed, 14 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv6/ip6_flowlabel.c b/net/ipv6/ip6_flowlabel.c
> index 1ab5ad0dcf24f..b864dfc87f779 100644
> --- a/net/ipv6/ip6_flowlabel.c
> +++ b/net/ipv6/ip6_flowlabel.c
> @@ -617,6 +617,7 @@ static int ipv6_flowlabel_get(struct sock *sk, struct in6_flowlabel_req *freq,
> struct ipv6_fl_socklist *sfl, *sfl1 = NULL;
> struct ip6_flowlabel *fl, *fl1 = NULL;
> struct net *net = sock_net(sk);
> + int lease_count = 0;
> int err;
>
> if (freq->flr_flags & IPV6_FL_F_REFLECT) {
> @@ -647,16 +648,20 @@ static int ipv6_flowlabel_get(struct sock *sk, struct in6_flowlabel_req *freq,
> err = -EEXIST;
> rcu_read_lock();
> for_each_sk_fl_rcu(sk, sfl) {
> + lease_count++;
> if (sfl->fl->label == freq->flr_label) {
> if (freq->flr_flags & IPV6_FL_F_EXCL) {
> rcu_read_unlock();
> goto done;
> }
> - fl1 = sfl->fl;
> - if (!atomic_inc_not_zero(&fl1->users))
> - fl1 = NULL;
> - break;
> + if (!fl1) {
> + fl1 = sfl->fl;
> + if (!atomic_inc_not_zero(&fl1->users))
> + fl1 = NULL;
> + }
> }
> + if (fl1 && lease_count >= FL_MAX_PER_SOCK)
> + break;
I find this hunk confusing. What do we gain from it? Unpriv users can
still walk the entire list in case of a miss. Also, we can get to the
recheck label from the fl_intern() path which is outside of this block.
How about something like [1]?
Also, please mention in the commit message that capable() is used
instead of ns_capable() in order to be consistent with mem_check() and
because we want to prevent unpriv users from consuming the host's memory
by creating a user ns and a netns where they have CAP_NET_ADMIN.
> }
> rcu_read_unlock();
>
> @@ -679,6 +684,11 @@ static int ipv6_flowlabel_get(struct sock *sk, struct in6_flowlabel_req *freq,
> err = -ENOMEM;
> if (!sfl1)
> goto release;
> + /* sockopt_lock_sock() serializes the count and fl_link(). */
> + err = -ENOBUFS;
> + if (lease_count >= FL_MAX_PER_SOCK &&
> + !capable(CAP_NET_ADMIN))
> + goto release;
> if (fl->linger > fl1->linger)
> fl1->linger = fl->linger;
> if ((long)(fl->expires - fl1->expires) > 0)
[1]
diff --git a/net/ipv6/ip6_flowlabel.c b/net/ipv6/ip6_flowlabel.c
index 1ab5ad0dcf24..006585dc8b5c 100644
--- a/net/ipv6/ip6_flowlabel.c
+++ b/net/ipv6/ip6_flowlabel.c
@@ -461,6 +461,21 @@ fl_create(struct net *net, struct sock *sk, struct in6_flowlabel_req *freq,
return NULL;
}
+static bool fl_sock_at_lease_limit(const struct sock *sk)
+{
+ const struct ipv6_fl_socklist *sfl;
+ int count = 0;
+
+ rcu_read_lock();
+ for_each_sk_fl_rcu(sk, sfl) {
+ if (++count >= FL_MAX_PER_SOCK)
+ break;
+ }
+ rcu_read_unlock();
+
+ return count >= FL_MAX_PER_SOCK;
+}
+
static int mem_check(struct sock *sk)
{
const int unpriv_total_limit = FL_MAX_SIZE - (FL_MAX_SIZE / 4);
@@ -679,6 +694,10 @@ static int ipv6_flowlabel_get(struct sock *sk, struct in6_flowlabel_req *freq,
err = -ENOMEM;
if (!sfl1)
goto release;
+ err = -ENOBUFS;
+ if (fl_sock_at_lease_limit(sk) &&
+ !capable(CAP_NET_ADMIN))
+ goto release;
if (fl->linger > fl1->linger)
fl1->linger = fl->linger;
if ((long)(fl->expires - fl1->expires) > 0)
prev parent reply other threads:[~2026-08-24 15:22 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-22 8:41 [PATCH net v3 0/1] ipv6: flowlabel: cap duplicate leases per socket Zhiling
2026-08-22 8:41 ` [PATCH net v3 1/1] " Zhiling
2026-08-24 15:22 ` Ido Schimmel [this message]
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=20260824152214.GA1112115@shredder \
--to=idosch@nvidia.com \
--cc=ZhilingZouzhilinz@nebusec.ai \
--cc=davem@davemloft.net \
--cc=dsahern@kernel.org \
--cc=edumazet@google.com \
--cc=horms@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=vega@nebusec.ai \
--cc=zhilinz@nebusec.ai \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.