diff mbox

[1/3] driver core: check notifier_call_chain return value

Message ID 20180227140926.22996-2-benjamin.gaignard@st.com (mailing list archive)
State New, archived
Headers show

Commit Message

Benjamin Gaignard Feb. 27, 2018, 2:09 p.m. UTC
When being notified that a driver is about to be bind a listener
could return NOTIFY_BAD.
Check the return to be sure that the driver could be bind.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
 drivers/base/dd.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

Comments

Greg Kroah-Hartman March 15, 2018, 5:10 p.m. UTC | #1
On Tue, Feb 27, 2018 at 03:09:24PM +0100, Benjamin Gaignard wrote:
> When being notified that a driver is about to be bind a listener
> could return NOTIFY_BAD.
> Check the return to be sure that the driver could be bind.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> ---
>  drivers/base/dd.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index de6fd092bf2f..9275f2c0fed2 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -304,9 +304,12 @@ static int driver_sysfs_add(struct device *dev)
>  {
>  	int ret;
>  
> -	if (dev->bus)
> -		blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
> -					     BUS_NOTIFY_BIND_DRIVER, dev);
> +	if (dev->bus) {
> +		if (blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
> +						 BUS_NOTIFY_BIND_DRIVER, dev) ==
> +						 NOTIFY_BAD)
> +			return -EINVAL;

checkpatch does not complain about this?

And what is going to break when we enable this, as we have never checked
this before?

thanks,

greg k-h
Benjamin Gaignard March 16, 2018, 8:53 a.m. UTC | #2
2018-03-15 18:10 GMT+01:00 Greg KH <gregkh@linuxfoundation.org>:
> On Tue, Feb 27, 2018 at 03:09:24PM +0100, Benjamin Gaignard wrote:
>> When being notified that a driver is about to be bind a listener
>> could return NOTIFY_BAD.
>> Check the return to be sure that the driver could be bind.
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>> ---
>>  drivers/base/dd.c | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
>> index de6fd092bf2f..9275f2c0fed2 100644
>> --- a/drivers/base/dd.c
>> +++ b/drivers/base/dd.c
>> @@ -304,9 +304,12 @@ static int driver_sysfs_add(struct device *dev)
>>  {
>>       int ret;
>>
>> -     if (dev->bus)
>> -             blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
>> -                                          BUS_NOTIFY_BIND_DRIVER, dev);
>> +     if (dev->bus) {
>> +             if (blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
>> +                                              BUS_NOTIFY_BIND_DRIVER, dev) ==
>> +                                              NOTIFY_BAD)
>> +                     return -EINVAL;
>
> checkpatch does not complain about this?

No (even with --strict), it should be indented with tabs

>
> And what is going to break when we enable this, as we have never checked
> this before?

I could have miss some occurences but when greping with BUS_NOTIFY_*
patern I haven't any problematic cases.
When notifiers don't care of the message they almost all return
NOTIFY_DONE, some return NOTIFY_OK but none
return NOTIFY_BAD. That I wrote the test like "== NOTIFY_BAD" and not
"!= NOTIFY_OK".

I have checked this list of files (I hope I haven't forgot any occurence)
arch/powerpc/kernel/isa-bridge.c
arch/powerpc/kernel/iommu.c
arch/powerpc/kernel/dma-swiotlb.c
arch/powerpc/platforms/pasemi/setup.c
arch/powerpc/platforms/512x/pdm360ng.c
arch/powerpc/platforms/cell/iommu.c
arch/arm/mach-mvebu/coherency.c
arch/arm/common/sa1111.c
arch/arm/mach-keystone/keystone.c -> this one return NOTIFY_BAD if dev
is NULL which is never the case.
arch/arm/mach-highbank/highbank.c
arch/arm/mach-shmobile/regulator-quirk-rcar-gen2.c
arch/arm/mach-omap2/omap_device.c
drivers/base/power/clock_ops.c
drivers/platform/x86/silead_dmi.c
drivers/input/serio/i8042.c
drivers/input/mouse/psmouse-smbus.c
drivers/w1/w1.c
drivers/media/platform/soc_camera/sh_mobile_ceu_camera.c
drivers/i2c/i2c-dev.c drivers/acpi/acpi_lpss.c
drivers/gpu/vga/vgaarb.c
drivers/xen/pci.c drivers/xen/xen-pciback/pci_stub.c
drivers/xen/arm-device.c
drivers/iommu/s390-iommu.c
drivers/iommu/iommu.c
drivers/iommu/dmar.c
drivers/iommu/intel-iommu.c
drivers/usb/core/usb.c
drivers/s390/cio/ccwgroup.c
drivers/net/ethernet/ibm/emac/core.c

Benjamin

>
> thanks,
>
> greg k-h
diff mbox

Patch

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index de6fd092bf2f..9275f2c0fed2 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -304,9 +304,12 @@  static int driver_sysfs_add(struct device *dev)
 {
 	int ret;
 
-	if (dev->bus)
-		blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
-					     BUS_NOTIFY_BIND_DRIVER, dev);
+	if (dev->bus) {
+		if (blocking_notifier_call_chain(&dev->bus->p->bus_notifier,
+						 BUS_NOTIFY_BIND_DRIVER, dev) ==
+						 NOTIFY_BAD)
+			return -EINVAL;
+	}
 
 	ret = sysfs_create_link(&dev->driver->p->kobj, &dev->kobj,
 				kobject_name(&dev->kobj));