| Message ID | 20170706221902.GA29514@embeddedgus (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Darren Hart |
| Headers | show |
On Thu, Jul 06, 2017 at 05:19:02PM -0500, Gustavo A. R. Silva wrote: > Check return value from call to devm_kzalloc() > in order to prevent a NULL pointer dereference. > > This issue was detected using Coccinelle and the following semantic patch: > > @@ > expression x; > identifier fld; > @@ > > * x = devm_kzalloc(...); > ... when != x == NULL > x->fld > > Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com> These checks should be added in the interest of code correctness. devm_kzalloc() can fail (even if it's extremely unlikely in practice) so we should check for this. Reviewed-by: Jonathan Woithe <jwoithe@just42.net> > --- > drivers/platform/x86/fujitsu-laptop.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c > index c1a8528..593a350 100644 > --- a/drivers/platform/x86/fujitsu-laptop.c > +++ b/drivers/platform/x86/fujitsu-laptop.c > @@ -695,6 +695,9 @@ static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device) > if (call_fext_func(device, > FUNC_LEDS, 0x0, 0x0, 0x0) & LOGOLAMP_POWERON) { > led = devm_kzalloc(&device->dev, sizeof(*led), GFP_KERNEL); > + if (!led) > + return -ENOMEM; > + > led->name = "fujitsu::logolamp"; > led->brightness_set_blocking = logolamp_set; > led->brightness_get = logolamp_get; > @@ -707,6 +710,9 @@ static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device) > FUNC_LEDS, 0x0, 0x0, 0x0) & KEYBOARD_LAMPS) && > (call_fext_func(device, FUNC_BUTTONS, 0x0, 0x0, 0x0) == 0x0)) { > led = devm_kzalloc(&device->dev, sizeof(*led), GFP_KERNEL); > + if (!led) > + return -ENOMEM; > + > led->name = "fujitsu::kblamps"; > led->brightness_set_blocking = kblamps_set; > led->brightness_get = kblamps_get; > @@ -723,6 +729,9 @@ static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device) > */ > if (call_fext_func(device, FUNC_BUTTONS, 0x0, 0x0, 0x0) & BIT(24)) { > led = devm_kzalloc(&device->dev, sizeof(*led), GFP_KERNEL); > + if (!led) > + return -ENOMEM; > + > led->name = "fujitsu::radio_led"; > led->brightness_set_blocking = radio_led_set; > led->brightness_get = radio_led_get; > @@ -741,6 +750,9 @@ static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device) > (call_fext_func(device, > FUNC_LEDS, 0x2, ECO_LED, 0x0) != UNSUPPORTED_CMD)) { > led = devm_kzalloc(&device->dev, sizeof(*led), GFP_KERNEL); > + if (!led) > + return -ENOMEM; > + > led->name = "fujitsu::eco_led"; > led->brightness_set_blocking = eco_led_set; > led->brightness_get = eco_led_get; > -- > 2.5.0
On Fri, Jul 07, 2017 at 09:49:00AM +0930, Jonathan Woithe wrote: > On Thu, Jul 06, 2017 at 05:19:02PM -0500, Gustavo A. R. Silva wrote: > > Check return value from call to devm_kzalloc() > > in order to prevent a NULL pointer dereference. > > > > This issue was detected using Coccinelle and the following semantic patch: > > > > @@ > > expression x; > > identifier fld; > > @@ > > > > * x = devm_kzalloc(...); > > ... when != x == NULL > > x->fld > > > > Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com> > > These checks should be added in the interest of code correctness. > devm_kzalloc() can fail (even if it's extremely unlikely in practice) so we > should check for this. > > Reviewed-by: Jonathan Woithe <jwoithe@just42.net> Thanks Gustavo and Jonathan, Queued to testing.
Hi Darren, Jonathan, Quoting Darren Hart <dvhart@infradead.org>: > On Fri, Jul 07, 2017 at 09:49:00AM +0930, Jonathan Woithe wrote: >> On Thu, Jul 06, 2017 at 05:19:02PM -0500, Gustavo A. R. Silva wrote: >> > Check return value from call to devm_kzalloc() >> > in order to prevent a NULL pointer dereference. >> > >> > This issue was detected using Coccinelle and the following semantic patch: >> > >> > @@ >> > expression x; >> > identifier fld; >> > @@ >> > >> > * x = devm_kzalloc(...); >> > ... when != x == NULL >> > x->fld >> > >> > Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com> >> >> These checks should be added in the interest of code correctness. >> devm_kzalloc() can fail (even if it's extremely unlikely in practice) so we >> should check for this. >> >> Reviewed-by: Jonathan Woithe <jwoithe@just42.net> > > Thanks Gustavo and Jonathan, > > Queued to testing. > Glad to help. :) Thanks -- Gustavo A. R. Silva
diff --git a/drivers/platform/x86/fujitsu-laptop.c b/drivers/platform/x86/fujitsu-laptop.c index c1a8528..593a350 100644 --- a/drivers/platform/x86/fujitsu-laptop.c +++ b/drivers/platform/x86/fujitsu-laptop.c @@ -695,6 +695,9 @@ static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device) if (call_fext_func(device, FUNC_LEDS, 0x0, 0x0, 0x0) & LOGOLAMP_POWERON) { led = devm_kzalloc(&device->dev, sizeof(*led), GFP_KERNEL); + if (!led) + return -ENOMEM; + led->name = "fujitsu::logolamp"; led->brightness_set_blocking = logolamp_set; led->brightness_get = logolamp_get; @@ -707,6 +710,9 @@ static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device) FUNC_LEDS, 0x0, 0x0, 0x0) & KEYBOARD_LAMPS) && (call_fext_func(device, FUNC_BUTTONS, 0x0, 0x0, 0x0) == 0x0)) { led = devm_kzalloc(&device->dev, sizeof(*led), GFP_KERNEL); + if (!led) + return -ENOMEM; + led->name = "fujitsu::kblamps"; led->brightness_set_blocking = kblamps_set; led->brightness_get = kblamps_get; @@ -723,6 +729,9 @@ static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device) */ if (call_fext_func(device, FUNC_BUTTONS, 0x0, 0x0, 0x0) & BIT(24)) { led = devm_kzalloc(&device->dev, sizeof(*led), GFP_KERNEL); + if (!led) + return -ENOMEM; + led->name = "fujitsu::radio_led"; led->brightness_set_blocking = radio_led_set; led->brightness_get = radio_led_get; @@ -741,6 +750,9 @@ static int acpi_fujitsu_laptop_leds_register(struct acpi_device *device) (call_fext_func(device, FUNC_LEDS, 0x2, ECO_LED, 0x0) != UNSUPPORTED_CMD)) { led = devm_kzalloc(&device->dev, sizeof(*led), GFP_KERNEL); + if (!led) + return -ENOMEM; + led->name = "fujitsu::eco_led"; led->brightness_set_blocking = eco_led_set; led->brightness_get = eco_led_get;
Check return value from call to devm_kzalloc() in order to prevent a NULL pointer dereference. This issue was detected using Coccinelle and the following semantic patch: @@ expression x; identifier fld; @@ * x = devm_kzalloc(...); ... when != x == NULL x->fld Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com> --- drivers/platform/x86/fujitsu-laptop.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)