1. Patchew
  2. QEMU
  3. View series

[PATCH-for-5.0] tools/virtiofsd/passthrough_ll: Fix double close()

Philippe Mathieu-Daudé posted 1 patch 4 years ago
Test docker-mingw@fedora passed
Test docker-quick@centos7 passed
Test checkpatch passed
Test FreeBSD passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200321120654.7985-1-philmd@redhat.com
Maintainers: "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>
tools/virtiofsd/passthrough_ll.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
[PATCH-for-5.0] tools/virtiofsd/passthrough_ll: Fix double close()
Posted by Philippe Mathieu-Daudé 4 years ago 
On success, the fdopendir() call closes fd. Later on the error
path we try to close an already-closed fd. This can lead to
use-after-free. Fix by only closing the fd if the fdopendir()
call failed.

Cc: qemu-stable@nongnu.org
Fixes: 7c6b66027 (Import passthrough_ll from libfuse fuse-3.8.0)
Reported-by: Coverity (CID 1421933 USE_AFTER_FREE)
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 tools/virtiofsd/passthrough_ll.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 4f259aac70..4c35c95b25 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -1520,8 +1520,7 @@ out_err:
     if (d) {
         if (d->dp) {
             closedir(d->dp);
-        }
-        if (fd != -1) {
+        } else if (fd != -1) {
             close(fd);
         }
         free(d);
-- 
2.21.1
Re: [PATCH-for-5.0] tools/virtiofsd/passthrough_ll: Fix double close()
Posted by Philippe Mathieu-Daudé 4 years ago 
On 3/21/20 1:06 PM, Philippe Mathieu-Daudé wrote:
> On success, the fdopendir() call closes fd. Later on the error
> path we try to close an already-closed fd. This can lead to
> use-after-free. Fix by only closing the fd if the fdopendir()
> call failed.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: 7c6b66027 (Import passthrough_ll from libfuse fuse-3.8.0)

libfuse is correct, the bug was introduced in commit b39bce121b, so:

Fixes: b39bce121b (add dirp_map to hide lo_dirp pointers)

> Reported-by: Coverity (CID 1421933 USE_AFTER_FREE)
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>   tools/virtiofsd/passthrough_ll.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
> index 4f259aac70..4c35c95b25 100644
> --- a/tools/virtiofsd/passthrough_ll.c
> +++ b/tools/virtiofsd/passthrough_ll.c
> @@ -1520,8 +1520,7 @@ out_err:
>       if (d) {
>           if (d->dp) {
>               closedir(d->dp);
> -        }
> -        if (fd != -1) {
> +        } else if (fd != -1) {
>               close(fd);
>           }
>           free(d);
>
Re: [PATCH-for-5.0] tools/virtiofsd/passthrough_ll: Fix double close()
Posted by Dr. David Alan Gilbert 4 years ago 
* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> On 3/21/20 1:06 PM, Philippe Mathieu-Daudé wrote:
> > On success, the fdopendir() call closes fd. Later on the error
> > path we try to close an already-closed fd. This can lead to
> > use-after-free. Fix by only closing the fd if the fdopendir()
> > call failed.
> > 
> > Cc: qemu-stable@nongnu.org
> > Fixes: 7c6b66027 (Import passthrough_ll from libfuse fuse-3.8.0)
> 
> libfuse is correct, the bug was introduced in commit b39bce121b, so:
> 
> Fixes: b39bce121b (add dirp_map to hide lo_dirp pointers)

Queued with that tweak

> > Reported-by: Coverity (CID 1421933 USE_AFTER_FREE)
> > Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > ---
> >   tools/virtiofsd/passthrough_ll.c | 3 +--
> >   1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
> > index 4f259aac70..4c35c95b25 100644
> > --- a/tools/virtiofsd/passthrough_ll.c
> > +++ b/tools/virtiofsd/passthrough_ll.c
> > @@ -1520,8 +1520,7 @@ out_err:
> >       if (d) {
> >           if (d->dp) {
> >               closedir(d->dp);
> > -        }
> > -        if (fd != -1) {
> > +        } else if (fd != -1) {
> >               close(fd);
> >           }
> >           free(d);
> > 
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
Re: [PATCH-for-5.0] tools/virtiofsd/passthrough_ll: Fix double close()
Posted by Stefan Hajnoczi 4 years ago 
On Sat, Mar 21, 2020 at 01:06:54PM +0100, Philippe Mathieu-Daudé wrote:
> On success, the fdopendir() call closes fd. Later on the error
> path we try to close an already-closed fd. This can lead to
> use-after-free. Fix by only closing the fd if the fdopendir()
> call failed.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: 7c6b66027 (Import passthrough_ll from libfuse fuse-3.8.0)
> Reported-by: Coverity (CID 1421933 USE_AFTER_FREE)
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  tools/virtiofsd/passthrough_ll.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)

Thanks!

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.