| Message ID | 20240916151206.947484-13-adam.duskett@amarulasolutions.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | selinux-packages: bump to 3.7 | expand |
Hello Adam, On Mon, 16 Sep 2024 17:12:05 +0200 Adam Duskett <adam.duskett@amarulasolutions.com> wrote: > Tested with qemu_x86_64_defconfig. start, stop, restart, reload, and rotate > all work with busybox ash shell. > > Signed-off-by: Adam Duskett <adam.duskett@amarulasolutions.com> > --- > package/audit/S02auditd | 44 ++++++++++++++++++++++------------------- > 1 file changed, 24 insertions(+), 20 deletions(-) Thanks for this rework. As part of this, could you rework S02auditd to follow the canonical example of package/busybox/S01syslogd ? There's been an effort from Fiona to start aligning our init scripts, and this effort is really good, so I'd rather see rework of other init scripts go in the direction that Fiona has initiated. I've added Fiona in the loop so that hopefully she can help if your specific init script raises some specific questions. Thanks a lot! Thomas
Hi Adam, Thomas! Am 26.10.24 um 18:09 schrieb Thomas Petazzoni: > Hello Adam, > > On Mon, 16 Sep 2024 17:12:05 +0200 > Adam Duskett <adam.duskett@amarulasolutions.com> wrote: > >> Tested with qemu_x86_64_defconfig. start, stop, restart, reload, and rotate >> all work with busybox ash shell. >> >> Signed-off-by: Adam Duskett <adam.duskett@amarulasolutions.com> >> --- >> package/audit/S02auditd | 44 ++++++++++++++++++++++------------------- >> 1 file changed, 24 insertions(+), 20 deletions(-) > > Thanks for this rework. As part of this, could you rework S02auditd to > follow the canonical example of package/busybox/S01syslogd ? There's > been an effort from Fiona to start aligning our init scripts, and this > effort is really good, so I'd rather see rework of other init scripts > go in the direction that Fiona has initiated. > > I've added Fiona in the loop so that hopefully she can help if your > specific init script raises some specific questions. I took a look at that patch [1], general issues I see: * Critical (though pre-existing, just restructured in the patch): reload and rotate use numeric signals. 1 seems to be pretty consistent (HUP), but according to signal(7) 10 can actually have different meanings across different architectures. Use the symbolic names instead, both to make sure you get the right one and for readability. Or maybe it'd make sense to use auditctl? * There's a "/dev/nulll" in the reload function. For the canonical style, things to consider: * On stop, check that the daemon process is actually gone before returning. Otherwise restart might fail because the new instance is started before the old one has actually stopped (I've seen that with a lot of services). * Action functions should return a success (or failure) code, usually the return code of the relevant start-stop-daemon action, and the last one should be the return code of the init script. That way it's possible to check success when calling the init script from automated tools (e.g. other scripts). * Use long form options where possible for clarity. While you're tidying up the script, it might be good to break some of the very long lines, too. I guess I should put some of the above into the docs some time. :-) Best regards, Fiona [1] https://patchwork.ozlabs.org/project/buildroot/patch/20240916151206.947484-13-adam.duskett@amarulasolutions.com/
diff --git a/package/audit/S02auditd b/package/audit/S02auditd index dd3dc22d6d..b23c49a125 100644 --- a/package/audit/S02auditd +++ b/package/audit/S02auditd @@ -8,27 +8,22 @@ # will be sent to syslog. # -NAME=auditd -DAEMON=/usr/sbin/${NAME} -CONFIG=/etc/audit/auditd.conf -PIDFILE=/var/run/${NAME}.pid +DAEMON="auditd" +PIDFILE="/var/run/${DAEMON}.pid" start(){ - printf "Starting ${NAME}: " + printf "Starting %s: " "${DAEMON}" # Create dir to store log files in if one doesn't exist. Create # the directory with SELinux permissions if possible - command -v selabel_lookup >/dev/null 2>&1 - if [ $? = 0 ]; then - mkdir -p /var/log/audit -Z `selabel_lookup -b file -k /var/log/audit | cut -d ' ' -f 3` + if command -v selabel_lookup >/dev/null 2>&1; then + mkdir -p /var/log/audit -Z "$(selabel_lookup -b file -k /var/log/audit | cut -d ' ' -f 3)" else mkdir -p /var/log/audit fi # Run audit daemon executable - start-stop-daemon -S -q -p ${PIDFILE} --exec ${DAEMON} - - if [ $? = 0 ]; then + if start-stop-daemon -S -q -p "${PIDFILE}" --exec /usr/sbin/"${DAEMON}"; then # Load the default rules test -f /etc/audit/rules.d/audit.rules && /usr/sbin/auditctl -R /etc/audit/rules.d/audit.rules >/dev/null echo "OK" @@ -38,22 +33,31 @@ start(){ } stop(){ - printf "Stopping ${NAME}: " + printf "Stopping %s: " "${DAEMON}" - start-stop-daemon -K -q -p ${PIDFILE} - [ $? = 0 ] && echo "OK" || echo "FAIL" + if start-stop-daemon -K -q -p "${PIDFILE}"; then + echo "OK" + else + echo "FAIL" + fi } reload(){ - printf "Reloading ${NAME} configuration: " - start-stop-daemon --stop -s 1 -p ${PIDFILE} 1>/dev/null - [ $? = 0 ] && echo "OK" || echo "FAIL" + printf "Reloading %s configuration: " "${DAEMON}" + if start-stop-daemon --stop -s 1 -p "${PIDFILE}" 1>/dev/nulll; then + echo "OK" + else + echo "FAIL" + fi } rotate(){ - printf "Rotating ${NAME} logs: " - start-stop-daemon --stop -s 10 -p ${PIDFILE} 1>/dev/null - [ $? = 0 ] && echo "OK" || echo "FAIL" + printf "Rotating %s logs: " "${DAEMON}" + if start-stop-daemon --stop -s 10 -p "${PIDFILE}" 1>/dev/null; then + echo "OK" + else + echo "FAIL" + fi } case "$1" in
Tested with qemu_x86_64_defconfig. start, stop, restart, reload, and rotate all work with busybox ash shell. Signed-off-by: Adam Duskett <adam.duskett@amarulasolutions.com> --- package/audit/S02auditd | 44 ++++++++++++++++++++++------------------- 1 file changed, 24 insertions(+), 20 deletions(-)