From: Bartosz Golaszewski <[email protected]>
Andy brought to my attention the fact that users allocating an array of
equally sized elements should check if the size multiplication doesn't
overflow. This is why we have helpers like kmalloc_array().
However we don't have krealloc_array() equivalent and there are many
users who do their own multiplication when calling krealloc() for arrays.
This series provides krealloc_array() and uses it in a couple places.
A separate series will follow adding devm_krealloc_array() which is
needed in the xilinx adc driver.
Bartosz Golaszewski (8):
mm: slab: provide krealloc_array()
ALSA: pcm: use krealloc_array()
vhost: vringh: use krealloc_array()
pinctrl: use krealloc_array()
edac: ghes: use krealloc_array()
drm: atomic: use krealloc_array()
hwtracing: intel: use krealloc_array()
dma-buf: use krealloc_array()
drivers/dma-buf/sync_file.c | 4 ++--
drivers/edac/ghes_edac.c | 4 ++--
drivers/gpu/drm/drm_atomic.c | 3 ++-
drivers/hwtracing/intel_th/msu.c | 2 +-
drivers/pinctrl/pinctrl-utils.c | 2 +-
drivers/vhost/vringh.c | 3 ++-
include/linux/slab.h | 11 +++++++++++
sound/core/pcm_lib.c | 4 ++--
8 files changed, 23 insertions(+), 10 deletions(-)
--
2.29.1
From: Bartosz Golaszewski <[email protected]>
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
drivers/hwtracing/intel_th/msu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwtracing/intel_th/msu.c b/drivers/hwtracing/intel_th/msu.c
index 3a77551fb4fc..7d95242db900 100644
--- a/drivers/hwtracing/intel_th/msu.c
+++ b/drivers/hwtracing/intel_th/msu.c
@@ -2002,7 +2002,7 @@ nr_pages_store(struct device *dev, struct device_attribute *attr,
}
nr_wins++;
- rewin = krealloc(win, sizeof(*win) * nr_wins, GFP_KERNEL);
+ rewin = krealloc_array(win, nr_wins, sizeof(*win), GFP_KERNEL);
if (!rewin) {
kfree(win);
return -ENOMEM;
--
2.29.1
From: Bartosz Golaszewski <[email protected]>
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
drivers/vhost/vringh.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
index 8bd8b403f087..08a0e1c842df 100644
--- a/drivers/vhost/vringh.c
+++ b/drivers/vhost/vringh.c
@@ -198,7 +198,8 @@ static int resize_iovec(struct vringh_kiov *iov, gfp_t gfp)
flag = (iov->max_num & VRINGH_IOV_ALLOCATED);
if (flag)
- new = krealloc(iov->iov, new_num * sizeof(struct iovec), gfp);
+ new = krealloc_array(iov->iov, new_num,
+ sizeof(struct iovec), gfp);
else {
new = kmalloc_array(new_num, sizeof(struct iovec), gfp);
if (new) {
--
2.29.1
From: Bartosz Golaszewski <[email protected]>
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
drivers/pinctrl/pinctrl-utils.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pinctrl/pinctrl-utils.c b/drivers/pinctrl/pinctrl-utils.c
index f2bcbf62c03d..93df0d4c0a24 100644
--- a/drivers/pinctrl/pinctrl-utils.c
+++ b/drivers/pinctrl/pinctrl-utils.c
@@ -39,7 +39,7 @@ int pinctrl_utils_reserve_map(struct pinctrl_dev *pctldev,
if (old_num >= new_num)
return 0;
- new_map = krealloc(*map, sizeof(*new_map) * new_num, GFP_KERNEL);
+ new_map = krealloc_array(*map, new_num, sizeof(*new_map), GFP_KERNEL);
if (!new_map) {
dev_err(pctldev->dev, "krealloc(map) failed\n");
return -ENOMEM;
--
2.29.1
From: Bartosz Golaszewski <[email protected]>
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
drivers/edac/ghes_edac.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
index a918ca93e4f7..6d1ddecbf0da 100644
--- a/drivers/edac/ghes_edac.c
+++ b/drivers/edac/ghes_edac.c
@@ -207,8 +207,8 @@ static void enumerate_dimms(const struct dmi_header *dh, void *arg)
if (!hw->num_dimms || !(hw->num_dimms % 16)) {
struct dimm_info *new;
- new = krealloc(hw->dimms, (hw->num_dimms + 16) * sizeof(struct dimm_info),
- GFP_KERNEL);
+ new = krealloc_array(hw->dimms, hw->num_dimms + 16,
+ sizeof(struct dimm_info), GFP_KERNEL);
if (!new) {
WARN_ON_ONCE(1);
return;
--
2.29.1
From: Bartosz Golaszewski <[email protected]>
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
sound/core/pcm_lib.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index d531e1bc2b81..c6f8f9e252e0 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -1129,8 +1129,8 @@ int snd_pcm_hw_rule_add(struct snd_pcm_runtime *runtime, unsigned int cond,
if (constrs->rules_num >= constrs->rules_all) {
struct snd_pcm_hw_rule *new;
unsigned int new_rules = constrs->rules_all + 16;
- new = krealloc(constrs->rules, new_rules * sizeof(*c),
- GFP_KERNEL);
+ new = krealloc_array(constrs->rules, new_rules,
+ sizeof(*c), GFP_KERNEL);
if (!new) {
va_end(args);
return -ENOMEM;
--
2.29.1
From: Bartosz Golaszewski <[email protected]>
When allocating an array of elements, users should check for
multiplication overflow or preferably use one of the provided helpers
like: kmalloc_array().
There's no krealloc_array() counterpart but there are many users who use
regular krealloc() to reallocate arrays. Let's provide an actual
krealloc_array() implementation.
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
include/linux/slab.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/include/linux/slab.h b/include/linux/slab.h
index dd6897f62010..0e6683affee7 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -592,6 +592,17 @@ static inline void *kmalloc_array(size_t n, size_t size, gfp_t flags)
return __kmalloc(bytes, flags);
}
+static __must_check inline void *
+krealloc_array(void *p, size_t new_n, size_t new_size, gfp_t flags)
+{
+ size_t bytes;
+
+ if (unlikely(check_mul_overflow(new_n, new_size, &bytes)))
+ return NULL;
+
+ return krealloc(p, bytes, flags);
+}
+
/**
* kcalloc - allocate memory for an array. The memory is set to zero.
* @n: number of elements.
--
2.29.1
From: Bartosz Golaszewski <[email protected]>
Use the helper that checks for overflows internally instead of manually
calculating the size of the new array.
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
drivers/gpu/drm/drm_atomic.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 58527f151984..09ad6a2ec17b 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -960,7 +960,8 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state,
struct __drm_connnectors_state *c;
int alloc = max(index + 1, config->num_connector);
- c = krealloc(state->connectors, alloc * sizeof(*state->connectors), GFP_KERNEL);
+ c = krealloc_array(state->connectors, alloc,
+ sizeof(*state->connectors), GFP_KERNEL);
if (!c)
return ERR_PTR(-ENOMEM);
--
2.29.1
On Tue, 27 Oct 2020 13:17:19 +0100,
Bartosz Golaszewski wrote:
>
> From: Bartosz Golaszewski <[email protected]>
>
> Use the helper that checks for overflows internally instead of manually
> calculating the size of the new array.
>
> Signed-off-by: Bartosz Golaszewski <[email protected]>
Reviewed-by: Takashi Iwai <[email protected]>
thanks,
Takashi
On Tue, Oct 27, 2020 at 01:17:20PM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> Use the helper that checks for overflows internally instead of manually
> calculating the size of the new array.
>
> Signed-off-by: Bartosz Golaszewski <[email protected]>
No problem with the patch, it does introduce some symmetry in the code.
Acked-by: Michael S. Tsirkin <[email protected]>
> ---
> drivers/vhost/vringh.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
> index 8bd8b403f087..08a0e1c842df 100644
> --- a/drivers/vhost/vringh.c
> +++ b/drivers/vhost/vringh.c
> @@ -198,7 +198,8 @@ static int resize_iovec(struct vringh_kiov *iov, gfp_t gfp)
>
> flag = (iov->max_num & VRINGH_IOV_ALLOCATED);
> if (flag)
> - new = krealloc(iov->iov, new_num * sizeof(struct iovec), gfp);
> + new = krealloc_array(iov->iov, new_num,
> + sizeof(struct iovec), gfp);
> else {
> new = kmalloc_array(new_num, sizeof(struct iovec), gfp);
> if (new) {
> --
> 2.29.1
On Tue, 2020-10-27 at 11:28 -0400, Michael S. Tsirkin wrote:
> On Tue, Oct 27, 2020 at 01:17:20PM +0100, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <[email protected]>
> >
> > Use the helper that checks for overflows internally instead of manually
> > calculating the size of the new array.
> >
> > Signed-off-by: Bartosz Golaszewski <[email protected]>
>
> No problem with the patch, it does introduce some symmetry in the code.
Perhaps more symmetry by using kmemdup
---
drivers/vhost/vringh.c | 23 ++++++++++-------------
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
index 8bd8b403f087..99222a3651cd 100644
--- a/drivers/vhost/vringh.c
+++ b/drivers/vhost/vringh.c
@@ -191,26 +191,23 @@ static int move_to_indirect(const struct vringh *vrh,
static int resize_iovec(struct vringh_kiov *iov, gfp_t gfp)
{
struct kvec *new;
- unsigned int flag, new_num = (iov->max_num & ~VRINGH_IOV_ALLOCATED) * 2;
+ size_t new_num = (iov->max_num & ~VRINGH_IOV_ALLOCATED) * 2;
+ size_t size;
if (new_num < 8)
new_num = 8;
- flag = (iov->max_num & VRINGH_IOV_ALLOCATED);
- if (flag)
- new = krealloc(iov->iov, new_num * sizeof(struct iovec), gfp);
- else {
- new = kmalloc_array(new_num, sizeof(struct iovec), gfp);
- if (new) {
- memcpy(new, iov->iov,
- iov->max_num * sizeof(struct iovec));
- flag = VRINGH_IOV_ALLOCATED;
- }
- }
+ if (unlikely(check_mul_overflow(new_num, sizeof(struct iovec), &size)))
+ return -ENOMEM;
+
+ if (iov->max_num & VRINGH_IOV_ALLOCATED)
+ new = krealloc(iov->iov, size, gfp);
+ else
+ new = kmemdup(iov->iov, size, gfp);
if (!new)
return -ENOMEM;
iov->iov = new;
- iov->max_num = (new_num | flag);
+ iov->max_num = new_num | VRINGH_IOV_ALLOCATED;
return 0;
}
On Tue, Oct 27, 2020 at 5:50 PM Joe Perches <[email protected]> wrote:
>
> On Tue, 2020-10-27 at 11:28 -0400, Michael S. Tsirkin wrote:
> > On Tue, Oct 27, 2020 at 01:17:20PM +0100, Bartosz Golaszewski wrote:
> > > From: Bartosz Golaszewski <[email protected]>
> > >
> > > Use the helper that checks for overflows internally instead of manually
> > > calculating the size of the new array.
> > >
> > > Signed-off-by: Bartosz Golaszewski <[email protected]>
> >
> > No problem with the patch, it does introduce some symmetry in the code.
>
> Perhaps more symmetry by using kmemdup
> ---
> drivers/vhost/vringh.c | 23 ++++++++++-------------
> 1 file changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
> index 8bd8b403f087..99222a3651cd 100644
> --- a/drivers/vhost/vringh.c
> +++ b/drivers/vhost/vringh.c
> @@ -191,26 +191,23 @@ static int move_to_indirect(const struct vringh *vrh,
> static int resize_iovec(struct vringh_kiov *iov, gfp_t gfp)
> {
> struct kvec *new;
> - unsigned int flag, new_num = (iov->max_num & ~VRINGH_IOV_ALLOCATED) * 2;
> + size_t new_num = (iov->max_num & ~VRINGH_IOV_ALLOCATED) * 2;
> + size_t size;
>
> if (new_num < 8)
> new_num = 8;
>
> - flag = (iov->max_num & VRINGH_IOV_ALLOCATED);
> - if (flag)
> - new = krealloc(iov->iov, new_num * sizeof(struct iovec), gfp);
> - else {
> - new = kmalloc_array(new_num, sizeof(struct iovec), gfp);
> - if (new) {
> - memcpy(new, iov->iov,
> - iov->max_num * sizeof(struct iovec));
> - flag = VRINGH_IOV_ALLOCATED;
> - }
> - }
> + if (unlikely(check_mul_overflow(new_num, sizeof(struct iovec), &size)))
> + return -ENOMEM;
> +
The whole point of using helpers such as kmalloc_array() is not doing
these checks manually.
Bartosz
> + if (iov->max_num & VRINGH_IOV_ALLOCATED)
> + new = krealloc(iov->iov, size, gfp);
> + else
> + new = kmemdup(iov->iov, size, gfp);
> if (!new)
> return -ENOMEM;
> iov->iov = new;
> - iov->max_num = (new_num | flag);
> + iov->max_num = new_num | VRINGH_IOV_ALLOCATED;
> return 0;
> }
>
>
>
On Tue, 2020-10-27 at 17:58 +0100, Bartosz Golaszewski wrote:
> On Tue, Oct 27, 2020 at 5:50 PM Joe Perches <[email protected]> wrote:
> >
> > On Tue, 2020-10-27 at 11:28 -0400, Michael S. Tsirkin wrote:
> > > On Tue, Oct 27, 2020 at 01:17:20PM +0100, Bartosz Golaszewski wrote:
> > > > From: Bartosz Golaszewski <[email protected]>
> > > >
> > > > Use the helper that checks for overflows internally instead of manually
> > > > calculating the size of the new array.
> > > >
> > > > Signed-off-by: Bartosz Golaszewski <[email protected]>
> > >
> > > No problem with the patch, it does introduce some symmetry in the code.
> >
> > Perhaps more symmetry by using kmemdup
> > ---
> > ?drivers/vhost/vringh.c | 23 ++++++++++-------------
> > ?1 file changed, 10 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
> > index 8bd8b403f087..99222a3651cd 100644
> > --- a/drivers/vhost/vringh.c
> > +++ b/drivers/vhost/vringh.c
> > @@ -191,26 +191,23 @@ static int move_to_indirect(const struct vringh *vrh,
> > ?static int resize_iovec(struct vringh_kiov *iov, gfp_t gfp)
> > ?{
> > ????????struct kvec *new;
> > - unsigned int flag, new_num = (iov->max_num & ~VRINGH_IOV_ALLOCATED) * 2;
> > + size_t new_num = (iov->max_num & ~VRINGH_IOV_ALLOCATED) * 2;
> > + size_t size;
> >
> > ????????if (new_num < 8)
> > ????????????????new_num = 8;
> >
> > - flag = (iov->max_num & VRINGH_IOV_ALLOCATED);
> > - if (flag)
> > - new = krealloc(iov->iov, new_num * sizeof(struct iovec), gfp);
> > - else {
> > - new = kmalloc_array(new_num, sizeof(struct iovec), gfp);
> > - if (new) {
> > - memcpy(new, iov->iov,
> > - iov->max_num * sizeof(struct iovec));
> > - flag = VRINGH_IOV_ALLOCATED;
> > - }
> > - }
> > + if (unlikely(check_mul_overflow(new_num, sizeof(struct iovec), &size)))
> > + return -ENOMEM;
> > +
>
> The whole point of using helpers such as kmalloc_array() is not doing
> these checks manually.
Tradeoffs for in readability for overflow and not mistyping or doing
the multiplication of iov->max_num * sizeof(struct iovec) twice.
Just fyi:
the realloc doesn't do a multiplication overflow test as written so the
suggestion is slightly more resistant to defect.
On Tue, Oct 27, 2020 at 6:08 PM Joe Perches <[email protected]> wrote:
>
> On Tue, 2020-10-27 at 17:58 +0100, Bartosz Golaszewski wrote:
> > On Tue, Oct 27, 2020 at 5:50 PM Joe Perches <[email protected]> wrote:
> > >
> > > On Tue, 2020-10-27 at 11:28 -0400, Michael S. Tsirkin wrote:
> > > > On Tue, Oct 27, 2020 at 01:17:20PM +0100, Bartosz Golaszewski wrote:
> > > > > From: Bartosz Golaszewski <[email protected]>
> > > > >
> > > > > Use the helper that checks for overflows internally instead of manually
> > > > > calculating the size of the new array.
> > > > >
> > > > > Signed-off-by: Bartosz Golaszewski <[email protected]>
> > > >
> > > > No problem with the patch, it does introduce some symmetry in the code.
> > >
> > > Perhaps more symmetry by using kmemdup
> > > ---
> > > drivers/vhost/vringh.c | 23 ++++++++++-------------
> > > 1 file changed, 10 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
> > > index 8bd8b403f087..99222a3651cd 100644
> > > --- a/drivers/vhost/vringh.c
> > > +++ b/drivers/vhost/vringh.c
> > > @@ -191,26 +191,23 @@ static int move_to_indirect(const struct vringh *vrh,
> > > static int resize_iovec(struct vringh_kiov *iov, gfp_t gfp)
> > > {
> > > struct kvec *new;
> > > - unsigned int flag, new_num = (iov->max_num & ~VRINGH_IOV_ALLOCATED) * 2;
> > > + size_t new_num = (iov->max_num & ~VRINGH_IOV_ALLOCATED) * 2;
> > > + size_t size;
> > >
> > > if (new_num < 8)
> > > new_num = 8;
> > >
> > > - flag = (iov->max_num & VRINGH_IOV_ALLOCATED);
> > > - if (flag)
> > > - new = krealloc(iov->iov, new_num * sizeof(struct iovec), gfp);
> > > - else {
> > > - new = kmalloc_array(new_num, sizeof(struct iovec), gfp);
> > > - if (new) {
> > > - memcpy(new, iov->iov,
> > > - iov->max_num * sizeof(struct iovec));
> > > - flag = VRINGH_IOV_ALLOCATED;
> > > - }
> > > - }
> > > + if (unlikely(check_mul_overflow(new_num, sizeof(struct iovec), &size)))
> > > + return -ENOMEM;
> > > +
> >
> > The whole point of using helpers such as kmalloc_array() is not doing
> > these checks manually.
>
> Tradeoffs for in readability for overflow and not mistyping or doing
> the multiplication of iov->max_num * sizeof(struct iovec) twice.
>
It's out of scope for this series - I want to add users for
krealloc_array(), not refactor code I don't really know. If the
maintainer of this bit objects, it can be dropped.
> Just fyi:
>
> the realloc doesn't do a multiplication overflow test as written so the
> suggestion is slightly more resistant to defect.
>
I'm not sure what your point is. I used krealloc_array() exactly for
this reason - to add the overflow test.
BTW I suppose kmalloc_array() here can be replaced with
krealloc_array() if the original pointer is NULL the first time it's
called.
Bartosz
On 10/27/20 1:17 PM, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> When allocating an array of elements, users should check for
> multiplication overflow or preferably use one of the provided helpers
> like: kmalloc_array().
>
> There's no krealloc_array() counterpart but there are many users who use
> regular krealloc() to reallocate arrays. Let's provide an actual
> krealloc_array() implementation.
>
> Signed-off-by: Bartosz Golaszewski <[email protected]>
Makes sense.
Acked-by: Vlastimil Babka <[email protected]>
> ---
> include/linux/slab.h | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index dd6897f62010..0e6683affee7 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -592,6 +592,17 @@ static inline void *kmalloc_array(size_t n, size_t size, gfp_t flags)
> return __kmalloc(bytes, flags);
> }
>
> +static __must_check inline void *
> +krealloc_array(void *p, size_t new_n, size_t new_size, gfp_t flags)
> +{
> + size_t bytes;
> +
> + if (unlikely(check_mul_overflow(new_n, new_size, &bytes)))
> + return NULL;
> +
> + return krealloc(p, bytes, flags);
> +}
> +
> /**
> * kcalloc - allocate memory for an array. The memory is set to zero.
> * @n: number of elements.
>
On Tue, Oct 27, 2020 at 01:17:22PM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> Use the helper that checks for overflows internally instead of manually
> calculating the size of the new array.
>
> Signed-off-by: Bartosz Golaszewski <[email protected]>
> ---
> drivers/edac/ghes_edac.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
> index a918ca93e4f7..6d1ddecbf0da 100644
> --- a/drivers/edac/ghes_edac.c
> +++ b/drivers/edac/ghes_edac.c
> @@ -207,8 +207,8 @@ static void enumerate_dimms(const struct dmi_header *dh, void *arg)
> if (!hw->num_dimms || !(hw->num_dimms % 16)) {
> struct dimm_info *new;
>
> - new = krealloc(hw->dimms, (hw->num_dimms + 16) * sizeof(struct dimm_info),
> - GFP_KERNEL);
> + new = krealloc_array(hw->dimms, hw->num_dimms + 16,
> + sizeof(struct dimm_info), GFP_KERNEL);
> if (!new) {
> WARN_ON_ONCE(1);
> return;
> --
Sure, why not.
Acked-by: Borislav Petkov <[email protected]>
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On Tue, Oct 27, 2020 at 01:17:23PM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> Use the helper that checks for overflows internally instead of manually
> calculating the size of the new array.
>
> Signed-off-by: Bartosz Golaszewski <[email protected]>
Acked-by: Daniel Vetter <[email protected]>
I don't expect conflicts with this going through some other tree, so
please make that happen. Or resend once I can apply this to drm trees.
Thanks, Daniel
> ---
> drivers/gpu/drm/drm_atomic.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 58527f151984..09ad6a2ec17b 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -960,7 +960,8 @@ drm_atomic_get_connector_state(struct drm_atomic_state *state,
> struct __drm_connnectors_state *c;
> int alloc = max(index + 1, config->num_connector);
>
> - c = krealloc(state->connectors, alloc * sizeof(*state->connectors), GFP_KERNEL);
> + c = krealloc_array(state->connectors, alloc,
> + sizeof(*state->connectors), GFP_KERNEL);
> if (!c)
> return ERR_PTR(-ENOMEM);
>
> --
> 2.29.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
On Tue, Oct 27, 2020 at 01:17:18PM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> When allocating an array of elements, users should check for
> multiplication overflow or preferably use one of the provided helpers
> like: kmalloc_array().
>
> There's no krealloc_array() counterpart but there are many users who use
> regular krealloc() to reallocate arrays. Let's provide an actual
> krealloc_array() implementation.
>
> Signed-off-by: Bartosz Golaszewski <[email protected]>
> ---
> include/linux/slab.h | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index dd6897f62010..0e6683affee7 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -592,6 +592,17 @@ static inline void *kmalloc_array(size_t n, size_t size, gfp_t flags)
> return __kmalloc(bytes, flags);
> }
>
Can you please add kernel-doc here and a word or two about this function
to Documentation/core-api/memory-allocation.rst?
> +static __must_check inline void *
> +krealloc_array(void *p, size_t new_n, size_t new_size, gfp_t flags)
> +{
> + size_t bytes;
> +
> + if (unlikely(check_mul_overflow(new_n, new_size, &bytes)))
> + return NULL;
> +
> + return krealloc(p, bytes, flags);
> +}
> +
> /**
> * kcalloc - allocate memory for an array. The memory is set to zero.
> * @n: number of elements.
> --
> 2.29.1
>
>
--
Sincerely yours,
Mike.
On Tue, Oct 27, 2020 at 1:17 PM Bartosz Golaszewski <[email protected]> wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> Andy brought to my attention the fact that users allocating an array of
> equally sized elements should check if the size multiplication doesn't
> overflow. This is why we have helpers like kmalloc_array().
>
> However we don't have krealloc_array() equivalent and there are many
> users who do their own multiplication when calling krealloc() for arrays.
>
> This series provides krealloc_array() and uses it in a couple places.
>
> A separate series will follow adding devm_krealloc_array() which is
> needed in the xilinx adc driver.
The series:
Acked-by: Linus Walleij <[email protected]>
I really like this.
Yours,
Linus Walleij