[PATCH] [RFC] net: dsa: mv88e6xxx: Pre-initialize err in mv88e6xxx_port_bridge_join()

[PATCH] [RFC] net: dsa: mv88e6xxx: Pre-initialize err in mv88e6xxx_port_bridge_join()

[Geert Uytterhoeven]

drivers/net/dsa/mv88e6xxx.c: In function ‘mv88e6xxx_port_bridge_join’:
drivers/net/dsa/mv88e6xxx.c:2184: warning: ‘err’ may be used uninitialized in this function

If netdev_notifier_changeupper_info.upper_dev is ever NULL, the bridge
parameter will be NULL too, and the function will return an
uninitialized value.

Pre-initialize err to zero to fix this.

Fixes: 207afda1b5036009 ("net: dsa: mv88e6xxx: share the same default FDB")
Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
Can this actually happen?
---
 drivers/net/dsa/mv88e6xxx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c
index a2904029cccc2949..5e572b3510b9483a 100644
--- a/drivers/net/dsa/mv88e6xxx.c
+++ b/drivers/net/dsa/mv88e6xxx.c
@@ -2181,7 +2181,7 @@ int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port,
 			       struct net_device *bridge)
 {
 	struct mv88e6xxx_priv_state *ps = ds_to_priv(ds);
-	int i, err;
+	int i, err = 0;
 
 	mutex_lock(&ps->smi_mutex);
 
-- 
1.9.1

Re: [PATCH] [RFC] net: dsa: mv88e6xxx: Pre-initialize err in mv88e6xxx_port_bridge_join()

[Vivien Didelot]

Hi Geert,

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> drivers/net/dsa/mv88e6xxx.c: In function ‘mv88e6xxx_port_bridge_join’:
> drivers/net/dsa/mv88e6xxx.c:2184: warning: ‘err’ may be used uninitialized in this function

Interesting, I don't have those warnings on 207afda1b5036009...

> If netdev_notifier_changeupper_info.upper_dev is ever NULL, the bridge
> parameter will be NULL too, and the function will return an
> uninitialized value.
>
> Pre-initialize err to zero to fix this.
>
> Fixes: 207afda1b5036009 ("net: dsa: mv88e6xxx: share the same default FDB")
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> Can this actually happen?

bridge cannot be NULL here. Also ps->ports[port].bridge_dev is assigned
to it before entering the for loop, so _mv88e6xxx_port_based_vlan_map
will be called at least for this port.

Thanks,

        Vivien

Re: [PATCH] [RFC] net: dsa: mv88e6xxx: Pre-initialize err in mv88e6xxx_port_bridge_join()

[Geert Uytterhoeven]

Hi Vivien,

On Mon, Apr 25, 2016 at 5:03 PM, Vivien Didelot
<vivien.didelot@savoirfairelinux.com> wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>> drivers/net/dsa/mv88e6xxx.c: In function ‘mv88e6xxx_port_bridge_join’:
>> drivers/net/dsa/mv88e6xxx.c:2184: warning: ‘err’ may be used uninitialized in this function
>
> Interesting, I don't have those warnings on 207afda1b5036009...

It depends on the compiler version (still using 4.1.2) and options.

>> If netdev_notifier_changeupper_info.upper_dev is ever NULL, the bridge
>> parameter will be NULL too, and the function will return an
>> uninitialized value.
>>
>> Pre-initialize err to zero to fix this.
>>
>> Fixes: 207afda1b5036009 ("net: dsa: mv88e6xxx: share the same default FDB")
>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> ---
>> Can this actually happen?
>
> bridge cannot be NULL here. Also ps->ports[port].bridge_dev is assigned
> to it before entering the for loop, so _mv88e6xxx_port_based_vlan_map
> will be called at least for this port.

But there's no way the compiler can know that...

Gr{oetje,eeting}s,

                        Geert

Re: [PATCH] [RFC] net: dsa: mv88e6xxx: Pre-initialize err in mv88e6xxx_port_bridge_join()

[Vivien Didelot]

Hi Geert,

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> On Mon, Apr 25, 2016 at 5:03 PM, Vivien Didelot
> <vivien.didelot@savoirfairelinux.com> wrote:
>> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>>> drivers/net/dsa/mv88e6xxx.c: In function ‘mv88e6xxx_port_bridge_join’:
>>> drivers/net/dsa/mv88e6xxx.c:2184: warning: ‘err’ may be used uninitialized in this function
>>
>> Interesting, I don't have those warnings on 207afda1b5036009...
>
> It depends on the compiler version (still using 4.1.2) and options.
>
>>> If netdev_notifier_changeupper_info.upper_dev is ever NULL, the bridge
>>> parameter will be NULL too, and the function will return an
>>> uninitialized value.
>>>
>>> Pre-initialize err to zero to fix this.
>>>
>>> Fixes: 207afda1b5036009 ("net: dsa: mv88e6xxx: share the same default FDB")
>>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>>> ---
>>> Can this actually happen?
>>
>> bridge cannot be NULL here. Also ps->ports[port].bridge_dev is assigned
>> to it before entering the for loop, so _mv88e6xxx_port_based_vlan_map
>> will be called at least for this port.
>
> But there's no way the compiler can know that...

Or maybe it can in new configurations. Anyway, this fix doesn't hurt,
with a relevant commit message, I'd ack it.

Thanks,

        Vivien

Re: [PATCH] [RFC] net: dsa: mv88e6xxx: Pre-initialize err in mv88e6xxx_port_bridge_join()

[Geert Uytterhoeven]

Hi Vivien,

On Mon, Apr 25, 2016 at 7:31 PM, Vivien Didelot
<vivien.didelot@savoirfairelinux.com> wrote:
> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>> On Mon, Apr 25, 2016 at 5:03 PM, Vivien Didelot
>> <vivien.didelot@savoirfairelinux.com> wrote:
>>> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>>>> drivers/net/dsa/mv88e6xxx.c: In function ‘mv88e6xxx_port_bridge_join’:
>>>> drivers/net/dsa/mv88e6xxx.c:2184: warning: ‘err’ may be used uninitialized in this function
>>>
>>> Interesting, I don't have those warnings on 207afda1b5036009...
>>
>> It depends on the compiler version (still using 4.1.2) and options.
>>
>>>> If netdev_notifier_changeupper_info.upper_dev is ever NULL, the bridge
>>>> parameter will be NULL too, and the function will return an
>>>> uninitialized value.
>>>>
>>>> Pre-initialize err to zero to fix this.
>>>>
>>>> Fixes: 207afda1b5036009 ("net: dsa: mv88e6xxx: share the same default FDB")
>>>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>>>> ---
>>>> Can this actually happen?
>>>
>>> bridge cannot be NULL here. Also ps->ports[port].bridge_dev is assigned
>>> to it before entering the for loop, so _mv88e6xxx_port_based_vlan_map
>>> will be called at least for this port.
>>
>> But there's no way the compiler can know that...
>
> Or maybe it can in new configurations. Anyway, this fix doesn't hurt,
> with a relevant commit message, I'd ack it.

What would you consider a relevant commit message?

Thanks!

Gr{oetje,eeting}s,

                        Geert

Re: [PATCH] [RFC] net: dsa: mv88e6xxx: Pre-initialize err in mv88e6xxx_port_bridge_join()

[Vivien Didelot]

Hi Geert,

Geert Uytterhoeven <geert@linux-m68k.org> writes:

> Hi Vivien,
>
> On Mon, Apr 25, 2016 at 7:31 PM, Vivien Didelot
> <vivien.didelot@savoirfairelinux.com> wrote:
>> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>>> On Mon, Apr 25, 2016 at 5:03 PM, Vivien Didelot
>>> <vivien.didelot@savoirfairelinux.com> wrote:
>>>> Geert Uytterhoeven <geert@linux-m68k.org> writes:
>>>>> drivers/net/dsa/mv88e6xxx.c: In function ‘mv88e6xxx_port_bridge_join’:
>>>>> drivers/net/dsa/mv88e6xxx.c:2184: warning: ‘err’ may be used uninitialized in this function
>>>>
>>>> Interesting, I don't have those warnings on 207afda1b5036009...
>>>
>>> It depends on the compiler version (still using 4.1.2) and options.
>>>
>>>>> If netdev_notifier_changeupper_info.upper_dev is ever NULL, the bridge
>>>>> parameter will be NULL too, and the function will return an
>>>>> uninitialized value.
>>>>>
>>>>> Pre-initialize err to zero to fix this.
>>>>>
>>>>> Fixes: 207afda1b5036009 ("net: dsa: mv88e6xxx: share the same default FDB")
>>>>> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
>>>>> ---
>>>>> Can this actually happen?
>>>>
>>>> bridge cannot be NULL here. Also ps->ports[port].bridge_dev is assigned
>>>> to it before entering the for loop, so _mv88e6xxx_port_based_vlan_map
>>>> will be called at least for this port.
>>>
>>> But there's no way the compiler can know that...
>>
>> Or maybe it can in new configurations. Anyway, this fix doesn't hurt,
>> with a relevant commit message, I'd ack it.
>
> What would you consider a relevant commit message?

bridge being NULL is not the reason why err can eventually get returned
uninitialized. The GCC version would be great too, I have no such
warning here with 5.3.0.

Thanks,
-v