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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Dan Carpenter <dan.carpenter@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
>  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



[Index of Archives]     [Kernel Development]     [Kernel Announce]     [Kernel Newbies]     [Linux Networking Development]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Device Mapper]

  Powered by Linux