linux-kernel - Re: [PATCH] staging: vchiq_arm: Fix missing refcount decrement in error path for fw

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: <8c0bbde9-aba9-433f-b36b-2d467f6a1b66@gmail.com>
Date: Mon, 14 Oct 2024 10:15:25 +0200
From: Javier Carrasco <javier.carrasco.cruz@...il.com>
To: Dan Carpenter <dan.carpenter@...aro.org>
Cc: Florian Fainelli <florian.fainelli@...adcom.com>,
 Broadcom internal kernel review list
 <bcm-kernel-feedback-list@...adcom.com>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Stefan Wahren <wahrenst@....net>, Umang Jain <umang.jain@...asonboard.com>,
 Laurent Pinchart <laurent.pinchart@...asonboard.com>,
 linux-rpi-kernel@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org,
 linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org,
 stable@...r.kernel.org
Subject: Re: [PATCH] staging: vchiq_arm: Fix missing refcount decrement in
 error path for fw_node

On 14/10/2024 10:12, Dan Carpenter wrote:
> On Mon, Oct 14, 2024 at 09:59:49AM +0200, Javier Carrasco wrote:
>> This approach is great as long as the maintainer accepts mid-scope
>> variable declaration and the goto instructions get refactored, as stated
>> in cleanup.h.
>>
>> The first point is not being that problematic so far, but the second one
>> is trickier, and we all have to take special care to avoid such issues,
>> even if they don't look dangerous in the current code, because adding a
>> goto where there cleanup attribute is already used can be overlooked as
>> well.
>>
> 
> To be honest, I don't really understand this paragraph.  I think maybe you're
> talking about if we declare the variable at the top and forget to initialize it
> to NULL?  It leads to an uninitialized variable if we exit the function before
> it is initialized.
> 

No, I am talking about declaring the variable mid-scope, and later on
adding a goto before that declaration in a different patch, let's say
far above the variable declaration. As soon as a goto is added, care
must be taken to make sure that we don't have variables with the cleanup
attribute in the scope. Just something to take into account.

>> Actually there are goto instructions in the function, but at least in
>> their current form they are as harmless as useless.
> 
> Yep.  Feel free to delete them.
> 
> regards,
> dan carpenter
>