* [PATCH 1/2] posix/group_member: Add group_member2 with error return.
2023-10-12 15:34 [PATCH 0/2] posix: Add group_member2 and deprecate group_member Joe Simmons-Talbott
@ 2023-10-12 15:34 ` Joe Simmons-Talbott
2023-10-12 15:34 ` [PATCH 2/2] posix: Deprecate group_member for Linux Joe Simmons-Talbott
2023-10-12 16:59 ` [PATCH 0/2] posix: Add group_member2 and deprecate group_member Joseph Myers
2 siblings, 0 replies; 5+ messages in thread
From: Joe Simmons-Talbott @ 2023-10-12 15:34 UTC (permalink / raw)
To: libc-alpha; +Cc: Joe Simmons-Talbott
Because group_member doesn't return an error status and we are trying to
get rid of alloca usage. Add a new group_member2 which returns -1 and
sets errno on error. The old group_member will be deprecated in a later
change.
---
include/unistd.h | 1 +
posix/Makefile | 1 +
posix/group_member.c | 32 +++++++++++++++++++++++++++++
posix/tst-group_member2.c | 42 +++++++++++++++++++++++++++++++++++++++
posix/unistd.h | 3 +++
5 files changed, 79 insertions(+)
create mode 100644 posix/tst-group_member2.c
diff --git a/include/unistd.h b/include/unistd.h
index e241603b81..39d5bda372 100644
--- a/include/unistd.h
+++ b/include/unistd.h
@@ -131,6 +131,7 @@ extern __gid_t __getegid (void) attribute_hidden;
extern int __getgroups (int __size, __gid_t __list[]) attribute_hidden;
libc_hidden_proto (__getpgid)
extern int __group_member (__gid_t __gid) attribute_hidden;
+extern int __group_member2 (__gid_t __gid) attribute_hidden;
extern int __setuid (__uid_t __uid);
extern int __setreuid (__uid_t __ruid, __uid_t __euid);
extern int __setgid (__gid_t __gid);
diff --git a/posix/Makefile b/posix/Makefile
index c36b9d981e..b46ff3259c 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -294,6 +294,7 @@ tests := \
tst-glob_symlinks \
tst-gnuglob \
tst-gnuglob64 \
+ tst-group_member2 \
tst-mmap \
tst-mmap-offset \
tst-nanosleep \
diff --git a/posix/group_member.c b/posix/group_member.c
index 22422b1f9f..6064eb7264 100644
--- a/posix/group_member.c
+++ b/posix/group_member.c
@@ -16,6 +16,8 @@
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
+#include <errno.h>
+#include <scratch_buffer.h>
#include <sys/types.h>
#include <unistd.h>
#include <stdlib.h>
@@ -47,3 +49,33 @@ __group_member (gid_t gid)
return 0;
}
weak_alias (__group_member, group_member)
+
+int
+__group_member2 (gid_t gid)
+{
+ int n;
+ gid_t *groups;
+ struct scratch_buffer buf;
+ scratch_buffer_init (&buf);
+
+ n = __getgroups (0, NULL);
+ if (!scratch_buffer_set_array_size (&buf, n, sizeof (*groups)))
+ {
+ errno = ENOMEM;
+ return -1;
+ }
+ groups = buf.data;
+
+ n = __getgroups (n, groups);
+
+ while (n-- > 0)
+ if (groups[n] == gid)
+ {
+ scratch_buffer_free (&buf);
+ return 1;
+ }
+
+ scratch_buffer_free (&buf);
+ return 0;
+}
+weak_alias (__group_member2, group_member2)
diff --git a/posix/tst-group_member2.c b/posix/tst-group_member2.c
new file mode 100644
index 0000000000..4a360c71da
--- /dev/null
+++ b/posix/tst-group_member2.c
@@ -0,0 +1,42 @@
+/* Basic tests for group_memmber2.
+ Copyright (C) 2023 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <alloca.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include <support/check.h>
+
+static int do_test (void)
+{
+ int n;
+ gid_t *groups;
+
+ n = getgroups (0, NULL);
+ groups = alloca (n * sizeof (*groups));
+ n = getgroups (n, groups);
+
+ while (n-- > 0)
+ TEST_COMPARE (1, group_member2 (groups[n]));
+
+ return EXIT_SUCCESS;
+}
+
+#include <support/test-driver.c>
+
diff --git a/posix/unistd.h b/posix/unistd.h
index 0477527a60..96816b186d 100644
--- a/posix/unistd.h
+++ b/posix/unistd.h
@@ -713,6 +713,9 @@ extern int getgroups (int __size, __gid_t __list[]) __THROW __wur
#ifdef __USE_GNU
/* Return nonzero iff the calling process is in group GID. */
extern int group_member (__gid_t __gid) __THROW;
+/* Return nonzero iff the calling process is in group GID. Return
+ -1 on error and set errno. */
+extern int group_member2 (__gid_t __gid) __THROW;
#endif
/* Set the user ID of the calling process to UID.
--
2.39.2
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH 2/2] posix: Deprecate group_member for Linux
2023-10-12 15:34 [PATCH 0/2] posix: Add group_member2 and deprecate group_member Joe Simmons-Talbott
2023-10-12 15:34 ` [PATCH 1/2] posix/group_member: Add group_member2 with error return Joe Simmons-Talbott
@ 2023-10-12 15:34 ` Joe Simmons-Talbott
2023-10-12 16:59 ` [PATCH 0/2] posix: Add group_member2 and deprecate group_member Joseph Myers
2 siblings, 0 replies; 5+ messages in thread
From: Joe Simmons-Talbott @ 2023-10-12 15:34 UTC (permalink / raw)
To: libc-alpha; +Cc: Joe Simmons-Talbott
The alloca usage in group_member could lead to stack overflow on Linux.
Removing the alloca usage would require group_member to handle the error
condition where memory could not be allocated and that cannot be done
since group_member returns a boolean. Thus deprecate group_member. Add
a testcase.
---
posix/Makefile | 3 ++
posix/group_member.h | 29 ++++++++++++++++++
posix/tst-group_member.c | 41 ++++++++++++++++++++++++++
posix/unistd.h | 9 ++----
sysdeps/unix/sysv/linux/group_member.h | 27 +++++++++++++++++
5 files changed, 103 insertions(+), 6 deletions(-)
create mode 100644 posix/group_member.h
create mode 100644 posix/tst-group_member.c
create mode 100644 sysdeps/unix/sysv/linux/group_member.h
diff --git a/posix/Makefile b/posix/Makefile
index b46ff3259c..09308d4568 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -295,6 +295,7 @@ tests := \
tst-gnuglob \
tst-gnuglob64 \
tst-group_member2 \
+ tst-group_member \
tst-mmap \
tst-mmap-offset \
tst-nanosleep \
@@ -616,6 +617,8 @@ bug-glob1-ARGS = "$(objpfx)"
tst-execvp3-ARGS = --test-dir=$(objpfx)
CFLAGS-tst-spawn3.c += -DOBJPFX=\"$(objpfx)\"
+CFLAGS-tst-group_member.c += -Wno-error=deprecated-declarations
+
# Test voluntarily overflows struct dirent
CFLAGS-bug-glob2.c += $(no-fortify-source)
diff --git a/posix/group_member.h b/posix/group_member.h
new file mode 100644
index 0000000000..cc3c3ba603
--- /dev/null
+++ b/posix/group_member.h
@@ -0,0 +1,29 @@
+/* Copyright (C) 2023 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#ifndef _GROUP_MEMBER_H
+#define _GROUP_MEMBER_H 1
+
+#ifdef __USE_GNU
+/* Return nonzero iff the calling process is in group GID. */
+extern int group_member (__gid_t __gid) __THROW;
+/* Return nonzero iff the calling process is in group GID. Return
+ -1 on error and set errno. */
+extern int group_member2 (__gid_t __gid) __THROW;
+#endif
+
+#endif /* GROUP_MEMBER_H */
diff --git a/posix/tst-group_member.c b/posix/tst-group_member.c
new file mode 100644
index 0000000000..7f70841832
--- /dev/null
+++ b/posix/tst-group_member.c
@@ -0,0 +1,41 @@
+/* Basic tests for group_member.
+ Copyright (C) 2023 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <alloca.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+#include <support/check.h>
+
+static int do_test (void)
+{
+ int n;
+ gid_t *groups;
+
+ n = getgroups (0, NULL);
+ groups = alloca (n * sizeof (*groups));
+ n = getgroups (n, groups);
+
+ while (n-- > 0)
+ TEST_COMPARE (1, group_member(groups[n]));
+
+ return EXIT_SUCCESS;
+}
+
+#include <support/test-driver.c>
diff --git a/posix/unistd.h b/posix/unistd.h
index 96816b186d..d73a9eeea2 100644
--- a/posix/unistd.h
+++ b/posix/unistd.h
@@ -710,13 +710,10 @@ extern __gid_t getegid (void) __THROW;
of its supplementary groups in LIST and return the number written. */
extern int getgroups (int __size, __gid_t __list[]) __THROW __wur
__fortified_attr_access (__write_only__, 2, 1);
+
#ifdef __USE_GNU
-/* Return nonzero iff the calling process is in group GID. */
-extern int group_member (__gid_t __gid) __THROW;
-/* Return nonzero iff the calling process is in group GID. Return
- -1 on error and set errno. */
-extern int group_member2 (__gid_t __gid) __THROW;
-#endif
+# include <group_member.h>
+#endif
/* Set the user ID of the calling process to UID.
If the calling process is the super-user, set the real
diff --git a/sysdeps/unix/sysv/linux/group_member.h b/sysdeps/unix/sysv/linux/group_member.h
new file mode 100644
index 0000000000..97a9f30dfe
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/group_member.h
@@ -0,0 +1,27 @@
+/* Copyright (C) 2023 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#ifndef _GROUP_MEMBER_H
+#define _GROUP_MEMBER_H 1
+
+#ifdef __USE_GNU
+/* Return nonzero iff the calling process is in group GID. Deprecated */
+extern int group_member (__gid_t __gid) __THROW
+ __attribute_deprecated_msg__ ("may overflow the stack");
+#endif
+
+#endif /* GROUP_MEMBER_H */
--
2.39.2
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH 0/2] posix: Add group_member2 and deprecate group_member
2023-10-12 15:34 [PATCH 0/2] posix: Add group_member2 and deprecate group_member Joe Simmons-Talbott
2023-10-12 15:34 ` [PATCH 1/2] posix/group_member: Add group_member2 with error return Joe Simmons-Talbott
2023-10-12 15:34 ` [PATCH 2/2] posix: Deprecate group_member for Linux Joe Simmons-Talbott
@ 2023-10-12 16:59 ` Joseph Myers
2023-10-12 19:44 ` Joe Simmons-Talbott
2 siblings, 1 reply; 5+ messages in thread
From: Joseph Myers @ 2023-10-12 16:59 UTC (permalink / raw)
To: Joe Simmons-Talbott; +Cc: libc-alpha
On Thu, 12 Oct 2023, Joe Simmons-Talbott wrote:
> Add group_member2 which uses a scratch_buffer rather than alloca and
> returns -1 on error and sets errno. Replace the alloca usage in
> group_member with a scratch_buffer and call __fortify_fail when
> allocation fails. Deprecate group_member for Linux.
How was this tested? It's missing a Versions update so I wouldn't have
expected the tests to link (also missing an addition of group_member.h to
the list of installed headers in posix/Makefile, abilist updates for all
architectures and a NEWS update for the new user-visible function; since
group_member isn't documented in the manual, it's probably reasonable that
it doesn't add documentation for group_member2).
If group_member.h isn't intended to be a public API itself (if user code
isn't intended to do #include <group_member.h>) then it should be
bits/group_member.h instead with appropriate tests that it's only included
from <unistd.h> and not directly from user code.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 0/2] posix: Add group_member2 and deprecate group_member
2023-10-12 16:59 ` [PATCH 0/2] posix: Add group_member2 and deprecate group_member Joseph Myers
@ 2023-10-12 19:44 ` Joe Simmons-Talbott
0 siblings, 0 replies; 5+ messages in thread
From: Joe Simmons-Talbott @ 2023-10-12 19:44 UTC (permalink / raw)
To: Joseph Myers; +Cc: libc-alpha
On Thu, Oct 12, 2023 at 04:59:51PM +0000, Joseph Myers wrote:
> On Thu, 12 Oct 2023, Joe Simmons-Talbott wrote:
>
> > Add group_member2 which uses a scratch_buffer rather than alloca and
> > returns -1 on error and sets errno. Replace the alloca usage in
> > group_member with a scratch_buffer and call __fortify_fail when
> > allocation fails. Deprecate group_member for Linux.
>
> How was this tested? It's missing a Versions update so I wouldn't have
> expected the tests to link (also missing an addition of group_member.h to
> the list of installed headers in posix/Makefile, abilist updates for all
> architectures and a NEWS update for the new user-visible function; since
> group_member isn't documented in the manual, it's probably reasonable that
> it doesn't add documentation for group_member2).
>
> If group_member.h isn't intended to be a public API itself (if user code
> isn't intended to do #include <group_member.h>) then it should be
> bits/group_member.h instead with appropriate tests that it's only included
> from <unistd.h> and not directly from user code.
Sorry, this was from me combining two git branches and one wasn't ready
to submit yet. Thanks for pointing out what was missing. I'll post a
v2 with the missing parts soon.
What are folk's thoughts on whether to make group_member.h public or
not? I don't have strong feelings either way.
Thanks,
Joe
^ permalink raw reply [flat|nested] 5+ messages in thread