public inbox for systemtap@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Fix "make install" in certain fakeroot environments
@ 2019-01-27  4:22 David Ward
  2019-01-27  5:08 ` David Ward
  0 siblings, 1 reply; 6+ messages in thread
From: David Ward @ 2019-01-27  4:22 UTC (permalink / raw)
  To: systemtap

Do not abort "make install" if the stapusr group cannot be found or
created, or it cannot be set as the owner of installed executables,
even when the effective user ID is 0.

These operations may not be possible in some fakeroot environments
that are used to build distribution binary packages. (I discovered
this issue on Arch Linux using "makechrootpkg".) The package build
files often specify these operations directly: see systemtap.spec.

Otherwise, use fewer commands to achieve the same result as before.
---
 stapbpf/Makefile.am | 13 +++++--------
 stapbpf/Makefile.in | 13 +++++--------
 staprun/Makefile.am | 12 +++++-------
 staprun/Makefile.in | 12 +++++-------
 4 files changed, 20 insertions(+), 30 deletions(-)

diff --git a/stapbpf/Makefile.am b/stapbpf/Makefile.am
index fa2035dab..7f3511806 100644
--- a/stapbpf/Makefile.am
+++ b/stapbpf/Makefile.am
@@ -39,13 +39,10 @@ BUILT_SOURCES += git_version.stamp
 git_version.stamp ../git_version.h:
 	$(MAKE) -C .. $(notdir $@)
 
-# Why the "id -u" condition?  This way, an unprivileged user can run
-# make install, and have "sudo stap ...." or "sudo stapbpf ...." work later.
+# Why the "-" at the start of the line?  This way, an unprivileged user can run
+# "make install", and have "sudo stap ...." or "sudo stapbpf ...." work later.
 install-exec-hook:
-	if [ `id -u` -eq 0 ]; then \
-		getent group stapusr >/dev/null || groupadd -g 156 -r stapusr 2>/dev/null || groupadd -r stapusr; \
-		getent group stapusr >/dev/null \
-		&& chgrp stapusr "$(DESTDIR)$(bindir)/stapbpf" \
-		&& chmod 04110 "$(DESTDIR)$(bindir)/stapbpf"; \
-	fi
+	-(getent group stapusr >/dev/null || groupadd -f -g 156 -r stapusr) \
+	 && chgrp stapusr "$(DESTDIR)$(bindir)/stapbpf" \
+	 && chmod 04110 "$(DESTDIR)$(bindir)/stapbpf"
 endif
diff --git a/stapbpf/Makefile.in b/stapbpf/Makefile.in
index f7f20b6ee..1e1b334e3 100644
--- a/stapbpf/Makefile.in
+++ b/stapbpf/Makefile.in
@@ -845,15 +845,12 @@ uninstall-man: uninstall-man8
 @HAVE_BPF_DECLS_TRUE@git_version.stamp ../git_version.h:
 @HAVE_BPF_DECLS_TRUE@	$(MAKE) -C .. $(notdir $@)
 
-# Why the "id -u" condition?  This way, an unprivileged user can run
-# make install, and have "sudo stap ...." or "sudo stapbpf ...." work later.
+# Why the "-" at the start of the line?  This way, an unprivileged user can run
+# "make install", and have "sudo stap ...." or "sudo stapbpf ...." work later.
 @HAVE_BPF_DECLS_TRUE@install-exec-hook:
-@HAVE_BPF_DECLS_TRUE@	if [ `id -u` -eq 0 ]; then \
-@HAVE_BPF_DECLS_TRUE@		getent group stapusr >/dev/null || groupadd -g 156 -r stapusr 2>/dev/null || groupadd -r stapusr; \
-@HAVE_BPF_DECLS_TRUE@		getent group stapusr >/dev/null \
-@HAVE_BPF_DECLS_TRUE@		&& chgrp stapusr "$(DESTDIR)$(bindir)/stapbpf" \
-@HAVE_BPF_DECLS_TRUE@		&& chmod 04110 "$(DESTDIR)$(bindir)/stapbpf"; \
-@HAVE_BPF_DECLS_TRUE@	fi
+@HAVE_BPF_DECLS_TRUE@	-(getent group stapusr >/dev/null || groupadd -f -g 156 -r stapusr) \
+@HAVE_BPF_DECLS_TRUE@	 && chgrp stapusr "$(DESTDIR)$(bindir)/stapbpf" \
+@HAVE_BPF_DECLS_TRUE@	 && chmod 04110 "$(DESTDIR)$(bindir)/stapbpf"
 
 # Tell versions [3.59,3.63) of GNU make to not export all variables.
 # Otherwise a system limit (for SysV at least) may be exceeded.
diff --git a/staprun/Makefile.am b/staprun/Makefile.am
index 73ddc41ff..288911f57 100644
--- a/staprun/Makefile.am
+++ b/staprun/Makefile.am
@@ -74,11 +74,9 @@ git_version.stamp ../git_version.h:
 	$(MAKE) -C .. $(notdir $@)
 
 
-# Why the "id -u" condition?  This way, an unprivileged user can run
-# make install, and have "sudo stap ...." or "sudo staprun ...." work later.
+# Why the "-" at the start of the line?  This way, an unprivileged user can run
+# "make install", and have "sudo stap ...." or "sudo stapbpf ...." work later.
 install-exec-hook:
-	if [ `id -u` -eq 0 ]; then \
-		getent group stapusr >/dev/null || groupadd -g 156 -r stapusr 2>/dev/null || groupadd -r stapusr; \
-		getent group stapusr >/dev/null && chgrp stapusr "$(DESTDIR)$(bindir)/staprun"; \
-		chmod 04110 "$(DESTDIR)$(bindir)/staprun"; \
-	fi
+	-(getent group stapusr >/dev/null || groupadd -f -g 156 -r stapusr) \
+	 && chgrp stapusr "$(DESTDIR)$(bindir)/staprun" \
+	 && chmod 04110 "$(DESTDIR)$(bindir)/staprun"
diff --git a/staprun/Makefile.in b/staprun/Makefile.in
index 6cb3966a5..4c00a7df8 100644
--- a/staprun/Makefile.in
+++ b/staprun/Makefile.in
@@ -1158,14 +1158,12 @@ uninstall-man: uninstall-man8
 git_version.stamp ../git_version.h:
 	$(MAKE) -C .. $(notdir $@)
 
-# Why the "id -u" condition?  This way, an unprivileged user can run
-# make install, and have "sudo stap ...." or "sudo staprun ...." work later.
+# Why the "-" at the start of the line?  This way, an unprivileged user can run
+# "make install", and have "sudo stap ...." or "sudo stapbpf ...." work later.
 install-exec-hook:
-	if [ `id -u` -eq 0 ]; then \
-		getent group stapusr >/dev/null || groupadd -g 156 -r stapusr 2>/dev/null || groupadd -r stapusr; \
-		getent group stapusr >/dev/null && chgrp stapusr "$(DESTDIR)$(bindir)/staprun"; \
-		chmod 04110 "$(DESTDIR)$(bindir)/staprun"; \
-	fi
+	-(getent group stapusr >/dev/null || groupadd -f -g 156 -r stapusr) \
+	 && chgrp stapusr "$(DESTDIR)$(bindir)/staprun" \
+	 && chmod 04110 "$(DESTDIR)$(bindir)/staprun"
 
 # Tell versions [3.59,3.63) of GNU make to not export all variables.
 # Otherwise a system limit (for SysV at least) may be exceeded.
-- 
2.20.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix "make install" in certain fakeroot environments
  2019-01-27  4:22 [PATCH] Fix "make install" in certain fakeroot environments David Ward
@ 2019-01-27  5:08 ` David Ward
  2019-01-30  5:38   ` David Ward
  0 siblings, 1 reply; 6+ messages in thread
From: David Ward @ 2019-01-27  5:08 UTC (permalink / raw)
  To: systemtap

On 1/26/19 10:25 PM, David Ward wrote:
> Do not abort "make install" if the stapusr group cannot be found or
> created, or it cannot be set as the owner of installed executables,
> even when the effective user ID is 0.
>
> These operations may not be possible in some fakeroot environments
> that are used to build distribution binary packages. (I discovered
> this issue on Arch Linux using "makechrootpkg".) The package build
> files often specify these operations directly: see systemtap.spec.
>
> Otherwise, use fewer commands to achieve the same result as before.

I believe this can be disregarded. It seems I was missing some 
information specific to Arch Linux, and I don't think this patch is 
otherwise necessary.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] Simplify creation of groups during installation
  2019-01-30  5:38   ` David Ward
  2019-01-30  5:38     ` [PATCH 2/2] Handle installation without stapusr group David Ward
@ 2019-01-30  5:38     ` David Ward
  2019-02-04 17:05     ` [PATCH] Fix "make install" in certain fakeroot environments Frank Ch. Eigler
  2 siblings, 0 replies; 6+ messages in thread
From: David Ward @ 2019-01-30  5:38 UTC (permalink / raw)
  To: systemtap

Use the "-f" option of "groupadd", rather than calling it a second
time if the desired GID is already in use.

Do not call "getent" twice. We know that a group exists if the first
call to "getent" returned successfully, or otherwise if "groupadd"
returned successfully.
---
 stapbpf/Makefile.am | 3 +--
 stapbpf/Makefile.in | 3 +--
 staprun/Makefile.am | 6 +++---
 staprun/Makefile.in | 6 +++---
 systemtap.spec      | 8 ++++----
 5 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/stapbpf/Makefile.am b/stapbpf/Makefile.am
index fa2035d..1626e4a 100644
--- a/stapbpf/Makefile.am
+++ b/stapbpf/Makefile.am
@@ -43,8 +43,7 @@ git_version.stamp ../git_version.h:
 # make install, and have "sudo stap ...." or "sudo stapbpf ...." work later.
 install-exec-hook:
 	if [ `id -u` -eq 0 ]; then \
-		getent group stapusr >/dev/null || groupadd -g 156 -r stapusr 2>/dev/null || groupadd -r stapusr; \
-		getent group stapusr >/dev/null \
+		(getent group stapusr >/dev/null || groupadd -f -g 156 -r stapusr) \
 		&& chgrp stapusr "$(DESTDIR)$(bindir)/stapbpf" \
 		&& chmod 04110 "$(DESTDIR)$(bindir)/stapbpf"; \
 	fi
diff --git a/stapbpf/Makefile.in b/stapbpf/Makefile.in
index f7f20b6..487da04 100644
--- a/stapbpf/Makefile.in
+++ b/stapbpf/Makefile.in
@@ -849,8 +849,7 @@ uninstall-man: uninstall-man8
 # make install, and have "sudo stap ...." or "sudo stapbpf ...." work later.
 @HAVE_BPF_DECLS_TRUE@install-exec-hook:
 @HAVE_BPF_DECLS_TRUE@	if [ `id -u` -eq 0 ]; then \
-@HAVE_BPF_DECLS_TRUE@		getent group stapusr >/dev/null || groupadd -g 156 -r stapusr 2>/dev/null || groupadd -r stapusr; \
-@HAVE_BPF_DECLS_TRUE@		getent group stapusr >/dev/null \
+@HAVE_BPF_DECLS_TRUE@		(getent group stapusr >/dev/null || groupadd -f -g 156 -r stapusr) \
 @HAVE_BPF_DECLS_TRUE@		&& chgrp stapusr "$(DESTDIR)$(bindir)/stapbpf" \
 @HAVE_BPF_DECLS_TRUE@		&& chmod 04110 "$(DESTDIR)$(bindir)/stapbpf"; \
 @HAVE_BPF_DECLS_TRUE@	fi
diff --git a/staprun/Makefile.am b/staprun/Makefile.am
index 73ddc41..f1a7a69 100644
--- a/staprun/Makefile.am
+++ b/staprun/Makefile.am
@@ -78,7 +78,7 @@ git_version.stamp ../git_version.h:
 # make install, and have "sudo stap ...." or "sudo staprun ...." work later.
 install-exec-hook:
 	if [ `id -u` -eq 0 ]; then \
-		getent group stapusr >/dev/null || groupadd -g 156 -r stapusr 2>/dev/null || groupadd -r stapusr; \
-		getent group stapusr >/dev/null && chgrp stapusr "$(DESTDIR)$(bindir)/staprun"; \
-		chmod 04110 "$(DESTDIR)$(bindir)/staprun"; \
+		(getent group stapusr >/dev/null || groupadd -f -g 156 -r stapusr) \
+		&& chgrp stapusr "$(DESTDIR)$(bindir)/staprun" \
+		&& chmod 04110 "$(DESTDIR)$(bindir)/staprun"; \
 	fi
diff --git a/staprun/Makefile.in b/staprun/Makefile.in
index 6cb3966..c518c7d 100644
--- a/staprun/Makefile.in
+++ b/staprun/Makefile.in
@@ -1162,9 +1162,9 @@ git_version.stamp ../git_version.h:
 # make install, and have "sudo stap ...." or "sudo staprun ...." work later.
 install-exec-hook:
 	if [ `id -u` -eq 0 ]; then \
-		getent group stapusr >/dev/null || groupadd -g 156 -r stapusr 2>/dev/null || groupadd -r stapusr; \
-		getent group stapusr >/dev/null && chgrp stapusr "$(DESTDIR)$(bindir)/staprun"; \
-		chmod 04110 "$(DESTDIR)$(bindir)/staprun"; \
+		(getent group stapusr >/dev/null || groupadd -f -g 156 -r stapusr) \
+		&& chgrp stapusr "$(DESTDIR)$(bindir)/staprun" \
+		&& chmod 04110 "$(DESTDIR)$(bindir)/staprun"; \
 	fi
 
 # Tell versions [3.59,3.63) of GNU make to not export all variables.
diff --git a/systemtap.spec b/systemtap.spec
index c3a9a32..26bfad0 100644
--- a/systemtap.spec
+++ b/systemtap.spec
@@ -767,13 +767,13 @@ done
 %endif
 
 %pre runtime
-getent group stapusr >/dev/null || groupadd -g 156 -r stapusr 2>/dev/null || groupadd -r stapusr
-getent group stapsys >/dev/null || groupadd -g 157 -r stapsys 2>/dev/null || groupadd -r stapsys
-getent group stapdev >/dev/null || groupadd -g 158 -r stapdev 2>/dev/null || groupadd -r stapdev
+getent group stapusr >/dev/null || groupadd -f -g 156 -r stapusr
+getent group stapsys >/dev/null || groupadd -f -g 157 -r stapsys
+getent group stapdev >/dev/null || groupadd -f -g 158 -r stapdev
 exit 0
 
 %pre server
-getent group stap-server >/dev/null || groupadd -g 155 -r stap-server 2>/dev/null || groupadd -r stap-server
+getent group stap-server >/dev/null || groupadd -f -g 155 -r stap-server
 getent passwd stap-server >/dev/null || \
   useradd -c "Systemtap Compile Server" -u 155 -g stap-server -d %{_localstatedir}/lib/stap-server -r -s /sbin/nologin stap-server 2>/dev/null || \
   useradd -c "Systemtap Compile Server" -g stap-server -d %{_localstatedir}/lib/stap-server -r -s /sbin/nologin stap-server
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix "make install" in certain fakeroot environments
  2019-01-27  5:08 ` David Ward
