The qemu_memfd_alloc_check() routine allocates the fd variable on stack.
This variable is initialized inside the qemu_memfd_alloc() function.
There are several cases when *fd will be left unintialized which can
lead to the unexpected close() in the qemu_memfd_free() call.

Set file descriptor to -1 before calling the qemu_memfd_alloc routine.

Signed-off-by: Dima Stepanov <dimastep@yandex-team.ru>
---
 util/memfd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/util/memfd.c b/util/memfd.c
index d248a53..6287946 100644
--- a/util/memfd.c
+++ b/util/memfd.c
@@ -187,6 +187,7 @@ bool qemu_memfd_alloc_check(void)
         int fd;
         void *ptr;
 
+        fd = -1;
         ptr = qemu_memfd_alloc("test", 4096, 0, &fd, NULL);
         memfd_check = ptr ? MEMFD_OK : MEMFD_KO;
         qemu_memfd_free(ptr, 4096, fd);
-- 
2.7.4

On Wed, Jun 13, 2018 at 10:19 AM, Dima Stepanov <dimastep@yandex-team.ru> wrote:
> The qemu_memfd_alloc_check() routine allocates the fd variable on stack.
> This variable is initialized inside the qemu_memfd_alloc() function.
> There are several cases when *fd will be left unintialized which can
> lead to the unexpected close() in the qemu_memfd_free() call.
>
> Set file descriptor to -1 before calling the qemu_memfd_alloc routine.
>
> Signed-off-by: Dima Stepanov <dimastep@yandex-team.ru>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  util/memfd.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/util/memfd.c b/util/memfd.c
> index d248a53..6287946 100644
> --- a/util/memfd.c
> +++ b/util/memfd.c
> @@ -187,6 +187,7 @@ bool qemu_memfd_alloc_check(void)
>          int fd;
>          void *ptr;
>
> +        fd = -1;
>          ptr = qemu_memfd_alloc("test", 4096, 0, &fd, NULL);
>          memfd_check = ptr ? MEMFD_OK : MEMFD_KO;
>          qemu_memfd_free(ptr, 4096, fd);
> --
> 2.7.4
>
>



-- 
Marc-André Lureau

On 2018-06-13 10:19, Dima Stepanov wrote:
> The qemu_memfd_alloc_check() routine allocates the fd variable on stack.
> This variable is initialized inside the qemu_memfd_alloc() function.
> There are several cases when *fd will be left unintialized which can
> lead to the unexpected close() in the qemu_memfd_free() call.
> 
> Set file descriptor to -1 before calling the qemu_memfd_alloc routine.
> 
> Signed-off-by: Dima Stepanov <dimastep@yandex-team.ru>
> ---
>  util/memfd.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/util/memfd.c b/util/memfd.c
> index d248a53..6287946 100644
> --- a/util/memfd.c
> +++ b/util/memfd.c
> @@ -187,6 +187,7 @@ bool qemu_memfd_alloc_check(void)
>          int fd;

(in case you respin: You could also write "int fd = -1;" in above line)

>          void *ptr;
>  
> +        fd = -1;
>          ptr = qemu_memfd_alloc("test", 4096, 0, &fd, NULL);
>          memfd_check = ptr ? MEMFD_OK : MEMFD_KO;
>          qemu_memfd_free(ptr, 4096, fd);
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

(FWIW: GCC complains with -O3 about this uninitialized variable, too, so
the build breaks with -Werror here - just noticed this today)