2021-09-11 10:34:38

by Len Baker

[permalink] [raw]
Subject: [PATCH] net: mana: Prefer struct_size over open coded arithmetic

As noted in the "Deprecated Interfaces, Language Features, Attributes,
and Conventions" documentation [1], size calculations (especially
multiplication) should not be performed in memory allocator (or similar)
function arguments due to the risk of them overflowing. This could lead
to values wrapping around and a smaller allocation being made than the
caller was expecting. Using those allocations could lead to linear
overflows of heap memory and other misbehaviors.

So, use the struct_size() helper to do the arithmetic instead of the
argument "size + count * size" in the kzalloc() function.

[1] https://www.kernel.org/doc/html/v5.14/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments

Signed-off-by: Len Baker <[email protected]>
---
drivers/net/ethernet/microsoft/mana/hw_channel.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 1a923fd99990..0efdc6c3c32a 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -398,9 +398,7 @@ static int mana_hwc_alloc_dma_buf(struct hw_channel_context *hwc, u16 q_depth,
int err;
u16 i;

- dma_buf = kzalloc(sizeof(*dma_buf) +
- q_depth * sizeof(struct hwc_work_request),
- GFP_KERNEL);
+ dma_buf = kzalloc(struct_size(dma_buf, reqs, q_depth), GFP_KERNEL);
if (!dma_buf)
return -ENOMEM;

--
2.25.1


2021-09-11 13:37:52

by Haiyang Zhang

[permalink] [raw]
Subject: RE: [PATCH] net: mana: Prefer struct_size over open coded arithmetic



> -----Original Message-----
> From: Len Baker <[email protected]>
> Sent: Saturday, September 11, 2021 6:28 AM
> To: KY Srinivasan <[email protected]>; Haiyang Zhang
> <[email protected]>; Stephen Hemminger <[email protected]>;
> Wei Liu <[email protected]>; Dexuan Cui <[email protected]>; David S.
> Miller <[email protected]>; Jakub Kicinski <[email protected]>; Sumit
> Semwal <[email protected]>; Christian K?nig
> <[email protected]>; Kees Cook <[email protected]>
> Cc: Len Baker <[email protected]>; Colin Ian King
> <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; linux-
> [email protected]; [email protected]; dri-
> [email protected]; [email protected]
> Subject: [PATCH] net: mana: Prefer struct_size over open coded
> arithmetic
>
> [Some people who received this message don't often get email from
> [email protected]. Learn why this is important at
> http://aka.ms/LearnAboutSenderIdentification.]
>
> As noted in the "Deprecated Interfaces, Language Features, Attributes,
> and Conventions" documentation [1], size calculations (especially
> multiplication) should not be performed in memory allocator (or similar)
> function arguments due to the risk of them overflowing. This could lead
> to values wrapping around and a smaller allocation being made than the
> caller was expecting. Using those allocations could lead to linear
> overflows of heap memory and other misbehaviors.
>
> So, use the struct_size() helper to do the arithmetic instead of the
> argument "size + count * size" in the kzalloc() function.
>
> [1]
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.ke
> rnel.org%2Fdoc%2Fhtml%2Fv5.14%2Fprocess%2Fdeprecated.html%23open-coded-
> arithmetic-in-allocator-
> arguments&amp;data=04%7C01%7Chaiyangz%40microsoft.com%7C1bf83c1204a34dae
> a6d308d9750eef16%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C6376695297
> 12931146%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJ
> BTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=PbYpBtyYfVfwwlxWSQx%2FiARc9
> mhb0J7bfD46%2F9q5oTw%3D&amp;reserved=0
>
> Signed-off-by: Len Baker <[email protected]>
> ---
> drivers/net/ethernet/microsoft/mana/hw_channel.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index 1a923fd99990..0efdc6c3c32a 100644
> --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> @@ -398,9 +398,7 @@ static int mana_hwc_alloc_dma_buf(struct
> hw_channel_context *hwc, u16 q_depth,
> int err;
> u16 i;
>
> - dma_buf = kzalloc(sizeof(*dma_buf) +
> - q_depth * sizeof(struct hwc_work_request),
> - GFP_KERNEL);
> + dma_buf = kzalloc(struct_size(dma_buf, reqs, q_depth),
> GFP_KERNEL);

Thanks!

Reviewed-by: Haiyang Zhang <[email protected]>

2021-09-19 02:52:14

by Len Baker

[permalink] [raw]
Subject: Re: [PATCH] net: mana: Prefer struct_size over open coded arithmetic

Hi,

On Sat, Sep 11, 2021 at 12:28:18PM +0200, Len Baker wrote:
> As noted in the "Deprecated Interfaces, Language Features, Attributes,
> and Conventions" documentation [1], size calculations (especially
> multiplication) should not be performed in memory allocator (or similar)
> function arguments due to the risk of them overflowing. This could lead
> to values wrapping around and a smaller allocation being made than the
> caller was expecting. Using those allocations could lead to linear
> overflows of heap memory and other misbehaviors.
>
> So, use the struct_size() helper to do the arithmetic instead of the
> argument "size + count * size" in the kzalloc() function.
>
> [1] https://www.kernel.org/doc/html/v5.14/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments
>
> Signed-off-by: Len Baker <[email protected]>
> ---
> drivers/net/ethernet/microsoft/mana/hw_channel.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> index 1a923fd99990..0efdc6c3c32a 100644
> --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> @@ -398,9 +398,7 @@ static int mana_hwc_alloc_dma_buf(struct hw_channel_context *hwc, u16 q_depth,
> int err;
> u16 i;
>
> - dma_buf = kzalloc(sizeof(*dma_buf) +
> - q_depth * sizeof(struct hwc_work_request),
> - GFP_KERNEL);
> + dma_buf = kzalloc(struct_size(dma_buf, reqs, q_depth), GFP_KERNEL);
> if (!dma_buf)
> return -ENOMEM;
>
> --
> 2.25.1
>

I have received a email from the linux-media subsystem telling that this
patch is not applicable. The email is the following:

Hello,

The following patch (submitted by you) has been updated in Patchwork:

* linux-media: net: mana: Prefer struct_size over open coded arithmetic
- http://patchwork.linuxtv.org/project/linux-media/patch/[email protected]/
- for: Linux Media kernel patches
was: New
now: Not Applicable

This email is a notification only - you do not need to respond.

The question is: Why it is not applicable?. I have no received any bad comment
and a "Reviewed-by:" tag from Haiyang Zhang. So, what is the reason for the
"Not Applicable" state?.

Regards,
Len

2021-09-19 06:45:29

by Kees Cook

[permalink] [raw]
Subject: Re: [PATCH] net: mana: Prefer struct_size over open coded arithmetic



On September 18, 2021 6:20:10 AM PDT, Len Baker <[email protected]> wrote:
>Hi,
>
>On Sat, Sep 11, 2021 at 12:28:18PM +0200, Len Baker wrote:
>> As noted in the "Deprecated Interfaces, Language Features, Attributes,
>> and Conventions" documentation [1], size calculations (especially
>> multiplication) should not be performed in memory allocator (or similar)
>> function arguments due to the risk of them overflowing. This could lead
>> to values wrapping around and a smaller allocation being made than the
>> caller was expecting. Using those allocations could lead to linear
>> overflows of heap memory and other misbehaviors.
>>
>> So, use the struct_size() helper to do the arithmetic instead of the
>> argument "size + count * size" in the kzalloc() function.
>>
>> [1] https://www.kernel.org/doc/html/v5.14/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments
>>
>> Signed-off-by: Len Baker <[email protected]>
>> ---
>> drivers/net/ethernet/microsoft/mana/hw_channel.c | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
>> index 1a923fd99990..0efdc6c3c32a 100644
>> --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
>> +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
>> @@ -398,9 +398,7 @@ static int mana_hwc_alloc_dma_buf(struct hw_channel_context *hwc, u16 q_depth,
>> int err;
>> u16 i;
>>
>> - dma_buf = kzalloc(sizeof(*dma_buf) +
>> - q_depth * sizeof(struct hwc_work_request),
>> - GFP_KERNEL);
>> + dma_buf = kzalloc(struct_size(dma_buf, reqs, q_depth), GFP_KERNEL);
>> if (!dma_buf)
>> return -ENOMEM;
>>
>> --
>> 2.25.1
>>
>
>I have received a email from the linux-media subsystem telling that this
>patch is not applicable. The email is the following:
>
>Hello,
>
>The following patch (submitted by you) has been updated in Patchwork:
>
> * linux-media: net: mana: Prefer struct_size over open coded arithmetic
> - http://patchwork.linuxtv.org/project/linux-media/patch/[email protected]/
> - for: Linux Media kernel patches
> was: New
> now: Not Applicable
>
>This email is a notification only - you do not need to respond.
>
>The question is: Why it is not applicable?. I have no received any bad comment
>and a "Reviewed-by:" tag from Haiyang Zhang. So, what is the reason for the
>"Not Applicable" state?.

That is the "Media" subsystem patch tracker. The patch appears to be for networking, so the Media tracker has marked it as "not applicable [to the media subsystem]".

The CC list for this patch seems rather wide (media, dri). I would have expected only netdev. Were you using scripts/get_maintainer.pl for getting addresses?

-Kees

2021-09-19 06:49:38

by Dexuan Cui

[permalink] [raw]
Subject: RE: [PATCH] net: mana: Prefer struct_size over open coded arithmetic

> From: Len Baker <[email protected]>
> Sent: Saturday, September 18, 2021 6:20 AM
> ...
> I have received a email from the linux-media subsystem telling that this
> patch is not applicable. The email is the following:
>
> Regards,
> Len

The patch is already in the net-next tree:
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=f11ee2ad25b22c2ee587045dd6999434375532f7

2021-09-19 09:05:27

by Len Baker

[permalink] [raw]
Subject: Re: [PATCH] net: mana: Prefer struct_size over open coded arithmetic

Hi Kees,

On Sat, Sep 18, 2021 at 06:51:51AM -0700, Kees Cook wrote:
>
>
> On September 18, 2021 6:20:10 AM PDT, Len Baker <[email protected]> wrote:
> >Hi,
> >
> >On Sat, Sep 11, 2021 at 12:28:18PM +0200, Len Baker wrote:
> >> As noted in the "Deprecated Interfaces, Language Features, Attributes,
> >> and Conventions" documentation [1], size calculations (especially
> >> multiplication) should not be performed in memory allocator (or similar)
> >> function arguments due to the risk of them overflowing. This could lead
> >> to values wrapping around and a smaller allocation being made than the
> >> caller was expecting. Using those allocations could lead to linear
> >> overflows of heap memory and other misbehaviors.
> >>
> >> So, use the struct_size() helper to do the arithmetic instead of the
> >> argument "size + count * size" in the kzalloc() function.
> >>
> >> [1] https://www.kernel.org/doc/html/v5.14/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments
> >>
> >> Signed-off-by: Len Baker <[email protected]>
> >> ---
> >> drivers/net/ethernet/microsoft/mana/hw_channel.c | 4 +---
> >> 1 file changed, 1 insertion(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> >> index 1a923fd99990..0efdc6c3c32a 100644
> >> --- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
> >> +++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
> >> @@ -398,9 +398,7 @@ static int mana_hwc_alloc_dma_buf(struct hw_channel_context *hwc, u16 q_depth,
> >> int err;
> >> u16 i;
> >>
> >> - dma_buf = kzalloc(sizeof(*dma_buf) +
> >> - q_depth * sizeof(struct hwc_work_request),
> >> - GFP_KERNEL);
> >> + dma_buf = kzalloc(struct_size(dma_buf, reqs, q_depth), GFP_KERNEL);
> >> if (!dma_buf)
> >> return -ENOMEM;
> >>
> >> --
> >> 2.25.1
> >>
> >
> >I have received a email from the linux-media subsystem telling that this
> >patch is not applicable. The email is the following:
> >
> >Hello,
> >
> >The following patch (submitted by you) has been updated in Patchwork:
> >
> > * linux-media: net: mana: Prefer struct_size over open coded arithmetic
> > - http://patchwork.linuxtv.org/project/linux-media/patch/[email protected]/
> > - for: Linux Media kernel patches
> > was: New
> > now: Not Applicable
> >
> >This email is a notification only - you do not need to respond.
> >
> >The question is: Why it is not applicable?. I have no received any bad comment
> >and a "Reviewed-by:" tag from Haiyang Zhang. So, what is the reason for the
> >"Not Applicable" state?.
>
> That is the "Media" subsystem patch tracker. The patch appears to be for networking, so the Media tracker has marked it as "not applicable [to the media subsystem]".
>
> The CC list for this patch seems rather wide (media, dri). I would have expected only netdev. Were you using scripts/get_maintainer.pl for getting addresses?

Yes, my workflow is scripts/checkpatch.pl and then scripts/get_maintainer.pl
before sending any patch :)

Regards,
Len
>
> -Kees

2021-09-19 12:57:27

by Len Baker

[permalink] [raw]
Subject: Re: [PATCH] net: mana: Prefer struct_size over open coded arithmetic

Hi Dexuan,

On Sat, Sep 18, 2021 at 05:06:16PM +0000, Dexuan Cui wrote:
> > From: Len Baker <[email protected]>
> > Sent: Saturday, September 18, 2021 6:20 AM
> > ...
> > I have received a email from the linux-media subsystem telling that this
> > patch is not applicable. The email is the following:
> >
> > Regards,
> > Len
>
> The patch is already in the net-next tree:
> https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=f11ee2ad25b22c2ee587045dd6999434375532f7

Thanks for the info.

Regards,
Len