All of lore.kernel.org
 help / color / mirror / Atom feed
From: Paul Moore <paul@paul-moore.com>
To: Richard Guy Briggs <rgb@redhat.com>
Cc: linux-kernel@vger.kernel.org, linux-audit@redhat.com,
	Rusty Russell <rusty@rustcorp.com.au>,
	Jessica Yu <jeyu@redhat.com>
Subject: Re: [RFC] [PATCH] audit: log module name on init_module
Date: Tue, 31 Jan 2017 06:59:45 -0500	[thread overview]
Message-ID: <CAHC9VhR-iW2AD+Dhg4zBwbnbFp2akjx-12cO-5qgJ_JJ1MokEw@mail.gmail.com> (raw)
In-Reply-To: <2f5850f7ba8026763eedf5ebabbb020929aab82c.1485461541.git.rgb@redhat.com>

On Thu, Jan 26, 2017 at 4:21 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> This adds a new auxiliary record MODULE_INIT to the SYSCALL event.
>
> We get finit_module for free since it made most sense to hook this in to
> load_module().
>
> https://github.com/linux-audit/audit-kernel/issues/7
> https://github.com/linux-audit/audit-kernel/wiki/RFE-Module-load-record-format

Consistency nit: capitalize the first letter in the wiki page words
(see the existing RFE pages)

> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  include/linux/audit.h      |   12 ++++++++++++
>  include/uapi/linux/audit.h |    1 +
>  kernel/audit.h             |    3 +++
>  kernel/auditsc.c           |   20 ++++++++++++++++++++
>  kernel/module.c            |    5 ++++-
>  5 files changed, 40 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 2be99b2..7bb23d5 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -360,6 +360,7 @@ extern int __audit_log_bprm_fcaps(struct linux_binprm *bprm,
>                                   const struct cred *old);
>  extern void __audit_log_capset(const struct cred *new, const struct cred *old);
>  extern void __audit_mmap_fd(int fd, int flags);
> +extern void __audit_module_init(char *name);
>
>  static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
>  {
> @@ -450,6 +451,12 @@ static inline void audit_mmap_fd(int fd, int flags)
>                 __audit_mmap_fd(fd, flags);
>  }
>
> +static inline void audit_module_init(char *name)
> +{
> +       if (!audit_dummy_context())
> +               __audit_module_init(name);
> +}

More on this below, but I was expecting the function above to named
audit_log_kern_module(), or something similar.

>  extern int audit_n_rules;
>  extern int audit_signals;
>  #else /* CONFIG_AUDITSYSCALL */
> @@ -561,6 +568,11 @@ static inline void audit_log_capset(const struct cred *new,
>  { }
>  static inline void audit_mmap_fd(int fd, int flags)
>  { }
> +
> +static inline void audit_module_init(char *name)
> +{
> +}
> +
>  static inline void audit_ptrace(struct task_struct *t)
>  { }
>  #define audit_n_rules 0
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 3f24110..4a328b4 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -111,6 +111,7 @@
>  #define AUDIT_PROCTITLE                1327    /* Proctitle emit event */
>  #define AUDIT_FEATURE_CHANGE   1328    /* audit log listing feature changes */
>  #define AUDIT_REPLACE          1329    /* Replace auditd if this packet unanswerd */
> +#define AUDIT_MODULE_INIT      1330    /* Module init event */
>
>  #define AUDIT_AVC              1400    /* SE Linux avc denial or grant */
>  #define AUDIT_SELINUX_ERR      1401    /* Internal SE Linux Errors */
> diff --git a/kernel/audit.h b/kernel/audit.h
> index 431444c..144b7eb 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -199,6 +199,9 @@ struct audit_context {
>                 struct {
>                         int                     argc;
>                 } execve;
> +               struct {
> +                       char                    *name;
> +               } module;
>         };
>         int fds[2];
>         struct audit_proctitle proctitle;
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index bb5f504..3e12678 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1172,6 +1172,14 @@ out:
>         kfree(buf_head);
>  }
>
> +static void audit_log_kern_module(struct audit_context *context,
> +                                 struct audit_buffer **ab)
> +{
> +       audit_log_format(*ab, " name=");
> +       audit_log_untrustedstring(*ab, context->module.name);
> +       kfree(context->module.name);
> +}
> +
>  static void show_special(struct audit_context *context, int *call_panic)
>  {
>         struct audit_buffer *ab;
> @@ -1268,6 +1276,9 @@ static void show_special(struct audit_context *context, int *call_panic)
>         case AUDIT_EXECVE: {
>                 audit_log_execve_info(context, &ab);
>                 break; }
> +       case AUDIT_MODULE_INIT:
> +               audit_log_kern_module(context, &ab);
> +               break;

With the exception of AUDIT_EXECVE everything else in show_special()
is simply open coded inside the show_special() function; considering
that audit_log_kern_module() is trivial it seems like a separate
function isn't really needed here.

>         }
>         audit_log_end(ab);
>  }
> @@ -2368,6 +2379,15 @@ void __audit_mmap_fd(int fd, int flags)
>         context->type = AUDIT_MMAP;
>  }
>
> +void __audit_module_init(char *name)
> +{
> +       struct audit_context *context = current->audit_context;
> +
> +       context->module.name = kmalloc(strlen(name) + 1, GFP_KERNEL);
> +       strcpy(context->module.name, name);
> +       context->type = AUDIT_MODULE_INIT;
> +}
> +
>  static void audit_log_task(struct audit_buffer *ab)
>  {
>         kuid_t auid, uid;
> diff --git a/kernel/module.c b/kernel/module.c
> index 529efae..678407e 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -61,6 +61,7 @@
>  #include <linux/pfn.h>
>  #include <linux/bsearch.h>
>  #include <linux/dynamic_debug.h>
> +#include <linux/audit.h>
>  #include <uapi/linux/module.h>
>  #include "module-internal.h"
>
> @@ -3593,6 +3594,8 @@ static int load_module(struct load_info *info, const char __user *uargs,
>                 goto free_copy;
>         }
>
> +       audit_module_init(mod->name);
> +
>         /* Reserve our place in the list. */
>         err = add_unformed_module(mod);
>         if (err)
> @@ -3681,7 +3684,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
>                        mod->name, after_dashes);
>         }
>
> -       /* Link in to syfs. */
> +       /* Link in to sysfs. */
>         err = mod_sysfs_setup(mod, info, mod->kp, mod->num_kp);
>         if (err < 0)
>                 goto coming_cleanup;
> --
> 1.7.1
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

