From: Alexandre Ferrieux <alexandre.ferrieux@gmail.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: edumazet@google.com, jhs@mojatatu.com, xiyou.wangcong@gmail.com,
jiri@resnulli.us, horms@kernel.org, netdev@vger.kernel.org
Subject: RFC: chasing all idr_remove() misses
Date: Thu, 14 Nov 2024 19:24:27 +0100 [thread overview]
Message-ID: <40bb5d4c-e21d-4eac-aec0-25b2f722be6d@orange.com> (raw)
In-Reply-To: <173147403002.787328.3694466422358304986.git-patchwork-notify@kernel.org>
Hi,
In the recent fix of u32's IDR leaks, one side remark is that the problem went
unnoticed for 7 years due to the NULL result from idr_remove() being ignored at
this call site.
Now, a cursory grep over the whole Linux tree shows 306 out of 386 call sites
(excluding those hidden in macros, if any) don't bother to extract the value
returned by idr_remove().
Indeed, a failed IDR removal is "mostly harmless" since IDs are not pointers so
the mismatch is detectable (and is detected, returning NULL). However, in racy
situations you may end up killing an innocent fresh entry, which may really
break things a bit later. And in all cases, a true bug is the root cause.
So, unless we have reasons to think cls_u32 was the only place where two ID
encodings might lend themselves to confusion, I'm wondering if it wouldn't make
sense to chase the issue more systematically:
- either with WARN_ON[_ONCE](idr_remove()==NULL) on each call site individually
(a year-long endeavor implying tens of maintainers)
- or with WARN_ON[_ONCE] just before returning NULL within idr_remove() itself,
or even radix_tree_delete_item().
Opinions ?
next prev parent reply other threads:[~2024-11-14 18:24 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-10 17:28 [PATCH net v7] net: sched: cls_u32: Fix u32's systematic failure to free IDR entries for hnodes Alexandre Ferrieux
2024-11-10 18:14 ` Victor Nogueira
2024-11-13 5:00 ` patchwork-bot+netdevbpf
2024-11-14 18:24 ` Alexandre Ferrieux [this message]
2024-11-19 3:51 ` RFC: chasing all idr_remove() misses Cong Wang
2024-11-19 3:57 ` Cong Wang
2024-11-19 6:46 ` Alexandre Ferrieux
2024-11-22 21:32 ` Cong Wang
-- strict thread matches above, loose matches on Subject: below --
2024-11-16 17:45 Alexandre Ferrieux
2024-11-16 19:43 ` Dan Carpenter
2024-11-17 15:07 ` Alexandre Ferrieux
2024-11-18 2:47 ` Matthew Wilcox
2024-11-18 11:33 ` Alexandre Ferrieux
2024-11-20 14:43 ` Alexandre Ferrieux
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=40bb5d4c-e21d-4eac-aec0-25b2f722be6d@orange.com \
--to=alexandre.ferrieux@gmail.com \
--cc=edumazet@google.com \
--cc=horms@kernel.org \
--cc=jhs@mojatatu.com \
--cc=jiri@resnulli.us \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=xiyou.wangcong@gmail.com \
/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.