[Buildroot] [PATCH v3 0/8] package/petitboot: misc fixes/enhancement

[Buildroot] [PATCH v3 0/8] package/petitboot: misc fixes/enhancement

[Reza Arbab]

The br2-external tree used to build OpenPOWER firmware has long carried
petitboot as a custom package[1]. Now that petitboot has been added to
buildroot proper, it would be nice to leverage the base package instead.

To make that transition easier, here is a set of patches which port over
some of the enhancements made to that external package.

[1] https://github.com/open-power/op-build/tree/master/openpower/package/petitboot
---
v3:
* Add a number of small fixes.

* Add user separation, so the UI runs as non-root.

* Remove udev rules that enabled some additional types of boot devices.
  These should more appropriately live outside of buildroot.

* Remove a sysctl.d file to silence kernel output. I think there's a
  bug upstream; see https://github.com/open-power/petitboot/pull/103

v2:
* Use Laurent's suggested additions to "run pb-console at boot" patch.

Reza Arbab (8):
  package/petitboot: fix menu comment
  package/petitboot: fix pb-discover pidfile creation
  package/petitboot: use default logfile dir
  package/petitboot: prefer kexec-lite on powerpc
  package/petitboot: fix shutdown
  package/petitboot: run petitboot UI on consoles
  package/petitboot: enable user separation
  package/petitboot: prefer UTF-8 support

 package/petitboot/Config.in      | 26 +++++++++++++++------
 package/petitboot/S15pb-discover | 13 +++++++----
 package/petitboot/kexec-restart  |  8 +++++++
 package/petitboot/pb-console     | 39 ++++++++++++++++++++++++++++++++
 package/petitboot/petitboot.mk   | 38 +++++++++++++++++++++++++++++--
 package/petitboot/shell_config   | 24 ++++++++++++++++++++
 package/petitboot/shell_profile  |  5 ++++
 system/Config.in                 |  2 +-
 8 files changed, 140 insertions(+), 15 deletions(-)
 create mode 100755 package/petitboot/kexec-restart
 create mode 100644 package/petitboot/pb-console
 create mode 100644 package/petitboot/shell_config
 create mode 100644 package/petitboot/shell_profile

-- 
2.39.3

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v3 1/8] package/petitboot: fix menu comment

[Reza Arbab]

The comment should appear if threads aren't enabled, not when they are.

Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
---
 package/petitboot/Config.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/package/petitboot/Config.in b/package/petitboot/Config.in
index 0b4dc762bbd8..b534b4900e88 100644
--- a/package/petitboot/Config.in
+++ b/package/petitboot/Config.in
@@ -26,6 +26,6 @@ comment "petitboot needs a uClibc or glibc toolchain w/ wchar, dynamic library,
 	depends on BR2_PACKAGE_KEXEC_ARCH_SUPPORTS
 	depends on BR2_USE_MMU
 	depends on !BR2_USE_WCHAR || BR2_STATIC_LIBS || \
-		BR2_TOOLCHAIN_HAS_THREADS || \
+		!BR2_TOOLCHAIN_HAS_THREADS || \
 		!(BR2_TOOLCHAIN_USES_UCLIBC || BR2_TOOLCHAIN_USES_GLIBC) || \
 		!BR2_PACKAGE_HAS_UDEV
-- 
2.39.3

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v3 2/8] package/petitboot: fix pb-discover pidfile creation

[Reza Arbab]

pb-discover does not create its own pid file. Handle the creation and
removal of the pid file in the init script.

Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
---
 package/petitboot/S15pb-discover | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/package/petitboot/S15pb-discover b/package/petitboot/S15pb-discover
index 7ecc12e99103..9b641298cafa 100644
--- a/package/petitboot/S15pb-discover
+++ b/package/petitboot/S15pb-discover
@@ -16,7 +16,7 @@ fi
 start() {
 	printf 'Starting %s: ' "$DAEMON"
 	# shellcheck disable=SC2086 # we need the word splitting
-	start-stop-daemon -S -q -b -p "$PIDFILE" -x "/usr/sbin/$DAEMON" \
+	start-stop-daemon -S -q -b -m -p "$PIDFILE" -x "/usr/sbin/$DAEMON" \
 		-- $PB_DISCOVER_ARGS
 	status=$?
 	if [ "$status" -eq 0 ]; then
@@ -32,6 +32,7 @@ stop() {
 	start-stop-daemon -K -q -p "$PIDFILE"
 	status=$?
 	if [ "$status" -eq 0 ]; then
+		rm -f "$PIDFILE"
 		echo "OK"
 	else
 		echo "FAIL"
-- 
2.39.3

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v3 3/8] package/petitboot: use default logfile dir

[Reza Arbab]

All the petitboot components assume /var/log/petitboot by default;
pb-console can also put multiple logs there and pb-sos collects that
directory when creating a diagnostic tarball.

Defer to this default when launching pb-discover. If someone wants to
override, let's call the file /etc/default/petitboot which makes more
sense to be shared by all the components.

Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
---
 package/petitboot/S15pb-discover | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/package/petitboot/S15pb-discover b/package/petitboot/S15pb-discover
index 9b641298cafa..71ab62d99859 100644
--- a/package/petitboot/S15pb-discover
+++ b/package/petitboot/S15pb-discover
@@ -2,12 +2,9 @@
 
 DAEMON="pb-discover"
 PIDFILE="/var/run/$DAEMON.pid"
-LOGFILE="/var/log/$DAEMON.log"
-
-PB_DISCOVER_ARGS="-l $LOGFILE"
 
 # shellcheck source=/dev/null
-[ -r "/etc/default/$DAEMON" ] && . "/etc/default/$DAEMON"
+[ -r "/etc/default/petitboot" ] && . "/etc/default/petitboot"
 
 if [ "$(pb-config debug)" = "enabled" ] ; then
 	PB_DISCOVER_ARGS="$PB_DISCOVER_ARGS --verbose"
@@ -15,6 +12,8 @@ fi
 
 start() {
 	printf 'Starting %s: ' "$DAEMON"
+	mkdir -p /var/log/petitboot
+
 	# shellcheck disable=SC2086 # we need the word splitting
 	start-stop-daemon -S -q -b -m -p "$PIDFILE" -x "/usr/sbin/$DAEMON" \
 		-- $PB_DISCOVER_ARGS
-- 
2.39.3

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v3 4/8] package/petitboot: prefer kexec-lite on powerpc

[Reza Arbab]

This is a better choice on devicetree-based platforms.

Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
---
 package/petitboot/Config.in | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/package/petitboot/Config.in b/package/petitboot/Config.in
index b534b4900e88..4981c165bf76 100644
--- a/package/petitboot/Config.in
+++ b/package/petitboot/Config.in
@@ -11,12 +11,11 @@ config BR2_PACKAGE_PETITBOOT
 	select BR2_PACKAGE_ELFUTILS
 	select BR2_PACKAGE_LVM2 # devmapper
 	select BR2_PACKAGE_NCURSES
-	# run-time dependency only
+	# run-time dependencies
 	select BR2_PACKAGE_KEXEC if !BR2_PACKAGE_KEXEC_LITE
-	# run-time dependency only
-	select BR2_PACKAGE_POWERPC_UTILS if ( BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le )
-	# run-time dependency only
+	select BR2_PACKAGE_KEXEC_LITE if ( BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le )
 	select BR2_PACKAGE_NVME if ( BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le )
+	select BR2_PACKAGE_POWERPC_UTILS if ( BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le )
 	help
 	  Petitboot is a small kexec-based bootloader
 
-- 
2.39.3

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v3 5/8] package/petitboot: fix shutdown

[Reza Arbab]

There are a few ways to set HOST_PROG_SHUTDOWN that enable us to
successfully kexec the new kernel chosen from the petitboot menu.

1. HOST_PROG_SHUTDOWN=/usr/libexec/petitboot/bb-kexec-reboot
    `- bb-kexec-reboot does 'exec kill -QUIT 1'
        |- init sends SIGTERM to pb-console
        |   `- pb-console does 'trap 'reset; echo "SIGTERM received, booting..."; sleep 2' SIGTERM'
        `- init runs 'null::restart:/usr/sbin/kexec-restart'
            `- kexec-restart does '/usr/sbin/kexec -f -e'

2. HOST_PROG_SHUTDOWN=/usr/sbin/kexec-restart
    `- kexec-restart does '/usr/sbin/kexec -f -e'

3. unset HOST_PROG_SHUTDOWN (default is /sbin/shutdown)
    `- petitboot tries, in order:
        |- 1. '/sbin/shutdown -r now'
        |      `- file not found
        |- 2. '/usr/sbin/kexec -e'
        `- 3. '/usr/sbin/kexec -e -f'

If we're using busybox init, do (1). This performs a proper shutdown
before the kexec, giving us nice console output with a terminal reset
and the "booting..." notification. Otherwise, fall back to (2).

Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
---
 package/petitboot/kexec-restart |  8 ++++++++
 package/petitboot/petitboot.mk  | 16 ++++++++++++++--
 2 files changed, 22 insertions(+), 2 deletions(-)
 create mode 100755 package/petitboot/kexec-restart

