From: Colin Ian King <colin.king@canonical.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Geoff Levand <geoff@infradead.org>,
"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] ps3-vuart: BUG_ON on null drv before dereferencing it on dev_dbg
Date: Tue, 01 Sep 2015 11:38:55 +0100 [thread overview]
Message-ID: <55E5803F.3090704@canonical.com> (raw)
In-Reply-To: <CAMuHMdUm-eRpPmoNARdPVsi9=4Fqxp8crfgOXZkVTdAFhNzngQ@mail.gmail.com>
On 01/09/15 11:35, Geert Uytterhoeven wrote:
> Hi Colin,
>
> On Tue, Sep 1, 2015 at 12:21 PM, Colin King <colin.king@canonical.com> wrote:
>> On the unlikely event that drv is null, the current code will
>> perform a null pointer dereference with it when printing a dev_dbg
>> message. Instead, the BUG_ON check on drv should be performed
>> before we emit the dev_dbg message.
>
> What about just removing the BUG_ON()?
>
> The system will crash anyway, providing a backtrace.
I personally think a BUG_ON() shows intention to try to catch an
unexpected issue in a standard way as opposed to just removing it and
then hitting an issue with a null ptr deference.
>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>> drivers/ps3/ps3-vuart.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/ps3/ps3-vuart.c b/drivers/ps3/ps3-vuart.c
>> index d6db822..632701a 100644
>> --- a/drivers/ps3/ps3-vuart.c
>> +++ b/drivers/ps3/ps3-vuart.c
>> @@ -1000,12 +1000,11 @@ static int ps3_vuart_probe(struct ps3_system_bus_device *dev)
>> dev_dbg(&dev->core, "%s:%d\n", __func__, __LINE__);
>>
>> drv = ps3_system_bus_dev_to_vuart_drv(dev);
>> + BUG_ON(!drv);
>>
>> dev_dbg(&dev->core, "%s:%d: (%s)\n", __func__, __LINE__,
>> drv->core.core.name);
>>
>> - BUG_ON(!drv);
>> -
>> if (dev->port_number >= PORT_COUNT) {
>> BUG();
>> return -EINVAL;
>
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
next prev parent reply other threads:[~2015-09-01 10:39 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-09-01 10:21 [PATCH] ps3-vuart: BUG_ON on null drv before dereferencing it on dev_dbg Colin King
2015-09-01 10:35 ` Geert Uytterhoeven
2015-09-01 10:38 ` Colin Ian King [this message]
2015-09-01 11:28 ` Geert Uytterhoeven
2015-09-01 17:20 ` Geoff Levand
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=55E5803F.3090704@canonical.com \
--to=colin.king@canonical.com \
--cc=geert@linux-m68k.org \
--cc=geoff@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.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 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.