Thread (12 messages) 12 messages, 3 authors, 2019-07-02

Re: [PATCH] net: stmmac: add sanity check to device_property_read_u32_array call

From: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Date: 2019-06-28 16:05:47
Also in: kernel-janitors, linux-arm-kernel, lkml

Hi Colin,

On Fri, Jun 28, 2019 at 10:32 AM Colin Ian King
[off-list ref] wrote:
On 28/06/2019 05:15, Martin Blumenstingl wrote:
quoted
On Tue, Jun 25, 2019 at 9:58 AM Colin Ian King [off-list ref] wrote:
quoted
On 25/06/2019 05:44, Martin Blumenstingl wrote:
quoted
Hi Colin,

On Thu, Jun 20, 2019 at 3:34 AM Martin Blumenstingl
[off-list ref] wrote:
quoted
Hi Colin,

On Wed, Jun 19, 2019 at 8:55 AM Colin Ian King [off-list ref] wrote:
quoted
On 19/06/2019 06:13, Martin Blumenstingl wrote:
quoted
Hi Colin,
quoted
Currently the call to device_property_read_u32_array is not error checked
leading to potential garbage values in the delays array that are then used
in msleep delays.  Add a sanity check to the property fetching.

Addresses-Coverity: ("Uninitialized scalar variable")
Signed-off-by: Colin Ian King <redacted>
I have also sent a patch [0] to fix initialize the array.
can you please look at my patch so we can work out which one to use?

my concern is that the "snps,reset-delays-us" property is optional,
the current dt-bindings documentation states that it's a required
property. in reality it isn't, there are boards (two examples are
mentioned in my patch: [0]) without it.

so I believe that the resulting behavior has to be:
1. don't delay if this property is missing (instead of delaying for
   <garbage value> ms)
2. don't error out if this property is missing

your patch covers #1, can you please check whether #2 is also covered?
I tested case #2 when submitting my patch and it worked fine (even
though I could not reproduce the garbage values which are being read
on some boards)
in the meantime I have tested your patch.
when I don't set the "snps,reset-delays-us" property then I get the
following error:
  invalid property snps,reset-delays-us

my patch has landed in the meantime: [0]
how should we proceed with your patch?
Your fix is good, so I think we should just drop/forget about my fix.
thank you for looking at the situation

as far I understand the -net/-net-next tree all commits are immutable
so if we want to remove your patch we need to send a revert
do you want me to do that (I can do it on Monday) or will you take care of that?


Martin
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help