| Message ID | 20180824153726.11815-1-idryomov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ceph: avoid a use-after-free in ceph_destroy_options() | expand |
On Fri, Aug 24, 2018 at 11:40 PM Ilya Dryomov <idryomov@gmail.com> wrote: > > syzbot reported a use-after-free in ceph_destroy_options(), called from > ceph_mount(). The problem was that create_fs_client() consumed the opt > pointer on some errors, but not on all of them. Make sure it always > consumes both libceph and ceph options. > > Reported-by: syzbot+8ab6f1042021b4eed062@syzkaller.appspotmail.com > Signed-off-by: Ilya Dryomov <idryomov@gmail.com> > --- > fs/ceph/super.c | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/fs/ceph/super.c b/fs/ceph/super.c > index 43ca3b763875..eab1359d0553 100644 > --- a/fs/ceph/super.c > +++ b/fs/ceph/super.c > @@ -602,6 +602,8 @@ static int extra_mon_dispatch(struct ceph_client *client, struct ceph_msg *msg) > > /* > * create a new fs client > + * > + * Success or not, this function consumes @fsopt and @opt. > */ > static struct ceph_fs_client *create_fs_client(struct ceph_mount_options *fsopt, > struct ceph_options *opt) > @@ -609,17 +611,20 @@ static struct ceph_fs_client *create_fs_client(struct ceph_mount_options *fsopt, > struct ceph_fs_client *fsc; > int page_count; > size_t size; > - int err = -ENOMEM; > + int err; > > fsc = kzalloc(sizeof(*fsc), GFP_KERNEL); > - if (!fsc) > - return ERR_PTR(-ENOMEM); > + if (!fsc) { > + err = -ENOMEM; > + goto fail; > + } > > fsc->client = ceph_create_client(opt, fsc); > if (IS_ERR(fsc->client)) { > err = PTR_ERR(fsc->client); > goto fail; > } > + opt = NULL; /* fsc->client now owns this */ > > fsc->client->extra_mon_dispatch = extra_mon_dispatch; > fsc->client->osdc.abort_on_full = true; > @@ -677,6 +682,9 @@ static struct ceph_fs_client *create_fs_client(struct ceph_mount_options *fsopt, > ceph_destroy_client(fsc->client); > fail: > kfree(fsc); > + if (opt) > + ceph_destroy_options(opt); > + destroy_mount_options(fsopt); > return ERR_PTR(err); > } > > @@ -1042,8 +1050,6 @@ static struct dentry *ceph_mount(struct file_system_type *fs_type, > fsc = create_fs_client(fsopt, opt); > if (IS_ERR(fsc)) { > res = ERR_CAST(fsc); > - destroy_mount_options(fsopt); > - ceph_destroy_options(opt); > goto out_final; > } > > -- > 2.14.4 > Acked-by: "Yan, Zheng" <zyan@redhat.com> Thanks Yan, Zheng
diff --git a/fs/ceph/super.c b/fs/ceph/super.c index 43ca3b763875..eab1359d0553 100644 --- a/fs/ceph/super.c +++ b/fs/ceph/super.c @@ -602,6 +602,8 @@ static int extra_mon_dispatch(struct ceph_client *client, struct ceph_msg *msg) /* * create a new fs client + * + * Success or not, this function consumes @fsopt and @opt. */ static struct ceph_fs_client *create_fs_client(struct ceph_mount_options *fsopt, struct ceph_options *opt) @@ -609,17 +611,20 @@ static struct ceph_fs_client *create_fs_client(struct ceph_mount_options *fsopt, struct ceph_fs_client *fsc; int page_count; size_t size; - int err = -ENOMEM; + int err; fsc = kzalloc(sizeof(*fsc), GFP_KERNEL); - if (!fsc) - return ERR_PTR(-ENOMEM); + if (!fsc) { + err = -ENOMEM; + goto fail; + } fsc->client = ceph_create_client(opt, fsc); if (IS_ERR(fsc->client)) { err = PTR_ERR(fsc->client); goto fail; } + opt = NULL; /* fsc->client now owns this */ fsc->client->extra_mon_dispatch = extra_mon_dispatch; fsc->client->osdc.abort_on_full = true; @@ -677,6 +682,9 @@ static struct ceph_fs_client *create_fs_client(struct ceph_mount_options *fsopt, ceph_destroy_client(fsc->client); fail: kfree(fsc); + if (opt) + ceph_destroy_options(opt); + destroy_mount_options(fsopt); return ERR_PTR(err); } @@ -1042,8 +1050,6 @@ static struct dentry *ceph_mount(struct file_system_type *fs_type, fsc = create_fs_client(fsopt, opt); if (IS_ERR(fsc)) { res = ERR_CAST(fsc); - destroy_mount_options(fsopt); - ceph_destroy_options(opt); goto out_final; }
syzbot reported a use-after-free in ceph_destroy_options(), called from ceph_mount(). The problem was that create_fs_client() consumed the opt pointer on some errors, but not on all of them. Make sure it always consumes both libceph and ceph options. Reported-by: syzbot+8ab6f1042021b4eed062@syzkaller.appspotmail.com Signed-off-by: Ilya Dryomov <idryomov@gmail.com> --- fs/ceph/super.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)