Re: [PATCH] i2c: cp2615: check for allocation failure in cp2615_i2c_recv()

Bence Csókás
2021-05-12T11:10:09Z

Dan Carpenter <dan.carpenter@oracle.com> ezt írta (időpont: 2021. máj.
12., Sze, 12:07):
>
> We need to add a check for if the kzalloc() fails.

That is correct, I missed that :/

>
> Fixes: 4a7695429ead ("i2c: cp2615: add i2c driver for Silicon Labs' CP2615 Digital Audio Bridge")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> drivers/i2c/busses/i2c-cp2615.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-cp2615.c b/drivers/i2c/busses/i2c-cp2615.c
> index 78cfecd1ea76..3ded28632e4c 100644
> --- a/drivers/i2c/busses/i2c-cp2615.c
> +++ b/drivers/i2c/busses/i2c-cp2615.c
> @@ -138,17 +138,23 @@ cp2615_i2c_send(struct usb_interface *usbif, struct cp2615_i2c_transfer *i2c_w)
> static int
> cp2615_i2c_recv(struct usb_interface *usbif, unsigned char tag, void *buf)
> {
> - struct cp2615_iop_msg *msg = kzalloc(sizeof(*msg), GFP_KERNEL);
> - struct cp2615_i2c_transfer_result *i2c_r = (struct cp2615_i2c_transfer_result *)&msg->data;

These two lines could stay as-is, since the invalid i2c_r will never be used.

> struct usb_device *usbdev = interface_to_usbdev(usbif);
> - int res = usb_bulk_msg(usbdev, usb_rcvbulkpipe(usbdev, IOP_EP_IN),
> - msg, sizeof(struct cp2615_iop_msg), NULL, 0);
> + struct cp2615_iop_msg *msg;
> + struct cp2615_i2c_transfer_result *i2c_r;
> + int res;
> +
> + msg = kzalloc(sizeof(*msg), GFP_KERNEL);
> + if (!msg)
> + return -ENOMEM;

You will want to also make `cp2615_init_iop_msg()` return -ENOMEM
instead of -EINVAL, for consistency's sake.

>
> + res = usb_bulk_msg(usbdev, usb_rcvbulkpipe(usbdev, IOP_EP_IN), msg,
> + sizeof(struct cp2615_iop_msg), NULL, 0);
> if (res < 0) {
> kfree(msg);
> return res;
> }
>
> + i2c_r = (struct cp2615_i2c_transfer_result *)&msg->data;
> if (msg->msg != htons(iop_I2cTransferResult) || i2c_r->tag != tag) {
> kfree(msg);
> return -EIO;
> --
> 2.30.2
>

Otherwise, the patch looks good, I shall sign off on the next version