Re: [PATCH net-XXX] net: dsa: qca8k: uninitialized variable in hw_control_get()

Re: [PATCH net-XXX] net: dsa: qca8k: uninitialized variable in hw_control_get()

[Christian Marangi]

On Mon, Jun 12, 2023 at 10:20:55AM +0300, Dan Carpenter wrote:
> The caller, netdev_trig_activate(), passes an uninitialized value for
> *rules.  This function sets bits to one but it doesn't zero out any
> bits so there is a potential for uninitialized data to be used.
> Zero out the *rules at the start of the function.
> 
> Fixes: e0256648c831 ("net: dsa: qca8k: implement hw_control ops")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>

Thanks for the fix but I wonder if this should be better fixed in
netdev_trig_activate? By setting the mode as 0 directly there?

I assume other dev implementing the get ops would do the same mistake.

-- 
	Ansuel

[PATCH net-XXX] net: dsa: qca8k: uninitialized variable in hw_control_get()

[Dan Carpenter]

The caller, netdev_trig_activate(), passes an uninitialized value for
*rules.  This function sets bits to one but it doesn't zero out any
bits so there is a potential for uninitialized data to be used.
Zero out the *rules at the start of the function.

Fixes: e0256648c831 ("net: dsa: qca8k: implement hw_control ops")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 drivers/net/dsa/qca/qca8k-leds.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/dsa/qca/qca8k-leds.c b/drivers/net/dsa/qca/qca8k-leds.c
index 6f02029b454b..772478156e4e 100644
--- a/drivers/net/dsa/qca/qca8k-leds.c
+++ b/drivers/net/dsa/qca/qca8k-leds.c
@@ -304,6 +304,8 @@ qca8k_cled_hw_control_get(struct led_classdev *ldev, unsigned long *rules)
 	u32 val;
 	int ret;
 
+	*rules = 0;
+
 	/* With hw control not active return err */
 	if (!qca8k_cled_hw_control_status(ldev))
 		return -EINVAL;
-- 
2.39.2

Re: [PATCH net-XXX] net: dsa: qca8k: uninitialized variable in hw_control_get()

[Dan Carpenter]

The net-XXX in the subject was supposed to be net-next btw.  I did check
that it only applied to net-next but I messed up the subject...  :/

On Sun, Jun 11, 2023 at 06:04:31PM +0200, Christian Marangi wrote:
> On Mon, Jun 12, 2023 at 10:20:55AM +0300, Dan Carpenter wrote:
> > The caller, netdev_trig_activate(), passes an uninitialized value for
> > *rules.  This function sets bits to one but it doesn't zero out any
> > bits so there is a potential for uninitialized data to be used.
> > Zero out the *rules at the start of the function.
> > 
> > Fixes: e0256648c831 ("net: dsa: qca8k: implement hw_control ops")
> > Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> 
> Thanks for the fix but I wonder if this should be better fixed in
> netdev_trig_activate? By setting the mode as 0 directly there?
> 
> I assume other dev implementing the get ops would do the same mistake.

Yes.  You're obviously right on this.  I'm not sure what I was thinking.

I will resend.

regards,
dan carpenter