diff --git a/package/petitboot/kexec-restart b/package/petitboot/kexec-restart
new file mode 100755
index 000000000000..62fbea114513
--- /dev/null
+++ b/package/petitboot/kexec-restart
@@ -0,0 +1,8 @@
+#!/bin/sh
+
+/usr/sbin/kexec -f -e
+
+while :
+do
+	sleep 1
+done
diff --git a/package/petitboot/petitboot.mk b/package/petitboot/petitboot.mk
index 0992111cb1ec..5c7913b560b1 100644
--- a/package/petitboot/petitboot.mk
+++ b/package/petitboot/petitboot.mk
@@ -21,7 +21,6 @@ PETITBOOT_CONF_OPTS = \
 	--without-twin-x11 \
 	$(if $(BR2_PACKAGE_BUSYBOX),--enable-busybox,--disable-busybox) \
 	HOST_PROG_KEXEC=/usr/sbin/kexec \
-	HOST_PROG_SHUTDOWN=/usr/libexec/petitboot/bb-kexec-reboot
 
 # HPA and Busybox tftp are supported. HPA tftp is part of Buildroot's tftpd
 # package.
@@ -46,6 +45,17 @@ else
 PETITBOOT_CONF_OPTS += --without-fdt
 endif
 
+ifeq ($(BR2_INIT_BUSYBOX),y)
+PETITBOOT_CONF_OPTS += HOST_PROG_SHUTDOWN=/usr/libexec/petitboot/bb-kexec-reboot
+define PETITBOOT_INITTAB
+	grep -q kexec-restart $(TARGET_DIR)/etc/inittab || \
+		printf "\nnull::restart:/usr/sbin/kexec-restart\n" >> $(TARGET_DIR)/etc/inittab
+endef
+PETITBOOT_TARGET_FINALIZE_HOOKS += PETITBOOT_INITTAB
+else
+PETITBOOT_CONF_OPTS += HOST_PROG_SHUTDOWN=/usr/sbin/kexec-restart
+endif
+
 define PETITBOOT_POST_INSTALL
 	$(INSTALL) -D -m 0755 $(@D)/utils/bb-kexec-reboot \
 		$(TARGET_DIR)/usr/libexec/petitboot/bb-kexec-reboot
@@ -53,8 +63,10 @@ define PETITBOOT_POST_INSTALL
 		$(TARGET_DIR)/etc/petitboot/boot.d/01-create-default-dtb
 	$(INSTALL) -D -m 0755 $(@D)/utils/hooks/90-sort-dtb \
 		$(TARGET_DIR)/etc/petitboot/boot.d/90-sort-dtb
-	$(INSTALL) -m 0755 -D $(PETITBOOT_PKGDIR)/S15pb-discover \
+	$(INSTALL) -D -m 0755 $(PETITBOOT_PKGDIR)/S15pb-discover \
 		$(TARGET_DIR)/etc/init.d/S15pb-discover
+	$(INSTALL) -D -m 0755 $(PETITBOOT_PKGDIR)/kexec-restart \
+		$(TARGET_DIR)/usr/sbin/kexec-restart
 	mkdir -p $(TARGET_DIR)/usr/share/udhcpc/default.script.d/
 	ln -sf /usr/sbin/pb-udhcpc \
 		$(TARGET_DIR)/usr/share/udhcpc/default.script.d/
-- 
2.39.3

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v3 6/8] package/petitboot: run petitboot UI on consoles

[Reza Arbab]

Display the petitboot UI instead of a login prompt, allowing the
configuration of custom tty(s) as we do for the login prompt.

petitboot already depends on udev, so let's use it instead of rcS to
launch pb-console. This has the advantage of easily wildcarding the list
of ttys ("hvc*") and enables hotplug devices ("ttyUSB0").

Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
---
 package/petitboot/Config.in    |  8 ++++++++
 package/petitboot/pb-console   | 36 ++++++++++++++++++++++++++++++++++
 package/petitboot/petitboot.mk | 10 ++++++++++
 system/Config.in               |  2 +-
 4 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100644 package/petitboot/pb-console

diff --git a/package/petitboot/Config.in b/package/petitboot/Config.in
index 4981c165bf76..5f1d91e77ecb 100644
--- a/package/petitboot/Config.in
+++ b/package/petitboot/Config.in
@@ -28,3 +28,11 @@ comment "petitboot needs a uClibc or glibc toolchain w/ wchar, dynamic library,
 		!BR2_TOOLCHAIN_HAS_THREADS || \
 		!(BR2_TOOLCHAIN_USES_UCLIBC || BR2_TOOLCHAIN_USES_GLIBC) || \
 		!BR2_PACKAGE_HAS_UDEV
+
+config BR2_PACKAGE_PETITBOOT_GETTY_PORT
+	string "TTY port(s)"
+	default "console"
+	depends on BR2_PACKAGE_PETITBOOT
+	help
+	  Specify a space-separated list of ports to run the petitboot UI on.
+	  Wildcards are allowed. Example: "hvc* ttys0 ttyS*"
diff --git a/package/petitboot/pb-console b/package/petitboot/pb-console
new file mode 100644
index 000000000000..407ff3b30232
--- /dev/null
+++ b/package/petitboot/pb-console
@@ -0,0 +1,36 @@
+#!/bin/sh
+
+DAEMON="pb-console"
+
+PB_CONSOLE_PORT=${2:-"console"}
+PB_CONSOLE_ARGS="--getty --detach -- -n -i 0 $PB_CONSOLE_PORT linux"
+
+# shellcheck source=/dev/null
+[ -r "/etc/default/petitboot" ] && . "/etc/default/petitboot"
+
+start() {
+	printf 'Starting %s on %s: ' "$DAEMON" "$PB_CONSOLE_PORT"
+	mkdir -p /var/log/petitboot
+
+	# shellcheck disable=SC2086 # we need the word splitting
+	start-stop-daemon -S -q -x "/usr/libexec/petitboot/$DAEMON" \
+		-- $PB_CONSOLE_ARGS
+	status=$?
+	if [ "$status" -eq 0 ]; then
+		echo "OK"
+	else
+		echo "FAIL"
+	fi
+	return "$status"
+}
+
+case "$1" in
+	start)
+		"$1";;
+	stop|restart|reload)
+		;;
+	*)
+		echo "Usage: $0 {start|stop|restart|reload} [port]"
+		exit 1
+		;;
+esac
diff --git a/package/petitboot/petitboot.mk b/package/petitboot/petitboot.mk
index 5c7913b560b1..ff87f3498734 100644
--- a/package/petitboot/petitboot.mk
+++ b/package/petitboot/petitboot.mk
@@ -56,6 +56,8 @@ else
 PETITBOOT_CONF_OPTS += HOST_PROG_SHUTDOWN=/usr/sbin/kexec-restart
 endif
 
+PETITBOOT_GETTY_PORT = $(patsubst %,'%',$(call qstrip,$(BR2_PACKAGE_PETITBOOT_GETTY_PORT)))
+
 define PETITBOOT_POST_INSTALL
 	$(INSTALL) -D -m 0755 $(@D)/utils/bb-kexec-reboot \
 		$(TARGET_DIR)/usr/libexec/petitboot/bb-kexec-reboot
@@ -67,6 +69,14 @@ define PETITBOOT_POST_INSTALL
 		$(TARGET_DIR)/etc/init.d/S15pb-discover
 	$(INSTALL) -D -m 0755 $(PETITBOOT_PKGDIR)/kexec-restart \
 		$(TARGET_DIR)/usr/sbin/kexec-restart
+	$(INSTALL) -D -m 0755 $(PETITBOOT_PKGDIR)/pb-console \
+		$(TARGET_DIR)/etc/init.d/pb-console
+
+	mkdir -p $(TARGET_DIR)/etc/udev/rules.d
+	(for port in $(PETITBOOT_GETTY_PORT); do \
+		printf 'SUBSYSTEM=="tty", KERNEL=="%s", RUN+="/etc/init.d/pb-console start $$name"\n' "$$port"; \
+	done) > $(TARGET_DIR)/etc/udev/rules.d/petitboot-console-ui.rules
+
 	mkdir -p $(TARGET_DIR)/usr/share/udhcpc/default.script.d/
 	ln -sf /usr/sbin/pb-udhcpc \
 		$(TARGET_DIR)/usr/share/udhcpc/default.script.d/
diff --git a/system/Config.in b/system/Config.in
index 24798dc06803..9587dd9ce4db 100644
--- a/system/Config.in
+++ b/system/Config.in
@@ -375,7 +375,7 @@ config BR2_SYSTEM_BIN_SH
 
 menuconfig BR2_TARGET_GENERIC_GETTY
 	bool "Run a getty (login prompt) after boot"
-	default y
+	default y if !BR2_PACKAGE_PETITBOOT
 
 if BR2_TARGET_GENERIC_GETTY
 config BR2_TARGET_GENERIC_GETTY_PORT
-- 
2.39.3

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v3 7/8] package/petitboot: enable user separation

[Reza Arbab]

Run the petitboot UI as an unprivileged user. This requires using the
agetty package instead of the busybox getty utility, running the initial
pb-console helper at user login rather than directly.

If sudo is installed, with a sudoers policy allowing petituser to
perform sudo with no password (or a blank password), the "drop to shell"
feature of petitboot will automatically become a root shell.

Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
---
 package/petitboot/Config.in      |  1 +
 package/petitboot/S15pb-discover |  4 +++-
 package/petitboot/pb-console     |  6 ++++--
 package/petitboot/petitboot.mk   | 12 ++++++++++++
 package/petitboot/shell_config   | 24 ++++++++++++++++++++++++
 package/petitboot/shell_profile  |  2 ++
 6 files changed, 46 insertions(+), 3 deletions(-)
 create mode 100644 package/petitboot/shell_config
 create mode 100644 package/petitboot/shell_profile

