Mailing List Archive: Linux: Kernel

[patch 3/3] ThinkPad ACPI: fix possible NULL pointer dereference

Index | Next | Previous | View Threaded

gorcunov at gmail

Apr 16, 2008, 10:44 AM

Post #1 of 7 (517 views)
Permalink

[patch 3/3] ThinkPad ACPI: fix possible NULL pointer dereference

Fix potential NULL pointer dereference if kstrdup failed

Signed-off-by: Cyrill Gorcunov <gorcunov [at] gmail>

---

Index: linux-2.6.git/drivers/misc/thinkpad_acpi.c
===================================================================
--- linux-2.6.git.orig/drivers/misc/thinkpad_acpi.c 2008-04-16 20:35:34.000000000 +0400
+++ linux-2.6.git/drivers/misc/thinkpad_acpi.c 2008-04-16 20:36:38.000000000 +0400
@@ -5826,7 +5826,7 @@ static void __init get_thinkpad_model_da

tp->model_str = kstrdup(dmi_get_system_info(DMI_PRODUCT_VERSION),
GFP_KERNEL);
- if (strnicmp(tp->model_str, "ThinkPad", 8) != 0) {
+ if (tp->model_str && strnicmp(tp->model_str, "ThinkPad", 8) != 0) {
kfree(tp->model_str);
tp->model_str = NULL;
}

--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

hmh at hmh

Apr 16, 2008, 1:52 PM

Post #2 of 7 (491 views)
Permalink

Re: [patch 3/3] ThinkPad ACPI: fix possible NULL pointer dereference [In reply to]

On Wed, 16 Apr 2008, Cyrill Gorcunov wrote:
> Fix potential NULL pointer dereference if kstrdup failed
>
> Signed-off-by: Cyrill Gorcunov <gorcunov [at] gmail>

Acked-by: Henrique de Moraes Holschuh <hmh [at] hmh>

> Index: linux-2.6.git/drivers/misc/thinkpad_acpi.c
> ===================================================================
> --- linux-2.6.git.orig/drivers/misc/thinkpad_acpi.c 2008-04-16 20:35:34.000000000 +0400
> +++ linux-2.6.git/drivers/misc/thinkpad_acpi.c 2008-04-16 20:36:38.000000000 +0400
> @@ -5826,7 +5826,7 @@ static void __init get_thinkpad_model_da
>
> tp->model_str = kstrdup(dmi_get_system_info(DMI_PRODUCT_VERSION),
> GFP_KERNEL);
> - if (strnicmp(tp->model_str, "ThinkPad", 8) != 0) {
> + if (tp->model_str && strnicmp(tp->model_str, "ThinkPad", 8) != 0) {
> kfree(tp->model_str);
> tp->model_str = NULL;
> }
>
> --

--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

pavel at ucw

Apr 18, 2008, 5:41 AM

Post #3 of 7 (476 views)
Permalink

Re: [patch 3/3] ThinkPad ACPI: fix possible NULL pointer dereference [In reply to]

Hi!

> Fix potential NULL pointer dereference if kstrdup failed
>
> Signed-off-by: Cyrill Gorcunov <gorcunov [at] gmail>
>
> ---
>
> Index: linux-2.6.git/drivers/misc/thinkpad_acpi.c
> ===================================================================
> --- linux-2.6.git.orig/drivers/misc/thinkpad_acpi.c 2008-04-16 20:35:34.000000000 +0400
> +++ linux-2.6.git/drivers/misc/thinkpad_acpi.c 2008-04-16 20:36:38.000000000 +0400
> @@ -5826,7 +5826,7 @@ static void __init get_thinkpad_model_da
>
> tp->model_str = kstrdup(dmi_get_system_info(DMI_PRODUCT_VERSION),
> GFP_KERNEL);
> - if (strnicmp(tp->model_str, "ThinkPad", 8) != 0) {
> + if (tp->model_str && strnicmp(tp->model_str, "ThinkPad", 8) != 0) {
> kfree(tp->model_str);
> tp->model_str = NULL;
> }

are you sure? This seems to assume machine is thinkpad if kstrdup
fails... which is very wrong.

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

gorcunov at gmail

Apr 18, 2008, 5:55 AM

Post #4 of 7 (477 views)
Permalink

Re: [patch 3/3] ThinkPad ACPI: fix possible NULL pointer dereference [In reply to]

[Pavel Machek - Fri, Apr 18, 2008 at 02:41:12PM +0200]
| Hi!
|
| > Fix potential NULL pointer dereference if kstrdup failed
| >
| > Signed-off-by: Cyrill Gorcunov <gorcunov [at] gmail>
| >
| > ---
| >
| > Index: linux-2.6.git/drivers/misc/thinkpad_acpi.c
| > ===================================================================
| > --- linux-2.6.git.orig/drivers/misc/thinkpad_acpi.c 2008-04-16 20:35:34.000000000 +0400
| > +++ linux-2.6.git/drivers/misc/thinkpad_acpi.c 2008-04-16 20:36:38.000000000 +0400
| > @@ -5826,7 +5826,7 @@ static void __init get_thinkpad_model_da
| >
| > tp->model_str = kstrdup(dmi_get_system_info(DMI_PRODUCT_VERSION),
| > GFP_KERNEL);
| > - if (strnicmp(tp->model_str, "ThinkPad", 8) != 0) {
| > + if (tp->model_str && strnicmp(tp->model_str, "ThinkPad", 8) != 0) {
| > kfree(tp->model_str);
| > tp->model_str = NULL;
| > }
|
| are you sure? This seems to assume machine is thinkpad if kstrdup
| fails... which is very wrong.

No, it's *not* wrong, look there we have

tp->model_str = kstrdup(dmi_get_system_info(DMI_PRODUCT_VERSION),
GFP_KERNEL);

lets assume we've got NULL here so probe_for_thinkpad() will check for it

is_thinkpad = (thinkpad_id.model_str != NULL);

Thanks for comment

- Cyrill -
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

gorcunov at gmail

Apr 18, 2008, 6:07 AM

Post #5 of 7 (478 views)
Permalink

Re: [patch 3/3] ThinkPad ACPI: fix possible NULL pointer dereference [In reply to]

[Pavel Machek - Fri, Apr 18, 2008 at 02:41:12PM +0200]
| Hi!
|
| > Fix potential NULL pointer dereference if kstrdup failed
| >
| > Signed-off-by: Cyrill Gorcunov <gorcunov [at] gmail>
| >
| > ---
| >
| > Index: linux-2.6.git/drivers/misc/thinkpad_acpi.c
| > ===================================================================
| > --- linux-2.6.git.orig/drivers/misc/thinkpad_acpi.c 2008-04-16 20:35:34.000000000 +0400
| > +++ linux-2.6.git/drivers/misc/thinkpad_acpi.c 2008-04-16 20:36:38.000000000 +0400
| > @@ -5826,7 +5826,7 @@ static void __init get_thinkpad_model_da
| >
| > tp->model_str = kstrdup(dmi_get_system_info(DMI_PRODUCT_VERSION),
| > GFP_KERNEL);
| > - if (strnicmp(tp->model_str, "ThinkPad", 8) != 0) {
| > + if (tp->model_str && strnicmp(tp->model_str, "ThinkPad", 8) != 0) {
| > kfree(tp->model_str);
| > tp->model_str = NULL;
| > }
|
| are you sure? This seems to assume machine is thinkpad if kstrdup
| fails... which is very wrong.
|

Oh, I see what do you mean - you mean that even if machine is ThinkPad
*but* kstrdup failed with my patch it would lead that the machine will
*not* be recognized as ThinkPad and that is not correct, agreed. But how
to preven from NULL dereference then? I think the current situation
brought by my patch would not lead to really critical problems *but*
it should be reorganized indeed! Thanks a lot, Pavel, for comments ;)

- Cyrill -
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

gorcunov at gmail

Apr 18, 2008, 6:53 AM

Post #6 of 7 (477 views)
Permalink

Re: [patch 3/3] ThinkPad ACPI: fix possible NULL pointer dereference [In reply to]

[Pavel Machek - Fri, Apr 18, 2008 at 02:41:12PM +0200]
| Hi!
|
| > Fix potential NULL pointer dereference if kstrdup failed
| >
| > Signed-off-by: Cyrill Gorcunov <gorcunov [at] gmail>
| >
| > ---
| >
| > Index: linux-2.6.git/drivers/misc/thinkpad_acpi.c
| > ===================================================================
| > --- linux-2.6.git.orig/drivers/misc/thinkpad_acpi.c 2008-04-16 20:35:34.000000000 +0400
| > +++ linux-2.6.git/drivers/misc/thinkpad_acpi.c 2008-04-16 20:36:38.000000000 +0400
| > @@ -5826,7 +5826,7 @@ static void __init get_thinkpad_model_da
| >
| > tp->model_str = kstrdup(dmi_get_system_info(DMI_PRODUCT_VERSION),
| > GFP_KERNEL);
| > - if (strnicmp(tp->model_str, "ThinkPad", 8) != 0) {
| > + if (tp->model_str && strnicmp(tp->model_str, "ThinkPad", 8) != 0) {
| > kfree(tp->model_str);
| > tp->model_str = NULL;
| > }
|
| are you sure? This seems to assume machine is thinkpad if kstrdup
| fails... which is very wrong.
|

Actually, my patch didn't bring any new into the current driver state,
just add additional check to prevent NULL deref, that's all, so I think
it's fine (but maybe additional printk with info would had been usefull).

- Cyrill -
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

hmh at hmh

Apr 18, 2008, 9:10 PM

Post #7 of 7 (465 views)
Permalink

Re: [patch 3/3] ThinkPad ACPI: fix possible NULL pointer dereference [In reply to]

On Fri, 18 Apr 2008, Cyrill Gorcunov wrote:
> Oh, I see what do you mean - you mean that even if machine is ThinkPad
> *but* kstrdup failed with my patch it would lead that the machine will
> *not* be recognized as ThinkPad and that is not correct, agreed. But how

That's acceptable. It is not worth bothering with this failure mode at
that point of the driver lifetime. Your patch makes it just keep
running, which is fine since if it can't kstrdup a small string, it will
abend with -ENOMEM soon enough when it tries to alocate other much
bigger structures.

Just in case, I will schedule a low-priority fix for later that will
-ENOMEM if kstrdup fails, but your patch is good enough a fix for now.

--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo [at] vger
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact lists@gossamer-threads.com