From 9c8fabffa2b15a4ada570c8c469ff0709d00b003 Mon Sep 17 00:00:00 2001 From: Richard Yao Date: Tue, 10 Jan 2023 15:44:35 -0500 Subject: [PATCH] Cleanup: Replace oldstyle struct hack with C99 flexible array members The Linux 5.16.14 kernel's coccicheck caught this. The semantic patch that caught it was: ./scripts/coccinelle/misc/flexible_array.cocci However, unlike the cases where the GNU zero length array extension had been used, coccicheck would not suggest patches for the older style single member arrays. That was good because blindly changing them would break size calculations in most cases. Therefore, this required care to make sure that we did not break size calculations. In the case of `indirect_split_t`, we use `offsetof(indirect_split_t, is_child[is->is_children])` to calculate size. This might be subtly wrong according to an old mailing list thread: https://inbox.sourceware.org/gcc-prs/20021226123454.27019.qmail@sources.redhat.com/T/ That is because the C99 specification should consider the flexible array members to start at the end of a structure, but compilers prefer to put padding at the end. A suggestion was made to allow compilers to allocate padding after the VLA like compilers already did: http://std.dkuug.dk/JTC1/SC22/WG14/www/docs/n983.htm However, upon thinking about it, whether or not we allocate end of structure padding does not matter, so using offsetof() to calculate the size of the structure is fine, so long as we do not mix it with sizeof() on structures with no array members. In the case that we mix them and padding causes offsetof(struct_t, vla_member[0]) to differ from sizeof(struct_t), we would be doing unsafe operations if we underallocate via `offsetof()` and then overcopy via sizeof(). Reviewed-by: Brian Behlendorf Signed-off-by: Richard Yao Closes #14372 --- module/lua/lfunc.h | 4 ++-- module/lua/lobject.h | 4 ++-- module/os/freebsd/zfs/zfs_debug.c | 4 ++-- module/os/linux/zfs/zfs_debug.c | 4 ++-- module/zfs/vdev_indirect.c | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/module/lua/lfunc.h b/module/lua/lfunc.h index ca0d3a3e0b..1dc6995ca9 100644 --- a/module/lua/lfunc.h +++ b/module/lua/lfunc.h @@ -12,10 +12,10 @@ #define sizeCclosure(n) (cast(int, sizeof(CClosure)) + \ - cast(int, sizeof(TValue)*((n)-1))) + cast(int, sizeof(TValue)*((n)))) #define sizeLclosure(n) (cast(int, sizeof(LClosure)) + \ - cast(int, sizeof(TValue *)*((n)-1))) + cast(int, sizeof(TValue *)*((n)))) LUAI_FUNC Proto *luaF_newproto (lua_State *L); diff --git a/module/lua/lobject.h b/module/lua/lobject.h index d29d0068c7..b7c6b41ac7 100644 --- a/module/lua/lobject.h +++ b/module/lua/lobject.h @@ -513,14 +513,14 @@ typedef struct UpVal { typedef struct CClosure { ClosureHeader; lua_CFunction f; - TValue upvalue[1]; /* list of upvalues */ + TValue upvalue[]; /* list of upvalues */ } CClosure; typedef struct LClosure { ClosureHeader; struct Proto *p; - UpVal *upvals[1]; /* list of upvalues */ + UpVal *upvals[]; /* list of upvalues */ } LClosure; diff --git a/module/os/freebsd/zfs/zfs_debug.c b/module/os/freebsd/zfs/zfs_debug.c index abb3c00331..78d50c6fd8 100644 --- a/module/os/freebsd/zfs/zfs_debug.c +++ b/module/os/freebsd/zfs/zfs_debug.c @@ -30,7 +30,7 @@ typedef struct zfs_dbgmsg { list_node_t zdm_node; time_t zdm_timestamp; uint_t zdm_size; - char zdm_msg[1]; /* variable length allocation */ + char zdm_msg[]; } zfs_dbgmsg_t; static list_t zfs_dbgmsgs; @@ -159,7 +159,7 @@ __zfs_dbgmsg(char *buf) DTRACE_PROBE1(zfs__dbgmsg, char *, buf); - size = sizeof (zfs_dbgmsg_t) + strlen(buf); + size = sizeof (zfs_dbgmsg_t) + strlen(buf) + 1; zdm = kmem_zalloc(size, KM_SLEEP); zdm->zdm_size = size; zdm->zdm_timestamp = gethrestime_sec(); diff --git a/module/os/linux/zfs/zfs_debug.c b/module/os/linux/zfs/zfs_debug.c index d187801423..b090ec684e 100644 --- a/module/os/linux/zfs/zfs_debug.c +++ b/module/os/linux/zfs/zfs_debug.c @@ -30,7 +30,7 @@ typedef struct zfs_dbgmsg { procfs_list_node_t zdm_node; uint64_t zdm_timestamp; uint_t zdm_size; - char zdm_msg[1]; /* variable length allocation */ + char zdm_msg[]; /* variable length allocation */ } zfs_dbgmsg_t; static procfs_list_t zfs_dbgmsgs; @@ -135,7 +135,7 @@ __set_error(const char *file, const char *func, int line, int err) void __zfs_dbgmsg(char *buf) { - uint_t size = sizeof (zfs_dbgmsg_t) + strlen(buf); + uint_t size = sizeof (zfs_dbgmsg_t) + strlen(buf) + 1; zfs_dbgmsg_t *zdm = kmem_zalloc(size, KM_SLEEP); zdm->zdm_size = size; zdm->zdm_timestamp = gethrestime_sec(); diff --git a/module/zfs/vdev_indirect.c b/module/zfs/vdev_indirect.c index b70e8edfaf..8c11a574ae 100644 --- a/module/zfs/vdev_indirect.c +++ b/module/zfs/vdev_indirect.c @@ -270,7 +270,7 @@ typedef struct indirect_split { */ indirect_child_t *is_good_child; - indirect_child_t is_child[1]; /* variable-length */ + indirect_child_t is_child[]; } indirect_split_t; /*