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@xxxxxxxxxxxxxxxxxxxxxxxxx>
> Link: https://syzkaller.appspot.com/bug?extid=048585f3f4227bb2b49b [1]
> Signed-off-by: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
> ---
> 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
