| Message ID | 20240606144608.97817-2-jean-philippe@linaro.org (mailing list archive) |
|---|---|
| State | Mainlined |
| Commit | 0ee14725471cea66e03e3cd4f4c582d759de502c |
| Headers | show |
| Series | mm/util: Swap kmemdup_array() arguments | expand |
On Thu, Jun 6, 2024 at 5:47 PM Jean-Philippe Brucker <jean-philippe@linaro.org> wrote: > > GCC 14.1 complains about the argument usage of kmemdup_array(): > > drivers/soc/tegra/fuse/fuse-tegra.c:130:65: error: 'kmemdup_array' sizes specified with 'sizeof' in the earlier argument and not in the later argument [-Werror=calloc-transposed-args] > 130 | fuse->lookups = kmemdup_array(fuse->soc->lookups, sizeof(*fuse->lookups), > | ^ > drivers/soc/tegra/fuse/fuse-tegra.c:130:65: note: earlier argument should specify number of elements, later size of each element > > The annotation introduced by commit 7d78a7773355 ("string: Add > additional __realloc_size() annotations for "dup" helpers") lets the > compiler think that kmemdup_array() follows the same format as calloc(), > with the number of elements preceding the size of one element. So we > could simply swap the arguments to __realloc_size() to get rid of that > warning, but it seems cleaner to instead have kmemdup_array() follow the > same format as krealloc_array(), memdup_array_user(), calloc() etc. LGTM, it seems we don't have many users (yet), this looks like a comprehensive change. Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
On Thu, 06 Jun 2024 15:46:09 +0100, Jean-Philippe Brucker wrote: > GCC 14.1 complains about the argument usage of kmemdup_array(): > > drivers/soc/tegra/fuse/fuse-tegra.c:130:65: error: 'kmemdup_array' sizes specified with 'sizeof' in the earlier argument and not in the later argument [-Werror=calloc-transposed-args] > 130 | fuse->lookups = kmemdup_array(fuse->soc->lookups, sizeof(*fuse->lookups), > | ^ > drivers/soc/tegra/fuse/fuse-tegra.c:130:65: note: earlier argument should specify number of elements, later size of each element > > [...] Applied to for-next/hardening, thanks! [1/1] mm/util: Swap kmemdup_array() arguments https://git.kernel.org/kees/c/0ee14725471c Take care,
On Thu, Jun 6, 2024 at 6:56 PM Kees Cook <kees@kernel.org> wrote: > On Thu, 06 Jun 2024 15:46:09 +0100, Jean-Philippe Brucker wrote: [...] > Applied to for-next/hardening, thanks! Btw, is it possible to get this for v6.10, so we may start enabling it for others?
On Thu, Jun 06, 2024 at 08:35:13PM +0300, Andy Shevchenko wrote: > On Thu, Jun 6, 2024 at 6:56 PM Kees Cook <kees@kernel.org> wrote: > > On Thu, 06 Jun 2024 15:46:09 +0100, Jean-Philippe Brucker wrote: > > [...] > > > Applied to for-next/hardening, thanks! > > Btw, is it possible to get this for v6.10, so we may start enabling it > for others? Which others do you mean?
On Thu, Jun 6, 2024 at 8:46 PM Kees Cook <kees@kernel.org> wrote: > > On Thu, Jun 06, 2024 at 08:35:13PM +0300, Andy Shevchenko wrote: > > On Thu, Jun 6, 2024 at 6:56 PM Kees Cook <kees@kernel.org> wrote: > > > On Thu, 06 Jun 2024 15:46:09 +0100, Jean-Philippe Brucker wrote: > > > > [...] > > > > > Applied to for-next/hardening, thanks! > > > > Btw, is it possible to get this for v6.10, so we may start enabling it > > for others? > > Which others do you mean? There are a lot of users of kmemdup(x*y) which I want to convert sooner than later to kmemdup_array(x,y). FWIW, this patch even has a Fixes tag!
On Thu, Jun 06, 2024 at 08:48:37PM +0300, Andy Shevchenko wrote: > On Thu, Jun 6, 2024 at 8:46 PM Kees Cook <kees@kernel.org> wrote: > > > > On Thu, Jun 06, 2024 at 08:35:13PM +0300, Andy Shevchenko wrote: > > > On Thu, Jun 6, 2024 at 6:56 PM Kees Cook <kees@kernel.org> wrote: > > > > On Thu, 06 Jun 2024 15:46:09 +0100, Jean-Philippe Brucker wrote: > > > > > > [...] > > > > > > > Applied to for-next/hardening, thanks! > > > > > > Btw, is it possible to get this for v6.10, so we may start enabling it > > > for others? > > > > Which others do you mean? > > There are a lot of users of kmemdup(x*y) which I want to convert > sooner than later to kmemdup_array(x,y). Ah-ha, I see what you mean. Well, I'm not sure we can do v6.10 for this because rc2 is behind us, and that's what most subsystems merge to. I can land the patch for rc3 so there will be no warnings in Linus's tree/-next, but conversions in subsystem trees will gain warnings, I think...
On Thu, Jun 06, 2024 at 11:10:43AM -0700, Kees Cook wrote: > On Thu, Jun 06, 2024 at 08:48:37PM +0300, Andy Shevchenko wrote: > > On Thu, Jun 6, 2024 at 8:46 PM Kees Cook <kees@kernel.org> wrote: > > > > > > On Thu, Jun 06, 2024 at 08:35:13PM +0300, Andy Shevchenko wrote: > > > > On Thu, Jun 6, 2024 at 6:56 PM Kees Cook <kees@kernel.org> wrote: > > > > > On Thu, 06 Jun 2024 15:46:09 +0100, Jean-Philippe Brucker wrote: > > > > > > > > [...] > > > > > > > > > Applied to for-next/hardening, thanks! > > > > > > > > Btw, is it possible to get this for v6.10, so we may start enabling it > > > > for others? > > > > > > Which others do you mean? > > > > There are a lot of users of kmemdup(x*y) which I want to convert > > sooner than later to kmemdup_array(x,y). > > Ah-ha, I see what you mean. Well, I'm not sure we can do v6.10 for this > because rc2 is behind us, and that's what most subsystems merge to. I > can land the patch for rc3 so there will be no warnings in Linus's > tree/-next, but conversions in subsystem trees will gain warnings, I > think... I see, but v6.10-rc3 is still better than v6.11-rc1. Some of the subsystems are okay to merge rcX to their for-next if needed.
diff --git a/include/linux/string.h b/include/linux/string.h index 60168aa2af075..9edace076ddbf 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -289,7 +289,7 @@ extern void *kmemdup_noprof(const void *src, size_t len, gfp_t gfp) __realloc_si extern void *kvmemdup(const void *src, size_t len, gfp_t gfp) __realloc_size(2); extern char *kmemdup_nul(const char *s, size_t len, gfp_t gfp); -extern void *kmemdup_array(const void *src, size_t element_size, size_t count, gfp_t gfp) +extern void *kmemdup_array(const void *src, size_t count, size_t element_size, gfp_t gfp) __realloc_size(2, 3); /* lib/argv_split.c */ diff --git a/drivers/soc/tegra/fuse/fuse-tegra.c b/drivers/soc/tegra/fuse/fuse-tegra.c index b6bfd6729df39..d276672838465 100644 --- a/drivers/soc/tegra/fuse/fuse-tegra.c +++ b/drivers/soc/tegra/fuse/fuse-tegra.c @@ -127,8 +127,8 @@ static void tegra_fuse_print_sku_info(struct tegra_sku_info *tegra_sku_info) static int tegra_fuse_add_lookups(struct tegra_fuse *fuse) { - fuse->lookups = kmemdup_array(fuse->soc->lookups, sizeof(*fuse->lookups), - fuse->soc->num_lookups, GFP_KERNEL); + fuse->lookups = kmemdup_array(fuse->soc->lookups, fuse->soc->num_lookups, + sizeof(*fuse->lookups), GFP_KERNEL); if (!fuse->lookups) return -ENOMEM; diff --git a/lib/fortify_kunit.c b/lib/fortify_kunit.c index f9cc467334ce3..e17d520f532cf 100644 --- a/lib/fortify_kunit.c +++ b/lib/fortify_kunit.c @@ -374,7 +374,7 @@ static const char * const test_strs[] = { for (i = 0; i < ARRAY_SIZE(test_strs); i++) { \ len = strlen(test_strs[i]); \ KUNIT_EXPECT_EQ(test, __builtin_constant_p(len), 0); \ - checker(len, kmemdup_array(test_strs[i], len, 1, gfp), \ + checker(len, kmemdup_array(test_strs[i], 1, len, gfp), \ kfree(p)); \ checker(len, kmemdup(test_strs[i], len, gfp), \ kfree(p)); \ diff --git a/mm/util.c b/mm/util.c index c9e519e6811f5..6682097372efc 100644 --- a/mm/util.c +++ b/mm/util.c @@ -139,14 +139,14 @@ EXPORT_SYMBOL(kmemdup_noprof); * kmemdup_array - duplicate a given array. * * @src: array to duplicate. - * @element_size: size of each element of array. * @count: number of elements to duplicate from array. + * @element_size: size of each element of array. * @gfp: GFP mask to use. * * Return: duplicated array of @src or %NULL in case of error, * result is physically contiguous. Use kfree() to free. */ -void *kmemdup_array(const void *src, size_t element_size, size_t count, gfp_t gfp) +void *kmemdup_array(const void *src, size_t count, size_t element_size, gfp_t gfp) { return kmemdup(src, size_mul(element_size, count), gfp); }
GCC 14.1 complains about the argument usage of kmemdup_array(): drivers/soc/tegra/fuse/fuse-tegra.c:130:65: error: 'kmemdup_array' sizes specified with 'sizeof' in the earlier argument and not in the later argument [-Werror=calloc-transposed-args] 130 | fuse->lookups = kmemdup_array(fuse->soc->lookups, sizeof(*fuse->lookups), | ^ drivers/soc/tegra/fuse/fuse-tegra.c:130:65: note: earlier argument should specify number of elements, later size of each element The annotation introduced by commit 7d78a7773355 ("string: Add additional __realloc_size() annotations for "dup" helpers") lets the compiler think that kmemdup_array() follows the same format as calloc(), with the number of elements preceding the size of one element. So we could simply swap the arguments to __realloc_size() to get rid of that warning, but it seems cleaner to instead have kmemdup_array() follow the same format as krealloc_array(), memdup_array_user(), calloc() etc. Fixes: 7d78a7773355 ("string: Add additional __realloc_size() annotations for "dup" helpers") Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org> --- include/linux/string.h | 2 +- drivers/soc/tegra/fuse/fuse-tegra.c | 4 ++-- lib/fortify_kunit.c | 2 +- mm/util.c | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) base-commit: 2df0193e62cf887f373995fb8a91068562784adc