| Message ID | caba05850df644814d75d5de0574c62ce90e8789.1611971959.git.tamas@tklengyel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/debug: fix page-overflow bug in dbg_rw_guest_mem | expand |
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@tklengyel.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> Sorry for breaking this...
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@tklengyel.com> > > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> 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. Jan
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@tklengyel.com> >> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> > 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.), 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? ~Andrew
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@tklengyel.com> >>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> >> 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
On Mon, Feb 1, 2021 at 7:29 AM Jan Beulich <jbeulich@suse.com> 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@tklengyel.com> > >>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> > >> 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
diff --git a/xen/arch/x86/debug.c b/xen/arch/x86/debug.c index 4356039ed2..f32d4b0bcc 100644 --- a/xen/arch/x86/debug.c +++ b/xen/arch/x86/debug.c @@ -112,10 +112,11 @@ static unsigned int dbg_rw_guest_mem(struct domain *dp, void * __user gaddr, void * __user buf, unsigned int len, bool toaddr, uint64_t pgd3) { + unsigned long addr = (unsigned long)gaddr; + while ( len > 0 ) { char *va; - unsigned long addr = (unsigned long)gaddr; mfn_t mfn; gfn_t gfn = INVALID_GFN; unsigned long pagecnt;
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@tklengyel.com> --- xen/arch/x86/debug.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)