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

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Mon, Feb 1, 2021 at 7:29 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> 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.

IMHO this should be backported cause it breaks use of gdbsx for all
affected releases.

Tamas