[PATCH]: vfs_glusterfs: Use after free.

[Ira Cooper]

On Friday, March 4, 2016, Jeremy Allison <jra at samba.org> wrote:

> On Fri, Mar 04, 2016 at 06:18:27PM -0500, Ira Cooper wrote:
> > This is a pure crash bug as far as I can see, but one that needs to be
> > fixed.
> >
> > BUG: https://bugzilla.samba.org/show_bug.cgi?id=11774
> >
> > Patch is enclosed.
> >
> > I'll provide backports as soon as a final version is accepted.
>
> OK, took me a lot of staring to understand,
> but I finally did :-).
>
> LGTM. Pushed.
>
>
>
> > From 42e3d35883a78eebf4c765a83d6dc87758595730 Mon Sep 17 00:00:00 2001
> > From: Ira Cooper <ira at samba.org <javascript:;>>
> > Date: Fri, 4 Mar 2016 18:00:07 -0500
> > Subject: [PATCH] vfs_glusterfs: Fix use after free in AIO callback.
> >
> > The wrapper->state pointer is not getting NULLed during free
> > allowing use of freed memory, causing a crash.
> >
> > Thanks to Red Hat for discovering this issue.
> >
> > BUG: https://bugzilla.samba.org/show_bug.cgi?id=11774
> >
> > Signed-off-by: Ira Copper <ira at samba.org <javascript:;>>
> > Reviewed-by: Poornima G <pgurusid at redhat.com <javascript:;>>
> > Tested-by: Christopher Blum <cblum at redhat.com <javascript:;>>
> > ---
> >  source3/modules/vfs_glusterfs.c | 15 +++++++--------
> >  1 file changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/source3/modules/vfs_glusterfs.c
> b/source3/modules/vfs_glusterfs.c
> > index c98e480..2008342 100644
> > --- a/source3/modules/vfs_glusterfs.c
> > +++ b/source3/modules/vfs_glusterfs.c
> > @@ -507,7 +507,9 @@ struct glusterfs_aio_state {
> >
> >  static int aio_wrapper_destructor(struct glusterfs_aio_wrapper *wrap)
> >  {
> > -     wrap->state->cancelled = true;
> > +     if (wrap->state != NULL) {
> > +             wrap->state->cancelled = true;
> > +     }
> >
> >       return 0;
> >  }
> > @@ -744,7 +746,6 @@ static struct tevent_req
> *vfs_gluster_pwrite_send(struct vfs_handle_struct
> >  static ssize_t vfs_gluster_recv(struct tevent_req *req,
> >                               struct vfs_aio_state *vfs_aio_state)
> >  {
> > -     struct glusterfs_aio_state *state = NULL;
> >       struct glusterfs_aio_wrapper *wrapper = NULL;
> >       int ret = 0;
> >
> > @@ -754,9 +755,7 @@ static ssize_t vfs_gluster_recv(struct tevent_req
> *req,
> >               return -1;
> >       }
> >
> > -     state = wrapper->state;
> > -
> > -     if (state == NULL) {
> > +     if (wrapper->state == NULL) {
> >               return -1;
> >       }
> >
> > @@ -764,12 +763,12 @@ static ssize_t vfs_gluster_recv(struct tevent_req
> *req,
> >               return -1;
> >       }
> >
> > -     *vfs_aio_state = state->vfs_aio_state;
> > -     ret = state->ret;
> > +     *vfs_aio_state = wrapper->state->vfs_aio_state;
> > +     ret = wrapper->state->ret;
> >
> >       /* Clean up the state, it is in a NULL context. */
> >
> > -     TALLOC_FREE(state);
> > +     TALLOC_FREE(wrapper->state);
> >
> >       return ret;
> >  }
> > --
> > 2.5.0
>
>
Yeah, it becomes real obvious if you fix vts_gluster_recv without making
the other fix. :)

In fact that is how I actually proved what the bug was to myself.

Thanks,

-Ira