linux-kernel - Re: drivers/iio/accel/kionix-kx022a.c:1053 kx022a_probe_internal() warn: passing zero to 'dev_err

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:   Fri, 12 May 2023 08:41:07 +0300
From:   Dan Carpenter <dan.carpenter@...aro.org>
To:     Matti Vaittinen <mazziesaccount@...il.com>
Cc:     oe-kbuild@...ts.linux.dev,
        Matti Vaittinen <matti.vaittinen@...rohmeurope.com>,
        lkp@...el.com, oe-kbuild-all@...ts.linux.dev,
        linux-kernel@...r.kernel.org,
        Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Subject: Re: drivers/iio/accel/kionix-kx022a.c:1053 kx022a_probe_internal()
 warn: passing zero to 'dev_err_probe'

On Fri, May 12, 2023 at 08:16:02AM +0300, Matti Vaittinen wrote:
> > 7c1d1677b3227c Matti Vaittinen 2022-10-24  1046  	irq = fwnode_irq_get_byname(fwnode, "INT1");
> > 7c1d1677b3227c Matti Vaittinen 2022-10-24  1047  	if (irq > 0) {
> > 7c1d1677b3227c Matti Vaittinen 2022-10-24  1048  		data->inc_reg = KX022A_REG_INC1;
> > 7c1d1677b3227c Matti Vaittinen 2022-10-24  1049  		data->ien_reg = KX022A_REG_INC4;
> > 7c1d1677b3227c Matti Vaittinen 2022-10-24  1050  	} else {
> > 7c1d1677b3227c Matti Vaittinen 2022-10-24  1051  		irq = fwnode_irq_get_byname(fwnode, "INT2");
> > 7c1d1677b3227c Matti Vaittinen 2022-10-24  1052  		if (irq <= 0)
> >                                                                      ^^^^^^^^
> > This code assumes fwnode_irq_get_byname() can return zero.
> > 
> > 7c1d1677b3227c Matti Vaittinen 2022-10-24 @1053  			return dev_err_probe(dev, irq, "No suitable IRQ\n");
> > 
> > But fortunately, it can't otherwise this would return success.
> > 
> 
> Ouch. Actually, the fwnode_irq_get_byname() can return zero on device-tree
> mapping error. I did actually send a patch to address this some time ago:
> https://lore.kernel.org/lkml/cover.1666710197.git.mazziesaccount@gmail.com/
> 

Ah.  I just went by the documentation instead of looking at the code.

Originally a bunch of irq functions return NO_IRQ on error which was a
design mistake...  irq_of_parse_and_map() still returns zero on error
instead of negative error codes.  I wrote a check for that yesterday.

drivers/gpu/drm/msm/dsi/dsi_host.c:1949 msm_dsi_host_init() warn: irq_of_parse_and_map() returns zero on failure
drivers/dma/ti/edma.c:2405 edma_probe() warn: irq_of_parse_and_map() returns zero on failure
drivers/dma/ti/edma.c:2421 edma_probe() warn: irq_of_parse_and_map() returns zero on failure
drivers/net/ethernet/xilinx/ll_temac_main.c:1570 temac_probe() warn: irq_of_parse_and_map() returns zero on failure
drivers/net/ethernet/xilinx/ll_temac_main.c:1573 temac_probe() warn: irq_of_parse_and_map() returns zero on failure
drivers/ata/sata_mv.c:4094 mv_platform_probe() warn: irq_of_parse_and_map() returns zero on failure

It would be good if we could apply your patch, otherwise I could create
an explicit check for fwnode_irq_get_byname() returns.

Ideally, everything would just return negative error codes on error.
It's harder to change irq_of_parse_and_map() because it doesn't return a
mix of errors and zero, but just zero.  We would have to rename it to
something else, otherwise backports would get messed up.

regards,
dan carpenter