@ 2019-01-30  5:38   ` David Ward
  2019-01-30  5:38     ` [PATCH 2/2] Handle installation without stapusr group David Ward
                       ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: David Ward @ 2019-01-30  5:38 UTC (permalink / raw)
  To: systemtap

On 1/27/19 12:08 AM, David Ward wrote:
> I believe this can be disregarded. It seems I was missing some
> information specific to Arch Linux, and I don't think this patch is
> otherwise necessary.

I reacted too quickly to information I came across after sending, which
led me to think I was doing something wrong. I still believe this change
should be made.

I examined other software that gives users additional privileges when
they are a member of a unique group (example: PulseAudio checks if a
user is in the groups "pulse-access" or "pulse-rt"). In each case I saw,
the groups are not created by running "make install" or equivalent. They
are created by the distribution package's pre/post-install script, or
manually by the user after a direct source code build and install.

I adjusted this to leave the UID 0 check in place (because the group may
already exist), and I split this change into two to make it easier to
read.

David Ward (2):
  Simplify creation of groups during installation
  Handle installation without stapusr group

 stapbpf/Makefile.am | 7 +++----
 stapbpf/Makefile.in | 7 +++----
 staprun/Makefile.am | 8 ++++----
 staprun/Makefile.in | 8 ++++----
 systemtap.spec      | 8 ++++----
 5 files changed, 18 insertions(+), 20 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] Handle installation without stapusr group
  2019-01-30  5:38   ` David Ward
@ 2019-01-30  5:38     ` David Ward
  2019-01-30  5:38     ` [PATCH 1/2] Simplify creation of groups during installation David Ward
  2019-02-04 17:05     ` [PATCH] Fix "make install" in certain fakeroot environments Frank Ch. Eigler
  2 siblings, 0 replies; 6+ messages in thread
