linux-kernel - Re: [PATCH 7/7] arch/unicore32/kernel/dma.c: ensure arguments to request_irq and free

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: <alpine.DEB.2.02.1203130921250.1974@localhost6.localdomain6>
Date:	Tue, 13 Mar 2012 09:23:41 +0100 (CET)
From:	Julia Lawall <julia.lawall@...6.fr>
To:	Guan Xuetao <gxt@...c.pku.edu.cn>
cc:	Julia Lawall <julia.lawall@...6.fr>,
	kernel-janitors@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 7/7] arch/unicore32/kernel/dma.c: ensure arguments to
 request_irq and free_irq are compatible

On Tue, 13 Mar 2012, Guan Xuetao wrote:

> On Mon, 2012-03-12 at 06:27 +0100, Julia Lawall wrote:
>> On Mon, 12 Mar 2012, Guan Xuetao wrote:
>>
>>> On Sun, 2012-03-11 at 20:36 +0100, Julia Lawall wrote:
>>>> From: Julia Lawall <Julia.Lawall@...6.fr>
>>>>
>>>> Convert calls to free_irq so that the second argument is the same as the
>>>> last argument of the corresponding call to request_irq, rather than the
>>>> second to last.  Without this property, free_irq does nothing.
>>>>
>>>> Signed-off-by: Julia Lawall <Julia.Lawall@...6.fr>
>>>>
>>>> ---
>>>>  arch/unicore32/kernel/dma.c |    2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/unicore32/kernel/dma.c b/arch/unicore32/kernel/dma.c
>>>> index ae441bc..c813fec 100644
>>>> --- a/arch/unicore32/kernel/dma.c
>>>> +++ b/arch/unicore32/kernel/dma.c
>>>> @@ -173,7 +173,7 @@ int __init puv3_init_dma(void)
>>>>  	ret = request_irq(IRQ_DMAERR, dma_err_handler, 0, "DMAERR", NULL);
>>>>  	if (ret) {
>>>>  		printk(KERN_CRIT "Can't register IRQ for DMAERR\n");
>>>> -		free_irq(IRQ_DMA, "DMA");
>>>> +		free_irq(IRQ_DMA, NULL);
>>>>  		return ret;
>>>>  	}
>>>>
>>> Yeah, it's an obvious mistake. Thanks.
>>> Because the dma device is just located inside PKUnity-3 SoC, and
>>> request_irq() should always return 0, I prefer to remove this free_irq()
>>> line.
>>
>> Remove the whole if test I guess.  Is there a nce way to indicate that the
>> return value is not needed (eg for the benefit of future bug finding
>> rules)?
>>
>> julia
> In this case, removing the line containing free_irq() is well enough,
> because IRQ_DMA can work even when IRQ_DMAERR doesn't work. And we need
> printk and error return value to get potential logical bug information.

I'm not completely sure to understand.  The point is that the first 
request_irq can never fail, so we don't need to clean up when the second 
one fails?  Because the lack of cleaning up will not cause the first one 
to fail the next time?  free_irq removes an action from a list and does a 
module_put.  Are these operations both not needed?

thanks,julia
--
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/