| Message ID | 20220723043229.2953386-1-williamsukatube@163.com |
|---|---|
| State | New |
| Headers | show |
| Series | i2c: cp2615: check the return value of kzalloc() | expand |
Dear William, thank you for your insight. However, you will find that the first access of the variable `msg` is a call to `cp2615_init_iop_msg()` which, on line 85, checks for the variable being NULL, and if so, returns -EINVAL, which in turn causes `cp2615_check_iop()` to return too. In fact, the same mechanism allows for `cp2615_i2c_send()` to also not have to worry about `kzalloc()` returning NULL. Though you could argue that in these cases, `cp2615_init_iop_msg()` should return -ENOMEM instead of -EINVAL, and you would be correct. Cheers, Bence <williamsukatube@163.com> ezt írta (időpont: 2022. júl. 23., Szo, 6:33): > > From: William Dean <williamsukatube@gmail.com> > > kzalloc() is a memory allocation function which can return NULL when > some internal memory errors happen. So it is better to check the > return value of it to prevent potential wrong memory access or > memory leak. > > Fixes: 4a7695429eade ("i2c: cp2615: add i2c driver for Silicon Labs' CP2615 Digital Audio Bridge") > Reported-by: Hacash Robot <hacashRobot@santino.com> > Signed-off-by: William Dean <williamsukatube@gmail.com> > --- > drivers/i2c/busses/i2c-cp2615.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-cp2615.c b/drivers/i2c/busses/i2c-cp2615.c > index 3ded28632e4c..7c9403346615 100644 > --- a/drivers/i2c/busses/i2c-cp2615.c > +++ b/drivers/i2c/busses/i2c-cp2615.c > @@ -171,11 +171,17 @@ cp2615_i2c_recv(struct usb_interface *usbif, unsigned char tag, void *buf) > /* Checks if the IOP is functional by querying the part's ID */ > static int cp2615_check_iop(struct usb_interface *usbif) > { > - struct cp2615_iop_msg *msg = kzalloc(sizeof(*msg), GFP_KERNEL); > - struct cp2615_iop_accessory_info *info = (struct cp2615_iop_accessory_info *)&msg->data; > + struct cp2615_iop_msg *msg; > + struct cp2615_iop_accessory_info *info; > struct usb_device *usbdev = interface_to_usbdev(usbif); > - int res = cp2615_init_iop_msg(msg, iop_GetAccessoryInfo, NULL, 0); > + int res; > + > + msg = kzalloc(sizeof(*msg), GFP_KERNEL); > + if (!msg) > + return -ENOMEM; > > + info = (struct cp2615_iop_accessory_info *)&msg->data; > + res = cp2615_init_iop_msg(msg, iop_GetAccessoryInfo, NULL, 0); > if (res) > goto out; > > -- > 2.25.1 > >
diff --git a/drivers/i2c/busses/i2c-cp2615.c b/drivers/i2c/busses/i2c-cp2615.c index 3ded28632e4c..7c9403346615 100644 --- a/drivers/i2c/busses/i2c-cp2615.c +++ b/drivers/i2c/busses/i2c-cp2615.c @@ -171,11 +171,17 @@ cp2615_i2c_recv(struct usb_interface *usbif, unsigned char tag, void *buf) /* Checks if the IOP is functional by querying the part's ID */ static int cp2615_check_iop(struct usb_interface *usbif) { - struct cp2615_iop_msg *msg = kzalloc(sizeof(*msg), GFP_KERNEL); - struct cp2615_iop_accessory_info *info = (struct cp2615_iop_accessory_info *)&msg->data; + struct cp2615_iop_msg *msg; + struct cp2615_iop_accessory_info *info; struct usb_device *usbdev = interface_to_usbdev(usbif); - int res = cp2615_init_iop_msg(msg, iop_GetAccessoryInfo, NULL, 0); + int res; + + msg = kzalloc(sizeof(*msg), GFP_KERNEL); + if (!msg) + return -ENOMEM; + info = (struct cp2615_iop_accessory_info *)&msg->data; + res = cp2615_init_iop_msg(msg, iop_GetAccessoryInfo, NULL, 0); if (res) goto out;