| 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
| ||
|
Message-ID: <572377B6.9020608@bingham.xyz>
Date: Fri, 29 Apr 2016 16:03:18 +0100
From: Kieran Bingham <kieran@...uared.org.uk>
To: Dom Cote <buzdelabuz2@...il.com>
Cc: "J. Kiszka" <jan.kiszka@...mens.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Fix issue with dmesg.py and python 3.X
Hi Dom,
On 08/04/16 04:33, Dom Cote wrote:
> Hi Kieran,
>
> Thanks for the feedback.
>
> I am going to need to build a version of gdb with python 2.7 to see what
> the differences are, and try to abstract them in the best possible way.
>
> Then I will send a new patch and incorporate what you just highlighted
> in it.
I was just wondering if you had chance to look at any of this ? (I was
looking at it again myself)
> Regards
>
> Dom
>
>
> On Thu, Apr 7, 2016 at 11:02 PM, Kieran Bingham
> <kieran.bingham@...aro.org <mailto:kieran.bingham@...aro.org>> wrote:
>
> Hi Dom,
>
> I've just tested your patch quickly, and it generates an error on
> Python 2.7
>
>
> On 05/04/16 19:38, Dom Cote wrote:
> > When using GDB built with python 2.7,
> >
> > Inferior.read_memory (address, length)
> >
> > returns a buffer object. When using GDB built with python 3.X,
> > it returns a memoryview object, which cannot be added to another
> > memoryview object.
> >
> > Replace the addition (+) of 2 python memoryview objects
> > with the addition of 2 'bytes' objects.
> >
> > Create a read_memoryview() function that always return a memoryview
> > object.
>
> The change to memoryview appears to work - but I don't think it needs to
> be indirected by a function definition?
>
>
> > Change the read_u16 function so it doesn't need to use ord()
> > anymore.
>
> This change is separate to the memoryview object, and should be in it's
> own patch. One patch should fix one thing independently.
>
> For example, the change to memoryview object appears to be functional,
> and the read_u16 is not. If these changes are in separate patches, then
> working changes can be accepted sooner, and tested easier.
>
>
> > Tested with Python 3.4 and gdb 7.7
> >
> > Signed-off-by: Dom Cote <buzdelabuz2@...il.com <mailto:buzdelabuz2@...il.com>>
> > ---
> > scripts/gdb/linux/dmesg.py | 9 +++++----
> > scripts/gdb/linux/utils.py | 9 +++++++--
> > 2 files changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/scripts/gdb/linux/dmesg.py b/scripts/gdb/linux/dmesg.py
> > index 927d0d2a3145..96f4732157d8 100644
> > --- a/scripts/gdb/linux/dmesg.py
> > +++ b/scripts/gdb/linux/dmesg.py
> > @@ -33,11 +33,12 @@ class LxDmesg(gdb.Command):
> > if log_first_idx < log_next_idx:
> > log_buf_2nd_half = -1
> > length = log_next_idx - log_first_idx
> > - log_buf = inf.read_memory(start, length)
> > + log_buf = utils.read_memoryview(inf, start, length).tobytes()
>
> This looks like it could just call memoryview() directly ...
I just looked at the line sizes:
- log_buf = inf.read_memory(start, length)
+ log_buf = utils.read_memoryview(inf, start, length).tobytes()
+ log_buf = memoryview(inf.read_memory(start, length)).tobytes()
There's not a lot in it, and saving one char is probably not enough
reason to add a function call.
>
> > else:
> > log_buf_2nd_half = log_buf_len - log_first_idx
> > - log_buf = inf.read_memory(start, log_buf_2nd_half) + \
> > - inf.read_memory(log_buf_addr, log_next_idx)
> > + a = utils.read_memoryview(inf, start, log_buf_2nd_half)
> > + b = utils.read_memoryview(inf, log_buf_addr, log_next_idx)
>
> Likewise here I presume ...
>
> > + log_buf = a.tobytes() + b.tobytes()
> >
> > pos = 0
> > while pos < log_buf.__len__():
> > @@ -53,7 +54,7 @@ class LxDmesg(gdb.Command):
> > text = log_buf[pos + 16:pos + 16 + text_len]
> > time_stamp = utils.read_u64(log_buf[pos:pos + 8])
> >
> > - for line in memoryview(text).tobytes().splitlines():
> > + for line in text.splitlines():
>
> This looks like a separate change, not related to the bug fix?
> If this is an improvement, rather than a fix - it should probably also
> have it's own patch. (at first glance it looks like an improvement :D )
>
> I just haven't seen yet if it depends on the other change.
>
> > gdb.write("[{time:12.6f}] {line}\n".format(
> > time=time_stamp / 1000000000.0,
> > line=line))
> > diff --git a/scripts/gdb/linux/utils.py b/scripts/gdb/linux/utils.py
> > index 0893b326a28b..c2b779e7bd26 100644
> > --- a/scripts/gdb/linux/utils.py
> > +++ b/scripts/gdb/linux/utils.py
> > @@ -87,11 +87,16 @@ def get_target_endianness():
> > return target_endianness
> >
> >
> > +# Compat between GDB built with python 2.7 vs 3.X
> > +def read_memoryview(inf, start, length):
> > + return memoryview(inf.read_memory(start, length))
>
> This simply always returns a memoryview object, so why not just change
> the respective lines, which you have already had to modify to call/use
> memoryview directly?
>
> > +
> > +
> > def read_u16(buffer):
> > if get_target_endianness() == LITTLE_ENDIAN:
> > - return ord(buffer[0]) + (ord(buffer[1]) << 8)
> > + return buffer[0] + (buffer[1] << 8)
> > else:
> > - return ord(buffer[1]) + (ord(buffer[0]) << 8)
> > + return buffer[1] + (buffer[0] << 8)
>
> This breaks for me, but returning these lines to use ord() shows that
> the memoryview changes appear to work.
>
> What was the need for changing these lines? Does it cause a break on
> 3.x?
>
> This causes the following error on 2.7 for me:
>
> (gdb) lx-dmesg
> Traceback (most recent call last):
> File
> "/home/linuxembedded/linaro/lkd/openst-lkd/objects/arm/qemu-arm/linux/scripts/gdb/linux/dmesg.py",
> line 45, in invoke
> length = utils.read_u16(log_buf[pos + 8:pos + 10])
> File
> "/home/linuxembedded/linaro/lkd/openst-lkd/objects/arm/qemu-arm/linux/scripts/gdb/linux/utils.py",
> line 97, in read_u16
> return buffer[0] + (buffer[1] << 8)
> TypeError: unsupported operand type(s) for <<: 'str' and 'int'
> Error occurred in Python command: unsupported operand type(s) for <<:
> 'str' and 'int'
I guess we're going to have to do something along the lines of
PY2 = (sys.version_info[0] == 2)
if PY2:
# Do Py2 compatible code
else:
# Do Py3+ stuff
to support both targets. (Ugh...) But this isn't the first time I've
been asked if we support both Python 2 and Python 3 so I guess it will
become an issue.
Anyway, let me know if there's anything I can do to help / test.
--
Regards
Kieran Bingham
Powered by blists - more mailing lists