From: David Ward @ 2019-01-30  5:38 UTC (permalink / raw)
  To: systemtap

Do not cause "make install" to return an error if the stapusr group
cannot be found or created (even as root); continue without setting
the ownership or mode of the installed executables. This may happen
when building distribution packages using fakeroot (it was observed
on Arch Linux). This step is often performed directly in the build
files of the distribution package instead (such as systemtap.spec).
---
 stapbpf/Makefile.am | 6 +++---
 stapbpf/Makefile.in | 6 +++---
 staprun/Makefile.am | 6 +++---
 staprun/Makefile.in | 6 +++---
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/stapbpf/Makefile.am b/stapbpf/Makefile.am
index 1626e4a..ba18697 100644
--- a/stapbpf/Makefile.am
+++ b/stapbpf/Makefile.am
@@ -42,9 +42,9 @@ git_version.stamp ../git_version.h:
 # Why the "id -u" condition?  This way, an unprivileged user can run
 # make install, and have "sudo stap ...." or "sudo stapbpf ...." work later.
 install-exec-hook:
-	if [ `id -u` -eq 0 ]; then \
-		(getent group stapusr >/dev/null || groupadd -f -g 156 -r stapusr) \
-		&& chgrp stapusr "$(DESTDIR)$(bindir)/stapbpf" \
+	if [ `id -u` -eq 0 ] && (getent group stapusr >/dev/null \
+	                         || groupadd -f -g 156 -r stapusr); then \
+		chgrp stapusr "$(DESTDIR)$(bindir)/stapbpf" \
 		&& chmod 04110 "$(DESTDIR)$(bindir)/stapbpf"; \
 	fi
 endif
