| Message ID | 20221108061113.35964-1-yuehaibing@huawei.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | phy: usb: sunplus: Fix memleak in update_disc_vol() | expand |
On Tue, Nov 08, 2022 at 02:11:13PM +0800, YueHaibing wrote: > 'otp_v' is allocated in nvmem_cell_read(), it should be freed > before return. > > Fixes: 99d9ccd97385 ("phy: usb: Add USB2.0 phy driver for Sunplus SP7021") > Signed-off-by: YueHaibing <yuehaibing@huawei.com> > --- > drivers/phy/sunplus/phy-sunplus-usb2.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/phy/sunplus/phy-sunplus-usb2.c b/drivers/phy/sunplus/phy-sunplus-usb2.c > index e827b79f6d49..450cf8e6d7fb 100644 > --- a/drivers/phy/sunplus/phy-sunplus-usb2.c > +++ b/drivers/phy/sunplus/phy-sunplus-usb2.c > @@ -96,6 +96,7 @@ static int update_disc_vol(struct sp_usbphy *usbphy) > set = *(otp_v + 1); > set = (set << (sizeof(char) * 8)) | *otp_v; > set = (set >> usbphy->disc_vol_addr_off) & J_DISC; > + kfree(otp_v); > } > > if (IS_ERR(otp_v) || set == 0) How did you test this? Just by looking at this tiny diff, this seems to be wrong, please fix your tools and your review process to catch errors like this. thanks, greg k-h
On 2022/11/8 14:38, Greg KH wrote: > On Tue, Nov 08, 2022 at 02:11:13PM +0800, YueHaibing wrote: >> 'otp_v' is allocated in nvmem_cell_read(), it should be freed >> before return. >> >> Fixes: 99d9ccd97385 ("phy: usb: Add USB2.0 phy driver for Sunplus SP7021") >> Signed-off-by: YueHaibing <yuehaibing@huawei.com> >> --- >> drivers/phy/sunplus/phy-sunplus-usb2.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/phy/sunplus/phy-sunplus-usb2.c b/drivers/phy/sunplus/phy-sunplus-usb2.c >> index e827b79f6d49..450cf8e6d7fb 100644 >> --- a/drivers/phy/sunplus/phy-sunplus-usb2.c >> +++ b/drivers/phy/sunplus/phy-sunplus-usb2.c >> @@ -96,6 +96,7 @@ static int update_disc_vol(struct sp_usbphy *usbphy) >> set = *(otp_v + 1); >> set = (set << (sizeof(char) * 8)) | *otp_v; >> set = (set >> usbphy->disc_vol_addr_off) & J_DISC; >> + kfree(otp_v); >> } >> >> if (IS_ERR(otp_v) || set == 0) > How did you test this? > > Just by looking at this tiny diff, this seems to be wrong, please fix > your tools and your review process to catch errors like this. sorry, I misread the code, will be more careful next time. > > thanks, > > greg k-h > .
diff --git a/drivers/phy/sunplus/phy-sunplus-usb2.c b/drivers/phy/sunplus/phy-sunplus-usb2.c index e827b79f6d49..450cf8e6d7fb 100644 --- a/drivers/phy/sunplus/phy-sunplus-usb2.c +++ b/drivers/phy/sunplus/phy-sunplus-usb2.c @@ -96,6 +96,7 @@ static int update_disc_vol(struct sp_usbphy *usbphy) set = *(otp_v + 1); set = (set << (sizeof(char) * 8)) | *otp_v; set = (set >> usbphy->disc_vol_addr_off) & J_DISC; + kfree(otp_v); } if (IS_ERR(otp_v) || set == 0)
'otp_v' is allocated in nvmem_cell_read(), it should be freed before return. Fixes: 99d9ccd97385 ("phy: usb: Add USB2.0 phy driver for Sunplus SP7021") Signed-off-by: YueHaibing <yuehaibing@huawei.com> --- drivers/phy/sunplus/phy-sunplus-usb2.c | 1 + 1 file changed, 1 insertion(+)