Re: [PATCH ghak84 v3] audit: purge audit_log_string from the intra-kernel audit API

audit_log_string() was inteded to be an internal audit function and since there are only two internal uses, remove them. Purge all external uses of it by restructuring code to use an existing audit_log_format() or using audit_log_format(). Please see the upstream issue https://github.com/linux-audit/audit-kernel/issues/84 Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; --- Passes audit-testsuite. Changelog: v3 - fix two warning: non-void function does not return a value in all control paths Reported-by: kernel test robot <lkp(a)intel.com&gt; v2 - restructure to piggyback on existing audit_log_format() calls, checking quoting needs for each. v1 Vlad Dronov - https://github.com/nefigtut/audit-kernel/commit/dbbcba46335a002f44b058741... include/linux/audit.h | 5 ----- kernel/audit.c | 4 ++-- security/apparmor/audit.c | 10 ++++------ security/apparmor/file.c | 25 +++++++------------------ security/apparmor/ipc.c | 46 +++++++++++++++++++++++----------------------- security/apparmor/net.c | 14 ++++++++------ security/lsm_audit.c | 4 ++-- 7 files changed, 46 insertions(+), 62 deletions(-)

diff --git a/security/apparmor/audit.c b/security/apparmor/audit.c index 597732503815..335b5b8d300b 100644 --- a/security/apparmor/audit.c +++ b/security/apparmor/audit.c @@ -57,18 +57,16 @@ static void audit_pre(struct audit_buffer *ab, void *ca) struct common_audit_data *sa = ca; if (aa_g_audit_header) { - audit_log_format(ab, "apparmor="); - audit_log_string(ab, aa_audit_type[aad(sa)->type]); + audit_log_format(ab, "apparmor=%s", + aa_audit_type[aad(sa)->type]); } if (aad(sa)->op) { - audit_log_format(ab, " operation="); - audit_log_string(ab, aad(sa)->op); + audit_log_format(ab, " operation=%s", aad(sa)->op); }

if (aad(sa)->info) { - audit_log_format(ab, " info="); - audit_log_string(ab, aad(sa)->info); + audit_log_format(ab, " info=\"%s\"", aad(sa)->info); if (aad(sa)->error) audit_log_format(ab, " error=%d", aad(sa)->error); } diff --git a/security/apparmor/file.c b/security/apparmor/file.c index 9a2d14b7c9f8..70f27124d051 100644 --- a/security/apparmor/file.c +++ b/security/apparmor/file.c @@ -35,20 +35,6 @@ static u32 map_mask_to_chr_mask(u32 mask) } /** - * audit_file_mask - convert mask to permission string - * @buffer: buffer to write string to (NOT NULL) - * @mask: permission mask to convert - */ -static void audit_file_mask(struct audit_buffer *ab, u32 mask) -{ - char str[10]; - - aa_perm_mask_to_str(str, sizeof(str), aa_file_perm_chrs, - map_mask_to_chr_mask(mask)); - audit_log_string(ab, str); -} - -/** * file_audit_cb - call back for file specific audit fields * @ab: audit_buffer (NOT NULL) * @va: audit struct to audit values of (NOT NULL) @@ -57,14 +43,17 @@ static void file_audit_cb(struct audit_buffer *ab, void *va) { struct common_audit_data *sa = va; kuid_t fsuid = current_fsuid(); + char str[10]; if (aad(sa)->request & AA_AUDIT_FILE_MASK) { - audit_log_format(ab, " requested_mask="); - audit_file_mask(ab, aad(sa)->request); + aa_perm_mask_to_str(str, sizeof(str), aa_file_perm_chrs, + map_mask_to_chr_mask(aad(sa)->request)); + audit_log_format(ab, " requested_mask=%s", str); } if (aad(sa)->denied & AA_AUDIT_FILE_MASK) { - audit_log_format(ab, " denied_mask="); - audit_file_mask(ab, aad(sa)->denied); + aa_perm_mask_to_str(str, sizeof(str), aa_file_perm_chrs, + map_mask_to_chr_mask(aad(sa)->denied)); + audit_log_format(ab, " denied_mask=%s", str); }

if (aad(sa)->request & AA_AUDIT_FILE_MASK) { audit_log_format(ab, " fsuid=%d", diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c index 4ecedffbdd33..fe431731883f 100644 --- a/security/apparmor/ipc.c +++ b/security/apparmor/ipc.c @@ -20,25 +20,23 @@ /** * audit_ptrace_mask - convert mask to permission string - * @buffer: buffer to write string to (NOT NULL) * @mask: permission mask to convert + * + * Returns: pointer to static string */ -static void audit_ptrace_mask(struct audit_buffer *ab, u32 mask) +static const char *audit_ptrace_mask(u32 mask) { switch (mask) { case MAY_READ: - audit_log_string(ab, "read"); - break; + return "read"; case MAY_WRITE: - audit_log_string(ab, "trace"); - break; + return "trace"; case AA_MAY_BE_READ: - audit_log_string(ab, "readby"); - break; + return "readby"; case AA_MAY_BE_TRACED: - audit_log_string(ab, "tracedby"); - break; + return "tracedby"; } + return ""; } /* call back to audit ptrace fields */ @@ -47,12 +45,12 @@ static void audit_ptrace_cb(struct audit_buffer *ab, void *va) struct common_audit_data *sa = va; if (aad(sa)->request & AA_PTRACE_PERM_MASK) { - audit_log_format(ab, " requested_mask="); - audit_ptrace_mask(ab, aad(sa)->request); + audit_log_format(ab, " requested_mask=%s", + audit_ptrace_mask(aad(sa)->request)); if (aad(sa)->denied & AA_PTRACE_PERM_MASK) { - audit_log_format(ab, " denied_mask="); - audit_ptrace_mask(ab, aad(sa)->denied); + audit_log_format(ab, " denied_mask=%s", + audit_ptrace_mask(aad(sa)->denied)); }