Messages in this thread | | | Date | Mon, 8 May 2023 18:01:24 -0400 | Subject | Re: [PATCH] x86/amd_nb: Check for invalid SMN reads | From | Yazen Ghannam <> |
| |
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
| |