mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Paolo Abeni <pabeni@redhat.com>
To: Noah Weinberger <NoahWeinberger2003@proton.me>,
	"jhs@mojatatu.com" <jhs@mojatatu.com>,
	"jiri@resnulli.us" <jiri@resnulli.us>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"edumazet@kernel.org" <edumazet@kernel.org>,
	"kuba@kernel.org" <kuba@kernel.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] net/sched: cls_u32: drop ht_down ref on hardware knode failure
Date: Mon, 9 Nov 2026 09:03:38 +0100	[thread overview]
Message-ID: <17bd863d-0ea5-4318-8d05-4fc786fcf37d@redhat.com> (raw)
In-Reply-To: <se_YnKsUIDyPpX0CSaIOaKht61bpYFwFGdmOcphFkus7Rk6NW3JnQsqMNGmDOjF1R4yTPaR_yeEK8clojDnh5Orl93J0kiHNJT7R3PkCxWQ=@proton.me>

On 11/9/26 00:51, Noah Weinberger wrote:
> Hello,
> 
> u32_set_parms() takes a reference on the TCA_U32_LINK hash table
> (refcount_inc at the end of the link block), but two of the three
> paths that can fail after that point never drop it:
> 
> 1. Fresh knode (u32_change): when u32_replace_hw_knode() fails, the
>     errunbind path unbinds the filter and then frees the node with
>     plain kfree(); n->ht_down keeps its reference forever.
> 
> 2. Replace path: u32_set_parms() has already re-pointed new->ht_down
> to the newly linked table B and dropped the old table A, but the
> failure block then takes an extra refcount_inc() on A (the old
> node's ht_down) as if __u32_destroy_key(new) would drop A - it drops
> B instead. The extra reference on A is never released, and since the
> old node survives this error path holding its original reference,
> A's refcount is permanently one too high.
> 
> Each failed attempt (a TC_SETUP_CLSU32-capable driver rejecting the
> rule) therefore leaks one reference; the linked table can never be
> deleted afterwards (u32_delete returns -EBUSY while refcount > 1) and
> is never freed with its hash array.
> 
> The patch drops n->ht_down on the fresh-node error path and removes
> the bogus compensating increment on the replace path, leaving
> __u32_destroy_key(new) as the single place that drops what the node
> holds - which is correct for both the LINK and no-LINK cases.
> 
> Affected version/commit: mainline 8a65162514fbad21a2c58ffed65f42b27e92b163 (v7.4-rc2 window, 2026-11-08), net/sched/cls_u32.c:794 (refcount_inc in u32_set_parms), :958-965 (replace path: compensating refcount_inc targets the old node's table while __u32_destroy_key drops new->ht_down, leaking one reference), :1191-1202 (fresh path: u32_set_parms then u32_replace_hw_knode), :1222-1238 (errunbind path frees n with plain kfree() and never drops n->ht_down), :406-414 (__u32_destroy_key drops n->ht_down)
> Longstanding error-path bug (the compensating-inc pattern predates the reviewed window).
> 
> This was found during an assisted (AI-supported) static source review of
> mainline 8a65162514fbad21a2c58ffed65f42b27e92b163 (v7.4-rc2 window, 2026-11-08). The patch below has NOT been compiled and has
> NOT been runtime tested, and no reproducer was produced - the triggering
> conditions are described above instead. Please treat this as a report for
> verification rather than a battle-tested fix.
You should invest a little more time here, asking the LLM model to generate
a reproduced, and actually testing the patch vs it. ATM we are flooded with
tons of reports of mixed quality. Half-hearted theoretical things will
have hard time to get any actual processing. Also you should cc the netdev
ML.

/P


      reply	other threads:[~2026-11-09  8:03 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-08 23:51 Noah Weinberger
2026-11-09  8:03 ` Paolo Abeni [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=17bd863d-0ea5-4318-8d05-4fc786fcf37d@redhat.com \
    --to=pabeni@redhat.com \
    --cc=NoahWeinberger2003@proton.me \
    --cc=davem@davemloft.net \
    --cc=edumazet@kernel.org \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /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

all inboxes | Powered by JetHome®