1. Patchew
  2. linux
  3. [PATCH v5 0/4] Add ARRAY_END(), and use it to fix off-by-one bugs
  4. View patch

[PATCH v5 4/4] mm: Use ARRAY_END() instead of open-coding it

Alejandro Colomar posted 4 patches 1 month, 4 weeks ago
There is a newer version of this series
[PATCH v5 4/4] mm: Use ARRAY_END() instead of open-coding it
Posted by Alejandro Colomar 1 month, 4 weeks ago 
Cc: Kees Cook <kees@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Alejandro Colomar <alx@kernel.org>
---
 mm/kmemleak.c      | 2 +-
 mm/memcontrol-v1.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/kmemleak.c b/mm/kmemleak.c
index 1ac56ceb29b6..fe33f2edfe07 100644
--- a/mm/kmemleak.c
+++ b/mm/kmemleak.c
@@ -510,7 +510,7 @@ static void mem_pool_free(struct kmemleak_object *object)
 {
 	unsigned long flags;
 
-	if (object < mem_pool || object >= mem_pool + ARRAY_SIZE(mem_pool)) {
+	if (object < mem_pool || object >= ARRAY_END(mem_pool)) {
 		kmem_cache_free(object_cache, object);
 		return;
 	}
diff --git a/mm/memcontrol-v1.c b/mm/memcontrol-v1.c
index 6eed14bff742..b2f37bd939fa 100644
--- a/mm/memcontrol-v1.c
+++ b/mm/memcontrol-v1.c
@@ -1794,7 +1794,7 @@ static int memcg_numa_stat_show(struct seq_file *m, void *v)
 
 	mem_cgroup_flush_stats(memcg);
 
-	for (stat = stats; stat < stats + ARRAY_SIZE(stats); stat++) {
+	for (stat = stats; stat < ARRAY_END(stats); stat++) {
 		seq_printf(m, "%s=%lu", stat->name,
 			   mem_cgroup_nr_lru_pages(memcg, stat->lru_mask,
 						   false));
@@ -1805,7 +1805,7 @@ static int memcg_numa_stat_show(struct seq_file *m, void *v)
 		seq_putc(m, '\n');
 	}
 
-	for (stat = stats; stat < stats + ARRAY_SIZE(stats); stat++) {
+	for (stat = stats; stat < ARRAY_END(stats); stat++) {
 
 		seq_printf(m, "hierarchical_%s=%lu", stat->name,
 			   mem_cgroup_nr_lru_pages(memcg, stat->lru_mask,
-- 
2.51.0
Re: [PATCH v5 4/4] mm: Use ARRAY_END() instead of open-coding it
Posted by Kees Cook 1 month, 4 weeks ago 

On December 11, 2025 7:46:49 AM GMT+09:00, Alejandro Colomar <alx@kernel.org> wrote:
>Cc: Kees Cook <kees@kernel.org>
>Cc: Linus Torvalds <torvalds@linux-foundation.org>
>Signed-off-by: Alejandro Colomar <alx@kernel.org>

Hm, this seems to be missing a commit log body?

Are there other open-coded instances that could be replaced? This seems like a great task for a coccinelle script.

-Kees

>---
> mm/kmemleak.c      | 2 +-
> mm/memcontrol-v1.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/mm/kmemleak.c b/mm/kmemleak.c
>index 1ac56ceb29b6..fe33f2edfe07 100644
>--- a/mm/kmemleak.c
>+++ b/mm/kmemleak.c
>@@ -510,7 +510,7 @@ static void mem_pool_free(struct kmemleak_object *object)
> {
> 	unsigned long flags;
> 
>-	if (object < mem_pool || object >= mem_pool + ARRAY_SIZE(mem_pool)) {
>+	if (object < mem_pool || object >= ARRAY_END(mem_pool)) {
> 		kmem_cache_free(object_cache, object);
> 		return;
> 	}
>diff --git a/mm/memcontrol-v1.c b/mm/memcontrol-v1.c
>index 6eed14bff742..b2f37bd939fa 100644
>--- a/mm/memcontrol-v1.c
>+++ b/mm/memcontrol-v1.c
>@@ -1794,7 +1794,7 @@ static int memcg_numa_stat_show(struct seq_file *m, void *v)
> 
> 	mem_cgroup_flush_stats(memcg);
> 
>-	for (stat = stats; stat < stats + ARRAY_SIZE(stats); stat++) {
>+	for (stat = stats; stat < ARRAY_END(stats); stat++) {
> 		seq_printf(m, "%s=%lu", stat->name,
> 			   mem_cgroup_nr_lru_pages(memcg, stat->lru_mask,
> 						   false));
>@@ -1805,7 +1805,7 @@ static int memcg_numa_stat_show(struct seq_file *m, void *v)
> 		seq_putc(m, '\n');
> 	}
> 
>-	for (stat = stats; stat < stats + ARRAY_SIZE(stats); stat++) {
>+	for (stat = stats; stat < ARRAY_END(stats); stat++) {
> 
> 		seq_printf(m, "hierarchical_%s=%lu", stat->name,
> 			   mem_cgroup_nr_lru_pages(memcg, stat->lru_mask,

-- 
Kees Cook
Re: [PATCH v5 4/4] mm: Use ARRAY_END() instead of open-coding it
Posted by Alejandro Colomar 1 month, 4 weeks ago 
Hi Kees,

On Thu, Dec 11, 2025 at 08:18:56AM +0900, Kees Cook wrote:
> 
> 
> On December 11, 2025 7:46:49 AM GMT+09:00, Alejandro Colomar <alx@kernel.org> wrote:
> >Cc: Kees Cook <kees@kernel.org>
> >Cc: Linus Torvalds <torvalds@linux-foundation.org>
> >Signed-off-by: Alejandro Colomar <alx@kernel.org>
> 
> Hm, this seems to be missing a commit log body?

Actually, there's not much to it.  The patch uses ARRAY_END() where it
was being open-coded.  There aren't any bugs in this code, so it's
purely cosmetic (and of course, to prevent future issues, in case the
code is modified).  Maybe I could say precisely that.  What would you
say here?

> Are there other open-coded instances that could be replaced? This seems like a great task for a coccinelle script.

There are many, but I wanted to keep them out of this initial patch set,
to make it easy to apply.  When this one is applied, I could work on a
second round that replaces more of them with coccinelle.  This is just
for showing that this is beneficial, and to make sure that you ask for
more.  :)

Also, it's easier if there are few maintainers that would block an
initial patch set.  If restrict the patch set to a few files, I don't
have to deal with many of them.  Once I get used to this, I'll deal with
all of them.

> 
> -Kees

Have a lovely night!
Alex

> 
> >---
> > mm/kmemleak.c      | 2 +-
> > mm/memcontrol-v1.c | 4 ++--
> > 2 files changed, 3 insertions(+), 3 deletions(-)
> >
> >diff --git a/mm/kmemleak.c b/mm/kmemleak.c
> >index 1ac56ceb29b6..fe33f2edfe07 100644
> >--- a/mm/kmemleak.c
> >+++ b/mm/kmemleak.c
> >@@ -510,7 +510,7 @@ static void mem_pool_free(struct kmemleak_object *object)
> > {
> > 	unsigned long flags;
> > 
> >-	if (object < mem_pool || object >= mem_pool + ARRAY_SIZE(mem_pool)) {
> >+	if (object < mem_pool || object >= ARRAY_END(mem_pool)) {
> > 		kmem_cache_free(object_cache, object);
> > 		return;
> > 	}
> >diff --git a/mm/memcontrol-v1.c b/mm/memcontrol-v1.c
> >index 6eed14bff742..b2f37bd939fa 100644
> >--- a/mm/memcontrol-v1.c
> >+++ b/mm/memcontrol-v1.c
> >@@ -1794,7 +1794,7 @@ static int memcg_numa_stat_show(struct seq_file *m, void *v)
> > 
> > 	mem_cgroup_flush_stats(memcg);
> > 
> >-	for (stat = stats; stat < stats + ARRAY_SIZE(stats); stat++) {
> >+	for (stat = stats; stat < ARRAY_END(stats); stat++) {
> > 		seq_printf(m, "%s=%lu", stat->name,
> > 			   mem_cgroup_nr_lru_pages(memcg, stat->lru_mask,
> > 						   false));
> >@@ -1805,7 +1805,7 @@ static int memcg_numa_stat_show(struct seq_file *m, void *v)
> > 		seq_putc(m, '\n');
> > 	}
> > 
> >-	for (stat = stats; stat < stats + ARRAY_SIZE(stats); stat++) {
> >+	for (stat = stats; stat < ARRAY_END(stats); stat++) {
> > 
> > 		seq_printf(m, "hierarchical_%s=%lu", stat->name,
> > 			   mem_cgroup_nr_lru_pages(memcg, stat->lru_mask,
> 
> -- 
> Kees Cook

-- 
<https://www.alejandro-colomar.es>
Re: [PATCH v5 4/4] mm: Use ARRAY_END() instead of open-coding it
Posted by Kees Cook 1 month, 4 weeks ago 
On Thu, Dec 11, 2025 at 01:21:59AM +0100, Alejandro Colomar wrote:
> Hi Kees,
> 
> On Thu, Dec 11, 2025 at 08:18:56AM +0900, Kees Cook wrote:
> > 
> > 
> > On December 11, 2025 7:46:49 AM GMT+09:00, Alejandro Colomar <alx@kernel.org> wrote:
> > >Cc: Kees Cook <kees@kernel.org>
> > >Cc: Linus Torvalds <torvalds@linux-foundation.org>
> > >Signed-off-by: Alejandro Colomar <alx@kernel.org>
> > 
> > Hm, this seems to be missing a commit log body?
> 
> Actually, there's not much to it.  The patch uses ARRAY_END() where it
> was being open-coded.  There aren't any bugs in this code, so it's
> purely cosmetic (and of course, to prevent future issues, in case the
> code is modified).  Maybe I could say precisely that.  What would you
> say here?

Yup, that would be perfect. A what/why, even a single sentence, is a
minimum for commit log bodies.

> > Are there other open-coded instances that could be replaced? This seems like a great task for a coccinelle script.
> 
> There are many, but I wanted to keep them out of this initial patch set,
> to make it easy to apply.  When this one is applied, I could work on a
> second round that replaces more of them with coccinelle.  This is just
> for showing that this is beneficial, and to make sure that you ask for
> more.  :)
> 
> Also, it's easier if there are few maintainers that would block an
> initial patch set.  If restrict the patch set to a few files, I don't
> have to deal with many of them.  Once I get used to this, I'll deal with
> all of them.

Sounds good!

-Kees

-- 
Kees Cook
Re: [PATCH v5 4/4] mm: Use ARRAY_END() instead of open-coding it
Posted by Alejandro Colomar 1 month, 2 weeks ago 
Hi Kees, Andrew,

On Wed, Dec 10, 2025 at 05:37:13PM -0800, Kees Cook wrote:
> > > Are there other open-coded instances that could be replaced? This seems like a great task for a coccinelle script.
> > 
> > There are many, but I wanted to keep them out of this initial patch set,
> > to make it easy to apply.  When this one is applied, I could work on a
> > second round that replaces more of them with coccinelle.  This is just
> > for showing that this is beneficial, and to make sure that you ask for
> > more.  :)
> > 
> > Also, it's easier if there are few maintainers that would block an
> > initial patch set.  If restrict the patch set to a few files, I don't
> > have to deal with many of them.  Once I get used to this, I'll deal with
> > all of them.
> 
> Sounds good!

Now that the first patch set has been merged, I'm working on a second
round.

I've written a semantic patch:

	$ cat src/spatch/array_end.sp 
	@@
	expression a;
	@@

	- a + ARRAY_SIZE(a)
	+ ARRAY_END(a)

	@@
	expression a;
	@@

	- ARRAY_SIZE(a) + a
	+ ARRAY_END(a)

	@@
	expression a;
	@@

	- &a[0] + ARRAY_SIZE(a)
	+ ARRAY_END(a)

	@@
	expression a;
	@@

	- ARRAY_SIZE(a) + &a[0]
	+ ARRAY_END(a)

	@@
	expression a;
	@@

	- &a[ARRAY_SIZE(a)]
	+ ARRAY_END(a)

Apart from the cases covered by this semantic patch, I've found one case
which needs manual intervention (I don't know if a semantic patch can
handle this case):

	diff --git i/fs/proc/base.c w/fs/proc/base.c
	index c5114e9a0323..e99c8cad49c3 100644
	--- i/fs/proc/base.c
	+++ w/fs/proc/base.c
	@@ -2876,7 +2876,7 @@ static struct dentry *proc_##LSM##_attr_dir_lookup(struct inode *dir, \
	 { \
		return proc_pident_lookup(dir, dentry, \
					  LSM##_attr_dir_stuff, \
	-                                 LSM##_attr_dir_stuff + ARRAY_SIZE(LSM##_attr_dir_stuff)); \
	+                                 ARRAY_END(LSM##_attr_dir_stuff)); \
	 } \
	 \
	 static const struct inode_operations proc_##LSM##_attr_dir_inode_ops = { \

Anyway, I made sure there's only one of these.  There are no similar
cases in the kernel.

Here's a summary of what the semantic patch finds:

 arch/arm/kernel/hibernate.c                               | 2 +-
 arch/arm/kernel/reboot.c                                  | 2 +-
 arch/arm/mach-omap1/clock_data.c                          | 2 +-
 arch/arm64/kvm/sys_regs.c                                 | 2 +-
 arch/mips/fw/sni/sniprom.c                                | 2 +-
 arch/mips/include/asm/sgiarcs.h                           | 2 +-
 arch/riscv/purgatory/purgatory.c                          | 2 +-
 arch/s390/purgatory/purgatory.c                           | 2 +-
 arch/x86/kernel/cpu/hypervisor.c                          | 2 +-
 arch/x86/purgatory/purgatory.c                            | 2 +-
 block/bio.c                                               | 2 +-
 crypto/adiantum.c                                         | 2 +-
 drivers/base/regmap/regmap-spi-avmm.c                     | 2 +-
 drivers/cpufreq/sa1110-cpufreq.c                          | 2 +-
 drivers/firmware/efi/libstub/vsprintf.c                   | 2 +-
 drivers/gpu/drm/i915/selftests/i915_vma.c                 | 2 +-
 drivers/gpu/drm/xe/xe_guc_ads.c                           | 2 +-
 drivers/md/bcache/btree.c                                 | 6 +++---
 drivers/md/bcache/util.h                                  | 2 +-
 drivers/md/dm-raid.c                                      | 6 +++---
 drivers/mtd/devices/mtd_dataflash.c                       | 2 +-
 drivers/net/ethernet/marvell/mvneta.c                     | 2 +-
 drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c           | 2 +-
 drivers/net/ethernet/sfc/falcon/nic.c                     | 8 ++++----
 drivers/net/ethernet/sfc/nic.c                            | 8 ++++----
 drivers/net/ethernet/sfc/siena/nic.c                      | 8 ++++----
 drivers/net/wireless/intel/iwlwifi/mei/net.c              | 4 ++--
 drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c         | 3 +--
 drivers/tty/serial/msm_serial.c                           | 2 +-
 fs/erofs/zdata.c                                          | 4 ++--
 fs/proc/base.c                                            | 6 +++---
 fs/proc/namespaces.c                                      | 2 +-
 kernel/bpf/log.c                                          | 2 +-
 lib/crypto/tests/hash-test-template.h                     | 4 ++--
 mm/kmemleak.c                                             | 2 +-
 mm/memcontrol-v1.c                                        | 4 ++--
 net/ipv4/tcp_input.c                                      | 4 ++--
 tools/bpf/bpftool/tracelog.c                              | 2 +-
 tools/testing/selftests/bpf/prog_tests/select_reuseport.c | 4 ++--
 tools/testing/selftests/bpf/prog_tests/sk_lookup.c        | 8 ++++----
 tools/testing/selftests/bpf/prog_tests/sockmap_listen.c   | 8 ++++----
 tools/testing/selftests/bpf/prog_tests/sockmap_redir.c    | 8 ++++----
 42 files changed, 72 insertions(+), 73 deletions(-)

I applied the semantic patch as

            $ time find * -type f \
            | grep '\.[ch]$' \
            | xargs grep -l ARRAY_SIZE \
            | xargs grep -L '^#define ARRAY_END' \
            | xargs grep -l \
		-e '+ *ARRAY_SIZE' \
		-e 'ARRAY_SIZE(.*) *+' \
		-e '&.*ARRAY_SIZE' \
            | xargs spatch --sp-file ~/src/spatch/array_end.sp --in-place;

How should I proceed with this?  Should I send separate patches for each
subtree?  Or should I just send the semantic patch and let maintainers
run it?  Or should I senda big patch?

Also, I don't know if all of those files are able to use
include/linux/array_size.h.  Do tools/ need a separate definition?


Have a lovely day!
Alex

-- 
<https://www.alejandro-colomar.es>
Re: [PATCH v5 4/4] mm: Use ARRAY_END() instead of open-coding it
Posted by Kees Cook 1 month, 2 weeks ago 
On Sun, Dec 21, 2025 at 03:07:14PM +0100, Alejandro Colomar wrote:
> Hi Kees, Andrew,
> 
> On Wed, Dec 10, 2025 at 05:37:13PM -0800, Kees Cook wrote:
> > > > Are there other open-coded instances that could be replaced? This seems like a great task for a coccinelle script.
> > > 
> > > There are many, but I wanted to keep them out of this initial patch set,
> > > to make it easy to apply.  When this one is applied, I could work on a
> > > second round that replaces more of them with coccinelle.  This is just
> > > for showing that this is beneficial, and to make sure that you ask for
> > > more.  :)
> > > 
> > > Also, it's easier if there are few maintainers that would block an
> > > initial patch set.  If restrict the patch set to a few files, I don't
> > > have to deal with many of them.  Once I get used to this, I'll deal with
> > > all of them.
> > 
> > Sounds good!
> 
> Now that the first patch set has been merged, I'm working on a second
> round.
> 
> I've written a semantic patch:
> 
> 	$ cat src/spatch/array_end.sp 
> 	@@
> 	expression a;
> 	@@
> 
> 	- a + ARRAY_SIZE(a)
> 	+ ARRAY_END(a)
> 
> 	@@
> 	expression a;
> 	@@
> 
> 	- ARRAY_SIZE(a) + a
> 	+ ARRAY_END(a)

I think you can add parens which will be silently removed but gain you
the commutative behavior:

@@
expression a;
@@

- (ARRAY_SIZE(a) + a)
+ ARRAY_END(a)

I *think* that'll cover "a + ARRAY_SIZE(a)" too.

Anyway, looks good! You could send it directly to Linus at the end of
the next rc1, and he may take it. If not, you'll want to split the patch
up and send to subsystems after ARRAY_END is in Linus's tree. I use this
tool to split a large single patch into per-subsystem patches:
https://github.com/kees/kernel-tools/blob/trunk/split-on-maintainer

-- 
Kees Cook
Re: [PATCH v5 4/4] mm: Use ARRAY_END() instead of open-coding it
Posted by Alejandro Colomar 1 month, 2 weeks ago 
Hi Kees,

On Mon, Dec 22, 2025 at 03:21:21PM -0800, Kees Cook wrote:
> > Now that the first patch set has been merged, I'm working on a second
> > round.
> > 
> > I've written a semantic patch:
> > 
> > 	$ cat src/spatch/array_end.sp 
> > 	@@
> > 	expression a;
> > 	@@
> > 
> > 	- a + ARRAY_SIZE(a)
> > 	+ ARRAY_END(a)
> > 
> > 	@@
> > 	expression a;
> > 	@@
> > 
> > 	- ARRAY_SIZE(a) + a
> > 	+ ARRAY_END(a)
> 
> I think you can add parens which will be silently removed but gain you
> the commutative behavior:

Ahhh, thanks!  I was wondering how I could get commutative behavior.

> @@
> expression a;
> @@
> 
> - (ARRAY_SIZE(a) + a)
> + ARRAY_END(a)
> 
> I *think* that'll cover "a + ARRAY_SIZE(a)" too.

Yup, it works.  :)

> Anyway, looks good!

Thanks!

> You could send it directly to Linus at the end of
> the next rc1, and he may take it.

I'll send a draft before that, just for you to review the actual patch.

> If not, you'll want to split the patch
> up and send to subsystems after ARRAY_END is in Linus's tree. I use this
> tool to split a large single patch into per-subsystem patches:
> https://github.com/kees/kernel-tools/blob/trunk/split-on-maintainer

Okay.  I can do both, anyway.


Have a lovely night!
Alex

-- 
<https://www.alejandro-colomar.es>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.