-- 
paul moore
www.paul-moore.com

WARNING: multiple messages have this Message-ID (diff)
From: Paul Moore <paul@paul-moore.com>
To: Richard Guy Briggs <rgb@redhat.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>,
	linux-audit@redhat.com, linux-kernel@vger.kernel.org,
	Jessica Yu <jeyu@redhat.com>
Subject: Re: [RFC] [PATCH] audit: log module name on init_module
Date: Tue, 31 Jan 2017 06:59:45 -0500	[thread overview]
Message-ID: <CAHC9VhR-iW2AD+Dhg4zBwbnbFp2akjx-12cO-5qgJ_JJ1MokEw@mail.gmail.com> (raw)
In-Reply-To: <2f5850f7ba8026763eedf5ebabbb020929aab82c.1485461541.git.rgb@redhat.com>

On Thu, Jan 26, 2017 at 4:21 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> This adds a new auxiliary record MODULE_INIT to the SYSCALL event.
>
> We get finit_module for free since it made most sense to hook this in to
> load_module().
>
> https://github.com/linux-audit/audit-kernel/issues/7
> https://github.com/linux-audit/audit-kernel/wiki/RFE-Module-load-record-format

Consistency nit: capitalize the first letter in the wiki page words
(see the existing RFE pages)

> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  include/linux/audit.h      |   12 ++++++++++++
>  include/uapi/linux/audit.h |    1 +
>  kernel/audit.h             |    3 +++
>  kernel/auditsc.c           |   20 ++++++++++++++++++++
>  kernel/module.c            |    5 ++++-
>  5 files changed, 40 insertions(+), 1 deletions(-)
>
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 2be99b2..7bb23d5 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -360,6 +360,7 @@ extern int __audit_log_bprm_fcaps(struct linux_binprm *bprm,
>                                   const struct cred *old);
>  extern void __audit_log_capset(const struct cred *new, const struct cred *old);
>  extern void __audit_mmap_fd(int fd, int flags);
> +extern void __audit_module_init(char *name);
>
>  static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
>  {
> @@ -450,6 +451,12 @@ static inline void audit_mmap_fd(int fd, int flags)
>                 __audit_mmap_fd(fd, flags);
>  }
>
> +static inline void audit_module_init(char *name)
> +{
> +       if (!audit_dummy_context())
> +               __audit_module_init(name);
> +}

More on this below, but I was expecting the function above to named
audit_log_kern_module(), or something similar.

