| Message ID | 20230623211457.102544-24-Julia.Lawall@inria.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | use array_size | expand |
Julia, Thanks for your patch. On 6/24/23 5:14 AM, 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> > > --- > drivers/staging/media/ipu3/ipu3-mmu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/media/ipu3/ipu3-mmu.c b/drivers/staging/media/ipu3/ipu3-mmu.c > index cb9bf5fb29a5..9c4adb815c94 100644 > --- a/drivers/staging/media/ipu3/ipu3-mmu.c > +++ b/drivers/staging/media/ipu3/ipu3-mmu.c > @@ -464,7 +464,7 @@ struct imgu_mmu_info *imgu_mmu_init(struct device *parent, void __iomem *base) > * Allocate the array of L2PT CPU pointers, initialized to zero, > * which means the dummy L2PT allocated above. > */ > - mmu->l2pts = vzalloc(IPU3_PT_PTES * sizeof(*mmu->l2pts)); > + mmu->l2pts = vzalloc(array_size(IPU3_PT_PTES, sizeof(*mmu->l2pts))); > if (!mmu->l2pts) > goto fail_l2pt; > > Reviewed-by: Bingbu Cao <bingbu.cao@intel.com>
On Fri, 23 Jun 2023, 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> > > --- > drivers/staging/media/ipu3/ipu3-mmu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/staging/media/ipu3/ipu3-mmu.c b/drivers/staging/media/ipu3/ipu3-mmu.c > index cb9bf5fb29a5..9c4adb815c94 100644 > --- a/drivers/staging/media/ipu3/ipu3-mmu.c > +++ b/drivers/staging/media/ipu3/ipu3-mmu.c > @@ -464,7 +464,7 @@ struct imgu_mmu_info *imgu_mmu_init(struct device *parent, void __iomem *base) > * Allocate the array of L2PT CPU pointers, initialized to zero, > * which means the dummy L2PT allocated above. > */ > - mmu->l2pts = vzalloc(IPU3_PT_PTES * sizeof(*mmu->l2pts)); > + mmu->l2pts = vzalloc(array_size(IPU3_PT_PTES, sizeof(*mmu->l2pts))); > if (!mmu->l2pts) > goto fail_l2pt; I think that this patch can be dropped. Since it is a multiplcation of two constants, if there is an overflow, I guess the compiler would detect it? julia
Hi Julia, Bingbu, On Tue, Jun 27, 2023 at 07:35:47PM +0200, Julia Lawall wrote: > > > On Fri, 23 Jun 2023, 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> > > > > --- > > drivers/staging/media/ipu3/ipu3-mmu.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/staging/media/ipu3/ipu3-mmu.c b/drivers/staging/media/ipu3/ipu3-mmu.c > > index cb9bf5fb29a5..9c4adb815c94 100644 > > --- a/drivers/staging/media/ipu3/ipu3-mmu.c > > +++ b/drivers/staging/media/ipu3/ipu3-mmu.c > > @@ -464,7 +464,7 @@ struct imgu_mmu_info *imgu_mmu_init(struct device *parent, void __iomem *base) > > * Allocate the array of L2PT CPU pointers, initialized to zero, > > * which means the dummy L2PT allocated above. > > */ > > - mmu->l2pts = vzalloc(IPU3_PT_PTES * sizeof(*mmu->l2pts)); > > + mmu->l2pts = vzalloc(array_size(IPU3_PT_PTES, sizeof(*mmu->l2pts))); > > if (!mmu->l2pts) > > goto fail_l2pt; > > I think that this patch can be dropped. Since it is a multiplcation of > two constants, if there is an overflow, I guess the compiler would detect > it? Indeed. vcalloc() would be perhaps nicer but the original isn't wrong either.
diff --git a/drivers/staging/media/ipu3/ipu3-mmu.c b/drivers/staging/media/ipu3/ipu3-mmu.c index cb9bf5fb29a5..9c4adb815c94 100644 --- a/drivers/staging/media/ipu3/ipu3-mmu.c +++ b/drivers/staging/media/ipu3/ipu3-mmu.c @@ -464,7 +464,7 @@ struct imgu_mmu_info *imgu_mmu_init(struct device *parent, void __iomem *base) * Allocate the array of L2PT CPU pointers, initialized to zero, * which means the dummy L2PT allocated above. */ - mmu->l2pts = vzalloc(IPU3_PT_PTES * sizeof(*mmu->l2pts)); + mmu->l2pts = vzalloc(array_size(IPU3_PT_PTES, sizeof(*mmu->l2pts))); if (!mmu->l2pts) goto fail_l2pt;
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/staging/media/ipu3/ipu3-mmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)