diff --git a/package/petitboot/Config.in b/package/petitboot/Config.in
index 5f1d91e77ecb..0f965e71e628 100644
--- a/package/petitboot/Config.in
+++ b/package/petitboot/Config.in
@@ -16,6 +16,7 @@ config BR2_PACKAGE_PETITBOOT
 	select BR2_PACKAGE_KEXEC_LITE if ( BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le )
 	select BR2_PACKAGE_NVME if ( BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le )
 	select BR2_PACKAGE_POWERPC_UTILS if ( BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le )
+	select BR2_PACKAGE_UTIL_LINUX_AGETTY
 	help
 	  Petitboot is a small kexec-based bootloader
 
diff --git a/package/petitboot/S15pb-discover b/package/petitboot/S15pb-discover
index 71ab62d99859..a37e33521f1a 100644
--- a/package/petitboot/S15pb-discover
+++ b/package/petitboot/S15pb-discover
@@ -12,7 +12,9 @@ fi
 
 start() {
 	printf 'Starting %s: ' "$DAEMON"
-	mkdir -p /var/log/petitboot
+	# shellcheck disable=SC2174 # only apply -m to deepest dir
+	mkdir -p -m 0775 /var/log/petitboot
+	chown root:petitgroup /var/log/petitboot
 
 	# shellcheck disable=SC2086 # we need the word splitting
 	start-stop-daemon -S -q -b -m -p "$PIDFILE" -x "/usr/sbin/$DAEMON" \
diff --git a/package/petitboot/pb-console b/package/petitboot/pb-console
index 407ff3b30232..eea40163d02f 100644
--- a/package/petitboot/pb-console
+++ b/package/petitboot/pb-console
@@ -3,14 +3,16 @@
 DAEMON="pb-console"
 
 PB_CONSOLE_PORT=${2:-"console"}
-PB_CONSOLE_ARGS="--getty --detach -- -n -i 0 $PB_CONSOLE_PORT linux"
+PB_CONSOLE_ARGS="--getty=/sbin/agetty --detach -- -a petituser -n -i $PB_CONSOLE_PORT linux"
 
 # shellcheck source=/dev/null
 [ -r "/etc/default/petitboot" ] && . "/etc/default/petitboot"
 
 start() {
 	printf 'Starting %s on %s: ' "$DAEMON" "$PB_CONSOLE_PORT"
-	mkdir -p /var/log/petitboot
+	# shellcheck disable=SC2174 # only apply -m to deepest dir
+	mkdir -p -m 0775 /var/log/petitboot
+	chown root:petitgroup /var/log/petitboot
 
 	# shellcheck disable=SC2086 # we need the word splitting
 	start-stop-daemon -S -q -x "/usr/libexec/petitboot/$DAEMON" \
diff --git a/package/petitboot/petitboot.mk b/package/petitboot/petitboot.mk
index ff87f3498734..5b517eb3b1a6 100644
--- a/package/petitboot/petitboot.mk
+++ b/package/petitboot/petitboot.mk
@@ -71,6 +71,10 @@ define PETITBOOT_POST_INSTALL
 		$(TARGET_DIR)/usr/sbin/kexec-restart
 	$(INSTALL) -D -m 0755 $(PETITBOOT_PKGDIR)/pb-console \
 		$(TARGET_DIR)/etc/init.d/pb-console
+	$(INSTALL) -D -m 0755 $(PETITBOOT_PKGDIR)/shell_config \
+		$(TARGET_DIR)/home/petituser/.shrc
+	$(INSTALL) -D -m 0755 $(PETITBOOT_PKGDIR)/shell_profile \
+		$(TARGET_DIR)/home/petituser/.profile
 
 	mkdir -p $(TARGET_DIR)/etc/udev/rules.d
 	(for port in $(PETITBOOT_GETTY_PORT); do \
@@ -84,4 +88,12 @@ endef
 
 PETITBOOT_POST_INSTALL_TARGET_HOOKS += PETITBOOT_POST_INSTALL
 
+define PETITBOOT_USERS
+	petituser -1 petitgroup -1 * /home/petituser /bin/sh - petitboot user
+endef
+
+define PETITBOOT_PERMISSIONS
+	/var/petitboot d 775 root petitgroup - - - - -
+endef
+
 $(eval $(autotools-package))
diff --git a/package/petitboot/shell_config b/package/petitboot/shell_config
new file mode 100644
index 000000000000..b10b95baae6c
--- /dev/null
+++ b/package/petitboot/shell_config
@@ -0,0 +1,24 @@
+#!/bin/sh
+
+try_sudo() {
+	[ -x "$(command -v sudo)" ] || return
+	sudo -K
+	echo | sudo -S /bin/true >/dev/null 2>&1 || return
+
+	echo "No password required, running as root."
+	sudo -i
+	sudo -K
+	exit
+}
+
+reset
+
+echo "Exiting petitboot. Type 'exit' to return."
+echo "You may run 'pb-sos' to gather diagnostic data."
+
+if [ "$(id -u)" != "0" ]; then
+	try_sudo
+	export PS1='$ '
+else
+	export PS1='# '
+fi
diff --git a/package/petitboot/shell_profile b/package/petitboot/shell_profile
new file mode 100644
index 000000000000..1ca5e6364dba
--- /dev/null
+++ b/package/petitboot/shell_profile
@@ -0,0 +1,2 @@
+export ENV="/home/petituser/.shrc"
+exec /usr/libexec/petitboot/pb-console
-- 
2.39.3

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

[Buildroot] [PATCH v3 8/8] package/petitboot: prefer UTF-8 support

[Reza Arbab]

The petitboot UI looks much nicer in a Unicode locale:

* Items in the language selection submenu use multibyte Unicode
  characters. In other locales, they say "Unable to display text in this
  locale".

* The combination of TERM=linux with a UTF-8 locale is required to
  trigger a special-case workaround in ncurses code[1]. Without
  this, line-drawing characters in the menu look like q's.

Add a reminder that a UTF-8 locale should be generated for things to
look right. Assume C.UTF-8 by default, allowing $LANG to be overridden
(by /etc/default/petitboot or otherwise) if something else is desired.

[1] https://invisible-island.net/ncurses/ncurses.faq.html#no_line_drawing

Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
---
 package/petitboot/Config.in      | 8 ++++++--
 package/petitboot/S15pb-discover | 1 +
 package/petitboot/pb-console     | 1 +
 package/petitboot/shell_profile  | 3 +++
 4 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/package/petitboot/Config.in b/package/petitboot/Config.in
index 0f965e71e628..a449ed87872c 100644
--- a/package/petitboot/Config.in
+++ b/package/petitboot/Config.in
@@ -4,6 +4,7 @@ config BR2_PACKAGE_PETITBOOT
 	depends on BR2_PACKAGE_KEXEC_ARCH_SUPPORTS
 	depends on BR2_USE_MMU # lvm2
 	depends on BR2_USE_WCHAR # elfutils
+	depends on BR2_ENABLE_LOCALE
 	depends on !BR2_STATIC_LIBS # elfutils, lvm2
 	depends on BR2_TOOLCHAIN_HAS_THREADS # elfutils, lvm2
 	depends on BR2_TOOLCHAIN_USES_UCLIBC || BR2_TOOLCHAIN_USES_GLIBC # elfutils
@@ -11,6 +12,7 @@ config BR2_PACKAGE_PETITBOOT
 	select BR2_PACKAGE_ELFUTILS
 	select BR2_PACKAGE_LVM2 # devmapper
 	select BR2_PACKAGE_NCURSES
+	select BR2_PACKAGE_NCURSES_WCHAR
 	# run-time dependencies
 	select BR2_PACKAGE_KEXEC if !BR2_PACKAGE_KEXEC_LITE
 	select BR2_PACKAGE_KEXEC_LITE if ( BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le )
@@ -20,12 +22,14 @@ config BR2_PACKAGE_PETITBOOT
 	help
 	  Petitboot is a small kexec-based bootloader
 
+	  NOTE: petitboot needs a working UTF-8 locale (BR2_GENERATE_LOCALE)
+
 	  http://www.kernel.org/pub/linux/kernel/people/geoff/petitboot/petitboot.html
 
-comment "petitboot needs a uClibc or glibc toolchain w/ wchar, dynamic library, threads, udev /dev management"
+comment "petitboot needs a uClibc or glibc toolchain w/ wchar, locale, dynamic library, threads, udev /dev management"
 	depends on BR2_PACKAGE_KEXEC_ARCH_SUPPORTS
 	depends on BR2_USE_MMU
-	depends on !BR2_USE_WCHAR || BR2_STATIC_LIBS || \
+	depends on !BR2_USE_WCHAR || !BR2_ENABLE_LOCALE || BR2_STATIC_LIBS || \
 		!BR2_TOOLCHAIN_HAS_THREADS || \
 		!(BR2_TOOLCHAIN_USES_UCLIBC || BR2_TOOLCHAIN_USES_GLIBC) || \
 		!BR2_PACKAGE_HAS_UDEV
diff --git a/package/petitboot/S15pb-discover b/package/petitboot/S15pb-discover
index a37e33521f1a..15b428ce0e42 100644
--- a/package/petitboot/S15pb-discover
+++ b/package/petitboot/S15pb-discover
@@ -15,6 +15,7 @@ start() {
 	# shellcheck disable=SC2174 # only apply -m to deepest dir
 	mkdir -p -m 0775 /var/log/petitboot
 	chown root:petitgroup /var/log/petitboot
+	export LANG="${LANG:-C.UTF-8}"
 
 	# shellcheck disable=SC2086 # we need the word splitting
 	start-stop-daemon -S -q -b -m -p "$PIDFILE" -x "/usr/sbin/$DAEMON" \
diff --git a/package/petitboot/pb-console b/package/petitboot/pb-console
index eea40163d02f..55e5462f0457 100644
--- a/package/petitboot/pb-console
+++ b/package/petitboot/pb-console
@@ -13,6 +13,7 @@ start() {
 	# shellcheck disable=SC2174 # only apply -m to deepest dir
 	mkdir -p -m 0775 /var/log/petitboot
 	chown root:petitgroup /var/log/petitboot
+	export LANG="${LANG:-C.UTF-8}"
 
 	# shellcheck disable=SC2086 # we need the word splitting
 	start-stop-daemon -S -q -x "/usr/libexec/petitboot/$DAEMON" \
diff --git a/package/petitboot/shell_profile b/package/petitboot/shell_profile
index 1ca5e6364dba..6bbe49e6d113 100644
--- a/package/petitboot/shell_profile
+++ b/package/petitboot/shell_profile
@@ -1,2 +1,5 @@
+[ -r "/etc/default/petitboot" ] && . "/etc/default/petitboot"
 export ENV="/home/petituser/.shrc"
+export LANG="${LANG:-C.UTF-8}"
+
 exec /usr/libexec/petitboot/pb-console
-- 
2.39.3

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 4/8] package/petitboot: prefer kexec-lite on powerpc

[Arnout Vandecappelle via buildroot]

  Hi Reza,

On 09/10/2023 17:17, Reza Arbab wrote:
> This is a better choice on devicetree-based platforms.
> 
> Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
> ---
>   package/petitboot/Config.in | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/package/petitboot/Config.in b/package/petitboot/Config.in
> index b534b4900e88..4981c165bf76 100644
> --- a/package/petitboot/Config.in
> +++ b/package/petitboot/Config.in
> @@ -11,12 +11,11 @@ config BR2_PACKAGE_PETITBOOT
>   	select BR2_PACKAGE_ELFUTILS
>   	select BR2_PACKAGE_LVM2 # devmapper
>   	select BR2_PACKAGE_NCURSES
> -	# run-time dependency only
> +	# run-time dependencies

  This is an unrelated change (sorting the runtime dependencies alphabetically 
and removing the repetition of the comment). Ideally, this should have been a 
separate commit. Failing that, it should be explained in the commit message. 
I've done that.

>   	select BR2_PACKAGE_KEXEC if !BR2_PACKAGE_KEXEC_LITE
> -	# run-time dependency only
> -	select BR2_PACKAGE_POWERPC_UTILS if ( BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le )
> -	# run-time dependency only
> +	select BR2_PACKAGE_KEXEC_LITE if ( BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le )

  If the user has already selected kexec, then we'll have both kexec and 
kexec-lite. I don't think that that is a good idea. So instead, I injected a 
separate commit that introduces BR2_PACKAGE_KEXEC_LITE_ARCH_SUPPORTS, and use 
that to simplify the condition here and to only select kexec-lite if kexec is 
not selected:

	select BR2_PACKAGE_KEXEC if !BR2_PACKAGE_KEXEC_LITE_ARCH_SUPPORTS
	select BR2_PACKAGE_KEXEC_LITE if BR2_PACKAGE_KEXEC_LITE_ARCH_SUPPORTS && 
!BR2_PACKAGE_KEXEC



  Regards,
  Arnout

>   	select BR2_PACKAGE_NVME if ( BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le )
> +	select BR2_PACKAGE_POWERPC_UTILS if ( BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le )
>   	help
>   	  Petitboot is a small kexec-based bootloader
>   
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 5/8] package/petitboot: fix shutdown

[Arnout Vandecappelle via buildroot]

  Hi Reza,

On 09/10/2023 17:17, Reza Arbab wrote:
> There are a few ways to set HOST_PROG_SHUTDOWN that enable us to
> successfully kexec the new kernel chosen from the petitboot menu.
> 
> 1. HOST_PROG_SHUTDOWN=/usr/libexec/petitboot/bb-kexec-reboot
>      `- bb-kexec-reboot does 'exec kill -QUIT 1'
>          |- init sends SIGTERM to pb-console
>          |   `- pb-console does 'trap 'reset; echo "SIGTERM received, booting..."; sleep 2' SIGTERM'
>          `- init runs 'null::restart:/usr/sbin/kexec-restart'
>              `- kexec-restart does '/usr/sbin/kexec -f -e'
> 
> 2. HOST_PROG_SHUTDOWN=/usr/sbin/kexec-restart
>      `- kexec-restart does '/usr/sbin/kexec -f -e'
> 
> 3. unset HOST_PROG_SHUTDOWN (default is /sbin/shutdown)
>      `- petitboot tries, in order:
>          |- 1. '/sbin/shutdown -r now'
>          |      `- file not found
>          |- 2. '/usr/sbin/kexec -e'
>          `- 3. '/usr/sbin/kexec -e -f'
> 
> If we're using busybox init, do (1). This performs a proper shutdown
> before the kexec, giving us nice console output with a terminal reset
> and the "booting..." notification. Otherwise, fall back to (2).

  This doesn't explain _why_ this is the proper approach, or why the current one 
needs to be fixed.

  It seems to me that (1) would be the proper thing to do for sysvinit, openrc 
and systemd based init as well (not that anyone would be crazy enough to put 
systemd in a petitboot image, I hope...). So, if I understand correctly, what 
needs to be fixed is when init is not an actual init, but something trivial like 
a single shell script. So at the very least, the condition should change I think.

  But then, BR2_INIT_NONE is not a reliable indication that PID 1 is not a real 
init. So I think it would be better to instead decide at runtime which approach 
to take. That could be done for instance by looking at /proc/1/exe and comparing 
it with /sbin/init - if they are the same, we have a real init; if not, we 
probably have a shell or some such.

  But maybe I misunderstand the reasoning completely.

  Also, why don't we simply always do (2)? The only difference is the message on 
pb-console, and normally it would anyway be pb-console that triggers the boot so 
it would already have shown something, I expect?

> 
> Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
> ---
>   package/petitboot/kexec-restart |  8 ++++++++
>   package/petitboot/petitboot.mk  | 16 ++++++++++++++--
>   2 files changed, 22 insertions(+), 2 deletions(-)
>   create mode 100755 package/petitboot/kexec-restart
> 
> diff --git a/package/petitboot/kexec-restart b/package/petitboot/kexec-restart
> new file mode 100755
> index 000000000000..62fbea114513
> --- /dev/null
> +++ b/package/petitboot/kexec-restart
> @@ -0,0 +1,8 @@
> +#!/bin/sh
> +
> +/usr/sbin/kexec -f -e
> +
> +while :
> +do
> +	sleep 1
> +done
> diff --git a/package/petitboot/petitboot.mk b/package/petitboot/petitboot.mk
> index 0992111cb1ec..5c7913b560b1 100644
> --- a/package/petitboot/petitboot.mk
> +++ b/package/petitboot/petitboot.mk
> @@ -21,7 +21,6 @@ PETITBOOT_CONF_OPTS = \
>   	--without-twin-x11 \
>   	$(if $(BR2_PACKAGE_BUSYBOX),--enable-busybox,--disable-busybox) \
>   	HOST_PROG_KEXEC=/usr/sbin/kexec \

  This leaves a trailing backslash here, as reported by check-package.

> -	HOST_PROG_SHUTDOWN=/usr/libexec/petitboot/bb-kexec-reboot
>   
>   # HPA and Busybox tftp are supported. HPA tftp is part of Buildroot's tftpd
>   # package.
> @@ -46,6 +45,17 @@ else
>   PETITBOOT_CONF_OPTS += --without-fdt
>   endif
>   
> +ifeq ($(BR2_INIT_BUSYBOX),y)
> +PETITBOOT_CONF_OPTS += HOST_PROG_SHUTDOWN=/usr/libexec/petitboot/bb-kexec-reboot
> +define PETITBOOT_INITTAB
> +	grep -q kexec-restart $(TARGET_DIR)/etc/inittab || \
> +		printf "\nnull::restart:/usr/sbin/kexec-restart\n" >> $(TARGET_DIR)/etc/inittab

  So, if I understand correctly, previously this wasn't booting _at all_? I 
mean, it would just reboot instead of kexec'ing like it should?


  I've marked the patch as Changes Requested.

  Regards,
  Arnout


> +endef
> +PETITBOOT_TARGET_FINALIZE_HOOKS += PETITBOOT_INITTAB
> +else
> +PETITBOOT_CONF_OPTS += HOST_PROG_SHUTDOWN=/usr/sbin/kexec-restart
> +endif
> +
>   define PETITBOOT_POST_INSTALL
>   	$(INSTALL) -D -m 0755 $(@D)/utils/bb-kexec-reboot \
>   		$(TARGET_DIR)/usr/libexec/petitboot/bb-kexec-reboot
> @@ -53,8 +63,10 @@ define PETITBOOT_POST_INSTALL
>   		$(TARGET_DIR)/etc/petitboot/boot.d/01-create-default-dtb
>   	$(INSTALL) -D -m 0755 $(@D)/utils/hooks/90-sort-dtb \
>   		$(TARGET_DIR)/etc/petitboot/boot.d/90-sort-dtb
> -	$(INSTALL) -m 0755 -D $(PETITBOOT_PKGDIR)/S15pb-discover \
> +	$(INSTALL) -D -m 0755 $(PETITBOOT_PKGDIR)/S15pb-discover \
>   		$(TARGET_DIR)/etc/init.d/S15pb-discover
> +	$(INSTALL) -D -m 0755 $(PETITBOOT_PKGDIR)/kexec-restart \
> +		$(TARGET_DIR)/usr/sbin/kexec-restart
>   	mkdir -p $(TARGET_DIR)/usr/share/udhcpc/default.script.d/
>   	ln -sf /usr/sbin/pb-udhcpc \
>   		$(TARGET_DIR)/usr/share/udhcpc/default.script.d/
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 6/8] package/petitboot: run petitboot UI on consoles

[Arnout Vandecappelle via buildroot]

On 09/10/2023 17:17, Reza Arbab wrote:
> Display the petitboot UI instead of a login prompt, allowing the
> configuration of custom tty(s) as we do for the login prompt.
> 
> petitboot already depends on udev, so let's use it instead of rcS to
> launch pb-console. This has the advantage of easily wildcarding the list
> of ttys ("hvc*") and enables hotplug devices ("ttyUSB0").

  Indeed, hotplug devices is a good reason to use udev!

> 
> Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
> ---
>   package/petitboot/Config.in    |  8 ++++++++
>   package/petitboot/pb-console   | 36 ++++++++++++++++++++++++++++++++++
>   package/petitboot/petitboot.mk | 10 ++++++++++
>   system/Config.in               |  2 +-
>   4 files changed, 55 insertions(+), 1 deletion(-)
>   create mode 100644 package/petitboot/pb-console
> 
> diff --git a/package/petitboot/Config.in b/package/petitboot/Config.in
> index 4981c165bf76..5f1d91e77ecb 100644
> --- a/package/petitboot/Config.in
> +++ b/package/petitboot/Config.in
> @@ -28,3 +28,11 @@ comment "petitboot needs a uClibc or glibc toolchain w/ wchar, dynamic library,
>   		!BR2_TOOLCHAIN_HAS_THREADS || \
>   		!(BR2_TOOLCHAIN_USES_UCLIBC || BR2_TOOLCHAIN_USES_GLIBC) || \
>   		!BR2_PACKAGE_HAS_UDEV
> +
> +config BR2_PACKAGE_PETITBOOT_GETTY_PORT
> +	string "TTY port(s)"
> +	default "console"
> +	depends on BR2_PACKAGE_PETITBOOT
> +	help
> +	  Specify a space-separated list of ports to run the petitboot UI on.

  This line was too long, as reported by check-package.

> +	  Wildcards are allowed. Example: "hvc* ttys0 ttyS*"

[snip]
> diff --git a/system/Config.in b/system/Config.in
> index 24798dc06803..9587dd9ce4db 100644
> --- a/system/Config.in
> +++ b/system/Config.in
> @@ -375,7 +375,7 @@ config BR2_SYSTEM_BIN_SH
>   
>   menuconfig BR2_TARGET_GENERIC_GETTY
>   	bool "Run a getty (login prompt) after boot"
> -	default y
> +	default y if !BR2_PACKAGE_PETITBOOT

  Since the getty would conflict, I wonder if it shouldn't be a `depends on` 
instead of just a default. On the other hand, you could imagine a getty on one 
port and a pb-console on another port, so I guess it's OK.

  Regards,
  Arnout

>   
>   if BR2_TARGET_GENERIC_GETTY
>   config BR2_TARGET_GENERIC_GETTY_PORT
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 7/8] package/petitboot: enable user separation

[Arnout Vandecappelle via buildroot]

On 09/10/2023 17:17, Reza Arbab wrote:
> Run the petitboot UI as an unprivileged user. This requires using the
> agetty package instead of the busybox getty utility, running the initial
> pb-console helper at user login rather than directly.

  That sounds counterproductive though? It means you have to log in before the 
boot menu is displayed? Or perhaps I misunderstand the statement here.

  It's also not clear why it would need agetty instead of busybox getty.

  This doesn't sound like something that should be done by default.


> If sudo is installed, with a sudoers policy allowing petituser to
> perform sudo with no password (or a blank password), the "drop to shell"
> feature of petitboot will automatically become a root shell.

  It seems to me that the logical thing to do would be to drop into an actual 
getty, which asks for a login and password.

> 
> Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
> ---
>   package/petitboot/Config.in      |  1 +
>   package/petitboot/S15pb-discover |  4 +++-
>   package/petitboot/pb-console     |  6 ++++--
>   package/petitboot/petitboot.mk   | 12 ++++++++++++
>   package/petitboot/shell_config   | 24 ++++++++++++++++++++++++
>   package/petitboot/shell_profile  |  2 ++
>   6 files changed, 46 insertions(+), 3 deletions(-)
>   create mode 100644 package/petitboot/shell_config
>   create mode 100644 package/petitboot/shell_profile
> 
> diff --git a/package/petitboot/Config.in b/package/petitboot/Config.in
> index 5f1d91e77ecb..0f965e71e628 100644
> --- a/package/petitboot/Config.in
> +++ b/package/petitboot/Config.in
> @@ -16,6 +16,7 @@ config BR2_PACKAGE_PETITBOOT
>   	select BR2_PACKAGE_KEXEC_LITE if ( BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le )
>   	select BR2_PACKAGE_NVME if ( BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le )
>   	select BR2_PACKAGE_POWERPC_UTILS if ( BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le )
> +	select BR2_PACKAGE_UTIL_LINUX_AGETTY
>   	help
>   	  Petitboot is a small kexec-based bootloader
>   
> diff --git a/package/petitboot/S15pb-discover b/package/petitboot/S15pb-discover
> index 71ab62d99859..a37e33521f1a 100644
> --- a/package/petitboot/S15pb-discover
> +++ b/package/petitboot/S15pb-discover
> @@ -12,7 +12,9 @@ fi
>   
>   start() {
>   	printf 'Starting %s: ' "$DAEMON"
> -	mkdir -p /var/log/petitboot
> +	# shellcheck disable=SC2174 # only apply -m to deepest dir
> +	mkdir -p -m 0775 /var/log/petitboot
> +	chown root:petitgroup /var/log/petitboot

  Why is it owned by root and not petituser?

>   
>   	# shellcheck disable=SC2086 # we need the word splitting
>   	start-stop-daemon -S -q -b -m -p "$PIDFILE" -x "/usr/sbin/$DAEMON" \
> diff --git a/package/petitboot/pb-console b/package/petitboot/pb-console
> index 407ff3b30232..eea40163d02f 100644
> --- a/package/petitboot/pb-console
> +++ b/package/petitboot/pb-console
> @@ -3,14 +3,16 @@
>   DAEMON="pb-console"
>   
>   PB_CONSOLE_PORT=${2:-"console"}
> -PB_CONSOLE_ARGS="--getty --detach -- -n -i 0 $PB_CONSOLE_PORT linux"
> +PB_CONSOLE_ARGS="--getty=/sbin/agetty --detach -- -a petituser -n -i $PB_CONSOLE_PORT linux"
>   
>   # shellcheck source=/dev/null
>   [ -r "/etc/default/petitboot" ] && . "/etc/default/petitboot"
>   
>   start() {
>   	printf 'Starting %s on %s: ' "$DAEMON" "$PB_CONSOLE_PORT"
> -	mkdir -p /var/log/petitboot
> +	# shellcheck disable=SC2174 # only apply -m to deepest dir
> +	mkdir -p -m 0775 /var/log/petitboot
> +	chown root:petitgroup /var/log/petitboot
>   
>   	# shellcheck disable=SC2086 # we need the word splitting
>   	start-stop-daemon -S -q -x "/usr/libexec/petitboot/$DAEMON" \
> diff --git a/package/petitboot/petitboot.mk b/package/petitboot/petitboot.mk
> index ff87f3498734..5b517eb3b1a6 100644
> --- a/package/petitboot/petitboot.mk
> +++ b/package/petitboot/petitboot.mk
> @@ -71,6 +71,10 @@ define PETITBOOT_POST_INSTALL
>   		$(TARGET_DIR)/usr/sbin/kexec-restart
>   	$(INSTALL) -D -m 0755 $(PETITBOOT_PKGDIR)/pb-console \
>   		$(TARGET_DIR)/etc/init.d/pb-console
> +	$(INSTALL) -D -m 0755 $(PETITBOOT_PKGDIR)/shell_config \
> +		$(TARGET_DIR)/home/petituser/.shrc
> +	$(INSTALL) -D -m 0755 $(PETITBOOT_PKGDIR)/shell_profile \
> +		$(TARGET_DIR)/home/petituser/.profile
>   
>   	mkdir -p $(TARGET_DIR)/etc/udev/rules.d
>   	(for port in $(PETITBOOT_GETTY_PORT); do \
> @@ -84,4 +88,12 @@ endef
>   
>   PETITBOOT_POST_INSTALL_TARGET_HOOKS += PETITBOOT_POST_INSTALL
>   
> +define PETITBOOT_USERS
> +	petituser -1 petitgroup -1 * /home/petituser /bin/sh - petitboot user

  Are petitgroup and petituser standard names? If not, we normally use the 
package name as username and group name, i.e.

	petitboot -1 petitboot -1 ...


  Also, does this user really need a home directory and a shell? It really 
should be a system user, no? It's only when it falls into the shell that you 
need an actual shell...

> +endef
> +
> +define PETITBOOT_PERMISSIONS
> +	/var/petitboot d 775 root petitgroup - - - - -

  What is /var/petitboot used for?

> +endef
> +
>   $(eval $(autotools-package))
> diff --git a/package/petitboot/shell_config b/package/petitboot/shell_config
> new file mode 100644
> index 000000000000..b10b95baae6c
> --- /dev/null
> +++ b/package/petitboot/shell_config
> @@ -0,0 +1,24 @@
> +#!/bin/sh
> +
> +try_sudo() {
> +	[ -x "$(command -v sudo)" ] || return
> +	sudo -K
> +	echo | sudo -S /bin/true >/dev/null 2>&1 || return
> +
> +	echo "No password required, running as root."
> +	sudo -i
> +	sudo -K
> +	exit
> +}
> +
> +reset
> +
> +echo "Exiting petitboot. Type 'exit' to return."
> +echo "You may run 'pb-sos' to gather diagnostic data."
> +
> +if [ "$(id -u)" != "0" ]; then
> +	try_sudo
> +	export PS1='$ '
> +else
> +	export PS1='# '
> +fi
> diff --git a/package/petitboot/shell_profile b/package/petitboot/shell_profile
> new file mode 100644
> index 000000000000..1ca5e6364dba
> --- /dev/null
> +++ b/package/petitboot/shell_profile
> @@ -0,0 +1,2 @@
> +export ENV="/home/petituser/.shrc"

  This needs a bit more explanation. Is ENV something that is used by 
pb-console? How is it evaluated?

  Marked as Changes Requested.

  Regards,
  Arnout

> +exec /usr/libexec/petitboot/pb-console
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 8/8] package/petitboot: prefer UTF-8 support

[Arnout Vandecappelle via buildroot]

On 09/10/2023 17:17, Reza Arbab wrote:
> The petitboot UI looks much nicer in a Unicode locale:
> 
> * Items in the language selection submenu use multibyte Unicode
>    characters. In other locales, they say "Unable to display text in this
>    locale".
> 
> * The combination of TERM=linux with a UTF-8 locale is required to
>    trigger a special-case workaround in ncurses code[1]. Without
>    this, line-drawing characters in the menu look like q's.
> 
> Add a reminder that a UTF-8 locale should be generated for things to
> look right. Assume C.UTF-8 by default, allowing $LANG to be overridden
> (by /etc/default/petitboot or otherwise) if something else is desired.

  So, this improves things, but they're not really necessary. I think it's 
better then to just put in the help text that it looks better with UTF-8 locale 
enabled.

> 
> [1] https://invisible-island.net/ncurses/ncurses.faq.html#no_line_drawing
> 
> Signed-off-by: Reza Arbab <arbab@linux.ibm.com>
> ---
>   package/petitboot/Config.in      | 8 ++++++--
>   package/petitboot/S15pb-discover | 1 +
>   package/petitboot/pb-console     | 1 +
>   package/petitboot/shell_profile  | 3 +++
>   4 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/package/petitboot/Config.in b/package/petitboot/Config.in
> index 0f965e71e628..a449ed87872c 100644
> --- a/package/petitboot/Config.in
> +++ b/package/petitboot/Config.in
> @@ -4,6 +4,7 @@ config BR2_PACKAGE_PETITBOOT
>   	depends on BR2_PACKAGE_KEXEC_ARCH_SUPPORTS
>   	depends on BR2_USE_MMU # lvm2
>   	depends on BR2_USE_WCHAR # elfutils
> +	depends on BR2_ENABLE_LOCALE
>   	depends on !BR2_STATIC_LIBS # elfutils, lvm2
>   	depends on BR2_TOOLCHAIN_HAS_THREADS # elfutils, lvm2
>   	depends on BR2_TOOLCHAIN_USES_UCLIBC || BR2_TOOLCHAIN_USES_GLIBC # elfutils
> @@ -11,6 +12,7 @@ config BR2_PACKAGE_PETITBOOT
>   	select BR2_PACKAGE_ELFUTILS
>   	select BR2_PACKAGE_LVM2 # devmapper
>   	select BR2_PACKAGE_NCURSES
> +	select BR2_PACKAGE_NCURSES_WCHAR
>   	# run-time dependencies
>   	select BR2_PACKAGE_KEXEC if !BR2_PACKAGE_KEXEC_LITE
>   	select BR2_PACKAGE_KEXEC_LITE if ( BR2_powerpc || BR2_powerpc64 || BR2_powerpc64le )
> @@ -20,12 +22,14 @@ config BR2_PACKAGE_PETITBOOT
>   	help
>   	  Petitboot is a small kexec-based bootloader
>   
> +	  NOTE: petitboot needs a working UTF-8 locale (BR2_GENERATE_LOCALE)
> +
>   	  http://www.kernel.org/pub/linux/kernel/people/geoff/petitboot/petitboot.html
>   
> -comment "petitboot needs a uClibc or glibc toolchain w/ wchar, dynamic library, threads, udev /dev management"
> +comment "petitboot needs a uClibc or glibc toolchain w/ wchar, locale, dynamic library, threads, udev /dev management"
>   	depends on BR2_PACKAGE_KEXEC_ARCH_SUPPORTS
>   	depends on BR2_USE_MMU
> -	depends on !BR2_USE_WCHAR || BR2_STATIC_LIBS || \
> +	depends on !BR2_USE_WCHAR || !BR2_ENABLE_LOCALE || BR2_STATIC_LIBS || \
>   		!BR2_TOOLCHAIN_HAS_THREADS || \
>   		!(BR2_TOOLCHAIN_USES_UCLIBC || BR2_TOOLCHAIN_USES_GLIBC) || \
>   		!BR2_PACKAGE_HAS_UDEV
> diff --git a/package/petitboot/S15pb-discover b/package/petitboot/S15pb-discover
> index a37e33521f1a..15b428ce0e42 100644
> --- a/package/petitboot/S15pb-discover
> +++ b/package/petitboot/S15pb-discover
> @@ -15,6 +15,7 @@ start() {
>   	# shellcheck disable=SC2174 # only apply -m to deepest dir
>   	mkdir -p -m 0775 /var/log/petitboot
>   	chown root:petitgroup /var/log/petitboot
> +	export LANG="${LANG:-C.UTF-8}"

  I think it's better to rely on the default locale being set to C.UTF-8. Hm, it 
seems we don't actually set a default locale though... Perhaps we should add a 
system menu entry to do exactly that. Though I'm not sure if it's even possible 
with uClibc (glibc has /etc/locale.conf).

  Regards,
  Arnout

>   
>   	# shellcheck disable=SC2086 # we need the word splitting
>   	start-stop-daemon -S -q -b -m -p "$PIDFILE" -x "/usr/sbin/$DAEMON" \
> diff --git a/package/petitboot/pb-console b/package/petitboot/pb-console
> index eea40163d02f..55e5462f0457 100644
> --- a/package/petitboot/pb-console
> +++ b/package/petitboot/pb-console
> @@ -13,6 +13,7 @@ start() {
>   	# shellcheck disable=SC2174 # only apply -m to deepest dir
>   	mkdir -p -m 0775 /var/log/petitboot
>   	chown root:petitgroup /var/log/petitboot
> +	export LANG="${LANG:-C.UTF-8}"
>   
>   	# shellcheck disable=SC2086 # we need the word splitting
>   	start-stop-daemon -S -q -x "/usr/libexec/petitboot/$DAEMON" \
> diff --git a/package/petitboot/shell_profile b/package/petitboot/shell_profile
> index 1ca5e6364dba..6bbe49e6d113 100644
> --- a/package/petitboot/shell_profile
> +++ b/package/petitboot/shell_profile
> @@ -1,2 +1,5 @@
> +[ -r "/etc/default/petitboot" ] && . "/etc/default/petitboot"
>   export ENV="/home/petituser/.shrc"
> +export LANG="${LANG:-C.UTF-8}"
> +
>   exec /usr/libexec/petitboot/pb-console
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 0/8] package/petitboot: misc fixes/enhancement

[Arnout Vandecappelle via buildroot]

On 09/10/2023 17:17, Reza Arbab wrote:
> The br2-external tree used to build OpenPOWER firmware has long carried
> petitboot as a custom package[1]. Now that petitboot has been added to
> buildroot proper, it would be nice to leverage the base package instead.
> 
> To make that transition easier, here is a set of patches which port over
> some of the enhancements made to that external package.
> 
> [1] https://github.com/open-power/op-build/tree/master/openpower/package/petitboot
> ---
> v3:
> * Add a number of small fixes.
> 
> * Add user separation, so the UI runs as non-root.
> 
> * Remove udev rules that enabled some additional types of boot devices.
>    These should more appropriately live outside of buildroot.
> 
> * Remove a sysctl.d file to silence kernel output. I think there's a
>    bug upstream; see https://github.com/open-power/petitboot/pull/103
> 
> v2:
> * Use Laurent's suggested additions to "run pb-console at boot" patch.
> 
> Reza Arbab (8):
>    package/petitboot: fix menu comment
>    package/petitboot: fix pb-discover pidfile creation
>    package/petitboot: use default logfile dir
>    package/petitboot: prefer kexec-lite on powerpc
>    package/petitboot: fix shutdown
>    package/petitboot: run petitboot UI on consoles
>    package/petitboot: enable user separation
>    package/petitboot: prefer UTF-8 support

  Series applied to master, thanks, except for the ones where I wrote that 
they're Changes Requested.

  Regards,
  Arnout

> 
>   package/petitboot/Config.in      | 26 +++++++++++++++------
>   package/petitboot/S15pb-discover | 13 +++++++----
>   package/petitboot/kexec-restart  |  8 +++++++
>   package/petitboot/pb-console     | 39 ++++++++++++++++++++++++++++++++
>   package/petitboot/petitboot.mk   | 38 +++++++++++++++++++++++++++++--
>   package/petitboot/shell_config   | 24 ++++++++++++++++++++
>   package/petitboot/shell_profile  |  5 ++++
>   system/Config.in                 |  2 +-
>   8 files changed, 140 insertions(+), 15 deletions(-)
>   create mode 100755 package/petitboot/kexec-restart
>   create mode 100644 package/petitboot/pb-console
>   create mode 100644 package/petitboot/shell_config
>   create mode 100644 package/petitboot/shell_profile
> 
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 5/8] package/petitboot: fix shutdown

[Reza Arbab]

On Sun, Nov 05, 2023 at 06:57:47PM +0100, Arnout Vandecappelle wrote:
>On 09/10/2023 17:17, Reza Arbab wrote:
>>There are a few ways to set HOST_PROG_SHUTDOWN that enable us to
>>successfully kexec the new kernel chosen from the petitboot menu.
>>
>>1. HOST_PROG_SHUTDOWN=/usr/libexec/petitboot/bb-kexec-reboot
>>     `- bb-kexec-reboot does 'exec kill -QUIT 1'
>>         |- init sends SIGTERM to pb-console
>>         |   `- pb-console does 'trap 'reset; echo "SIGTERM received, booting..."; sleep 2' SIGTERM'
>>         `- init runs 'null::restart:/usr/sbin/kexec-restart'
>>             `- kexec-restart does '/usr/sbin/kexec -f -e'
[...]
> It seems to me that (1) would be the proper thing to do for sysvinit, 
>openrc and systemd based init as well (not that anyone would be crazy 
>enough to put systemd in a petitboot image, I hope...). So, if I 
>understand correctly, what needs to be fixed is when init is not an 
>actual init, but something trivial like a single shell script. So at 
>the very least, the condition should change I think.
>
> But then, BR2_INIT_NONE is not a reliable indication that PID 1 is 
>not a real init. So I think it would be better to instead decide at 
>runtime which approach to take. That could be done for instance by 
>looking at /proc/1/exe and comparing it with /sbin/init - if they are 
>the same, we have a real init; if not, we probably have a shell or 
>some such.

Good idea! I'll set HOST_PROG_SHUTDOWN=kexec-restart unconditionally, 
and make that script smart enough to accomodate all the init options.

> Also, why don't we simply always do (2)? The only difference is the 
>message on pb-console, and normally it would anyway be pb-console that 
>triggers the boot so it would already have shown something, I expect?

Well, just in general it's better to terminate things properly before 
kexec. But also, pb-console doesn't really trigger the boot per se--it's 
pb-discover doing that in the background, so the SIGTERM pb-console 
receives is considered the final notice that we're going to boot.

What's nicer than the "booting..." notification is actually the 
preceding terminal reset, so you exit the ncurses visual mode,  
scrolling is raw rather than being confined to the window ncurses set 
up, etc.

>>@@ -46,6 +45,17 @@ else
>>  PETITBOOT_CONF_OPTS += --without-fdt
>>  endif
>>+ifeq ($(BR2_INIT_BUSYBOX),y)
>>+PETITBOOT_CONF_OPTS += HOST_PROG_SHUTDOWN=/usr/libexec/petitboot/bb-kexec-reboot
>>+define PETITBOOT_INITTAB
>>+	grep -q kexec-restart $(TARGET_DIR)/etc/inittab || \
>>+		printf "\nnull::restart:/usr/sbin/kexec-restart\n" >> $(TARGET_DIR)/etc/inittab
>
> So, if I understand correctly, previously this wasn't booting _at 
>all_? I mean, it would just reboot instead of kexec'ing like it 
>should?

Yes, at the moment we are doing `kill -QUIT 1` but without the above 
"restart" entry to do the kexec.

-- 
Reza Arbab
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 7/8] package/petitboot: enable user separation

[Reza Arbab]

On Sun, Nov 05, 2023 at 07:26:16PM +0100, Arnout Vandecappelle wrote:
>On 09/10/2023 17:17, Reza Arbab wrote:
>>Run the petitboot UI as an unprivileged user. This requires using the
>>agetty package instead of the busybox getty utility, running the initial
>>pb-console helper at user login rather than directly.
>
> That sounds counterproductive though? It means you have to log in 
>before the boot menu is displayed? Or perhaps I misunderstand the 
>statement here.
>
> It's also not clear why it would need agetty instead of busybox getty.

Sorry, I didn't explain it very well. The chain goes like this:

1. /etc/init.d/pb-console start console
2. /usr/libexec/petitboot/pb-console --getty --detach -- -n -i 0 console linux
3. /sbin/getty -l/usr/libexec/petitboot/pb-console -n -i 0 console linux
4. /usr/libexec/petitboot/pb-console
5. /usr/sbin/petiboot-nc
  
After the change:

1. /etc/init.d/pb-console start console
2. /usr/libexec/petitboot/pb-console --getty=/sbin/agetty --detach -- -a petituser -n -i console linux
3. /sbin/agetty -a petituser -n -i console linux
4. /home/petituser/.profile
5. /usr/libexec/petitboot/pb-console
6. /usr/sbin/petiboot-nc

Because we're using the -a (autologin) feature of agetty, everything 
from (4) down is running as petituser. If you select "drop to shell" 
from the menu, you then also do

7. /home/petituser/.shrc (the $ENV set in .profile)
8. /bin/sh

>>If sudo is installed, with a sudoers policy allowing petituser to
>>perform sudo with no password (or a blank password), the "drop to shell"
>>feature of petitboot will automatically become a root shell.
>
> It seems to me that the logical thing to do would be to drop into an 
>actual getty, which asks for a login and password.

At this point we're already running getty and (auto)logged in as
petituser. The sudo integration allos system-specific flexibility in 
if/how the the shell may be elevated. As petituser, the shell has 
permission to collect diagnostics/logs by running pb-sos.

>>--- a/package/petitboot/S15pb-discover
>>+++ b/package/petitboot/S15pb-discover
>>@@ -12,7 +12,9 @@ fi
>>  start() {
>>  	printf 'Starting %s: ' "$DAEMON"
>>-	mkdir -p /var/log/petitboot
>>+	# shellcheck disable=SC2174 # only apply -m to deepest dir
>>+	mkdir -p -m 0775 /var/log/petitboot
>>+	chown root:petitgroup /var/log/petitboot
>
> Why is it owned by root and not petituser?

I don't see any reason why it couldn't be owned by petituser.

>>@@ -84,4 +88,12 @@ endef
>>  PETITBOOT_POST_INSTALL_TARGET_HOOKS += PETITBOOT_POST_INSTALL
>>+define PETITBOOT_USERS
>>+	petituser -1 petitgroup -1 * /home/petituser /bin/sh - petitboot user
>
> Are petitgroup and petituser standard names? If not, we normally use 
>the package name as username and group name, i.e.
>
>	petitboot -1 petitboot -1 ...

I think we could change petituser to petitboot, but there is a hardcoded 
reference to petitgroup in the code:

   discover/discover-server.c:     group = getgrnam("petitgroup");

>>+define PETITBOOT_PERMISSIONS
>>+	/var/petitboot d 775 root petitgroup - - - - -
>
> What is /var/petitboot used for?

That is where pb-discover mounts devices, looking for boot sources:

   /var/petitboot/mnt/dev/nvme0n1p1/grub2/grub.cfg
   /var/petitboot/mnt/dev/nvme0n1p1/vmlinux

-- 
Reza Arbab
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 8/8] package/petitboot: prefer UTF-8 support

[Reza Arbab]

On Sun, Nov 05, 2023 at 07:30:28PM +0100, Arnout Vandecappelle wrote:
>On 09/10/2023 17:17, Reza Arbab wrote:
>>Add a reminder that a UTF-8 locale should be generated for things to
>>look right. Assume C.UTF-8 by default, allowing $LANG to be overridden
>>(by /etc/default/petitboot or otherwise) if something else is desired.
>
> So, this improves things, but they're not really necessary. I think 
>it's better then to just put in the help text that it looks better 
>with UTF-8 locale enabled.

Will change.

>>--- a/package/petitboot/S15pb-discover
>>+++ b/package/petitboot/S15pb-discover
>>@@ -15,6 +15,7 @@ start() {
>>  	# shellcheck disable=SC2174 # only apply -m to deepest dir
>>  	mkdir -p -m 0775 /var/log/petitboot
>>  	chown root:petitgroup /var/log/petitboot
>>+	export LANG="${LANG:-C.UTF-8}"
>
> I think it's better to rely on the default locale being set to 
>C.UTF-8. Hm, it seems we don't actually set a default locale though... 
>Perhaps we should add a system menu entry to do exactly that. Though 
>I'm not sure if it's even possible with uClibc (glibc has 
>/etc/locale.conf).

No problem, I'll take this out. I think we can defer to the user to set 
a UTF-8 system locale. A config option for default locale would be nice 
indeed.

-- 
Reza Arbab
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 0/8] package/petitboot: misc fixes/enhancement

[Reza Arbab]

On Sun, Nov 05, 2023 at 07:31:04PM +0100, Arnout Vandecappelle wrote:
> Series applied to master, thanks, except for the ones where I wrote 
>that they're Changes Requested.

Appreciate your help in getting all this straightened out! I'll send 
another set to address what's remaining.

-- 
Reza Arbab
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 1/8] package/petitboot: fix menu comment

[Peter Korsgaard]

>>>>> "Reza" == Reza Arbab <arbab@linux.ibm.com> writes:

 > The comment should appear if threads aren't enabled, not when they are.
 > Signed-off-by: Reza Arbab <arbab@linux.ibm.com>

Committed to 2023.08.x, thanks.

-- 
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 3/8] package/petitboot: use default logfile dir

