Skip to content

Commit

Permalink
Cleanup: Replace oldstyle struct hack with C99 flexible array members
Browse files Browse the repository at this point in the history
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/[email protected]/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 <[email protected]>
Signed-off-by: Richard Yao <[email protected]>
Closes #14372
  • Loading branch information
ryao authored and tonyhutter committed Sep 20, 2023
1 parent ae0d644 commit a93c30a
Show file tree
Hide file tree
Showing 5 changed files with 9 additions and 9 deletions.
4 changes: 2 additions & 2 deletions module/lua/lfunc.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,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);
Expand Down
4 changes: 2 additions & 2 deletions module/lua/lobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -514,14 +514,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;


Expand Down
4 changes: 2 additions & 2 deletions module/os/freebsd/zfs/zfs_debug.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ typedef struct zfs_dbgmsg {
list_node_t zdm_node;
time_t zdm_timestamp;
int zdm_size;
char zdm_msg[1]; /* variable length allocation */
char zdm_msg[];
} zfs_dbgmsg_t;

list_t zfs_dbgmsgs;
Expand Down Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions module/os/linux/zfs/zfs_debug.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ typedef struct zfs_dbgmsg {
procfs_list_node_t zdm_node;
uint64_t zdm_timestamp;
int zdm_size;
char zdm_msg[1]; /* variable length allocation */
char zdm_msg[]; /* variable length allocation */
} zfs_dbgmsg_t;

procfs_list_t zfs_dbgmsgs;
Expand Down Expand Up @@ -134,7 +134,7 @@ __set_error(const char *file, const char *func, int line, int err)
void
__zfs_dbgmsg(char *buf)
{
int size = sizeof (zfs_dbgmsg_t) + strlen(buf);
int 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();
Expand Down
2 changes: 1 addition & 1 deletion module/zfs/vdev_indirect.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/*
Expand Down

0 comments on commit a93c30a

Please sign in to comment.