Vivek Goyal
2021-Mar-23 17:10 UTC
[PATCH] fuse: Fix a potential double free in virtio_fs_get_tree
On Mon, Mar 22, 2021 at 10:18:31PM -0700, Lv Yunlong wrote:> In virtio_fs_get_tree, fm is allocated by kzalloc() and > assigned to fsc->s_fs_info by fsc->s_fs_info=fm statement. > If the kzalloc() failed, it will goto err directly, so that > fsc->s_fs_info must be non-NULL and fm will be freed.sget_fc() will either consume fsc->s_fs_info in case a new super block is allocated and set fsc->s_fs_info. In that case we don't free fc or fm. Or, sget_fc() will return with fsc->s_fs_info set in case we already found a super block. In that case we need to free fc and fm. In case of error from sget_fc(), fc/fm need to be freed first and then error needs to be returned to caller. if (IS_ERR(sb)) return PTR_ERR(sb); If we allocated a new super block in sget_fc(), then next step is to initialize it. if (!sb->s_root) { err = virtio_fs_fill_super(sb, fsc); } If we run into errors here, then fc/fm need to be freed. So current code looks fine to me. Vivek> > But later fm is freed again when virtio_fs_fill_super() fialed. > I think the statement if (fsc->s_fs_info) {kfree(fm);} is > misplaced. > > My patch puts this statement in the correct palce to avoid > double free. > > Signed-off-by: Lv Yunlong <lyl2019 at mail.ustc.edu.cn> > --- > fs/fuse/virtio_fs.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/fs/fuse/virtio_fs.c b/fs/fuse/virtio_fs.c > index 8868ac31a3c0..727cf436828f 100644 > --- a/fs/fuse/virtio_fs.c > +++ b/fs/fuse/virtio_fs.c > @@ -1437,10 +1437,7 @@ static int virtio_fs_get_tree(struct fs_context *fsc) > > fsc->s_fs_info = fm; > sb = sget_fc(fsc, virtio_fs_test_super, set_anon_super_fc); > - if (fsc->s_fs_info) { > - fuse_conn_put(fc); > - kfree(fm); > - } > + > if (IS_ERR(sb)) > return PTR_ERR(sb); > > @@ -1457,6 +1454,11 @@ static int virtio_fs_get_tree(struct fs_context *fsc) > sb->s_flags |= SB_ACTIVE; > } > > + if (fsc->s_fs_info) { > + fuse_conn_put(fc); > + kfree(fm); > + } > + > WARN_ON(fsc->root); > fsc->root = dget(sb->s_root); > return 0; > -- > 2.25.1 > >