[Peter Korsgaard]

>>>>> "Reza" == Reza Arbab <arbab@linux.ibm.com> writes:

 > All the petitboot components assume /var/log/petitboot by default;
 > pb-console can also put multiple logs there and pb-sos collects that
 > directory when creating a diagnostic tarball.

 > Defer to this default when launching pb-discover. If someone wants to
 > override, let's call the file /etc/default/petitboot which makes more
 > sense to be shared by all the components.

 > Signed-off-by: Reza Arbab <arbab@linux.ibm.com>

Committed to 2023.08.x, thanks.

-- 
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 7/8] package/petitboot: enable user separation

[Arnout Vandecappelle via buildroot]

On 09/11/2023 17:16, Reza Arbab wrote:
> On Sun, Nov 05, 2023 at 07:26:16PM +0100, Arnout Vandecappelle wrote:
>> On 09/10/2023 17:17, Reza Arbab wrote:
>>> Run the petitboot UI as an unprivileged user. This requires using the
>>> agetty package instead of the busybox getty utility, running the initial
>>> pb-console helper at user login rather than directly.
>>
>> That sounds counterproductive though? It means you have to log in before the 
>> boot menu is displayed? Or perhaps I misunderstand the statement here.
>>
>> It's also not clear why it would need agetty instead of busybox getty.
> 
> Sorry, I didn't explain it very well. The chain goes like this:
> 
> 1. /etc/init.d/pb-console start console > 2. /usr/libexec/petitboot/pb-console --getty --detach -- -n -i 0 console linux
> 3. /sbin/getty -l/usr/libexec/petitboot/pb-console -n -i 0 console linux
> 4. /usr/libexec/petitboot/pb-console
> 5. /usr/sbin/petiboot-nc
> 
> After the change:
> 
> 1. /etc/init.d/pb-console start console
> 2. /usr/libexec/petitboot/pb-console --getty=/sbin/agetty --detach -- -a 
> petituser -n -i console linux
> 3. /sbin/agetty -a petituser -n -i console linux

  How did the -l option disappear here? Is that due to --getty= instead of --getty?

  Anyway, the only reason to use agetty is for the -a option. Can't we achieve 
