| Message ID | 20241119153941.8307-1-advaitdhamorikar@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [PATCH-next] i3c: master: Fix potentially uninit variable | expand |
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 >
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 > >
> 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
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);
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(+)