On Mon, Jul 13, 2020 at 3:52 PM Richard Guy Briggs <r...@redhat.com> wrote:
>
> 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 <r...@redhat.com>
> ---
> Passes audit-testsuite.
>
> Changelog:
> v4
> - use double quotes in all replaced audit_log_string() calls
>
> v3
> - fix two warning: non-void function does not return a value in all control
> paths
> Reported-by: kernel test robot <l...@intel.com>
>
> 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/dbbcba46335a002f44b05874153a85b9cc18aebf
>
> 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(-)
Thanks for restoring the quotes, just one question below ...
> diff --git a/security/apparmor/ipc.c b/security/apparmor/ipc.c
> index 4ecedffbdd33..fe36d112aad9 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 "";
Are we okay with this returning an empty string ("") in this case?
Should it be a question mark ("?")?
My guess is that userspace parsing should be okay since it still has
quotes, I'm just not sure if we wanted to use a question mark as we do
in other cases where the field value is empty/unknown.
--
paul moore
www.paul-moore.com