the same by using su? I.e. create a wrapper script around pb-console that does 
something like

exec su -s /usr/libexec/petitboot/pb-console petituser

> 4. /home/petituser/.profile

  Something I didn't touch in my earlier review: I don't think .profile is the 
right place to set this up. Either petituser is a normal user - in that case 
logging in through some other means (e.g. ssh, sudo) should give a normal shell. 
I assume you can tell pb-console which "shell" to use in the "drop to shell" case.


> 5. /usr/libexec/petitboot/pb-console
> 6. /usr/sbin/petiboot-nc
> 
> Because we're using the -a (autologin) feature of agetty, everything from (4) 
> down is running as petituser. If you select "drop to shell" from the menu, you 
> then also do
> 
> 7. /home/petituser/.shrc (the $ENV set in .profile)
> 8. /bin/sh
> 
>>> If sudo is installed, with a sudoers policy allowing petituser to
>>> perform sudo with no password (or a blank password), the "drop to shell"
>>> feature of petitboot will automatically become a root shell.
>>
>> It seems to me that the logical thing to do would be to drop into an actual 
>> getty, which asks for a login and password.
> 
> At this point we're already running getty and (auto)logged in as
> petituser. The sudo integration allos system-specific flexibility in if/how the 
> the shell may be elevated. As petituser, the shell has permission to collect 
> diagnostics/logs by running pb-sos.

  I'm anyway wrong - getty or login are not setuid, they need to be executed as 
