From: Xia Fukun xiafukun@huawei.com
stable inclusion from stable-v4.19.267 commit 33d2f83e3f2c1fdabb365d25bed3aa630041cbc0 category: bugfix bugzilla: 188002, https://gitee.com/openeuler/kernel/issues/I63UEU CVE: NA
--------------------------------
commit fc82bbf4dede758007763867d0282353c06d1121 upstream.
This is another old BUG_ON() that just shouldn't exist (see also commit a382f8fee42c: "signal handling: don't use BUG_ON() for debugging").
In fact, as Matthew Wilcox points out, this condition shouldn't really even result in a warning, since a negative id allocation result is just a normal allocation failure:
"I wonder if we should even warn here -- sure, the caller is trying to free something that wasn't allocated, but we don't warn for kfree(NULL)"
and goes on to point out how that current error check is only causing people to unnecessarily do their own index range checking before freeing it.
This was noted by Itay Iellin, because the bluetooth HCI socket cookie code does *not* do that range checking, and ends up just freeing the error case too, triggering the BUG_ON().
The HCI code requires CAP_NET_RAW, and seems to just result in an ugly splat, but there really is no reason to BUG_ON() here, and we have generally striven for allocation models where it's always ok to just do
free(alloc());
even if the allocation were to fail for some random reason (usually obviously that "random" reason being some resource limit).
Fixes: 88eca0207cf1 ("ida: simplified functions for id allocation") Reported-by: Itay Iellin ieitayie@gmail.com Suggested-by: Matthew Wilcox willy@infradead.org Signed-off-by: Linus Torvalds torvalds@linux-foundation.org Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org Signed-off-by: Xia Fukun xiafukun@huawei.com Signed-off-by: Yongqiang Liu liuyongqiang13@huawei.com --- lib/idr.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/idr.c b/lib/idr.c index 6ff3b1c36e0a..82c24a417dc6 100644 --- a/lib/idr.c +++ b/lib/idr.c @@ -573,7 +573,9 @@ void ida_free(struct ida *ida, unsigned int id) { unsigned long flags;
- BUG_ON((int)id < 0); + if ((int)id < 0) + return; + xa_lock_irqsave(&ida->ida_rt, flags); ida_remove(ida, id); xa_unlock_irqrestore(&ida->ida_rt, flags);