diff --git a/stapbpf/Makefile.in b/stapbpf/Makefile.in
index 487da04..e3a3146 100644
--- a/stapbpf/Makefile.in
+++ b/stapbpf/Makefile.in
@@ -848,9 +848,9 @@ uninstall-man: uninstall-man8
 # Why the "id -u" condition?  This way, an unprivileged user can run
 # make install, and have "sudo stap ...." or "sudo stapbpf ...." work later.
 @HAVE_BPF_DECLS_TRUE@install-exec-hook:
-@HAVE_BPF_DECLS_TRUE@	if [ `id -u` -eq 0 ]; then \
-@HAVE_BPF_DECLS_TRUE@		(getent group stapusr >/dev/null || groupadd -f -g 156 -r stapusr) \
-@HAVE_BPF_DECLS_TRUE@		&& chgrp stapusr "$(DESTDIR)$(bindir)/stapbpf" \
+@HAVE_BPF_DECLS_TRUE@	if [ `id -u` -eq 0 ] && (getent group stapusr >/dev/null \
+@HAVE_BPF_DECLS_TRUE@	                         || groupadd -f -g 156 -r stapusr); then \
+@HAVE_BPF_DECLS_TRUE@		chgrp stapusr "$(DESTDIR)$(bindir)/stapbpf" \
 @HAVE_BPF_DECLS_TRUE@		&& chmod 04110 "$(DESTDIR)$(bindir)/stapbpf"; \
 @HAVE_BPF_DECLS_TRUE@	fi
 