root, so petituser can't start them.

>>> --- a/package/petitboot/S15pb-discover
>>> +++ b/package/petitboot/S15pb-discover
>>> @@ -12,7 +12,9 @@ fi
>>>  start() {
>>>      printf 'Starting %s: ' "$DAEMON"
>>> -    mkdir -p /var/log/petitboot
>>> +    # shellcheck disable=SC2174 # only apply -m to deepest dir
>>> +    mkdir -p -m 0775 /var/log/petitboot
>>> +    chown root:petitgroup /var/log/petitboot
>>
>> Why is it owned by root and not petituser?
> 
> I don't see any reason why it couldn't be owned by petituser.
> 
>>> @@ -84,4 +88,12 @@ endef
>>>  PETITBOOT_POST_INSTALL_TARGET_HOOKS += PETITBOOT_POST_INSTALL
>>> +define PETITBOOT_USERS
>>> +    petituser -1 petitgroup -1 * /home/petituser /bin/sh - petitboot user
>>
>> Are petitgroup and petituser standard names? If not, we normally use the 
>> package name as username and group name, i.e.
>>
>>     petitboot -1 petitboot -1 ...
> 
> I think we could change petituser to petitboot, but there is a hardcoded 
> reference to petitgroup in the code:
> 
>    discover/discover-server.c:     group = getgrnam("petitgroup");

  Sounds like petitgroup is at least a standard name, and in that case petituser 
fits nicely (and I assume it is already used _somewhere_ at least). So OK to 
keep petituser/petitgroup. Please say something about this in the commit message 
(we want the history to show why we chose "petituser" rather than the more 
typical "petitboot" for the username).


>>> +define PETITBOOT_PERMISSIONS
>>> +    /var/petitboot d 775 root petitgroup - - - - -

  Given the information below, why is it in petitgroup? If pb-discover is just 
going to create directories there on which it can mount things, it's not very 
useful for pb-console to have write access to it, right? Unless pb-console needs 
to delete those directories or rename them?

>>
>> What is /var/petitboot used for?
> 
> That is where pb-discover mounts devices, looking for boot sources:
> 
>    /var/petitboot/mnt/dev/nvme0n1p1/grub2/grub.cfg
>    /var/petitboot/mnt/dev/nvme0n1p1/vmlinux

  Could you mention this in the commit message as well?

  The commit message is going to grow pretty large, but that's fine.

  Regards,
  Arnout

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH v3 7/8] package/petitboot: enable user separation

