Re: [PATCH] nilfs2: fix incomplete initialization in nilfs_direct_assign_p()

[Ryusuke Konishi <konishi.ryusuke-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On Sun, Mar 26, 2023 at 3:32 PM Tetsuo Handa wrote:
>
> syzbot is reporting uninit value at nilfs_add_checksums_on_logs() [1], for
> nilfs_direct_assign_p() from nilfs_direct_assign() from nilfs_bmap_assign()
> does not initialize "struct nilfs_binfo_dat"->bi_pad field.
>
> We need to initialize sizeof("union nilfs_binfo"->bi_dat) bytes if
> nilfs_write_dat_node_binfo() from nilfs_segctor_assign() copies it
> and nilfs_add_checksums_on_logs() passes it to CRC function.
>
> Reported-by: syzbot <syzbot+048585f3f4227bb2b49b-Pl5Pbv+GP7P466ipTTIvnc23WoclnBCf@public.gmane.orgm>
> Link: https://syzkaller.appspot.com/bug?extid=048585f3f4227bb2b49b [1]
> Signed-off-by: Tetsuo Handa <penguin-kernel-JPay3/Yim36HaxMnTkn67Xf5DAMn2ifp@public.gmane.org>
> ---
> I'm not sure whether this can fix the bug, for a reproducer is not yet
> available...
>
>  fs/nilfs2/direct.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/nilfs2/direct.c b/fs/nilfs2/direct.c
> index a35f2795b242..4358b4581ec4 100644
> --- a/fs/nilfs2/direct.c
> +++ b/fs/nilfs2/direct.c
> @@ -313,7 +313,8 @@ static int nilfs_direct_assign_p(struct nilfs_bmap *direct,
>         nilfs_direct_set_ptr(direct, key, blocknr);
>
>         binfo->bi_dat.bi_blkoff = cpu_to_le64(key);
> -       binfo->bi_dat.bi_level = 0;

> +       /* initialize bi_pad field together while assigning bi_level field */
> +       *(u64 *) &binfo->bi_dat.bi_level = (u64) 0;

Could you change this just to the initialization using bi_pad below?

     memset(binfo->bi_dat.bi_pad, 0, sizeof(binfo->bi_dat.bi_pad));

This is not efficient depending on the compiler, but I'd rather avoid
the non-intuitive initialization using the above cast and use a
straightforward initialization.

This does not eliminate the problem, but it does fix one, so I'll send
it upstream.

Thanks,
Ryusuke Konishi