| Message ID | 5dff3719-f827-45b6-a0d3-a00efed1099b@moroto.mountain (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-XXX] net: dsa: qca8k: uninitialized variable in hw_control_get() | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net |
| netdev/apply | fail | Patch does not apply to net |
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.
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
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;
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(+)