linux-kernel - Re: [PATCH 7/7] ASoC: sh: FSI:: don't check platform_get

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20091217094723.GB19548@pengutronix.de>
Date:	Thu, 17 Dec 2009 10:47:23 +0100
From:	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>
To:	Kuninori Morimoto <morimoto.kuninori@...esas.com>
Cc:	linux-kernel@...r.kernel.org, David Vrabel <dvrabel@...om.com>,
	Greg Kroah-Hartman <gregkh@...e.de>,
	Liam Girdwood <lrg@...mlogic.co.uk>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Jaroslav Kysela <perex@...ex.cz>, Takashi Iwai <tiwai@...e.de>,
	Paul Mundt <lethal@...ux-sh.org>, alsa-devel@...a-project.org
Subject: Re: [PATCH 7/7] ASoC: sh: FSI:: don't check platform_get_irq's
	return value against zero

On Thu, Dec 17, 2009 at 10:42:25AM +0900, Kuninori Morimoto wrote:
> 
> Dear Uwe
> 
> > platform_get_irq returns -ENXIO on failure, so !irq was probably
> > always true.  Better use (int)irq <= 0.  Note that a return value of
> > zero is still handled as error even though this could mean irq0.
> (snip)
> > -	if (!res || !irq) {
> > +	if (!res || (int)irq <= 0) {
> 
> Ohh..
> Thank you for checking.
> 
> Hmm..  now I tried to check about platform_get_irq in Linux kernel.
> In my easy check, I can find a lot of drivers which are...
> 
> o doesn't check irq value          ex) request_irq(platform_get_irq(...))
This is probably worth fixing.

> o checked irq but it have miss (?) ex) if (irq >= 0) OK
>                                        if (irq) OK
>                                        if (irq == -Exxx ) NG
For some drivers if (irq >= 0) is OK.  if (irq) is wrong.  if (irq ==
-Exxx ) IMHO isn't carefully enough.

> o checked irq but don't care zero  ex) if (irq < 0) NG
Same as if (irq >= 0)

> ---- it use !irq  ------------------------
> 
>  arch/avr32/mach-at32ap/extint.c :: 200
>  drivers/media/video/mx1_camera.c :: 651
>  drivers/media/video/sh_mobile_ceu_camera.c :: 1655
>  drivers/misc/atmel-ssc.c :: 110
>  drivers/net/can/at91_can.c :: 1071
>  drivers/net/sni_82596.c :: 112
>  drivers/pcmcia/bfin_cf_pcmcia.c :: 207
>  drivers/spi/spi_imx.c :: 553
>  drivers/spi/spi_mpc8xxx.c :: 885
>  drivers/usb/gadget/fsl_udc_core.c :: 2303
Most of these are fixed in this series.  I seem to have missed

	drivers/misc/atmel-ssc.c
	drivers/net/sni_82596.c
	drivers/usb/gadget/fsl_udc_core.c

and I found sound/soc/sh/fsi.c and drivers/spi/spi_imx.c should already
be fixed in Linus' Tree.

I wait to see the result for the already existing patches.  If they are
taken I might look for the others, too.  If you want to prepare some
more patches, feel free to do it.

Best regards
Uwe

-- 
Pengutronix e.K.                              | Uwe Kleine-König            |
Industrial Linux Solutions                    | http://www.pengutronix.de/  |
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/