linux-kernel - RE: [PATCH] soc: fsl: qe: Replace one-element array and use struct

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 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]

Date:   Mon, 25 May 2020 02:47:23 +0000
From:   Qiang Zhao <qiang.zhao@....com>
To:     Leo Li <leoyang.li@....com>, Leo Li <leoyang.li@....com>,
        Kees Cook <keescook@...omium.org>
CC:     "Gustavo A. R. Silva" <gustavoars@...nel.org>,
        linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
        "moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE" 
        <linux-arm-kernel@...ts.infradead.org>,
        lkml <linux-kernel@...r.kernel.org>,
        "Gustavo A. R. Silva" <gustavo@...eddedor.com>
Subject: RE: [PATCH] soc: fsl: qe: Replace one-element array and use
 struct_size() helper

On Wed, May 23, 2020 at 5:22 PM Li Yang <leoyang.li@....com>
> -----Original Message-----
> From: Li Yang <leoyang.li@....com>
> Sent: 2020年5月23日 5:22
> To: Kees Cook <keescook@...omium.org>
> Cc: Gustavo A. R. Silva <gustavoars@...nel.org>; Qiang Zhao
> <qiang.zhao@....com>; linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>;
> moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
> <linux-arm-kernel@...ts.infradead.org>; lkml <linux-kernel@...r.kernel.org>;
> Gustavo A. R. Silva <gustavo@...eddedor.com>
> Subject: Re: [PATCH] soc: fsl: qe: Replace one-element array and use
> struct_size() helper
> 
> On Wed, May 20, 2020 at 10:24 PM Kees Cook <keescook@...omium.org>
> wrote:
> >
> > On Wed, May 20, 2020 at 06:52:21PM -0500, Li Yang wrote:
> > > On Mon, May 18, 2020 at 5:57 PM Kees Cook <keescook@...omium.org>
> wrote:
> > > > Hm, looking at this code, I see a few other things that need to be
> > > > fixed:
> > > >
> > > > 1) drivers/tty/serial/ucc_uart.c does not do a be32_to_cpu() conversion
> > > >    on the length test (understandably, a little-endian system has never
> run
> > > >    this code since it's ppc specific), but it's still wrong:
> > > >
> > > >         if (firmware->header.length != fw->size) {
> > > >
> > > >    compare to the firmware loader:
> > > >
> > > >         length = be32_to_cpu(hdr->length);
> > > >
> > > > 2) drivers/soc/fsl/qe/qe.c does not perform bounds checking on the
> > > >    per-microcode offsets, so the uploader might send data outside the
> > > >    firmware buffer. Perhaps:
> > >
> > > We do validate the CRC for each microcode, it is unlikely the CRC
> > > check can pass if the offset or length is not correct.  But you are
> > > probably right that it will be safer to check the boundary and fail
> >
> > Right, but a malicious firmware file could still match CRC but trick
> > the kernel code.
> >
> > > quicker before we actually start the CRC check.  Will you come up
> > > with a formal patch or you want us to deal with it?
> >
> > It sounds like Gustavo will be sending one, though I don't think
> > either of us have the hardware to test it with, so if you could do
> > that part, that would be great! :)
> 
> That will be great.  I think Zhao Qiang can help with the testing part.
> 

Now the firmware are loaded in uboot, and kernel will do nothing for it.
So testing on it maybe need some extra codes both in driver and dts.
In the meanwhile, I am so busy on some high priority work that maybe test work 
could not be done in time.
Once I am free, I will do it.

Best Regards
Qiang Zhao