linux-kernel - Re: [PATCH 2/2] scripts/gdb: lx-dmesg: Use errors=replace for decoding

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1498238944.32344.34.camel@nxp.com>
Date:   Fri, 23 Jun 2017 20:29:04 +0300
From:   Leonard Crestez <leonard.crestez@....com>
To:     Jan Kiszka <jan.kiszka@...mens.com>
CC:     Kieran Bingham <kieran@...uared.org.uk>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] scripts/gdb: lx-dmesg: Use errors=replace for
 decoding

On Fri, 2017-06-23 at 18:02 +0200, Jan Kiszka wrote:
> On 2017-06-23 16:20, Leonard Crestez wrote:
> > 
> > It is never desirable lx-dmesg to fail on string decoding errors,
> > not
> > even if the log buffer is corrupt.
> > 
> > Signed-off-by: Leonard Crestez <leonard.crestez@....com>
> > ---
> >  scripts/gdb/linux/dmesg.py | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/scripts/gdb/linux/dmesg.py
> > b/scripts/gdb/linux/dmesg.py
> > index 6f8d2b2..d0cac58 100644
> > --- a/scripts/gdb/linux/dmesg.py
> > +++ b/scripts/gdb/linux/dmesg.py
> > @@ -52,13 +52,13 @@ class LxDmesg(gdb.Command):
> >                  continue
> >  
> >              text_len = utils.read_u16(log_buf[pos + 10:pos + 12])
> > -            text = log_buf[pos + 16:pos + 16 + text_len].decode()
> > +            text = log_buf[pos + 16:pos + 16 +
> > text_len].decode(errors='replace')
> pep8 should complain.
> 
> > 
> >              time_stamp = utils.read_u64(log_buf[pos:pos + 8])
> >  
> >              for line in text.splitlines():
> >                  gdb.write("[{time:12.6f}] {line}\n".format(
> >                      time=time_stamp / 1000000000.0,
> > -                    line=line))
> > +                    line=line.encode(errors='replace')))
> You only talk about "decoding" in the commit log, but here you encode
> back. An short explanation why this is also needed would be nice.
> 
Apparently .decode(errors='replace') will return an unicode string
where invalid characters are replaced with U+FFFD REPLACEMENT
CHARACTER. Attempting to encode that back to the default ascii encoding
of python2 throws an error, using errors='replace' results in a '?'
instead.

See: https://docs.python.org/2/library/codecs.html#codec-base-classes

In python3 the default encoding seems to be utf8 and errors='replace'
is not obviously required on the encode step.

I don't actually have a gdb version compiled with python3 support and
don't know if gdb.write always properly handles unicode in all cases.
Perhaps it might be better to also explicitly specify 'utf8' as the
encoding?

Linux does occasionally print unicode, for example the jffs2 driver
shows an copyright symbol at startup. Using errors='replace' everywhere
on python2 results in this output from lx-dmesg:

[    0.367578] jffs2: version 2.2. (NAND) ?? 2001-2006 Red Hat, Inc.

In theory if we use decode('utf8', errors='replace') and encode('utf8')
then errors='replace' would not be required on the encode side.
Honestly for debug code it might be preferable to do the safest
possible thing and go 'ascii' everywhere.

--
Regards,
Leonard