* [PATCH] Add the --with-tty-gid configure option [BZ #24941]
@ 2019-08-27 12:08 Florian Weimer
2019-10-31 20:41 ` Gabriel F. T. Gomes
2020-05-27 10:02 ` Florian Weimer
0 siblings, 2 replies; 3+ messages in thread
From: Florian Weimer @ 2019-08-27 12:08 UTC (permalink / raw)
To: libc-alpha
2019-08-27 Florian Weimer <fweimer@redhat.com>
[BZ #24941]
Add the --with-tty-gid configure option.
* config.make.in (define-tty-gid-cflags): New variable.
* configure.ac (--with-tty-gid): New option.
* manual/install.texi (Configuring and compiling): Document
--with-tty-gid.
* sysdeps/unix/Makefile [$(subdir) == login] (CFLAGS-grantpt.c):
Add $(define-tty-gid-cflags).
* sysdeps/unix/grantpt.c (grantpt): Use TTY_GID constant for gid
if defined.
* configure, INSTALL: Regeneration.
diff --git a/INSTALL b/INSTALL
index 16987cd048..48bbf7d613 100644
--- a/INSTALL
+++ b/INSTALL
@@ -106,6 +106,12 @@ if 'CFLAGS' is specified it must enable optimization. For example:
particular case and potentially change debugging information and
metadata only).
+'--with-tty-gid=GID'
+ Use GID as the value for the group ID of the 'tty' group. This
+ enables a more efficient implementation of the 'grantpt' function,
+ and the function will also work reliably in a forked subprocess of
+ a multi-threaded process.
+
'--disable-shared'
Don't build shared libraries even if it is possible. Not all
systems support shared libraries; you need ELF support and
diff --git a/NEWS b/NEWS
index a64b89986a..ac302f739b 100644
--- a/NEWS
+++ b/NEWS
@@ -21,6 +21,11 @@ Major new features:
18661-1:2014 and TS 18661-3:2015 as amended by the resolution of
Clarification Request 13 to TS 18661-3.
+* The GNU C Library can now be configured with the option --with-tty-gid=5,
+ hard-coding the GID of group "tty" to 5 (the value on most GNU/Linux
+ systems). This enables a more efficient implementation of grantpt, which
+ also works reliably in a forked subprocess of a multi-threaded process.
+
Deprecated and removed features, and other changes affecting compatibility:
* The totalorder and totalordermag functions, and the corresponding
diff --git a/config.make.in b/config.make.in
index 2fed3da773..b04c1ece1b 100644
--- a/config.make.in
+++ b/config.make.in
@@ -112,6 +112,7 @@ BUILD_CC = @BUILD_CC@
CFLAGS = @CFLAGS@
CPPFLAGS-config = @CPPFLAGS@
CPPUNDEFS = @CPPUNDEFS@
+define-tty-gid-cflags = @define_tty_gid_cflags@
extra-nonshared-cflags = @extra_nonshared_cflags@
ASFLAGS-config = @ASFLAGS_config@
AR = @AR@
diff --git a/configure b/configure
index c773c487b5..687c6a135d 100755
--- a/configure
+++ b/configure
@@ -685,6 +685,7 @@ force_install
bindnow
hardcoded_path_in_tests
enable_timezone_tools
+define_tty_gid_cflags
extra_nonshared_cflags
use_default_link
sysheaders
@@ -765,6 +766,7 @@ with_selinux
with_headers
with_default_link
with_nonshared_cflags
+with_tty_gid
enable_sanity_checks
enable_shared
enable_profile
@@ -1487,6 +1489,7 @@ Optional Packages:
--with-default-link do not use explicit linker scripts
--with-nonshared-cflags=CFLAGS
build nonshared libraries with additional CFLAGS
+ --with-tty-gid=GID set the ID of the tty group to GID
--with-cpu=CPU select code for CPU variant
Some influential environment variables:
@@ -3354,6 +3357,16 @@ fi
+
+# Check whether --with-tty-gid was given.
+if test "${with_tty_gid+set}" = set; then :
+ withval=$with_tty_gid; define_tty_gid_cflags=-DTTY_GID=$withval
+else
+ define_tty_gid_cflags=
+fi
+
+
+
# Check whether --enable-sanity-checks was given.
if test "${enable_sanity_checks+set}" = set; then :
enableval=$enable_sanity_checks; enable_sanity=$enableval
diff --git a/configure.ac b/configure.ac
index 598ba6c4ae..483fe3d211 100644
--- a/configure.ac
+++ b/configure.ac
@@ -162,6 +162,13 @@ AC_ARG_WITH([nonshared-cflags],
[extra_nonshared_cflags=])
AC_SUBST(extra_nonshared_cflags)
+AC_ARG_WITH([tty-gid],
+ AC_HELP_STRING([--with-tty-gid=GID],
+ [set the ID of the tty group to GID]),
+ [define_tty_gid_cflags=-DTTY_GID=$withval],
+ [define_tty_gid_cflags=])
+AC_SUBST(define_tty_gid_cflags)
+
AC_ARG_ENABLE([sanity-checks],
AC_HELP_STRING([--disable-sanity-checks],
[really do not use threads (should not be used except in special situations) @<:@default=yes@:>@]),
diff --git a/manual/install.texi b/manual/install.texi
index b2d569ac5a..917f41aed9 100644
--- a/manual/install.texi
+++ b/manual/install.texi
@@ -131,6 +131,12 @@ that the objects in @file{libc_nonshared.a} are compiled with this flag
(although this will not affect the generated code in this particular
case and potentially change debugging information and metadata only).
+@item --with-tty-gid=@var{GID}
+Use @var{GID} as the value for the group ID of the @samp{tty} group.
+This enables a more efficient implementation of the @code{grantpt}
+function, and the function will also work reliably in a forked
+subprocess of a multi-threaded process.
+
@c disable static doesn't work currently
@c @item --disable-static
@c Don't build static libraries. Static libraries aren't that useful these
diff --git a/sysdeps/unix/Makefile b/sysdeps/unix/Makefile
index 93496480e9..f02351da5f 100644
--- a/sysdeps/unix/Makefile
+++ b/sysdeps/unix/Makefile
@@ -109,4 +109,8 @@ $(common-objpfx)s-%.d: $(..)sysdeps/unix/s-%.S \
postclean-generated += sysd-syscalls
-endif
+endif # $(subdir) == misc
+
+ifeq (login,$(subdir))
+CFLAGS-grantpt.c += $(define-tty-gid-cflags)
+endif # $(subdir) == login
diff --git a/sysdeps/unix/grantpt.c b/sysdeps/unix/grantpt.c
index 3214ab06de..57a29347d3 100644
--- a/sysdeps/unix/grantpt.c
+++ b/sysdeps/unix/grantpt.c
@@ -135,6 +135,10 @@ grantpt (int fd)
goto helper;
}
+#ifdef TTY_GID
+ /* TTY_GID is set by the --with-tty-gid configure option. */
+ enum { gid = TTY_GID };
+#else
static int tty_gid = -1;
if (__glibc_unlikely (tty_gid == -1))
{
@@ -154,6 +158,7 @@ grantpt (int fd)
tty_gid = p->gr_gid;
}
gid_t gid = tty_gid == -1 ? __getgid () : tty_gid;
+#endif
#if HAVE_PT_CHOWN
/* Make sure the group of the device is that special group. */
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Add the --with-tty-gid configure option [BZ #24941]
2019-08-27 12:08 [PATCH] Add the --with-tty-gid configure option [BZ #24941] Florian Weimer
@ 2019-10-31 20:41 ` Gabriel F. T. Gomes
2020-05-27 10:02 ` Florian Weimer
1 sibling, 0 replies; 3+ messages in thread
From: Gabriel F. T. Gomes @ 2019-10-31 20:41 UTC (permalink / raw)
To: Florian Weimer; +Cc: libc-alpha
Hi, Florian,
Thanks for, in the bug report [1], helping me understand the rationale for
this patch (the problem was well described here, anyway, I had just failed
to notice the details).
[1] https://sourceware.org/bugzilla/show_bug.cgi?id=24941#c3
On Tue, 27 Aug 2019, Florian Weimer wrote:
>2019-08-27 Florian Weimer <fweimer@redhat.com>
>
> [BZ #24941]
> Add the --with-tty-gid configure option.
> * config.make.in (define-tty-gid-cflags): New variable.
> * configure.ac (--with-tty-gid): New option.
> * manual/install.texi (Configuring and compiling): Document
> --with-tty-gid.
> * sysdeps/unix/Makefile [$(subdir) == login] (CFLAGS-grantpt.c):
> Add $(define-tty-gid-cflags).
> * sysdeps/unix/grantpt.c (grantpt): Use TTY_GID constant for gid
> if defined.
> * configure, INSTALL: Regeneration.
I thinks this needs a new commit message, preferably one that summarizes
why this patch is useful... Something in the lines of:
When, in multi-threaded applications, a forked process tries to call
grantpt, it could hang in an internal NSS lock (__nss_database_lookup2),
if the fork happened while NSS was used in another thread. This patch
adds the --with-tty-gid configure option, which avoids the hang, by
avoiding calls to NSS functions.
Other than that, the patch looks good to me.
Reviewed-by: Gabriel F. T. Gomes <gabrielftg@linux.ibm.com>
Thanks
>+@item --with-tty-gid=@var{GID}
>+Use @var{GID} as the value for the group ID of the @samp{tty} group.
>+This enables a more efficient implementation of the @code{grantpt}
>+function, and the function will also work reliably in a forked
>+subprocess of a multi-threaded process.
OK.
>+#ifdef TTY_GID
>+ /* TTY_GID is set by the --with-tty-gid configure option. */
>+ enum { gid = TTY_GID };
>+#else
> static int tty_gid = -1;
> if (__glibc_unlikely (tty_gid == -1))
> {
>@@ -154,6 +158,7 @@ grantpt (int fd)
> tty_gid = p->gr_gid;
> }
> gid_t gid = tty_gid == -1 ? __getgid () : tty_gid;
>+#endif
OK.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Add the --with-tty-gid configure option [BZ #24941]
2019-08-27 12:08 [PATCH] Add the --with-tty-gid configure option [BZ #24941] Florian Weimer
2019-10-31 20:41 ` Gabriel F. T. Gomes
@ 2020-05-27 10:02 ` Florian Weimer
1 sibling, 0 replies; 3+ messages in thread
From: Florian Weimer @ 2020-05-27 10:02 UTC (permalink / raw)
To: libc-alpha
I'm withdrawing this patch. I have a rewrite of grantpt which fixes
this as well.
Thanks,
Florian
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-05-27 10:02 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-27 12:08 [PATCH] Add the --with-tty-gid configure option [BZ #24941] Florian Weimer
2019-10-31 20:41 ` Gabriel F. T. Gomes
2020-05-27 10:02 ` Florian Weimer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).