linux-kernel - Re: [PATCH 2/4] staging: most: Replace calls to BUG_ON() with WARN

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAFLvi20e+D4984vypj7nVECFxh_AKVwGWTqCqfyFYJ9AV=Rvdg@mail.gmail.com>
Date:   Tue, 6 Mar 2018 01:23:18 -0800
From:   Quytelda Kahja <quytelda@...alin.org>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     christian.gromm@...rochip.com, devel@...verdev.osuosl.org,
        Michael.Fabry@...rochip.com, chris@...ineersdelight.de,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/4] staging: most: Replace calls to BUG_ON() with
 WARN_ONCE() and return.

> Are you sure this will work?
Well, my goal was just to replace the code that could crash the kernel
and let somebody with a better understanding of this particular driver
write the recovery code, if necessary.  It seemed from context that
the BUG_ON() calls were being used like assert() though, so I assumed
there wasn't really much recovery to be made from that problem.  If
you feel this doesn't improve the behavior of the driver, just drop
the patch.

Thank you,
Quytelda Kahja

On Thu, Mar 1, 2018 at 8:21 AM, Greg KH <gregkh@...uxfoundation.org> wrote:
> On Fri, Feb 23, 2018 at 11:58:33PM -0800, Quytelda Kahja wrote:
>> Replace calls to BUG_ON() used to check for NULL pointers with WARN_ONCE()
>> followed by a return.
>
> Are you sure this will work?
>
>>
>> Signed-off-by: Quytelda Kahja <quytelda@...alin.org>
>> ---
>>  drivers/staging/most/core.c | 13 ++++++++++---
>>  1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/staging/most/core.c b/drivers/staging/most/core.c
>> index 18157dd80324..3f65390a6e17 100644
>> --- a/drivers/staging/most/core.c
>> +++ b/drivers/staging/most/core.c
>> @@ -916,7 +916,11 @@ static void arm_mbo(struct mbo *mbo)
>>       unsigned long flags;
>>       struct most_channel *c;
>>
>> -     BUG_ON((!mbo) || (!mbo->context));
>> +     if (WARN_ONCE(!mbo || !mbo->context,
>> +                   "Bad mbo or missing channel reference.\n")) {
>> +             return;
>
> How is the code supposed to recover from this major problem?
>
>> +     }
>> +
>>       c = mbo->context;
>>
>>       if (c->is_poisoned) {
>> @@ -1001,7 +1005,7 @@ static int arm_mbo_chain(struct most_channel *c, int dir,
>>  void most_submit_mbo(struct mbo *mbo)
>>  {
>>       if (WARN_ONCE(!mbo || !mbo->context,
>> -                   "bad mbo or missing channel reference\n"))
>> +                   "Bad mbo or missing channel reference.\n"))
>
> You did something different here that you did not describe in your
> changelog :(
>
> thanks,
>
> greg k-h