| Message ID | 20240922105212.28099-1-kdipendra88@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Fix dereferencing freed memory 'fw' | expand |
Hi Dipendra, Thanks for the patch. You forgot the susbsytem and driver from your subject line: https://docs.kernel.org/process/submitting-patches.html#the-canonical-patch-format On Sun, Sep 22, 2024 at 10:52:11AM +0000, Dipendra Khadka wrote: > smatch reported dereferencing freed memory as below: > ''' > drivers/usb/typec/tipd/core.c:1196 tps6598x_apply_patch() error: dereferencing freed memory 'fw' > ''' > > Invoking release_firware(fw) just after checking ret. > > Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com> This is a use-after-free case, so please add the Fixes tag: Fixes: 916b8e5fa73d ("usb: typec: tipd: add error log to provide firmware name and size") > --- > drivers/usb/typec/tipd/core.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c > index ea768b19a7f1..eb5596e3406a 100644 > --- a/drivers/usb/typec/tipd/core.c > +++ b/drivers/usb/typec/tipd/core.c > @@ -1191,11 +1191,11 @@ static int tps6598x_apply_patch(struct tps6598x *tps) > dev_info(tps->dev, "Firmware update succeeded\n"); > > release_fw: > - release_firmware(fw); > if (ret) { > dev_err(tps->dev, "Failed to write patch %s of %zu bytes\n", > firmware_name, fw->size); > } > + release_firmware(fw); > > return ret; > }; > -- > 2.43.0 thanks,
On Sun, Sep 22, 2024 at 10:52:11AM +0000, Dipendra Khadka wrote: > smatch reported dereferencing freed memory as below: > ''' > drivers/usb/typec/tipd/core.c:1196 tps6598x_apply_patch() error: dereferencing freed memory 'fw' > ''' This is only my opinion, but I don't like those '''single triple quotes''' in commit messages - they are just confusing. You don't need to use any kind of quotation marks in cases like this IMO. thanks,
Hi Heikki, On Mon, 23 Sept 2024 at 14:20, Heikki Krogerus <heikki.krogerus@linux.intel.com> wrote: > > On Sun, Sep 22, 2024 at 10:52:11AM +0000, Dipendra Khadka wrote: > > smatch reported dereferencing freed memory as below: > > ''' > > drivers/usb/typec/tipd/core.c:1196 tps6598x_apply_patch() error: dereferencing freed memory 'fw' > > ''' > > This is only my opinion, but I don't like those '''single triple > quotes''' in commit messages - they are just confusing. You don't need > to use any kind of quotation marks in cases like this IMO. > Thanks for the feedback. I will send v2. > thanks, > > -- > heikki Best Regards, Dipendra Khadka
diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c index ea768b19a7f1..eb5596e3406a 100644 --- a/drivers/usb/typec/tipd/core.c +++ b/drivers/usb/typec/tipd/core.c @@ -1191,11 +1191,11 @@ static int tps6598x_apply_patch(struct tps6598x *tps) dev_info(tps->dev, "Firmware update succeeded\n"); release_fw: - release_firmware(fw); if (ret) { dev_err(tps->dev, "Failed to write patch %s of %zu bytes\n", firmware_name, fw->size); } + release_firmware(fw); return ret; };
smatch reported dereferencing freed memory as below: ''' drivers/usb/typec/tipd/core.c:1196 tps6598x_apply_patch() error: dereferencing freed memory 'fw' ''' Invoking release_firware(fw) just after checking ret. Signed-off-by: Dipendra Khadka <kdipendra88@gmail.com> --- drivers/usb/typec/tipd/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)