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] [day] [month] [year] [list]
Message-ID: <20230528182543.656da0d1@jic23-huawei>
Date: Sun, 28 May 2023 18:25:43 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Matti Vaittinen <mazziesaccount@...il.com>
Cc: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>, Andy Shevchenko
 <andriy.shevchenko@...ux.intel.com>, Daniel Scally <djrscally@...il.com>,
 Heikki Krogerus <heikki.krogerus@...ux.intel.com>, Sakari Ailus
 <sakari.ailus@...ux.intel.com>, Greg Kroah-Hartman
 <gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>,
 Wolfram Sang <wsa@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>,
 Michael Hennerich <Michael.Hennerich@...log.com>, Andreas Klinger
 <ak@...klinger.de>, Marcin Wojtas <mw@...ihalf.com>, Russell King
 <linux@...linux.org.uk>, "David S. Miller" <davem@...emloft.net>, Eric
 Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo
 Abeni <pabeni@...hat.com>, Jonathan Neuschäfer
 <j.neuschaefer@....net>, Linus Walleij <linus.walleij@...aro.org>, Paul
 Cercueil <paul@...pouillou.net>, Akhil R <akhilrajeev@...dia.com>,
 linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-i2c@...r.kernel.org, linux-iio@...r.kernel.org,
 netdev@...r.kernel.org, openbmc@...ts.ozlabs.org,
 linux-gpio@...r.kernel.org, linux-mips@...r.kernel.org
Subject: Re: [PATCH v6 1/8] drivers: fwnode: fix fwnode_irq_get[_byname]()

On Fri, 26 May 2023 09:35:30 +0300
Matti Vaittinen <mazziesaccount@...il.com> wrote:

> The fwnode_irq_get() and the fwnode_irq_get_byname() return 0 upon
> device-tree IRQ mapping failure. This is contradicting the
> fwnode_irq_get_byname() function documentation and can potentially be a
> source of errors like:
> 
> int probe(...) {
> 	...
> 
> 	irq = fwnode_irq_get_byname();
> 	if (irq <= 0)
> 		return irq;
> 
> 	...
> }
> 
> Here we do correctly check the return value from fwnode_irq_get_byname()
> but the driver probe will now return success. (There was already one
> such user in-tree).
> 
> Change the fwnode_irq_get_byname() to work as documented and make also the
> fwnode_irq_get() follow same common convention returning a negative errno
> upon failure.
> 
> Fixes: ca0acb511c21 ("device property: Add fwnode_irq_get_byname")
> Suggested-by: Sakari Ailus <sakari.ailus@...ux.intel.com>
> Suggested-by: Jonathan Cameron <jic23@...nel.org>
> Signed-off-by: Matti Vaittinen <mazziesaccount@...il.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@...il.com>
> 
This bothers me a little because there may be drivers that haven't been
caught yet that assume the zero value.   Still this is more consistent
with what I'd expect to happen, so fair enough

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>


> ---
> I dropped the existing reviewed-by tags because change to
> fwnode_irq_get() was added.
> 
> Revision history:
> v4 =>:
>  - No Changes
> v3 => v4:
>  - Change also the fwnode_irq_get()
> ---
>  drivers/base/property.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/base/property.c b/drivers/base/property.c
> index f6117ec9805c..8c40abed7852 100644
> --- a/drivers/base/property.c
> +++ b/drivers/base/property.c
> @@ -987,12 +987,18 @@ EXPORT_SYMBOL(fwnode_iomap);
>   * @fwnode:	Pointer to the firmware node
>   * @index:	Zero-based index of the IRQ
>   *
> - * Return: Linux IRQ number on success. Other values are determined
> - * according to acpi_irq_get() or of_irq_get() operation.
> + * Return: Linux IRQ number on success. Negative errno on failure.
>   */
>  int fwnode_irq_get(const struct fwnode_handle *fwnode, unsigned int index)
>  {
> -	return fwnode_call_int_op(fwnode, irq_get, index);
> +	int ret;
> +
> +	ret = fwnode_call_int_op(fwnode, irq_get, index);
> +	/* We treat mapping errors as invalid case */
> +	if (ret == 0)
> +		return -EINVAL;
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL(fwnode_irq_get);
>  


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