linux-kernel - Re: [PATCH] x86/amd_nb: Check for invalid SMN reads

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: <864f265d-08d3-88d8-c0c6-263ab0e10813@amd.com>
Date:   Mon, 8 May 2023 18:01:24 -0400
From:   Yazen Ghannam <yazen.ghannam@....com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     yazen.ghannam@....com, x86@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86/amd_nb: Check for invalid SMN reads

On 4/5/23 3:10 PM, Yazen Ghannam wrote:
> On 4/5/23 14:32, Yazen Ghannam wrote:
>> On 4/5/23 14:06, Borislav Petkov wrote:
>>> On Mon, Apr 03, 2023 at 05:36:40PM -0400, Yazen Ghannam wrote:
>>>> Yes, that's fine. Should I send another revision?
>>>
>>> On a second thought, I think we should do what you said in the write
>>> function too. Because the write can fail too. So if it can, we need to
>>> handle that potential error too.
>>>
>>> Care to send a new version which does this check in the read and in the
>>> write function? Basically what you had initially but with the write side
>>> check added too to amd_smn_write.
>>>
>>
>> Sure thing. I don't have a real test for the write path. But I'll test by
>> faking it.
>>
> 
> So I thought about it for a bit and quickly realized the "write and read back"
> method isn't robust when done here.
> 
> Possible issues:
> 1) Bits that are "Write-1-to-clear". In this case, we *don't* want the read to
> match the write.
> 2) Bits that are "Read-as-Zero"/"Writes-Ignored". We can't know this
> information here.
> 3) Bits that are "Reserved / Set to 1". Ditto above.
> 
> I think all these issues should be handled by the callers of amd_smn_write().
> They should do the "write and read back" check themselves, if needed.
> 
> For #1, they can see if their target bits got cleared.
> 
> For #2 and #3, they can check if their target bits got set as intended.
> 
> This matches what we do for rdmsr/wrmsr. As long as there's no #GP, then we're
> good, and the caller does their own checking.
> 
> The "PCI Error Response" for the SMN read is the only check that would apply
> to *any* SMN read. So I think that makes sense to do here instead of at each
> call site.
> 
> What do you think?
>

Hi Boris,

Just following up. What do you think about the points above? I can send
another revision for whichever way we need to go.

Thanks,
Yazen