* [PATCH] i2c: cp2615: handle allocation failure @ 2026-08-16 16:17 Triet Hoang 2026-08-18 11:17 ` [PATCH v2] " Triet Hoang 0 siblings, 1 reply; 9+ messages in thread From: Triet Hoang @ 2026-08-16 16:17 UTC (permalink / raw) To: bence98; +Cc: andi.shyti, linux-i2c, linux-kernel, Triet Hoang Check the result of kzalloc_obj() before dereferencing the allocated message structure. Signed-off-by: Triet Hoang <triet.hoang.dev@gmail.com> --- drivers/i2c/busses/i2c-cp2615.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/i2c/busses/i2c-cp2615.c b/drivers/i2c/busses/i2c-cp2615.c index 951de6249..178a35d0b 100644 --- a/drivers/i2c/busses/i2c-cp2615.c +++ b/drivers/i2c/busses/i2c-cp2615.c @@ -125,6 +125,9 @@ static int cp2615_i2c_send(struct usb_interface *usbif, struct cp2615_i2c_transfer *i2c_w) { struct cp2615_iop_msg *msg = kzalloc_obj(*msg); + if (!msg) + return -ENOMEM; + struct usb_device *usbdev = interface_to_usbdev(usbif); int res = cp2615_init_i2c_msg(msg, i2c_w); @@ -172,6 +175,9 @@ cp2615_i2c_recv(struct usb_interface *usbif, unsigned char tag, void *buf) static int cp2615_check_iop(struct usb_interface *usbif) { struct cp2615_iop_msg *msg = kzalloc_obj(*msg); + if (!msg) + return -ENOMEM; + struct cp2615_iop_accessory_info *info = (struct cp2615_iop_accessory_info *)&msg->data; struct usb_device *usbdev = interface_to_usbdev(usbif); int res = cp2615_init_iop_msg(msg, iop_GetAccessoryInfo, NULL, 0); -- 2.53.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v2] i2c: cp2615: handle allocation failure 2026-08-16 16:17 [PATCH] i2c: cp2615: handle allocation failure Triet Hoang @ 2026-08-18 11:17 ` Triet Hoang 2026-08-19 6:28 ` Markus Elfring ` (2 more replies) 0 siblings, 3 replies; 9+ messages in thread From: Triet Hoang @ 2026-08-18 11:17 UTC (permalink / raw) To: triet.hoang.dev; +Cc: andi.shyti, bence98, linux-i2c, linux-kernel Check the result of kzalloc_obj() and return -ENOMEM when the allocation fails instead of passing a NULL pointer to the message initialization helpers, which would return -EINVAL. Signed-off-by: Triet Hoang <triet.hoang.dev@gmail.com> --- Changes in v2: - Clarify the commit message to describe the actual behavior change. - Fix coding style regression --- drivers/i2c/busses/i2c-cp2615.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/i2c/busses/i2c-cp2615.c b/drivers/i2c/busses/i2c-cp2615.c index 951de6249834..c1275fcad636 100644 --- a/drivers/i2c/busses/i2c-cp2615.c +++ b/drivers/i2c/busses/i2c-cp2615.c @@ -128,6 +128,9 @@ cp2615_i2c_send(struct usb_interface *usbif, struct cp2615_i2c_transfer *i2c_w) struct usb_device *usbdev = interface_to_usbdev(usbif); int res = cp2615_init_i2c_msg(msg, i2c_w); + if (!msg) + return -ENOMEM; + if (!res) res = usb_bulk_msg(usbdev, usb_sndbulkpipe(usbdev, IOP_EP_OUT), msg, ntohs(msg->length), NULL, 0); @@ -176,6 +179,9 @@ static int cp2615_check_iop(struct usb_interface *usbif) struct usb_device *usbdev = interface_to_usbdev(usbif); int res = cp2615_init_iop_msg(msg, iop_GetAccessoryInfo, NULL, 0); + if (!msg) + return -ENOMEM; + if (res) goto out; -- 2.53.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2] i2c: cp2615: handle allocation failure 2026-08-18 11:17 ` [PATCH v2] " Triet Hoang @ 2026-08-19 6:28 ` Markus Elfring 2026-08-28 12:08 ` Bence Csókás 2026-08-28 17:09 ` Andi Shyti 2 siblings, 0 replies; 9+ messages in thread From: Markus Elfring @ 2026-08-19 6:28 UTC (permalink / raw) To: Triet Hoang, linux-i2c, Andi Shyti, Bence Csókás Cc: LKML, kernel-janitors > Check the result of kzalloc_obj() and return -ENOMEM > when the allocation fails instead of passing a NULL pointer > to the message initialization helpers, which would return -EINVAL. * How do you think about to add any tags (like “Fixes” and “Cc”) accordingly? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2#n145 https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?h=v7.2#n34 * How do you think about to increase the application of scope-based resource management? Regards, Markus ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] i2c: cp2615: handle allocation failure 2026-08-18 11:17 ` [PATCH v2] " Triet Hoang 2026-08-19 6:28 ` Markus Elfring @ 2026-08-28 12:08 ` Bence Csókás 2026-08-28 17:09 ` Andi Shyti 2026-08-28 17:09 ` Andi Shyti 2 siblings, 1 reply; 9+ messages in thread From: Bence Csókás @ 2026-08-28 12:08 UTC (permalink / raw) To: Triet Hoang Cc: andi.shyti, linux-i2c, linux-kernel, Markus Elfring, kernel-janitors Hi, On 8/18/26 13:17, Triet Hoang wrote: > Check the result of kzalloc_obj() and return -ENOMEM > when the allocation fails instead of passing a NULL pointer > to the message initialization helpers, which would return -EINVAL. > > Signed-off-by: Triet Hoang <triet.hoang.dev@gmail.com> Just change the -EINVAL to -ENOMEM. This is something which has been discussed before [1] as a potential to-do. [1] https://lore.kernel.org/lkml/CACCVKEFXOKhXmF3rcmKQ8-aMFZJLZOf+imsuBYKAQsWptrJG6Q@mail.gmail.com/ On 8/19/26 08:28, Markus Elfring wrote: > * How do you think about to increase the application of scope-based resource management? I second this, this old code could use some RAII modernization. Bence ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] i2c: cp2615: handle allocation failure 2026-08-28 12:08 ` Bence Csókás @ 2026-08-28 17:09 ` Andi Shyti 2026-08-28 20:12 ` Bence Csókás 0 siblings, 1 reply; 9+ messages in thread From: Andi Shyti @ 2026-08-28 17:09 UTC (permalink / raw) To: Bence Csókás Cc: Triet Hoang, linux-i2c, linux-kernel, Markus Elfring, kernel-janitors On Fri, Aug 28, 2026 at 02:08:19PM +0200, Bence Csókás wrote: > On 8/18/26 13:17, Triet Hoang wrote: > > Check the result of kzalloc_obj() and return -ENOMEM > > when the allocation fails instead of passing a NULL pointer > > to the message initialization helpers, which would return -EINVAL. > > > > Signed-off-by: Triet Hoang <triet.hoang.dev@gmail.com> > Just change the -EINVAL to -ENOMEM. This is something which has been > discussed before [1] as a potential to-do. > > [1] https://lore.kernel.org/lkml/CACCVKEFXOKhXmF3rcmKQ8-aMFZJLZOf+imsuBYKAQsWptrJG6Q@mail.gmail.com/ > > On 8/19/26 08:28, Markus Elfring wrote: > > * How do you think about to increase the application of scope-based > resource management? > > I second this, this old code could use some RAII modernization. is there a managed version for kzalloc_obj()? Andi ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] i2c: cp2615: handle allocation failure 2026-08-28 17:09 ` Andi Shyti @ 2026-08-28 20:12 ` Bence Csókás 0 siblings, 0 replies; 9+ messages in thread From: Bence Csókás @ 2026-08-28 20:12 UTC (permalink / raw) To: Andi Shyti Cc: Triet Hoang, linux-i2c, linux-kernel, Markus Elfring, kernel-janitors, Bence Csókás Hi, On 8/28/26 19:09, Andi Shyti wrote: > On Fri, Aug 28, 2026 at 02:08:19PM +0200, Bence Csókás wrote: >> On 8/19/26 08:28, Markus Elfring wrote: >>> * How do you think about to increase the application of scope-based >> resource management? >> >> I second this, this old code could use some RAII modernization. > > is there a managed version for kzalloc_obj()? Well I'm thinking, since msg's lifetime is only within the functions, it can be attributed with `__free(kfree)`. Anyways, in the curent state of this patch, I'm afraid I must NAK it. Bence ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] i2c: cp2615: handle allocation failure 2026-08-18 11:17 ` [PATCH v2] " Triet Hoang 2026-08-19 6:28 ` Markus Elfring 2026-08-28 12:08 ` Bence Csókás @ 2026-08-28 17:09 ` Andi Shyti 2026-08-28 20:40 ` Christophe JAILLET 2026-08-29 3:42 ` Triet Hoang 2 siblings, 2 replies; 9+ messages in thread From: Andi Shyti @ 2026-08-28 17:09 UTC (permalink / raw) To: Triet Hoang; +Cc: bence98, linux-i2c, linux-kernel Hi Triet, thank you for your patch. On Tue, Aug 18, 2026 at 11:17:01AM +0000, Triet Hoang wrote: > Check the result of kzalloc_obj() and return -ENOMEM > when the allocation fails instead of passing a NULL pointer > to the message initialization helpers, which would return -EINVAL. > > Signed-off-by: Triet Hoang <triet.hoang.dev@gmail.com> > --- > Changes in v2: > - Clarify the commit message to describe the actual behavior change. > - Fix coding style regression Next time please don't send your v2 as in reply to v1. It confuses me. > --- > drivers/i2c/busses/i2c-cp2615.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/i2c/busses/i2c-cp2615.c b/drivers/i2c/busses/i2c-cp2615.c > index 951de6249834..c1275fcad636 100644 > --- a/drivers/i2c/busses/i2c-cp2615.c > +++ b/drivers/i2c/busses/i2c-cp2615.c > @@ -128,6 +128,9 @@ cp2615_i2c_send(struct usb_interface *usbif, struct cp2615_i2c_transfer *i2c_w) > struct usb_device *usbdev = interface_to_usbdev(usbif); > int res = cp2615_init_i2c_msg(msg, i2c_w); > > + if (!msg) > + return -ENOMEM; > + Your patch looks good, but, as you are at it, can I ask you here a little effort? Personally I don't like and I find unreadable the form: struct cp2615_iop_msg *msg = kzalloc_obj(*msg); ... if (!msg) return -ENOMEM. Important assignments, like kzalloc_*(), shouldn't be made during declaration. I prefer the form: struct cp2615_iop_msg *msg; ... msg = kzalloc_obj(*msg); if (!msg) return -ENOMEM. Works for you? Do you mind updating in v3? Thanks, Andi > if (!res) > res = usb_bulk_msg(usbdev, usb_sndbulkpipe(usbdev, IOP_EP_OUT), > msg, ntohs(msg->length), NULL, 0); ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] i2c: cp2615: handle allocation failure 2026-08-28 17:09 ` Andi Shyti @ 2026-08-28 20:40 ` Christophe JAILLET 2026-08-29 3:42 ` Triet Hoang 1 sibling, 0 replies; 9+ messages in thread From: Christophe JAILLET @ 2026-08-28 20:40 UTC (permalink / raw) To: Andi Shyti, Triet Hoang; +Cc: bence98, linux-i2c, linux-kernel Le 28/08/2026 à 19:09, Andi Shyti a écrit : > Hi Triet, > > thank you for your patch. > > On Tue, Aug 18, 2026 at 11:17:01AM +0000, Triet Hoang wrote: >> Check the result of kzalloc_obj() and return -ENOMEM >> when the allocation fails instead of passing a NULL pointer >> to the message initialization helpers, which would return -EINVAL. >> >> Signed-off-by: Triet Hoang <triet.hoang.dev@gmail.com> >> --- >> Changes in v2: >> - Clarify the commit message to describe the actual behavior change. >> - Fix coding style regression > > Next time please don't send your v2 as in reply to v1. It > confuses me. > >> --- >> drivers/i2c/busses/i2c-cp2615.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/drivers/i2c/busses/i2c-cp2615.c b/drivers/i2c/busses/i2c-cp2615.c >> index 951de6249834..c1275fcad636 100644 >> --- a/drivers/i2c/busses/i2c-cp2615.c >> +++ b/drivers/i2c/busses/i2c-cp2615.c >> @@ -128,6 +128,9 @@ cp2615_i2c_send(struct usb_interface *usbif, struct cp2615_i2c_transfer *i2c_w) >> struct usb_device *usbdev = interface_to_usbdev(usbif); >> int res = cp2615_init_i2c_msg(msg, i2c_w); >> >> + if (!msg) >> + return -ENOMEM; >> + > > Your patch looks good, but, as you are at it, can I ask you here Yes, the patch looks good (and the proposal below even better), but is not a must have. In both cases, the error handling ends being done at [1]. This is not obvious at all when reading the code, but looks safe. Just my 2c. CJ [1]: https://elixir.bootlin.com/linux/v7.2/source/drivers/i2c/busses/i2c-cp2615.c#L85 > a little effort? Personally I don't like and I find unreadable > the form: > > struct cp2615_iop_msg *msg = kzalloc_obj(*msg); > ... > > if (!msg) > return -ENOMEM. > > Important assignments, like kzalloc_*(), shouldn't be made during > declaration. I prefer the form: > > > struct cp2615_iop_msg *msg; > ... > > msg = kzalloc_obj(*msg); > if (!msg) > return -ENOMEM. > > Works for you? Do you mind updating in v3? > > Thanks, > Andi > >> if (!res) >> res = usb_bulk_msg(usbdev, usb_sndbulkpipe(usbdev, IOP_EP_OUT), >> msg, ntohs(msg->length), NULL, 0); > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] i2c: cp2615: handle allocation failure 2026-08-28 17:09 ` Andi Shyti 2026-08-28 20:40 ` Christophe JAILLET @ 2026-08-29 3:42 ` Triet Hoang 1 sibling, 0 replies; 9+ messages in thread From: Triet Hoang @ 2026-08-29 3:42 UTC (permalink / raw) To: andi.shyti; +Cc: bence98, linux-i2c, linux-kernel, triet.hoang.dev On Fri, 28 Aug 2026 19:09:03 +0200 Andi Shyti wrote: > Next time please don't send your v2 as in reply to v1. It > confuses me. Sorry about that. Will keep it in my mind! > Your patch looks good, but, as you are at it, can I ask you here > a little effort? Personally I don't like and I find unreadable > the form: > > struct cp2615_iop_msg *msg = kzalloc_obj(*msg); > ... > > if (!msg) > return -ENOMEM. > > Important assignments, like kzalloc_*(), shouldn't be made during > declaration. I prefer the form: > > > struct cp2615_iop_msg *msg; > ... > > msg = kzalloc_obj(*msg); > if (!msg) > return -ENOMEM. > > Works for you? Do you mind updating in v3? Sure, no problem. I will update and send the v3. Thanks for your reviewing and suggestion! Regards, Triet ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-08-29 3:42 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-08-16 16:17 [PATCH] i2c: cp2615: handle allocation failure Triet Hoang 2026-08-18 11:17 ` [PATCH v2] " Triet Hoang 2026-08-19 6:28 ` Markus Elfring 2026-08-28 12:08 ` Bence Csókás 2026-08-28 17:09 ` Andi Shyti 2026-08-28 20:12 ` Bence Csókás 2026-08-28 17:09 ` Andi Shyti 2026-08-28 20:40 ` Christophe JAILLET 2026-08-29 3:42 ` Triet Hoang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox