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 <[email protected]>
---
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
Hi Geert,
Geert Uytterhoeven <[email protected]> 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 <[email protected]>
> ---
> 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
Hi Vivien,
On Mon, Apr 25, 2016 at 5:03 PM, Vivien Didelot
<[email protected]> wrote:
> Geert Uytterhoeven <[email protected]> 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 <[email protected]>
>> ---
>> 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
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert,
Geert Uytterhoeven <[email protected]> writes:
> On Mon, Apr 25, 2016 at 5:03 PM, Vivien Didelot
> <[email protected]> wrote:
>> Geert Uytterhoeven <[email protected]> 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 <[email protected]>
>>> ---
>>> 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
Hi Vivien,
On Mon, Apr 25, 2016 at 7:31 PM, Vivien Didelot
<[email protected]> wrote:
> Geert Uytterhoeven <[email protected]> writes:
>> On Mon, Apr 25, 2016 at 5:03 PM, Vivien Didelot
>> <[email protected]> wrote:
>>> Geert Uytterhoeven <[email protected]> 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 <[email protected]>
>>>> ---
>>>> 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
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Geert,
Geert Uytterhoeven <[email protected]> writes:
> Hi Vivien,
>
> On Mon, Apr 25, 2016 at 7:31 PM, Vivien Didelot
> <[email protected]> wrote:
>> Geert Uytterhoeven <[email protected]> writes:
>>> On Mon, Apr 25, 2016 at 5:03 PM, Vivien Didelot
>>> <[email protected]> wrote:
>>>> Geert Uytterhoeven <[email protected]> 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 <[email protected]>
>>>>> ---
>>>>> 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