diff --git a/staprun/Makefile.am b/staprun/Makefile.am
index f1a7a69..dd1c920 100644
--- a/staprun/Makefile.am
+++ b/staprun/Makefile.am
@@ -77,8 +77,8 @@ git_version.stamp ../git_version.h:
 # Why the "id -u" condition?  This way, an unprivileged user can run
 # make install, and have "sudo stap ...." or "sudo staprun ...." work later.
 install-exec-hook:
-	if [ `id -u` -eq 0 ]; then \
-		(getent group stapusr >/dev/null || groupadd -f -g 156 -r stapusr) \
-		&& chgrp stapusr "$(DESTDIR)$(bindir)/staprun" \
+	if [ `id -u` -eq 0 ] && (getent group stapusr >/dev/null \
+	                         || groupadd -f -g 156 -r stapusr); then \
+		chgrp stapusr "$(DESTDIR)$(bindir)/staprun" \
 		&& chmod 04110 "$(DESTDIR)$(bindir)/staprun"; \
 	fi
diff --git a/staprun/Makefile.in b/staprun/Makefile.in
index c518c7d..3536852 100644
--- a/staprun/Makefile.in
+++ b/staprun/Makefile.in
@@ -1161,9 +1161,9 @@ git_version.stamp ../git_version.h:
 # Why the "id -u" condition?  This way, an unprivileged user can run
 # make install, and have "sudo stap ...." or "sudo staprun ...." work later.
 install-exec-hook:
