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
| ||
|
Message-ID: <d6f8aa80-47f1-ccf2-8c85-6839f13058fb@maciej.szmigiero.name> Date: Wed, 27 Dec 2017 19:42:21 +0100 From: "Maciej S. Szmigiero" <mail@...iej.szmigiero.name> To: William Breathitt Gray <vilhelm.gray@...il.com> Cc: Linus Walleij <linus.walleij@...aro.org>, Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, Jonathan Cameron <jic23@...nel.org>, linux-kernel <linux-kernel@...r.kernel.org>, linux-gpio@...r.kernel.org Subject: Re: [PATCH v2] gpio: winbond: add driver On 27.12.2017 01:24, William Breathitt Gray wrote: > On Mon, Dec 25, 2017 at 03:48:16PM +0100, Maciej S. Szmigiero wrote: (..) >> All the existing ISA bus drivers seem to depend on CONFIG_ISA_BUS_API >> instead of selecting it but IMHO this is wrong because: >> 1) This Kconfig option doesn't really enable or disable any bus support >> but building of a library of some common boilerplate code. >> Libraries are normally selected by drivers needing them and only provided >> as an user-selectable option if there is a possibility that a out-of-tree >> module would need it, >> >> 2) On x86_64 this option (or rather, its parent option CONFIG_ISA_BUS) >> cannot be enabled without CONFIG_EXPERT, >> >> 3) This device isn't really a ISA bus device any more than, for example, >> a 8250 serial port or a PC-style parallel port and these don't need >> that an user explicitly enables "ISA bus support" in his kernel >> configuration. > > I can see what you mean about selecting ISA_BUS_API rather than having > it as a dependency for drivers. Part of the reason I added the > CONFIG_EXPERT dependency for CONFIG_ISA_BUS -- as well as having > CONFIG_ISA_BUS_API be a dependency for the drivers themselves -- was to > hide the ISA-style drivers which blindly poke at I/O port addresses, > lest a niave user enable all available drivers and unintentionally brick > their system when the drivers execute. > > I think there is still merit in masking dangerous drivers such as this, > since the expected behavior nowadays is for the driver to probe for the > device before poking at memory; since ISA-style communication lacks a > standard method of detecting devices in hardware, these devices > generally pose a danger when loaded by niave users. This driver accesses the same Super I/O chip as w83627ehf hwmon and w83627hf_wdt watchdog drivers. In addition to this, there are loads of other hwmon and watchdog drivers for x86 Super I/Os in the tree, most of them using the same probing and communication style. There are even existing GPIO drivers for some Super I/Os like gpio-it87 and gpio-f7188x. None of these drivers need CONFIG_EXPERT to be selected. Also, CONFIG_EXPERT is described as "Configure standard kernel features" and that "[it] allows certain base kernel options and settings to be disabled or tweaked" for "specialized environments". Enabling this driver is not about changing "standard kernel feature" or a "base kernel option [or] setting". > However, I think CONFIG_EXPERT by itself is sufficient enough masking > to help prevent niave users from enabling these drivers on a whim. > Linus and Jonathan, do you have any objections if I replace the > ISA_BUS_API dependencies on my drivers to respective select lines? I > think this would have the benefit of resolving the Kconfig recursive > dependency issue too. > >> >> To be clear I'm fine with converting this driver to use the ISA bus (in >> fact, I have already done so), but I think that currently this would be >> a regression from user-friendliness perspective due to the points above. > > Regardless of the Kconfig decisions we make, continue to utilize the ISA > bus driver in your code as this API is the correct one to use for your > LPC devices. Kconfig improvements can be made later on, separate from > the driver code, so there's no need to let that be a deciding factor in > getting the driver itself right -- although I do agree that having a > Kconfig setup that does not appeal solely to the masochists is an > important end goal. ;) I'm fine with using the ISA bus for this driver, however I cannot submit a driver for inclusion that causes an error during kernel configuration, while, on the other hand, I think (for the reasons described above) that it shouldn't be dependent on the CONFIG_EXPERT option. > > William Breathitt Gray Maciej Szmigiero
Powered by blists - more mailing lists