Thread (10 messages) 10 messages, 4 authors, 2020-05-26

RE: [PATCH] soc: fsl: qe: Replace one-element array and use struct_size() helper

From: Qiang Zhao <qiang.zhao@nxp.com>
Date: 2020-05-25 02:47:30
Also in: linuxppc-dev, lkml

On Wed, May 23, 2020 at 5:22 PM Li Yang [off-list ref]
-----Original Message-----
From: Li Yang <redacted>
Sent: 2020年5月23日 5:22
To: Kees Cook <redacted>
Cc: Gustavo A. R. Silva <gustavoars@kernel.org>; Qiang Zhao
[off-list ref]; linuxppc-dev [off-list ref];
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
[off-list ref]; lkml [off-list ref];
Gustavo A. R. Silva [off-list ref]
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 [off-list ref]
wrote:
quoted
On Wed, May 20, 2020 at 06:52:21PM -0500, Li Yang wrote:
quoted
On Mon, May 18, 2020 at 5:57 PM Kees Cook [off-list ref]
wrote:
quoted
quoted
quoted
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
quoted
quoted
quoted
   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.
quoted
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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help