linux-kernel - Re: [PATCH v1] drivers: make struct device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 10 Nov 2020 21:31:06 +0100
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>
Cc:     kernel@...gutronix.de, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] drivers: make struct device_driver::remove return void

On Tue, Nov 10, 2020 at 04:07:23PM +0100, Uwe Kleine-König wrote:
> The driver core doesn't check the return value of the remove callback
> because there is only little software can do when hardware disappears.
> 
> So change the callback to not return a value at all and adapt all users.
> The motivation for this change is that some driver authors have a
> misconception about failures in the remove callback. Making it void
> makes it pretty obvious that there is no error handling to be expected.
> 
> Most drivers were easy to convert as they returned 0 unconditionally, I
> added a warning to code paths that returned an error code (that was
> ignored already before).
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@...gutronix.de>
> ---
> Hello,
> 
> I expect that there are still a few driver that need adaption as I only
> build tested allmodconfig on ARM and amd64.
> 
> Best regards
> Uwe
> 
>  drivers/acpi/processor_driver.c         | 10 ++++------
>  drivers/amba/bus.c                      |  7 ++++---
>  drivers/base/platform.c                 | 13 +++++++------
>  drivers/bus/fsl-mc/fsl-mc-bus.c         |  7 ++-----
>  drivers/bus/mhi/core/init.c             |  6 ++----
>  drivers/char/hw_random/optee-rng.c      |  4 +---
>  drivers/char/tpm/tpm_ftpm_tee.c         |  8 ++++----
>  drivers/firmware/broadcom/tee_bnxt_fw.c |  4 +---
>  drivers/fsi/fsi-master-hub.c            |  4 +---
>  drivers/fsi/fsi-sbefifo.c               |  4 +---
>  drivers/fsi/fsi-scom.c                  |  4 +---
>  drivers/gpu/drm/drm_mipi_dsi.c          |  7 +++++--
>  drivers/gpu/host1x/bus.c                | 11 +++++++----
>  drivers/greybus/core.c                  |  4 +---
>  drivers/hsi/clients/cmt_speech.c        |  4 +---
>  drivers/hsi/clients/hsi_char.c          |  4 +---
>  drivers/hsi/clients/nokia-modem.c       |  4 +---
>  drivers/hsi/clients/ssi_protocol.c      |  4 +---
>  drivers/i2c/busses/i2c-fsi.c            |  4 +---
>  drivers/input/rmi4/rmi_bus.c            |  4 +---
>  drivers/input/rmi4/rmi_driver.c         |  4 +---
>  drivers/input/touchscreen/wm97xx-core.c |  7 +++----
>  drivers/mfd/ucb1400_core.c              |  3 +--
>  drivers/net/ethernet/3com/3c509.c       |  5 ++---
>  drivers/net/ethernet/3com/3c59x.c       |  3 +--
>  drivers/net/ethernet/dec/tulip/de4x5.c  |  4 +---
>  drivers/net/fddi/defxx.c                |  5 ++---
>  drivers/net/phy/mdio_device.c           |  4 +---
>  drivers/net/phy/phy_device.c            |  4 +---
>  drivers/pci/pcie/portdrv_core.c         |  5 ++---
>  drivers/scsi/advansys.c                 |  3 +--
>  drivers/scsi/aha1740.c                  |  4 +---
>  drivers/scsi/aic7xxx/aic7770_osm.c      |  3 +--
>  drivers/scsi/ch.c                       |  3 +--
>  drivers/scsi/sd.c                       |  6 ++----
>  drivers/scsi/ses.c                      |  3 +--
>  drivers/scsi/sim710.c                   |  3 +--
>  drivers/scsi/sr.c                       |  6 ++----
>  drivers/scsi/st.c                       |  5 ++---
>  drivers/siox/siox-core.c                |  6 ++++--
>  drivers/soundwire/bus_type.c            | 13 +++++++------
>  drivers/spi/spi.c                       |  8 +++++---
>  drivers/usb/core/driver.c               |  7 ++-----
>  drivers/visorbus/visorbus_main.c        |  5 +----
>  include/linux/device/driver.h           |  2 +-
>  sound/core/seq/oss/seq_oss_synth.c      |  6 ++----
>  sound/core/seq/oss/seq_oss_synth.h      |  2 +-
>  sound/core/seq/seq_midi.c               |  6 +++---
>  sound/drivers/opl3/opl3_seq.c           | 10 ++++++----
>  sound/drivers/opl4/opl4_seq.c           |  9 +++++----
>  sound/hda/ext/hdac_ext_bus.c            |  9 +++++++--
>  sound/isa/sb/emu8000_synth.c            |  5 ++---
>  sound/pci/emu10k1/emu10k1_synth.c       |  5 ++---
>  sound/pci/hda/hda_bind.c                | 11 +++++++----
>  54 files changed, 129 insertions(+), 172 deletions(-)

First off, that's a lot of drivers with a "raw" remove function, why is
anyone doing that?  It should all be wrapped up in the bus that the
drivers are on.

In digging, ugh, looks like there's some sound driver abuse here that
should be fixed up first, which will cause you to get these "for free",
and some busses should be fixed up as well.

This type of patch should only have to bus code, if things are right,
not individual drivers.  It's not your fault, but fixing that up will
make this patch easier as it will be in bisectable pieces :)

thanks,

greg k-h