Bug 267464 - periodic 310.accounting enables accounting even if it is disabled in rc.conf
Summary: periodic 310.accounting enables accounting even if it is disabled in rc.conf
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: conf (show other bugs)
Version: 13.1-RELEASE
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-bugs (Nobody)
URL:
Keywords: needs-qa
Depends on:
Blocks:
 
Reported: 2022-10-31 13:55 UTC by iasen.kostov
Modified: 2023-02-28 17:00 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description iasen.kostov 2022-10-31 13:55:32 UTC
Even when accounting_enable="NO" in /etc/rc.conf the periodic /etc/periodic/daily/310.accounting enables it every day at 3:00. The service /etc/rc.d/accounting checks for "if checkyesno accounting_enable" but it is useless because in 310.accouting is used /etc/rc.d/accounting onerotate_log and "one" forces accounting_enable="YES"
What is the purpose of forcing the accounting with the "one" ? And is there a way of fixing that other than pulling in /etc/defaults/rc.conf into 310.accounting and checking for accounting_enable="YES" ?

Currently a temporary workaround should be setting daily_accounting_enable="NO" in /etc/periodic.conf but that bug should be fixed because that behavior is unexpected (and i don't think it is documented as it is wrong) and servers should not behave unexpectedly.
Comment 1 iasen.kostov 2022-11-01 11:58:11 UTC
If /var/account is empty it doesn't happen. Which means once accounting is enabled you can't stop it unless you go and clear /var/account which of course is not documented anywhere as it is a unexpected behavior.
Comment 2 John Grafton 2022-11-17 18:03:34 UTC
I was able to replicate this bug on my system.  I think a simple fix is to check if the accounting system process is running in the periodic script. If not, exit the periodic script indicating the service is not running instead of attempting to rotate the logs.  onerotate_log turns accounting back on regardless of what accounting_enable is set to.

I tested this patch on my local system and the periodic log rotate exits and doesn't restart the accounting service if the [accounting] process is not running.

diff --git a/usr.sbin/periodic/etc/daily/310.accounting b/usr.sbin/periodic/etc/daily/310.accounting
index b0dd786447eb..6a47690a0a6a 100755
--- a/usr.sbin/periodic/etc/daily/310.accounting
+++ b/usr.sbin/periodic/etc/daily/310.accounting
@@ -18,6 +18,10 @@ case "$daily_accounting_enable" in
            echo '$daily_accounting_enable is set but /var/account/acct' \
                "doesn't exist"
            rc=2
+       elif ! pgrep -qS accounting
+       then
+           echo 'accounting service is not active'
+           rc=2
        elif [ -z "$daily_accounting_save" ]
        then
            echo '$daily_accounting_enable is set but ' \
Comment 3 John Grafton 2022-11-18 01:39:41 UTC
Ignore the last patch, it's too hacky.  Grepping for the system process is problematic due to the possibility of a process other than accounting returning true for the grep.

This patch uses the kern.acct_configured sysctl to determine if accounting is active instead.

diff --git a/usr.sbin/periodic/etc/daily/310.accounting b/usr.sbin/periodic/etc/daily/310.accounting
index b0dd786447eb..8eeed16f0516 100755
--- a/usr.sbin/periodic/etc/daily/310.accounting
+++ b/usr.sbin/periodic/etc/daily/310.accounting
@@ -18,6 +18,11 @@ case "$daily_accounting_enable" in
            echo '$daily_accounting_enable is set but /var/account/acct' \
                "doesn't exist"
            rc=2
+       elif [ $(sysctl -n kern.acct_configured) -eq 0 ]
+       then
+           echo '$daily_accounting_enable is set but' \
+           'process accounting is not active'
+           rc=2
        elif [ -z "$daily_accounting_save" ]
        then
            echo '$daily_accounting_enable is set but ' \
Comment 4 iasen.kostov 2022-11-18 13:32:52 UTC
(In reply to John Grafton from comment #3)
Checking if the accounting is configured/running is a great idea! That way even if it is enabled by hand it will still rotate the logs.
Comment 5 John Grafton 2022-11-18 16:15:16 UTC
Phabriactor review: https://reviews.freebsd.org/D37434
Comment 6 commit-hook freebsd_committer freebsd_triage 2023-02-28 17:00:43 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=9ab40bc40d4a07e1a9f3622a7779312ca2466b55

commit 9ab40bc40d4a07e1a9f3622a7779312ca2466b55
Author:     John Grafton <john.grafton@runbox.com>
AuthorDate: 2023-02-28 16:49:40 +0000
Commit:     Warner Losh <imp@FreeBSD.org>
CommitDate: 2023-02-28 16:59:34 +0000

    310.accounting: Verify process accounting is active before log rotation.

    This corrects a bug in which the daily periodic script '310.accounting'
    attempts to rotate logs via /etc/rc.d/accounting by calling
    onerotate_logs function. The rotate logs function turns accounting back
    on regardless of what acccounting_enable is set to in /etc/rc.conf. This
    is due to checkyesno always returning YES since rotate logs is called
    with the 'one' prefix.

    In effect, accounting will always be turned back on once a day even if
    it is disabled and stopped by hand. The fix was simple, just check if
    accounting is before rotating logs and if it is, don't attempt the
    rotate.

    PR: 267464
    Reviewed by: imp, hps (lgtm, not approval), Mina Galić
    Pull Request: https://github.com/freebsd/freebsd-src/pull/648
    Differential Revision: https://reviews.freebsd.org/D37434

 usr.sbin/periodic/etc/daily/310.accounting | 5 +++++
 1 file changed, 5 insertions(+)