Re: [PATCH] x86/debug: fix page-overflow bug in dbg_rw_guest_mem

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.02.2021 12:44, Andrew Cooper wrote:
> On 01/02/2021 09:37, Jan Beulich wrote:
>> On 30.01.2021 03:59, Andrew Cooper wrote:
>>> On 30/01/2021 01:59, Tamas K Lengyel wrote:
>>>> When using gdbsx dbg_rw_guest_mem is used to read/write guest memory. When 
>>>> the
>>>> buffer being accessed is on a page-boundary, the next page needs to be 
>>>> grabbed
>>>> to access the correct memory for the buffer's overflown parts. While
>>>> dbg_rw_guest_mem has logic to handle that, it broke with 229492e210a. 
>>>> Instead
>>>> of grabbing the next page the code right now is looping back to the
>>>> start of the first page. This results in errors like the following while 
>>>> trying
>>>> to use gdb with Linux' lx-dmesg:
>>>>
>>>> [    0.114457] PM: hibernation: Registered nosave memory: [mem
>>>> 0xfdfff000-0xffffffff]
>>>> [    0.114460] [mem 0x90000000-0xfbffffff] available for PCI demem 0
>>>> [    0.114462] f]f]
>>>> Python Exception <class 'ValueError'> embedded null character:
>>>> Error occurred in Python: embedded null character
>>>>
>>>> Fixing this bug by taking the variable assignment outside the loop.
>>>>
>>>> Signed-off-by: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
>>> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> I have to admit that I'm irritated: On January 14th I did submit
>> a patch ('x86/gdbsx: convert "user" to "guest" accesses') fixing this
>> as a side effect. I understand that one was taking care of more
>> issues here, but shouldn't that be preferred? Re-basing isn't going
>> to be overly difficult, but anyway.
> 
> I'm sorry.  That was sent during the period where I had no email access
> (hence I wasn't aware of it - I've been focusing on 4.15 work and this
> series wasn't pinged.),

Oh, so you had actually lost emails, rather than (as I did
understand so far) only getting them in a very delayed fashion?

Anyway, the first part of that series having been pretty close
to getting an XSA, I thought you were well aware that at least
that part is very clearly intended for 4.15. (I also did
mention it to you last week on irc, when you asked what wants
specifically looking at for 4.15.) Plus, besides the bringing
up of the topic on the last two or three community calls, over
all of January I've specifically avoided pinging _any_ of the
many patches I have pending, to avoid giving you the feel of
even more pressure.

> but it also isn't identified as a bugfix, or
> suitable for backporting in that form.
> 
> I apologise for the extra work caused unintentionally, but I think this
> is the correct way around WRT backports, is it not?

It didn't occur to me that there could be a consideration of
backporting here. But yes, if so wanted, maybe the split is
helpful. Otoh then the full change could as well be taken,
to stop the abuse of "user" accesses also in the stable trees.

Jan