-	if [ `id -u` -eq 0 ]; then \
-		(getent group stapusr >/dev/null || groupadd -f -g 156 -r stapusr) \
-		&& chgrp stapusr "$(DESTDIR)$(bindir)/staprun" \
+	if [ `id -u` -eq 0 ] && (getent group stapusr >/dev/null \
+	                         || groupadd -f -g 156 -r stapusr); then \
+		chgrp stapusr "$(DESTDIR)$(bindir)/staprun" \
 		&& chmod 04110 "$(DESTDIR)$(bindir)/staprun"; \
 	fi
 
-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Fix "make install" in certain fakeroot environments
  2019-01-30  5:38   ` David Ward
  2019-01-30  5:38     ` [PATCH 2/2] Handle installation without stapusr group David Ward
  2019-01-30  5:38     ` [PATCH 1/2] Simplify creation of groups during installation David Ward
@ 2019-02-04 17:05     ` Frank Ch. Eigler
  2 siblings, 0 replies; 6+ messages in thread
From: Frank Ch. Eigler @ 2019-02-04 17:05 UTC (permalink / raw)
  To: David Ward; +Cc: systemtap


david.ward wrote:

> I reacted too quickly to information I came across after sending, which
> led me to think I was doing something wrong. I still believe this change
> should be made.

Looks okay, merged, thanks!

- FChE

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-02-04 17:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-27  4:22 [PATCH] Fix "make install" in certain fakeroot environments David Ward
2019-01-27  5:08 ` David Ward
2019-01-30  5:38   ` David Ward
2019-01-30  5:38     ` [PATCH 2/2] Handle installation without stapusr group David Ward
2019-01-30  5:38     ` [PATCH 1/2] Simplify creation of groups during installation David Ward
2019-02-04 17:05     ` [PATCH] Fix "make install" in certain fakeroot environments Frank Ch. Eigler

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).