Skip to content

Commit

Permalink
ida: don't use BUG_ON() for debugging
Browse files Browse the repository at this point in the history
commit fc82bbf upstream.

This is another old BUG_ON() that just shouldn't exist (see also commit
a382f8f: "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: 88eca02 ("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>
  • Loading branch information
torvalds authored and gregkh committed Jul 12, 2022
1 parent 37995f0 commit e23cfb3
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion lib/idr.c
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,8 @@ void ida_free(struct ida *ida, unsigned int id)
struct ida_bitmap *bitmap;
unsigned long flags;

BUG_ON((int)id < 0);
if ((int)id < 0)
return;

xas_lock_irqsave(&xas, flags);
bitmap = xas_load(&xas);
Expand Down

0 comments on commit e23cfb3

Please sign in to comment.