i3c: master: Fix potentially uninit variable

[PATCH-next] i3c: master: Fix potentially uninit variable

Posted by Advait Dhamorikar 1 year, 2 months ago

devinfo is uninitialized if the condition is not satisfied,
add an else condition to prevent unexpected behaviour.

The variable will contain an arbitrary value left from earlier 
computations in `i3c_device_uevent`.

Signed-off-by: Advait Dhamorikar <advaitdhamorikar@gmail.com>
---
 drivers/i3c/master.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
index 42310c9a00c2..7594d3793eb0 100644
--- a/drivers/i3c/master.c
+++ b/drivers/i3c/master.c
@@ -284,6 +284,8 @@ static int i3c_device_uevent(const struct device *dev, struct kobj_uevent_env *e
 
 	if (i3cdev->desc)
 		devinfo = i3cdev->desc->info;
+	else
+		return -ENODEV;
 	manuf = I3C_PID_MANUF_ID(devinfo.pid);
 	part = I3C_PID_PART_ID(devinfo.pid);
 	ext = I3C_PID_EXTRA_INFO(devinfo.pid);
-- 
2.34.1

Re: [PATCH-next] i3c: master: Fix potentially uninit variable

Posted by Advait Dhamorikar 1 year, 2 months ago

Hello,

I would really appreciate it if I could get some feedback on this patch,
I would like to know if this approach is the right way to proceed.

Thanks and regards,
Advait

On Tue, 19 Nov 2024 at 21:09, Advait Dhamorikar
<advaitdhamorikar@gmail.com> wrote:
>
> devinfo is uninitialized if the condition is not satisfied,
> add an else condition to prevent unexpected behaviour.
>
> The variable will contain an arbitrary value left from earlier
> computations in `i3c_device_uevent`.
>
> Signed-off-by: Advait Dhamorikar <advaitdhamorikar@gmail.com>
> ---
>  drivers/i3c/master.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> index 42310c9a00c2..7594d3793eb0 100644
> --- a/drivers/i3c/master.c
> +++ b/drivers/i3c/master.c
> @@ -284,6 +284,8 @@ static int i3c_device_uevent(const struct device *dev, struct kobj_uevent_env *e
>
>         if (i3cdev->desc)
>                 devinfo = i3cdev->desc->info;
> +       else
> +               return -ENODEV;
>         manuf = I3C_PID_MANUF_ID(devinfo.pid);
>         part = I3C_PID_PART_ID(devinfo.pid);
>         ext = I3C_PID_EXTRA_INFO(devinfo.pid);
> --
> 2.34.1
>

Re: [PATCH-next] i3c: master: Fix potentially uninit variable

Posted by Alexandre Belloni 1 year, 2 months ago

On 27/11/2024 01:26:08+0530, Advait Dhamorikar wrote:
> Hello,
> 
> I would really appreciate it if I could get some feedback on this patch,
> I would like to know if this approach is the right way to proceed.
> 

Yes and no, I don't think -ENODEV is allowed here, see platform_uevent
and i2c_device_uevent.


> Thanks and regards,
> Advait
> 
> On Tue, 19 Nov 2024 at 21:09, Advait Dhamorikar
> <advaitdhamorikar@gmail.com> wrote:
> >
> > devinfo is uninitialized if the condition is not satisfied,
> > add an else condition to prevent unexpected behaviour.
> >
> > The variable will contain an arbitrary value left from earlier
> > computations in `i3c_device_uevent`.
> >
> > Signed-off-by: Advait Dhamorikar <advaitdhamorikar@gmail.com>
> > ---
> >  drivers/i3c/master.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> > index 42310c9a00c2..7594d3793eb0 100644
> > --- a/drivers/i3c/master.c
> > +++ b/drivers/i3c/master.c
> > @@ -284,6 +284,8 @@ static int i3c_device_uevent(const struct device *dev, struct kobj_uevent_env *e
> >
> >         if (i3cdev->desc)
> >                 devinfo = i3cdev->desc->info;
> > +       else
> > +               return -ENODEV;
> >         manuf = I3C_PID_MANUF_ID(devinfo.pid);
> >         part = I3C_PID_PART_ID(devinfo.pid);
> >         ext = I3C_PID_EXTRA_INFO(devinfo.pid);
> > --
> > 2.34.1
> >

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH-next] i3c: master: Fix potentially uninit variable

Posted by Advait Dhamorikar 1 year, 2 months ago

> Yes and no, I don't think -ENODEV is allowed here, see platform_uevent
> and i2c_device_uevent.

Thanks for the feedback. So if devinfo is uninitialized is it possible to
initialize the i3c modalias using OF and ACPI uevent_modalias?

My humble apologies if this is a trivial question but I'm not really familiar
with i3c and found the potential uninitialization using a static analyzer.

Thanks and regards,
Advait

On Wed, 27 Nov 2024 at 04:18, Alexandre Belloni
<alexandre.belloni@bootlin.com> wrote:
>
> On 27/11/2024 01:26:08+0530, Advait Dhamorikar wrote:
> > Hello,
> >
> > I would really appreciate it if I could get some feedback on this patch,
> > I would like to know if this approach is the right way to proceed.
> >
>
> Yes and no, I don't think -ENODEV is allowed here, see platform_uevent
> and i2c_device_uevent.
>
>
> > Thanks and regards,
> > Advait
> >
> > On Tue, 19 Nov 2024 at 21:09, Advait Dhamorikar
> > <advaitdhamorikar@gmail.com> wrote:
> > >
> > > devinfo is uninitialized if the condition is not satisfied,
> > > add an else condition to prevent unexpected behaviour.
> > >
> > > The variable will contain an arbitrary value left from earlier
> > > computations in `i3c_device_uevent`.
> > >
> > > Signed-off-by: Advait Dhamorikar <advaitdhamorikar@gmail.com>
> > > ---
> > >  drivers/i3c/master.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/i3c/master.c b/drivers/i3c/master.c
> > > index 42310c9a00c2..7594d3793eb0 100644
> > > --- a/drivers/i3c/master.c
> > > +++ b/drivers/i3c/master.c
> > > @@ -284,6 +284,8 @@ static int i3c_device_uevent(const struct device *dev, struct kobj_uevent_env *e
> > >
> > >         if (i3cdev->desc)
> > >                 devinfo = i3cdev->desc->info;
> > > +       else
> > > +               return -ENODEV;
> > >         manuf = I3C_PID_MANUF_ID(devinfo.pid);
> > >         part = I3C_PID_PART_ID(devinfo.pid);
> > >         ext = I3C_PID_EXTRA_INFO(devinfo.pid);
> > > --
> > > 2.34.1
> > >
>
> --
> Alexandre Belloni, co-owner and COO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com