| Message ID | 20240306123028.164155-1-rand.sec96@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] ssb: Fix potential NULL pointer dereference in ssb_device_uevent | expand |
Hi On Wed, 6 Mar 2024 at 13:32, Rand Deeb <rand.sec96@gmail.com> wrote: > > The ssb_device_uevent function first attempts to convert the 'dev' pointer > to 'struct ssb_device *'. However, it mistakenly dereferences 'dev' before > performing the NULL check, potentially leading to a NULL pointer > dereference if 'dev' is NULL. > > To fix this issue, this patch moves the NULL check before dereferencing the > 'dev' pointer, ensuring that the pointer is valid before attempting to use > it. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Signed-off-by: Rand Deeb <rand.sec96@gmail.com> > --- > drivers/ssb/main.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/ssb/main.c b/drivers/ssb/main.c > index b9934b9c2d70..070a99a4180c 100644 > --- a/drivers/ssb/main.c > +++ b/drivers/ssb/main.c > @@ -341,11 +341,13 @@ static int ssb_bus_match(struct device *dev, struct device_driver *drv) > > static int ssb_device_uevent(const struct device *dev, struct kobj_uevent_env *env) > { > - const struct ssb_device *ssb_dev = dev_to_ssb_dev(dev); > + const struct ssb_device *ssb_dev; > > if (!dev) > return -ENODEV; > > + ssb_dev = dev_to_ssb_dev(dev); > + The NULL check is what needs to be fixed/removed, not the code surrounding it. This function will be called from dev_uevent() [1] where dev cannot be NULL. So a NULL dereference cannot happen. Most other implementors of bus_type::uevent have no NULL check. To be precise, there is only one other implementor with a NULL check, rio_uevent(), and none of the other ones have one. See e.g. bcma_device_uevent(), memstick_uevent(), mips_cdmm_uevent(), or fsl_mc_bus_uevent(). [1] https://elixir.bootlin.com/linux/v6.7.8/source/drivers/base/core.c#L2590 Best Regards, Jonas
Rand Deeb <rand.sec96@gmail.com> wrote: > The ssb_device_uevent() function first attempts to convert the 'dev' pointer > to 'struct ssb_device *'. However, it mistakenly dereferences 'dev' before > performing the NULL check, potentially leading to a NULL pointer > dereference if 'dev' is NULL. > > To fix this issue, move the NULL check before dereferencing the 'dev' pointer, > ensuring that the pointer is valid before attempting to use it. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Signed-off-by: Rand Deeb <rand.sec96@gmail.com> Patch applied to wireless-next.git, thanks. 789c17185fb0 ssb: Fix potential NULL pointer dereference in ssb_device_uevent()
diff --git a/drivers/ssb/main.c b/drivers/ssb/main.c index b9934b9c2d70..070a99a4180c 100644 --- a/drivers/ssb/main.c +++ b/drivers/ssb/main.c @@ -341,11 +341,13 @@ static int ssb_bus_match(struct device *dev, struct device_driver *drv) static int ssb_device_uevent(const struct device *dev, struct kobj_uevent_env *env) { - const struct ssb_device *ssb_dev = dev_to_ssb_dev(dev); + const struct ssb_device *ssb_dev; if (!dev) return -ENODEV; + ssb_dev = dev_to_ssb_dev(dev); + return add_uevent_var(env, "MODALIAS=ssb:v%04Xid%04Xrev%02X", ssb_dev->id.vendor, ssb_dev->id.coreid,
The ssb_device_uevent function first attempts to convert the 'dev' pointer to 'struct ssb_device *'. However, it mistakenly dereferences 'dev' before performing the NULL check, potentially leading to a NULL pointer dereference if 'dev' is NULL. To fix this issue, this patch moves the NULL check before dereferencing the 'dev' pointer, ensuring that the pointer is valid before attempting to use it. Found by Linux Verification Center (linuxtesting.org) with SVACE. Signed-off-by: Rand Deeb <rand.sec96@gmail.com> --- drivers/ssb/main.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)