linux-kernel - Re: [PATCH v1] dmaengine: dmatest: Fix dmatest waiting less when interrupted

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]

Message-ID: <87wmcslwg4.fsf@intel.com>
Date: Thu, 13 Mar 2025 09:36:11 -0700
From: Vinicius Costa Gomes <vinicius.gomes@...el.com>
To: Nathan Lynch <nathan.lynch@....com>, Vinod Koul <vkoul@...nel.org>,
 dmaengine@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: dave.jiang@...el.com, kristen.c.accardi@...el.com, kernel test robot
 <oliver.sang@...el.com>
Subject: Re: [PATCH v1] dmaengine: dmatest: Fix dmatest waiting less when
 interrupted

Nathan Lynch <nathan.lynch@....com> writes:

> Hi Vinicius,
>
> Vinicius Costa Gomes <vinicius.gomes@...el.com> writes:
>> Nathan Lynch <nathan.lynch@....com> writes:
>>> Vinicius Costa Gomes <vinicius.gomes@...el.com> writes:
>>>> Change the "wait for operation finish" logic to take interrupts into
>>>> account.
>>>>
>>>> When using dmatest with idxd DMA engine, it's possible that during
>>>> longer tests, the interrupt notifying the finish of an operation
>>>> happens during wait_event_freezable_timeout(), which causes dmatest to
>>>> cleanup all the resources, some of which might still be in use.
>>>>
>>>> This fix ensures that the wait logic correctly handles interrupts,
>>>> preventing premature cleanup of resources.
>>>>
>>>> Reported-by: kernel test robot <oliver.sang@...el.com>
>>>> Closes: https://lore.kernel.org/oe-lkp/202502171134.8c403348-lkp@intel.com
>>>
>>> Given the report at the URL above I'm struggling to follow the rationale
>>> for this change. It looks like a use-after-free in idxd while
>>> resetting/unbinding the device, and I can't see how changing whether
>>> dmatest threads perform freezeable waits would change this.
>>>
>>
>> I think that the short version is that the reproducition script triggers
>> different problems on different platforms/configurations.
>>
>> The solution I proposed fixes a problem I was seeing on a SPR system, on
>> a GNR system (that I was only able to get later) I see something more similar
>> to this particular splat (currently working on the fix).
>>
>> Seeing this question, I realize that I should have added a note to the
>> commit detailing this.
>>
>> So I am planning on proposing two (this and another) fixes for the same
>> report, hoping that it's not that confusing/unusual.
>
> I'm still confused... why is wait_event_freezable_timeout() the wrong
> API for dmatest to use, and how could changing it to
> wait_event_timeout() cause it to "take interrupts into account" that it
> didn't before?
>

My understanding (and testing) is that wait_event_timeout() will block
for the duration even in the face of interrupts, 'freezable' will not.

> AFAIK the only change made here is that dmatest threads effectively
> become unfreezeable, which is contrary to prior authors' intentions:
>
> commit 981ed70d8e4f ("dmatest: make dmatest threads freezable")
> commit adfa543e7314 ("dmatest: don't use set_freezable_with_signal()")
>

Cheers,
-- 
Vinicius