From fcbd03d7e95c10974de901fa3b14a3146b9ad8b6 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Sun, 14 Dec 2025 13:46:30 +0100 Subject: [PATCH 1/4] lib/list.c: add_list(): realloc(3) to prevent memory leaks Each call to add_list() that resulted in adding a group to the list was leaking the old list. There's no need to allocate a fresh list. Otherwise, it would be incorrect to return the original pointer in some cases, which we already did. Signed-off-by: Alejandro Colomar --- lib/list.c | 31 +++++-------------------------- 1 file changed, 5 insertions(+), 26 deletions(-) diff --git a/lib/list.c b/lib/list.c index cf918f477f..8fc28130e5 100644 --- a/lib/list.c +++ b/lib/list.c @@ -13,6 +13,7 @@ #include #include "alloc/malloc.h" +#include "alloc/realloc.h" #include "prototypes.h" #include "defines.h" #include "string/strchr/strchrcnt.h" @@ -23,16 +24,11 @@ /* * add_list - add a member to a list of group members - * - * the array of member names is searched for the new member - * name, and if not present it is added to a freshly allocated - * list of users. */ /*@only@*/char ** add_list(/*@returned@*/ /*@only@*/char **list, const char *member) { int i; - char **tmp; assert (NULL != member); assert (NULL != list); @@ -41,34 +37,17 @@ add_list(/*@returned@*/ /*@only@*/char **list, const char *member) * Scan the list for the new name. Return the original list * pointer if it is present. */ - for (i = 0; list[i] != NULL; i++) { if (streq(list[i], member)) { return list; } } - /* - * Allocate a new list pointer large enough to hold all the - * old entries, and the new entries as well. - */ + list = xrealloc_T(list, i + 2, char *); + list[i] = xstrdup(member); + list[i+1] = NULL; - tmp = xmalloc_T(i + 2, char *); - - /* - * Copy the original list to the new list, then append the - * new member and NULL terminate the result. This new list - * is returned to the invoker. - */ - - for (i = 0; list[i] != NULL; i++) { - tmp[i] = list[i]; - } - - tmp[i] = xstrdup (member); - tmp[i+1] = NULL; - - return tmp; + return list; } /* From dc9dcb3f945c185c8868e545bc9e9668be3be35c Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Sun, 14 Dec 2025 14:31:10 +0100 Subject: [PATCH 2/4] lib/list.c: del_list(): Reimplement with memmove(3) and realloc(3) to avoid memory leaks Like del_list(), each call to del_list() that wasn't a no-op, was leaking the old list. There's no need to allocate a fresh list; we can use memmove(3) to delete the old member, and return the original pointer. We already return the original pointer in some cases, so this must be okay. Signed-off-by: Alejandro Colomar --- lib/list.c | 56 +++++++++++------------------------------------------- 1 file changed, 11 insertions(+), 45 deletions(-) diff --git a/lib/list.c b/lib/list.c index 8fc28130e5..f7b865acb2 100644 --- a/lib/list.c +++ b/lib/list.c @@ -8,7 +8,8 @@ #include "config.h" -#ident "$Id$" +#include +#include #include @@ -52,59 +53,24 @@ add_list(/*@returned@*/ /*@only@*/char **list, const char *member) /* * del_list - delete a member from a list of group members - * - * the array of member names is searched for the old member - * name, and if present it is deleted from a freshly allocated - * list of users. */ - /*@only@*/char ** del_list(/*@returned@*/ /*@only@*/char **list, const char *member) { - int i, j; - char **tmp; + int n, m; assert (NULL != member); assert (NULL != list); - /* - * Scan the list for the old name. Return the original list - * pointer if it is not present. - */ - - for (i = j = 0; list[i] != NULL; i++) { - if (!streq(list[i], member)) { - j++; - } - } - - if (j == i) { - return list; - } - - /* - * Allocate a new list pointer large enough to hold all the - * old entries. - */ - - tmp = xmalloc_T(j + 1, char *); - - /* - * Copy the original list except the deleted members to the - * new list, then NULL terminate the result. This new list - * is returned to the invoker. - */ + do { + for (n = 0; list[n] != NULL; n++) + continue; + for (m = 0; list[m] != NULL && !streq(list[m], member); m++) + continue; + memmove(&list[m], &list[m+1], (n-m) * sizeof(char *)); + } while (m != n); - for (i = j = 0; list[i] != NULL; i++) { - if (!streq(list[i], member)) { - tmp[j] = list[i]; - j++; - } - } - - tmp[j] = NULL; - - return tmp; + return xrealloc_T(list, n+1, char *); } /* From acfbbd9dca33fbb2ef6363d623b3934994e6bb67 Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Sun, 14 Dec 2025 16:17:28 +0100 Subject: [PATCH 3/4] lib/string/strcpy/: memmove_T(): Add API An interesting detail is that we require the second argument to be non-const, while the memmove(3) function gets a const void*. This is because the second argument should usually be just the same as the first one, plus some offset, and thus will have the same const qualification (that is, it will not be qualified). Signed-off-by: Alejandro Colomar --- lib/Makefile.am | 2 ++ lib/string/README | 3 +++ lib/string/strcpy/memmove.c | 7 +++++++ lib/string/strcpy/memmove.h | 26 ++++++++++++++++++++++++++ 4 files changed, 38 insertions(+) create mode 100644 lib/string/strcpy/memmove.c create mode 100644 lib/string/strcpy/memmove.h diff --git a/lib/Makefile.am b/lib/Makefile.am index c402ff02a3..2bdb7a132c 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -218,6 +218,8 @@ libshadow_la_SOURCES = \ string/strcmp/strneq.h \ string/strcmp/strprefix.c \ string/strcmp/strprefix.h \ + string/strcpy/memmove.c \ + string/strcpy/memmove.h \ string/strcpy/stpecpy.c \ string/strcpy/stpecpy.h \ string/strcpy/strncat.c \ diff --git a/lib/string/README b/lib/string/README index 9fbcbd842c..b75cc3ae11 100644 --- a/lib/string/README +++ b/lib/string/README @@ -207,6 +207,9 @@ strcpy/ - String copying MEMCPY() Like memcpy(3), but takes two arrays. + memmove_T() + Like memmove(3), but type safe. + sprintf/ - Formatted string creation aprintf() diff --git a/lib/string/strcpy/memmove.c b/lib/string/strcpy/memmove.c new file mode 100644 index 0000000000..d95a52315d --- /dev/null +++ b/lib/string/strcpy/memmove.c @@ -0,0 +1,7 @@ +// SPDX-FileCopyrightText: 2025, Alejandro Colomar +// SPDX-License-Identifier: BSD-3-Clause + + +#include "config.h" + +#include "string/strcpy/memmove.h" diff --git a/lib/string/strcpy/memmove.h b/lib/string/strcpy/memmove.h new file mode 100644 index 0000000000..aff610eae4 --- /dev/null +++ b/lib/string/strcpy/memmove.h @@ -0,0 +1,26 @@ +// SPDX-FileCopyrightText: 2025, Alejandro Colomar +// SPDX-License-Identifier: BSD-3-Clause + + +#ifndef SHADOW_INCLUDE_LIB_STRING_STRCPY_MEMMOVE_H_ +#define SHADOW_INCLUDE_LIB_STRING_STRCPY_MEMMOVE_H_ + + +#include "config.h" + +#include + +#include "sizeof.h" + + +// memmove_T - memory move type-safe +#define memmove_T(dst, src, n, T) memmove_T_(dst, src, n, typeas(T)) +#define memmove_T_(dst, src, n, T) \ +({ \ + _Generic(dst, T *: (void)0); \ + _Generic(src, T *: (void)0); \ + (T *){memmove(dst, src, (n) * sizeof(T))}; \ +}) + + +#endif // include guard From 76373568d3c0fb9a0e7ec85c77faa4acc4d6692b Mon Sep 17 00:00:00 2001 From: Alejandro Colomar Date: Sun, 14 Dec 2025 16:19:10 +0100 Subject: [PATCH 4/4] lib/list.c: del_list(): Use memmove_T() instead of its pattern This adds type safety. Signed-off-by: Alejandro Colomar --- lib/list.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/list.c b/lib/list.c index f7b865acb2..a6425f51e1 100644 --- a/lib/list.c +++ b/lib/list.c @@ -19,6 +19,7 @@ #include "defines.h" #include "string/strchr/strchrcnt.h" #include "string/strcmp/streq.h" +#include "string/strcpy/memmove.h" #include "string/strdup/strdup.h" #include "string/strtok/strsep2ls.h" @@ -67,7 +68,7 @@ del_list(/*@returned@*/ /*@only@*/char **list, const char *member) continue; for (m = 0; list[m] != NULL && !streq(list[m], member); m++) continue; - memmove(&list[m], &list[m+1], (n-m) * sizeof(char *)); + memmove_T(&list[m], &list[m+1], n-m, char *); } while (m != n); return xrealloc_T(list, n+1, char *);