[Reza Arbab]

On Fri, Nov 10, 2023 at 10:01:25AM +0100, Arnout Vandecappelle wrote:
>On 09/11/2023 17:16, Reza Arbab wrote:
>>On Sun, Nov 05, 2023 at 07:26:16PM +0100, Arnout Vandecappelle wrote:
>>>It's also not clear why it would need agetty instead of busybox getty.
>>
>>Sorry, I didn't explain it very well. The chain goes like this:
>>
>>1. /etc/init.d/pb-console start console
>>2. /usr/libexec/petitboot/pb-console --getty --detach -- -n -i 0 console linux
>>3. /sbin/getty -l/usr/libexec/petitboot/pb-console -n -i 0 console linux
>>4. /usr/libexec/petitboot/pb-console
>>5. /usr/sbin/petiboot-nc
>>
>>After the change:
>>
>>1. /etc/init.d/pb-console start console
>>2. /usr/libexec/petitboot/pb-console --getty=/sbin/agetty --detach -- -a petituser -n -i console linux
>>3. /sbin/agetty -a petituser -n -i console linux
>
> How did the -l option disappear here? Is that due to --getty= instead of --getty?

Not because of --getty= exactly, but you've got the right idea. The 
script does not add -l if it sees -a.

> Anyway, the only reason to use agetty is for the -a option. Can't we 
>achieve the same by using su? I.e. create a wrapper script around 
>pb-console that does something like
>
>exec su -s /usr/libexec/petitboot/pb-console petituser

