Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <24d27380-544e-66d1-1cb2-14eb87ce89ac@alu.unizg.hr>
Date:   Tue, 1 Aug 2023 11:57:19 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH v1 1/1] test_firmware: prevent race conditions by a
 correct implementation of locking
From:   Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
To:     Luis Chamberlain <mcgrof@kernel.org>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        linux-kernel@vger.kernel.org,
        Russ Weight <russell.h.weight@intel.com>,
        Takashi Iwai <tiwai@suse.de>,
        Tianfei Zhang <tianfei.zhang@intel.com>,
        Shuah Khan <shuah@kernel.org>,
        Colin Ian King <colin.i.king@gmail.com>,
        Randy Dunlap <rdunlap@infradead.org>,
        linux-kselftest@vger.kernel.org, stable@vger.kernel.org,
        Dan Carpenter <error27@gmail.com>
References: <20230731165018.8233-1-mirsad.todorovac@alu.unizg.hr>
 <ZMfvAhOfSP5UXN6l@bombadil.infradead.org>
 <0e3a740f-60dd-e657-8a5c-79b155fa62b3@alu.unizg.hr>
Content-Language: en-US, hr
In-Reply-To: <0e3a740f-60dd-e657-8a5c-79b155fa62b3@alu.unizg.hr>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 8/1/23 10:24, Mirsad Todorovac wrote:
> On 7/31/23 19:27, Luis Chamberlain wrote:
>> On Mon, Jul 31, 2023 at 06:50:19PM +0200, Mirsad Todorovac wrote:
>>> NOTE: This patch is tested against 5.4 stable
>>>
>>> NOTE: This is a patch for the 5.4 stable branch, not for the torvalds tree.
>>>
>>>        The torvalds tree, and stable tree 5.10, 5.15, 6.1 and 6.4 branches
>>>        were fixed in the separate
>>>        commit ID 4acfe3dfde68 ("test_firmware: prevent race conditions by a correct implementation of locking")
>>>        which was incompatible with 5.4
>>>
>>
>> The above part is not part of the original commit, you also forgot to
>> mention the upstream commit:
>>
>> [ Upstream commit 4acfe3dfde685a5a9eaec5555351918e2d7266a1 ]
> 
> Will fix. Actually, I wasn't sure if it was required, because this backported patch
> isn't verbatim equal to commit 4acfe3dfde685a5a9eaec5555351918e2d7266a1 .
> 
> Though they are cousins, addressing the same issue.
> 
> There is a race to be fixed, despite not all racy functions present in the original commit c92316bf8e948.
> 
>>> Fixes: c92316bf8e948 ("test_firmware: add batched firmware tests")
>>> Cc: Luis R. Rodriguez <mcgrof@kernel.org>
>>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> Cc: Russ Weight <russell.h.weight@intel.com>
>>> Cc: Takashi Iwai <tiwai@suse.de>
>>> Cc: Tianfei Zhang <tianfei.zhang@intel.com>
>>> Cc: Shuah Khan <shuah@kernel.org>
>>> Cc: Colin Ian King <colin.i.king@gmail.com>
>>> Cc: Randy Dunlap <rdunlap@infradead.org>
>>> Cc: linux-kselftest@vger.kernel.org
>>> Cc: stable@vger.kernel.org # v5.4
>>> Suggested-by: Dan Carpenter <error27@gmail.com>
>>
>> Here you can add the above note in brackets:
>>
>> [ explain your changes here from the original commit ]
>>
>> Then, I see two commits upstream on Linus tree which are also fixes
>> but not merged on v5.4, did you want those applied too?
> 
> These seem merged in the stable 5.4?
> 
> commit 75d9e00f65cd2e0f2ce9ceeb395f821976773489 test_firmware: fix a memory leak with reqs buffer
> commit 94f3bc7e84af2f17dbfbc7afe93991c2a6f2f25e test_firmware: fix the memory leak of the allocated firmware buffer
> 
> Maybe this commit should be backported instead:
> 
> test_firmware: return ENOMEM instead of ENOSPC on failed memory allocation
> [ Upstream commit 7dae593cd226a0bca61201cf85ceb9335cf63682 ]
> 
> It was also merged into 6.4, 6.1, 5.15 and 5.10 stable, but not on 5.4
> 
> I might also check whether the 4.19 and 4.14 are vulnerable to these memory leaks and this race
> (Yes, they are, so it might be prudent that we backport this fix.)

FYI, just checked, the patch applied w/o modifications to 4.19 and 4.14 LTS stable branches.

Mirsad

-- 
Mirsad Todorovac
Sistem inženjer
Grafički fakultet | Akademija likovnih umjetnosti
Sveučilište u Zagrebu

System engineer
Faculty of Graphic Arts | Academy of Fine Arts
University of Zagreb, Republic of Croatia