Linux I2C development
 help / color / mirror / Atom feed
* [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-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 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-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