>  extern int audit_n_rules;
>  extern int audit_signals;
>  #else /* CONFIG_AUDITSYSCALL */
> @@ -561,6 +568,11 @@ static inline void audit_log_capset(const struct cred *new,
>  { }
>  static inline void audit_mmap_fd(int fd, int flags)
>  { }
> +
> +static inline void audit_module_init(char *name)
> +{
> +}
> +
>  static inline void audit_ptrace(struct task_struct *t)
>  { }
>  #define audit_n_rules 0
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 3f24110..4a328b4 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -111,6 +111,7 @@
>  #define AUDIT_PROCTITLE                1327    /* Proctitle emit event */
>  #define AUDIT_FEATURE_CHANGE   1328    /* audit log listing feature changes */
>  #define AUDIT_REPLACE          1329    /* Replace auditd if this packet unanswerd */
> +#define AUDIT_MODULE_INIT      1330    /* Module init event */
>
>  #define AUDIT_AVC              1400    /* SE Linux avc denial or grant */
>  #define AUDIT_SELINUX_ERR      1401    /* Internal SE Linux Errors */
> diff --git a/kernel/audit.h b/kernel/audit.h
> index 431444c..144b7eb 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -199,6 +199,9 @@ struct audit_context {
>                 struct {
>                         int                     argc;
>                 } execve;
> +               struct {
> +                       char                    *name;
> +               } module;
>         };
>         int fds[2];
>         struct audit_proctitle proctitle;
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index bb5f504..3e12678 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1172,6 +1172,14 @@ out:
>         kfree(buf_head);
>  }
>
> +static void audit_log_kern_module(struct audit_context *context,
> +                                 struct audit_buffer **ab)
> +{
> +       audit_log_format(*ab, " name=");
> +       audit_log_untrustedstring(*ab, context->module.name);
> +       kfree(context->module.name);
> +}
> +
>  static void show_special(struct audit_context *context, int *call_panic)
>  {
>         struct audit_buffer *ab;
> @@ -1268,6 +1276,9 @@ static void show_special(struct audit_context *context, int *call_panic)
>         case AUDIT_EXECVE: {
>                 audit_log_execve_info(context, &ab);
>                 break; }
> +       case AUDIT_MODULE_INIT:
> +               audit_log_kern_module(context, &ab);
> +               break;

With the exception of AUDIT_EXECVE everything else in show_special()
is simply open coded inside the show_special() function; considering
that audit_log_kern_module() is trivial it seems like a separate
function isn't really needed here.

>         }
>         audit_log_end(ab);
>  }
> @@ -2368,6 +2379,15 @@ void __audit_mmap_fd(int fd, int flags)
>         context->type = AUDIT_MMAP;
>  }
>
> +void __audit_module_init(char *name)
> +{
> +       struct audit_context *context = current->audit_context;
> +
> +       context->module.name = kmalloc(strlen(name) + 1, GFP_KERNEL);
> +       strcpy(context->module.name, name);
> +       context->type = AUDIT_MODULE_INIT;
> +}
> +
>  static void audit_log_task(struct audit_buffer *ab)
>  {
>         kuid_t auid, uid;
> diff --git a/kernel/module.c b/kernel/module.c
> index 529efae..678407e 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -61,6 +61,7 @@
>  #include <linux/pfn.h>
>  #include <linux/bsearch.h>
>  #include <linux/dynamic_debug.h>
> +#include <linux/audit.h>
>  #include <uapi/linux/module.h>
>  #include "module-internal.h"
>
> @@ -3593,6 +3594,8 @@ static int load_module(struct load_info *info, const char __user *uargs,
>                 goto free_copy;
>         }
>
> +       audit_module_init(mod->name);
> +
>         /* Reserve our place in the list. */
>         err = add_unformed_module(mod);
>         if (err)
> @@ -3681,7 +3684,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
>                        mod->name, after_dashes);
>         }
>
> -       /* Link in to syfs. */
> +       /* Link in to sysfs. */
>         err = mod_sysfs_setup(mod, info, mod->kp, mod->num_kp);
>         if (err < 0)
>                 goto coming_cleanup;
> --
> 1.7.1
>
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

-- 
paul moore
www.paul-moore.com

  reply	other threads:[~2017-01-31 12:00 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-26 21:21 [RFC] [PATCH] audit: log module name on init_module Richard Guy Briggs
2017-01-26 21:21 ` Richard Guy Briggs
2017-01-31 11:59 ` Paul Moore [this message]
2017-01-31 11:59   ` Paul Moore
2017-01-31 12:36   ` Richard Guy Briggs
2017-01-31 12:36     ` Richard Guy Briggs
2017-01-31 16:07     ` Paul Moore
2017-01-31 20:02       ` Richard Guy Briggs
2017-01-31 20:02         ` Richard Guy Briggs
2017-02-04  0:18         ` Paul Moore
2017-02-04  0:18           ` Paul Moore
2017-02-04 13:27           ` Steve Grubb
2017-02-04 17:20             ` Richard Guy Briggs
2017-02-04 17:19           ` Richard Guy Briggs
2017-02-01  8:30       ` Steve Grubb
  -- strict thread matches above, loose matches on Subject: below --
2017-01-26 19:50 Richard Guy Briggs
2017-01-26 19:50 ` Richard Guy Briggs
2017-01-26 20:14 ` Richard Guy Briggs
2017-01-27  0:04 ` Rusty Russell
2017-01-27  0:04   ` Rusty Russell
2017-01-28 18:48   ` Richard Guy Briggs
2017-01-28 18:48     ` Richard Guy Briggs
2017-01-30 10:54 ` Steve Grubb
2017-01-30 10:54   ` Steve Grubb
2017-01-30 11:43   ` Richard Guy Briggs
2017-01-30 11:43     ` Richard Guy Briggs

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHC9VhR-iW2AD+Dhg4zBwbnbFp2akjx-12cO-5qgJ_JJ1MokEw@mail.gmail.com \
    --to=paul@paul-moore.com \
    --cc=jeyu@redhat.com \
    --cc=linux-audit@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rgb@redhat.com \
    --cc=rusty@rustcorp.com.au \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.