diff mbox series

net: mana: Prefer struct_size over open coded arithmetic

Message ID 20210911102818.3804-1-len.baker@gmx.com (mailing list archive)
State Handled Elsewhere
Headers show
Series net: mana: Prefer struct_size over open coded arithmetic | expand

Commit Message

Len Baker Sept. 11, 2021, 10:28 a.m. UTC 
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 <len.baker@gmx.com>
---
 drivers/net/ethernet/microsoft/mana/hw_channel.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

--
2.25.1

Comments

Haiyang Zhang Sept. 11, 2021, 1:36 p.m. UTC | #1 
> -----Original Message-----
> From: Len Baker <len.baker@gmx.com>
> Sent: Saturday, September 11, 2021 6:28 AM
> To: KY Srinivasan <kys@microsoft.com>; Haiyang Zhang
> <haiyangz@microsoft.com>; Stephen Hemminger <sthemmin@microsoft.com>;
> Wei Liu <wei.liu@kernel.org>; Dexuan Cui <decui@microsoft.com>; David S.
> Miller <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Sumit
> Semwal <sumit.semwal@linaro.org>; Christian König
> <christian.koenig@amd.com>; Kees Cook <keescook@chromium.org>
> Cc: Len Baker <len.baker@gmx.com>; Colin Ian King
> <colin.king@canonical.com>; linux-hardening@vger.kernel.org; linux-
> hyperv@vger.kernel.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-media@vger.kernel.org; dri-
> devel@lists.freedesktop.org; linaro-mm-sig@lists.linaro.org
> Subject: [PATCH] net: mana: Prefer struct_size over open coded
> arithmetic
> 
> [Some people who received this message don't often get email from
> len.baker@gmx.com. 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 <len.baker@gmx.com>
> ---
>  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 <haiyangz@microsoft.com>
Len Baker Sept. 18, 2021, 1:20 p.m. UTC | #2 
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 <len.baker@gmx.com>
> ---
>  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/20210911102818.3804-1-len.baker@gmx.com/
     - 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
Kees Cook Sept. 18, 2021, 1:51 p.m. UTC | #3 
On September 18, 2021 6:20:10 AM PDT, Len Baker <len.baker@gmx.com> 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 <len.baker@gmx.com>
>> ---
>>  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/20210911102818.3804-1-len.baker@gmx.com/
>     - 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
Dexuan Cui Sept. 18, 2021, 5:06 p.m. UTC | #4 
> From: Len Baker <len.baker@gmx.com>
> 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
Len Baker Sept. 18, 2021, 5:11 p.m. UTC | #5 
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 <len.baker@gmx.com> 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 <len.baker@gmx.com>
> >> ---
> >>  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/20210911102818.3804-1-len.baker@gmx.com/
> >     - 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
Len Baker Sept. 19, 2021, 8:27 a.m. UTC | #6 
Hi Dexuan,

On Sat, Sep 18, 2021 at 05:06:16PM +0000, Dexuan Cui wrote:
> > From: Len Baker <len.baker@gmx.com>
> > 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
diff mbox series

Patch

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;