Message ID | 20230623211457.102544-3-Julia.Lawall@inria.fr (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | use array_size | expand |
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>
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?
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/
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
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
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 > >
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);
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(-)