[02/26] octeon_ep: use array_size

Message ID 20230623211457.102544-3-Julia.Lawall@inria.fr
State New
Headers
Series use array_size |

Commit Message

Julia Lawall June 23, 2023, 9:14 p.m. UTC
  Use array_size to protect against multiplication overflows.

The changes were done using the following Coccinelle semantic patch:

// <smpl>
@@
    expression E1, E2;
    constant C1, C2;
    identifier alloc = {vmalloc,vzalloc};
@@
    
(
      alloc(C1 * C2,...)
|
      alloc(
-           (E1) * (E2)
+           array_size(E1, E2)
      ,...)
)
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@inria.fr>

---
 drivers/net/ethernet/marvell/octeon_ep/octep_rx.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Simon Horman June 24, 2023, 3:46 p.m. UTC | #1
On Fri, Jun 23, 2023 at 11:14:33PM +0200, Julia Lawall wrote:
> Use array_size to protect against multiplication overflows.
> 
> The changes were done using the following Coccinelle semantic patch:
> 
> // <smpl>
> @@
>     expression E1, E2;
>     constant C1, C2;
>     identifier alloc = {vmalloc,vzalloc};
> @@
>     
> (
>       alloc(C1 * C2,...)
> |
>       alloc(
> -           (E1) * (E2)
> +           array_size(E1, E2)
>       ,...)
> )
> // </smpl>
> 
> Signed-off-by: Julia Lawall <Julia.Lawall@inria.fr>

Reviewed-by: Simon Horman <simon.horman@corigine.com>
  
Jakub Kicinski June 24, 2023, 10:28 p.m. UTC | #2
On Fri, 23 Jun 2023 23:14:33 +0200 Julia Lawall wrote:
> -	oq->buff_info = vzalloc(oq->max_count * OCTEP_OQ_RECVBUF_SIZE);
> +	oq->buff_info = vzalloc(array_size(oq->max_count, OCTEP_OQ_RECVBUF_SIZE));

vcalloc seems to exist, is there a reason array_size() is preferred?
  
Christophe JAILLET June 25, 2023, 8:14 p.m. UTC | #3
Le 25/06/2023 à 00:28, Jakub Kicinski a écrit :
> On Fri, 23 Jun 2023 23:14:33 +0200 Julia Lawall wrote:
>> -	oq->buff_info = vzalloc(oq->max_count * OCTEP_OQ_RECVBUF_SIZE);
>> +	oq->buff_info = vzalloc(array_size(oq->max_count, OCTEP_OQ_RECVBUF_SIZE));
> 
> vcalloc seems to exist, is there a reason array_size() is preferred?

Hi,

just for your information, I've just sent [1].

CJ

[1]: 
https://lore.kernel.org/all/3484e46180dd2cf05d993ff1a78b481bc2ad1f71.1687723931.git.christophe.jaillet@wanadoo.fr/
  
Julia Lawall June 25, 2023, 8:25 p.m. UTC | #4
On Sun, 25 Jun 2023, Christophe JAILLET wrote:

> Le 25/06/2023 à 00:28, Jakub Kicinski a écrit :
> > On Fri, 23 Jun 2023 23:14:33 +0200 Julia Lawall wrote:
> > > -	oq->buff_info = vzalloc(oq->max_count * OCTEP_OQ_RECVBUF_SIZE);
> > > +	oq->buff_info = vzalloc(array_size(oq->max_count,
> > > OCTEP_OQ_RECVBUF_SIZE));
> >
> > vcalloc seems to exist, is there a reason array_size() is preferred?
>
> Hi,
>
> just for your information, I've just sent [1].
>
> CJ
>
> [1]:
> https://lore.kernel.org/all/3484e46180dd2cf05d993ff1a78b481bc2ad1f71.1687723931.git.christophe.jaillet@wanadoo.fr/

For some reason, I have only received Christophe's mail, not Jakub's...

In any case, thanks for pointing out the existence of these functions.  I
just redid what Kees did in 2018, when I guess these functions didn't
exist.  I will look more carefully to see what functions are now available
and resend the whole thing.

Thanks!

julia
  
Christophe JAILLET June 25, 2023, 8:32 p.m. UTC | #5
Le 25/06/2023 à 22:25, Julia Lawall a écrit :
> 
> 
> On Sun, 25 Jun 2023, Christophe JAILLET wrote:
> 
>> Le 25/06/2023 à 00:28, Jakub Kicinski a écrit :
>>> On Fri, 23 Jun 2023 23:14:33 +0200 Julia Lawall wrote:
>>>> -	oq->buff_info = vzalloc(oq->max_count * OCTEP_OQ_RECVBUF_SIZE);
>>>> +	oq->buff_info = vzalloc(array_size(oq->max_count,
>>>> OCTEP_OQ_RECVBUF_SIZE));
>>>
>>> vcalloc seems to exist, is there a reason array_size() is preferred?
>>
>> Hi,
>>
>> just for your information, I've just sent [1].
>>
>> CJ
>>
>> [1]:
>> https://lore.kernel.org/all/3484e46180dd2cf05d993ff1a78b481bc2ad1f71.1687723931.git.christophe.jaillet@wanadoo.fr/
> 
> For some reason, I have only received Christophe's mail, not Jakub's...
> 
> In any case, thanks for pointing out the existence of these functions.  I
> just redid what Kees did in 2018, when I guess these functions didn't
> exist.  I will look more carefully to see what functions are now available
> and resend the whole thing.

Hi,

should you want to go 1 step further and simplify some code:

git grep v[mz]alloc.*array_size\( | wc -l
174

CJ

> 
> Thanks!
> 
> julia
  
Julia Lawall June 25, 2023, 8:57 p.m. UTC | #6
On Sun, 25 Jun 2023, Christophe JAILLET wrote:

> Le 25/06/2023 à 22:25, Julia Lawall a écrit :
> >
> >
> > On Sun, 25 Jun 2023, Christophe JAILLET wrote:
> >
> > > Le 25/06/2023 à 00:28, Jakub Kicinski a écrit :
> > > > On Fri, 23 Jun 2023 23:14:33 +0200 Julia Lawall wrote:
> > > > > -	oq->buff_info = vzalloc(oq->max_count *
> > > > > OCTEP_OQ_RECVBUF_SIZE);
> > > > > +	oq->buff_info = vzalloc(array_size(oq->max_count,
> > > > > OCTEP_OQ_RECVBUF_SIZE));
> > > >
> > > > vcalloc seems to exist, is there a reason array_size() is preferred?
> > >
> > > Hi,
> > >
> > > just for your information, I've just sent [1].
> > >
> > > CJ
> > >
> > > [1]:
> > > https://lore.kernel.org/all/3484e46180dd2cf05d993ff1a78b481bc2ad1f71.1687723931.git.christophe.jaillet@wanadoo.fr/
> >
> > For some reason, I have only received Christophe's mail, not Jakub's...
> >
> > In any case, thanks for pointing out the existence of these functions.  I
> > just redid what Kees did in 2018, when I guess these functions didn't
> > exist.  I will look more carefully to see what functions are now available
> > and resend the whole thing.
>
> Hi,
>
> should you want to go 1 step further and simplify some code:
>
> git grep v[mz]alloc.*array_size\( | wc -l
> 174

Yes, thanks for the suggestion.

julia

>
> CJ
>
> >
> > Thanks!
> >
> > julia
>
>
  

Patch

diff --git a/drivers/net/ethernet/marvell/octeon_ep/octep_rx.c b/drivers/net/ethernet/marvell/octeon_ep/octep_rx.c
index 392d9b0da0d7..185b7e50ee77 100644
--- a/drivers/net/ethernet/marvell/octeon_ep/octep_rx.c
+++ b/drivers/net/ethernet/marvell/octeon_ep/octep_rx.c
@@ -158,7 +158,7 @@  static int octep_setup_oq(struct octep_device *oct, int q_no)
 		goto desc_dma_alloc_err;
 	}
 
-	oq->buff_info = vzalloc(oq->max_count * OCTEP_OQ_RECVBUF_SIZE);
+	oq->buff_info = vzalloc(array_size(oq->max_count, OCTEP_OQ_RECVBUF_SIZE));
 	if (unlikely(!oq->buff_info)) {
 		dev_err(&oct->pdev->dev,
 			"Failed to allocate buffer info for OQ-%d\n", q_no);