[v2] Fix leak in getaddrinfo introduced by the fix for CVE-2023-4806 [BZ #30843]
Checks
| Context |
Check |
Description |
| redhat-pt-bot/TryBot-apply_patch |
success
|
Patch applied to master at the time it was sent
|
| redhat-pt-bot/TryBot-32bit |
success
|
Build for i686
|
| linaro-tcwg-bot/tcwg_glibc_build--master-arm |
fail
|
Patch failed to apply
|
| linaro-tcwg-bot/tcwg_glibc_check--master-arm |
fail
|
Patch failed to apply
|
| linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 |
fail
|
Patch failed to apply
|
| linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 |
fail
|
Patch failed to apply
|
Commit Message
Hi,
This is the v2 of my previously posted patch, with all the comments from
Siddhesh taken into account.
Note that before the fix, I can see this in the mtrace log file:
> cat output/x86_64/build/x86_64/final-system/glibc-build/nss/mtrace-tst-nss-gai-hv2-canonname.out
Memory not freed:
-----------------
Address Size Caller
0x000055d96076b2a0 0x11 at 0x93e7c
and when adding the fix, this error goes away.
Cheers,
Romain
---
nss/Makefile | 17 +++++++++++++++++
nss/tst-nss-gai-hv2-canonname.c | 3 +++
sysdeps/posix/getaddrinfo.c | 4 +---
3 files changed, 21 insertions(+), 3 deletions(-)
Comments
On 2023-09-25 00:42, Romain Geissler wrote:
> Hi,
>
> This is the v2 of my previously posted patch, with all the comments from
> Siddhesh taken into account.
>
> Note that before the fix, I can see this in the mtrace log file:
>
> > cat output/x86_64/build/x86_64/final-system/glibc-build/nss/mtrace-tst-nss-gai-hv2-canonname.out
>
> Memory not freed:
> -----------------
> Address Size Caller
> 0x000055d96076b2a0 0x11 at 0x93e7c
Thanks, just a minor nit to fix and we can take this home:
>
>
> and when adding the fix, this error goes away.
>
> Cheers,
> Romain
>
> ---
> nss/Makefile | 17 +++++++++++++++++
> nss/tst-nss-gai-hv2-canonname.c | 3 +++
> sysdeps/posix/getaddrinfo.c | 4 +---
> 3 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/nss/Makefile b/nss/Makefile
> index e3d21e9a899..5553a2b2a7e 100644
> --- a/nss/Makefile
> +++ b/nss/Makefile
> @@ -148,6 +148,15 @@ endif
> extra-test-objs += nss_test1.os nss_test2.os nss_test_errno.os \
> nss_test_gai_hv2_canonname.os
>
> +ifeq ($(run-built-tests),yes)
> +ifneq (no,$(PERL))
> +tests-special += $(objpfx)mtrace-tst-nss-gai-hv2-canonname.out
> +endif
> +endif
> +
> +generated += mtrace-tst-nss-gai-hv2-canonname.out \
> + tst-nss-gai-hv2-canonname.mtrace
> +
> include ../Rules
>
> ifeq (yes,$(have-selinux))
> @@ -216,6 +225,14 @@ endif
> $(objpfx)tst-nss-files-alias-leak.out: $(objpfx)/libnss_files.so
> $(objpfx)tst-nss-files-alias-truncated.out: $(objpfx)/libnss_files.so
>
> +tst-nss-gai-hv2-canonname-ENV = MALLOC_TRACE=$(objpfx)tst-nss-gai-hv2-canonname.mtrace \
> + LD_PRELOAD=$(common-objpfx)/malloc/libc_malloc_debug.so
> +$(objpfx)mtrace-tst-nss-gai-hv2-canonname.out: $(objpfx)tst-nss-gai-hv2-canonname.out
> + { test -r $(objpfx)tst-nss-gai-hv2-canonname.mtrace \
> + || ( echo "tst-nss-gai-hv2-canonname.mtrace does not exist"; exit 77; ) \
> + && $(common-objpfx)malloc/mtrace $(objpfx)tst-nss-gai-hv2-canonname.mtrace; } > $@; \
> + $(evaluate-test)
Please try and fix these to 79 chars.
> +
> # Disable DT_RUNPATH on NSS tests so that the glibc internal NSS
> # functions can load testing NSS modules via DT_RPATH.
> LDFLAGS-tst-nss-test1 = -Wl,--disable-new-dtags
> diff --git a/nss/tst-nss-gai-hv2-canonname.c b/nss/tst-nss-gai-hv2-canonname.c
> index d5f10c07d6a..84feea616be 100644
> --- a/nss/tst-nss-gai-hv2-canonname.c
> +++ b/nss/tst-nss-gai-hv2-canonname.c
> @@ -21,6 +21,7 @@
> #include <netdb.h>
> #include <stdlib.h>
> #include <string.h>
> +#include <mcheck.h>
> #include <support/check.h>
> #include <support/xstdio.h>
> #include "nss/tst-nss-gai-hv2-canonname.h"
> @@ -41,6 +42,8 @@ static void do_prepare (int a, char **av)
> static int
> do_test (void)
> {
> + mtrace();
> +
> __nss_configure_lookup ("hosts", "test_gai_hv2_canonname");
>
> struct addrinfo hints = {};
> diff --git a/sysdeps/posix/getaddrinfo.c b/sysdeps/posix/getaddrinfo.c
> index b4e8ea3880a..69f38bbfb9e 100644
> --- a/sysdeps/posix/getaddrinfo.c
> +++ b/sysdeps/posix/getaddrinfo.c
> @@ -1196,9 +1196,7 @@ free_and_return:
> if (malloc_name)
> free ((char *) name);
> free (addrmem);
> - if (res.free_at)
> - free (res.at);
> - free (res.canon);
> + gaih_result_reset (&res);
>
> return result;
> }
@@ -148,6 +148,15 @@ endif
extra-test-objs += nss_test1.os nss_test2.os nss_test_errno.os \
nss_test_gai_hv2_canonname.os
+ifeq ($(run-built-tests),yes)
+ifneq (no,$(PERL))
+tests-special += $(objpfx)mtrace-tst-nss-gai-hv2-canonname.out
+endif
+endif
+
+generated += mtrace-tst-nss-gai-hv2-canonname.out \
+ tst-nss-gai-hv2-canonname.mtrace
+
include ../Rules
ifeq (yes,$(have-selinux))
@@ -216,6 +225,14 @@ endif
$(objpfx)tst-nss-files-alias-leak.out: $(objpfx)/libnss_files.so
$(objpfx)tst-nss-files-alias-truncated.out: $(objpfx)/libnss_files.so
+tst-nss-gai-hv2-canonname-ENV = MALLOC_TRACE=$(objpfx)tst-nss-gai-hv2-canonname.mtrace \
+ LD_PRELOAD=$(common-objpfx)/malloc/libc_malloc_debug.so
+$(objpfx)mtrace-tst-nss-gai-hv2-canonname.out: $(objpfx)tst-nss-gai-hv2-canonname.out
+ { test -r $(objpfx)tst-nss-gai-hv2-canonname.mtrace \
+ || ( echo "tst-nss-gai-hv2-canonname.mtrace does not exist"; exit 77; ) \
+ && $(common-objpfx)malloc/mtrace $(objpfx)tst-nss-gai-hv2-canonname.mtrace; } > $@; \
+ $(evaluate-test)
+
# Disable DT_RUNPATH on NSS tests so that the glibc internal NSS
# functions can load testing NSS modules via DT_RPATH.
LDFLAGS-tst-nss-test1 = -Wl,--disable-new-dtags
@@ -21,6 +21,7 @@
#include <netdb.h>
#include <stdlib.h>
#include <string.h>
+#include <mcheck.h>
#include <support/check.h>
#include <support/xstdio.h>
#include "nss/tst-nss-gai-hv2-canonname.h"
@@ -41,6 +42,8 @@ static void do_prepare (int a, char **av)
static int
do_test (void)
{
+ mtrace();
+
__nss_configure_lookup ("hosts", "test_gai_hv2_canonname");
struct addrinfo hints = {};
@@ -1196,9 +1196,7 @@ free_and_return:
if (malloc_name)
free ((char *) name);
free (addrmem);
- if (res.free_at)
- free (res.at);
- free (res.canon);
+ gaih_result_reset (&res);
return result;
}