It's tricky. "pb-console --getty" runs getty which runs pb-console 
again, so at least that first invocation has be done as root. Instead of 
a wrapper, it can be made to work by adding this at the right place in 
pb-console:

     # If we're root, rerun as petituser
     if [ "$(id -u)" = "0" ]; then
	exec su -s "$0" petituser $@
     fi

I'm not sure if patching pb-console is preferable to depending on 
"agetty -a" and .profile?

>>4. /home/petituser/.profile
>
> Something I didn't touch in my earlier review: I don't think .profile 
>is the right place to set this up. Either petituser is a normal user - 
>in that case logging in through some other means (e.g. ssh, sudo) 
>should give a normal shell.

In .profile, I can do

     if [ "$PPID" = "1" ]; then
	exec /usr/libexec/petitboot/pb-console
     fi

so it only runs when petituser is logging in by getty/agetty and not 
through ssh or sudo.

>>>>+define PETITBOOT_PERMISSIONS
>>>>+    /var/petitboot d 775 root petitgroup - - - - -
>
> Given the information below, why is it in petitgroup? If pb-discover 
>is just going to create directories there on which it can mount 
>things, it's not very useful for pb-console to have write access to 
>it, right? Unless pb-console needs to delete those directories or 
>rename them?

You're right, I think /var/petitboot is really only used by pb-discover 
(root), so we can probably drop this.

I'll revise and split all this stuff into more than one patch so it's 
easier to understand when looking through the git log.

-- 
Reza Arbab
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot