Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1216750 > unrolled thread
| Started by | Colin King <colin.king@canonical.com> |
|---|---|
| First post | 2015-09-01 12:30 +0200 |
| Last post | 2015-09-01 19:30 +0200 |
| Articles | 5 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH] ps3-vuart: BUG_ON on null drv before dereferencing it on dev_dbg Colin King <colin.king@canonical.com> - 2015-09-01 12:30 +0200
Re: [PATCH] ps3-vuart: BUG_ON on null drv before dereferencing it on dev_dbg Geert Uytterhoeven <geert@linux-m68k.org> - 2015-09-01 12:40 +0200
Re: [PATCH] ps3-vuart: BUG_ON on null drv before dereferencing it on dev_dbg Colin Ian King <colin.king@canonical.com> - 2015-09-01 12:40 +0200
Re: [PATCH] ps3-vuart: BUG_ON on null drv before dereferencing it on dev_dbg Geert Uytterhoeven <geert@linux-m68k.org> - 2015-09-01 13:30 +0200
Re: [PATCH] ps3-vuart: BUG_ON on null drv before dereferencing it on dev_dbg Geoff Levand <geoff@infradead.org> - 2015-09-01 19:30 +0200
| From | Colin King <colin.king@canonical.com> |
|---|---|
| Date | 2015-09-01 12:30 +0200 |
| Subject | [PATCH] ps3-vuart: BUG_ON on null drv before dereferencing it on dev_dbg |
| Message-ID | <q3QZc-3MN-5@gated-at.bofh.it> |
From: Colin Ian King <colin.king@canonical.com>
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.
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;
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Geert Uytterhoeven <geert@linux-m68k.org> |
|---|---|
| Date | 2015-09-01 12:40 +0200 |
| Message-ID | <q3R8R-3Y0-7@gated-at.bofh.it> |
| In reply to | #1216750 |
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.
> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Colin Ian King <colin.king@canonical.com> |
|---|---|
| Date | 2015-09-01 12:40 +0200 |
| Subject | Re: [PATCH] ps3-vuart: BUG_ON on null drv before dereferencing it on dev_dbg |
| Message-ID | <q3R8R-3Y0-5@gated-at.bofh.it> |
| In reply to | #1216754 |
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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Geert Uytterhoeven <geert@linux-m68k.org> |
|---|---|
| Date | 2015-09-01 13:30 +0200 |
| Message-ID | <q3RVg-57f-15@gated-at.bofh.it> |
| In reply to | #1216755 |
Hi Colin,
On Tue, Sep 1, 2015 at 12:38 PM, Colin Ian King
<colin.king@canonical.com> wrote:
> On 01/09/15 11:35, Geert Uytterhoeven wrote:
>> 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.
I think this BUG_ON() was purely used during development of the ps3_vuart
framework.
Note that the corresponding platform_drv_probe() also just dereferences its
driver pointer.
>>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Geoff Levand <geoff@infradead.org> |
|---|---|
| Date | 2015-09-01 19:30 +0200 |
| Subject | Re: [PATCH] ps3-vuart: BUG_ON on null drv before dereferencing it on dev_dbg |
| Message-ID | <q3XxE-4Jj-13@gated-at.bofh.it> |
| In reply to | #1216750 |
On Tue, 2015-09-01 at 11:21 +0100, Colin King 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. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> I applied this to my ps3-linux repo. I can update it if you decide to do the other fixes Geert mentioned. -Geoff -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web