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: <18761e12-822a-16b4-fdc5-0ac889b1693c@intel.com>
Date: Sat, 10 Feb 2024 17:49:34 -0800
From: Fenghua Yu <fenghua.yu@...el.com>
To: Dave Jiang <dave.jiang@...el.com>, Vinod Koul <vkoul@...nel.org>
CC: <dmaengine@...r.kernel.org>, linux-kernel <linux-kernel@...r.kernel.org>,
	Lingyan Guo <lingyan.guo@...el.com>
Subject: Re: [PATCH] dmaengine: idxd: Clear Event Log head in idxd upon
 completion of the Enable Device command

Hi, Dave,

On 2/9/24 12:17, Dave Jiang wrote:
> 
> 
> On 2/9/24 12:18 PM, Fenghua Yu wrote:
>> If Event Log is supported, upon completion of the Enable Device command,
>> the Event Log head in the variable idxd->evl->head should be cleared to
>> match the state of the EVLSTATUS register. But the variable is not reset
>> currently, leading mismatch of the variable and the register state.
>> The mismatch causes incorrect processing of Event Log entries.
>>
>> Fix the issue by clearing the variable after completion of the command.
> 
> Should this be done in idxd_device_clear_state() instead?

If clear evl->head in idxd_device_clear_state(), evl->head still 
mismatches head in EVLSTATUS in some cases.

For exmample, when a few event log entries are logged and then device is 
disabled, head in EVLSTATUS is still a valid non-zero value. Clearing 
evl->head in idxd_device_clear_state() when disabling device makes 
evl->head and head in EVLSTATUS mismatched.

I haven't thought a failure test case when they mismatch in these cases 
though.

But while thinking evl->head more, I wonder why is it even needed?

head of event log can always be read from EVLSTATUS instead of from its 
shadow evl->head. And reading head from EVLSTATUS won't degrade 
performance because tail is always read from EVLSTATUS whenever head is 
read (no matter from evl->head or from EVLSATUS).

To avoid any mismatch issue/trouble, I think the right fix is to remove 
head definition in struct idxd_evl and always read head from EVLSTATUS.

Do you think this is the right fix?

Thanks.

-Fenghua

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