Re: [PATCH][next] fs: omfs: Use flexible-array member in struct omfs_extent

Kees Cook
2023-07-17T15:21:35Z

On Fri, Jul 14, 2023 at 01:56:37PM -0600, Gustavo A. R. Silva wrote:
> There are some binary differences before and after changes, but this are
> expected due to the change in the size of 'struct omfs_extent' and the
> necessary adjusments.

For binary changes, I think commit logs should have more details. In
this case, I can figure it out:

> diff --git a/fs/omfs/file.c b/fs/omfs/file.c
> index de8f57ee39ec..6b580b9da8e3 100644
> --- a/fs/omfs/file.c
> +++ b/fs/omfs/file.c
> @@ -14,7 +14,7 @@ static u32 omfs_max_extents(struct omfs_sb_info *sbi, int offset)
> {
> return (sbi->s_sys_blocksize - offset -
> sizeof(struct omfs_extent)) /
> - sizeof(struct omfs_extent_entry) + 1;
> + sizeof(struct omfs_extent_entry);
> }

I think the original calculation meant to do:

return (sbi->s_sys_blocksize - offset -
(sizeof(struct omfs_extent) - sizeof(struct omfs_extent_entry))) /
sizeof(struct omfs_extent_entry);

So this binary difference looks correct. I rebuilt before/after this
patch with omfs_max_extents() marked as noinline, and all the binary
changes were isolated here, and did exactly as expected: the first half
is 16 smaller (size of struct omfs_extent_entry), and the final +1 is
removed:

- 2e1: lea -0x20(%rbx),%rax
+ 2e1: lea -0x10(%rbx),%rax
2e5: pop %rbx
2e6: pop %rbp
2e7: shr $0x4,%rax
- 2eb: add $0x1,%eax

So this looks correct to me. Thanks!

Reviewed-by: Kees Cook <keescook@chromium.org>

--
Kees Cook