Skip to content

Add ARRAY_END(), and use it to fix off-by-one bugs

Alejandro Colomar linux-mm lkml llvm oe-kbuild-all 6 versions – In review

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v6 Latest4 patches 1 replyIn review 3 of 4 applied
  2. v54 patches 6 repliesSuperseded
  3. v44 patches 0 repliesSuperseded
Earlier versions · 3
VersionPostedPatchesRepliesStatus
v34 patches2 repliesSuperseded
v24 patches1 replySuperseded
v13 patches10 repliesSuperseded

Patches in v6 3 of 4 applied

#Subject and filesSizeRepliesApplied
1/4array_size.h: Add ARRAY_END()drivers/block/floppy.c include/linux/array_size.h+6 -20 replies
2/4mm: Fix benign off-by-one bugsmm/kfence/kfence_test.c mm/kmsan/kmsan_test.c+3 -30 repliesApplied
3/4kernel: Fix off-by-one benign bugskernel/kcsan/kcsan_test.c+2 -20 repliesApplied
4/4mm: Use ARRAY_END() instead of open-coding itmm/kmemleak.c mm/memcontrol-v1.c+3 -31 replyApplied

Reviews trailers per patch

2 of 4 patches have a Reviewed-by or Acked-by.

PatchMarco ElverSeongJae Park
1/4
2/4A-b
3/4
4/4R-b

Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.

Read the v6 thread

Cover letter [PATCH v6 0/4] Add ARRAY_END(), and use it to fix off-by-one bugs

Add ARRAY_END(), and use it to fix off-by-one bugs

ARRAY_END() is a macro to calculate a pointer to one past the last element of an array argument. This is a very common pointer, which is used to iterate over all elements of an array:

        for (T *p = a; p < ARRAY_END(a); p++)
                ...

Of course, this pointer should never be dereferenced. A pointer one past the last element of an array should not be dereferenced; it's perfectly fine to hold such a pointer --and a good thing to do--, but the only thing it should be used for is comparing it with other pointers derived from the same array.

Due to how special these pointers are, it would be good to use consistent naming. It's common to name such a pointer 'end' --in fact, we have many such cases in the kernel--. C++ even standardized this name with std::end(). Let's try naming such pointers 'end', and try also avoid using 'end' for pointers that are not the result of ARRAY_END().

It has been incorrectly suggested that these pointers are dangerous, and that they should never be used, suggesting to use something like

	#define ARRAY_LAST(a)  ((a) + ARRAY_SIZE(a) - 1)

	for (T *p = a; p <= ARRAY_LAST(a); p++)
		...

This is bogus, as it doesn't scale down to arrays of 0 elements. In the case of an array of 0 elements, ARRAY_LAST() would underflow the pointer, which not only it can't be dereferenced, it can't even be held (it produces Undefined Behavior). That would be a footgun. Such arrays don't exist per the ISO C standard; however, GCC supports them as an extension (with partial support, though; GCC has a few bugs which need to be fixed).

This patch set fixes a few places where it was intended to use the array end (that is, one past the last element), but accidentally a pointer to the last element was used instead, thus wasting one byte.

It also replaces other places where the array end was correctly calculated with ARRAY_SIZE(), by using the simpler ARRAY_END().

Also, there was one drivers/ file that already defined this macro. We remove that definition, to not conflict with this one.

Cc: Kees Cook <[email protected]>
Cc: Christopher Bazley <[email protected]>
Cc: Rasmus Villemoes <[email protected]>
Cc: Marco Elver <[email protected]>
Cc: Michal Hocko <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: Al Viro <[email protected]>
Cc: Alexander Potapenko <[email protected]>
Cc: Dmitry Vyukov <[email protected]>
Cc: Jann Horn <[email protected]>
Cc: Andrew Morton <[email protected]>
Cc: "Maciej W. Rozycki" <[email protected]>
In-Reply-To: <[email protected]>

---

Hi,

v6 adds a commit log to the last commit. Everything else is unchanged. See range-diff below.

Have a lovely day!
Alex

Alejandro Colomar (4):
array_size.h: Add ARRAY_END()
mm: Fix benign off-by-one bugs
kernel: Fix off-by-one benign bugs
mm: Use ARRAY_END() instead of open-coding it

 drivers/block/floppy.c     | 2 --
 include/linux/array_size.h | 6 ++++++
 kernel/kcsan/kcsan_test.c  | 4 ++--
 mm/kfence/kfence_test.c    | 4 ++--
 mm/kmemleak.c              | 2 +-
 mm/kmsan/kmsan_test.c      | 2 +-
 mm/memcontrol-v1.c         | 4 ++--
 7 files changed, 14 insertions(+), 10 deletions(-)

Range-diff against v5:

1:  5973cfb67419 = 1:  5973cfb67419 array_size.h: Add ARRAY_END()
2:  9c38dd009c17 = 2:  9c38dd009c17 mm: Fix benign off-by-one bugs
3:  b4a945a4d40b = 3:  b4a945a4d40b kernel: Fix off-by-one benign bugs
4:  e7bde864b039 ! 4:  2335917d1238 mm: Use ARRAY_END() instead of open-coding it

@@ Metadata
## Commit message ##
mm: Use ARRAY_END() instead of open-coding it

+ There aren't any bugs in this code; it's purely cosmetic.
+
+ By using ARRAY_END(), we prevent future issues, in case the code is
+ modified; it has less moving parts. Also, it should be more readable
+ (and perhaps more importantly, greppable), as there are several ways of
+ writing an expression that gets the end of an array, which are unified
+ by this API name.
+

Signature · 1 line
2.51.0