| Message ID | 1415366560-27614-1-git-send-email-sudipm.mukherjee@gmail.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Sudip Mukherjee <sudipm.mukherjee@gmail.com> Date: Fri, 7 Nov 2014 18:52:40 +0530 > we were dereferencing dev to initialize pdata. but just after that we > have a BUG_ON(!dev). so we were basically dereferencing the pointer > first and then tesing it for NULL. > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org> You didn't even compile test this. Do not even bother fixing theoretical issues if you're going to be introducing a _REAL_ serious regression into the code with your "fix": drivers/net/usb/smsc95xx.c: In function ‘smsc95xx_resume’: drivers/net/usb/smsc95xx.c:1674:5: warning: ‘pdata’ is used uninitialized in this function [-Wuninitialized] u8 suspend_flags = pdata->suspend_flags; ^ So, instead of a theoretical issue, we now have a real guaranteed crash. You did not compile test this change, and you definitely did not functionally test this change either. Please do not do this ever again, thanks.
On Mon, Nov 10, 2014 at 02:22:23PM -0500, David Miller wrote: > From: Sudip Mukherjee <sudipm.mukherjee@gmail.com> > Date: Fri, 7 Nov 2014 18:52:40 +0530 > > > we were dereferencing dev to initialize pdata. but just after that we > > have a BUG_ON(!dev). so we were basically dereferencing the pointer > > first and then tesing it for NULL. > > > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org> > > You didn't even compile test this. > > Do not even bother fixing theoretical issues if you're going to be > introducing a _REAL_ serious regression into the code with your "fix": > > drivers/net/usb/smsc95xx.c: In function ‘smsc95xx_resume’: > drivers/net/usb/smsc95xx.c:1674:5: warning: ‘pdata’ is used uninitialized in this function [-Wuninitialized] > u8 suspend_flags = pdata->suspend_flags; > ^ > > So, instead of a theoretical issue, we now have a real guaranteed > crash. > > You did not compile test this change, and you definitely did not > functionally test this change either. > > Please do not do this ever again, thanks. extremely sorry for this. i have compile tested but don't know how i missed the new warning message. for any of my patch,if for some reason i am not able to compile test it, i mention that in the comments. sorry again. thanks sudip -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/usb/smsc95xx.c b/drivers/net/usb/smsc95xx.c index d07bf4c..3393238 100644 --- a/drivers/net/usb/smsc95xx.c +++ b/drivers/net/usb/smsc95xx.c @@ -1670,12 +1670,13 @@ done: static int smsc95xx_resume(struct usb_interface *intf) { struct usbnet *dev = usb_get_intfdata(intf); - struct smsc95xx_priv *pdata = (struct smsc95xx_priv *)(dev->data[0]); + struct smsc95xx_priv *pdata; u8 suspend_flags = pdata->suspend_flags; int ret; u32 val; BUG_ON(!dev); + pdata = (struct smsc95xx_priv *)(dev->data[0]); netdev_dbg(dev->net, "resume suspend_flags=0x%02x\n", suspend_flags);
we were dereferencing dev to initialize pdata. but just after that we have a BUG_ON(!dev). so we were basically dereferencing the pointer first and then tesing it for NULL. Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org> --- drivers/net/usb/smsc95xx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)