Re: [PATCH][next] xfs: Replace one-element arrays with flexible-array members

From: Kees Cook
Date: Fri Feb 03 2023 - 12:54:10 EST


On Thu, Feb 02, 2023 at 07:24:50PM -0600, Gustavo A. R. Silva wrote:
> One-element arrays are deprecated, and we are replacing them with flexible
> array members instead. So, replace one-element arrays with flexible-array
> members in structures xfs_attr_leaf_name_local and
> xfs_attr_leaf_name_remote.
>
> The only binary differences reported after the changes are all like
> these:
>
> fs/xfs/libxfs/xfs_attr_leaf.o
> _@@ -435,7 +435,7 @@
> 3b8: movzbl 0x2(%rbx),%eax
> 3bc: rol $0x8,%bp
> 3c0: movzwl %bp,%ebp
> - 3c3: lea 0x2(%rax,%rbp,1),%ebx
> + 3c3: lea 0x3(%rax,%rbp,1),%ebx
> 3c7: call 3cc <xfs_attr_leaf_entsize+0x8c>
> 3c8: R_X86_64_PLT32 __tsan_func_exit-0x4
> 3cc: or $0x3,%ebx
> _@@ -454,7 +454,7 @@
> 3ea: movzbl 0x8(%rbx),%ebx
> 3ee: call 3f3 <xfs_attr_leaf_entsize+0xb3>
> 3ef: R_X86_64_PLT32 __tsan_func_exit-0x4
> - 3f3: add $0xa,%ebx
> + 3f3: add $0xb,%ebx
> 3f6: or $0x3,%ebx
> 3f9: add $0x1,%ebx
> 3fc: mov %ebx,%eax
>
> similar changes in fs/xfs/scrub/attr.o and fs/xfs/xfs.o object files.

I usually turn off the sanitizers for the A/B build comparisons to make
it easier to read the results. It looks like it _grew_ in size here,
though?

> And the reason for this is because of the round_up() macro called in
> functions xfs_attr_leaf_entsize_remote() and xfs_attr_leaf_entsize_local(),
> which is compensanting for the one-byte reduction in size (due to the
> flex-array transformation) of structures xfs_attr_leaf_name_remote and
> xfs_attr_leaf_name_local. So, sizes remain the same before and after
> changes.
>
> This helps with the ongoing efforts to tighten the FORTIFY_SOURCE
> routines on memcpy() and help us make progress towards globally
> enabling -fstrict-flex-arrays=3 [1].
>
> Link: https://github.com/KSPP/linux/issues/79
> Link: https://github.com/KSPP/linux/issues/251
> Link: https://gcc.gnu.org/pipermail/gcc-patches/2022-October/602902.html [1]
> Signed-off-by: Gustavo A. R. Silva <gustavoars@xxxxxxxxxx>

If xfstests pass, this seems good to me. Thanks!

Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx>

--
Kees Cook