All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC] [PATCH] audit: log module name on init_module
@ 2017-01-26 19:50 ` Richard Guy Briggs
  0 siblings, 0 replies; 26+ messages in thread
From: Richard Guy Briggs @ 2017-01-26 19:50 UTC (permalink / raw)
  To: linux-kernel, linux-audit
  Cc: Richard Guy Briggs, Eric Paris, Paul Moore, Steve Grubb, Rusty Russell

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

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 476bc12..6222042 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -358,6 +358,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)
 {
@@ -434,6 +435,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);
+}
+
 extern int audit_n_rules;
 extern int audit_signals;
 #else /* CONFIG_AUDITSYSCALL */
@@ -539,6 +546,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 843540c..513c930 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -110,6 +110,7 @@
 #define AUDIT_SECCOMP		1326	/* Secure Computing event */
 #define AUDIT_PROCTITLE		1327	/* Proctitle emit event */
 #define AUDIT_FEATURE_CHANGE	1328	/* audit log listing feature changes */
+#define AUDIT_MODULE_INIT	1329	/* 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 de6cbb7..cf86486 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -198,6 +198,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 b86cc04..93967b8 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1168,6 +1168,14 @@ static void audit_log_execve_info(struct audit_context *context,
 	kfree(buf);
 }
 
+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;
@@ -1264,6 +1272,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;
 	}
 	audit_log_end(ab);
 }
@@ -2356,6 +2367,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 8f051a1..214ba85 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -59,6 +59,7 @@
 #include <linux/jump_label.h>
 #include <linux/pfn.h>
 #include <linux/bsearch.h>
+#include <linux/audit.h>
 #include <uapi/linux/module.h>
 #include "module-internal.h"
 
@@ -3441,6 +3442,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)
@@ -3525,7 +3528,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 bug_cleanup;
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [RFC] [PATCH] audit: log module name on init_module
@ 2017-01-26 19:50 ` Richard Guy Briggs
  0 siblings, 0 replies; 26+ messages in thread
From: Richard Guy Briggs @ 2017-01-26 19:50 UTC (permalink / raw)
  To: linux-kernel, linux-audit; +Cc: Richard Guy Briggs, Rusty Russell

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

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 476bc12..6222042 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -358,6 +358,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)
 {
@@ -434,6 +435,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);
+}
+
 extern int audit_n_rules;
 extern int audit_signals;
 #else /* CONFIG_AUDITSYSCALL */
@@ -539,6 +546,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 843540c..513c930 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -110,6 +110,7 @@
 #define AUDIT_SECCOMP		1326	/* Secure Computing event */
 #define AUDIT_PROCTITLE		1327	/* Proctitle emit event */
 #define AUDIT_FEATURE_CHANGE	1328	/* audit log listing feature changes */
+#define AUDIT_MODULE_INIT	1329	/* 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 de6cbb7..cf86486 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -198,6 +198,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 b86cc04..93967b8 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1168,6 +1168,14 @@ static void audit_log_execve_info(struct audit_context *context,
 	kfree(buf);
 }
 
+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;
@@ -1264,6 +1272,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;
 	}
 	audit_log_end(ab);
 }
@@ -2356,6 +2367,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 8f051a1..214ba85 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -59,6 +59,7 @@
 #include <linux/jump_label.h>
 #include <linux/pfn.h>
 #include <linux/bsearch.h>
+#include <linux/audit.h>
 #include <uapi/linux/module.h>
 #include "module-internal.h"
 
@@ -3441,6 +3442,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)
@@ -3525,7 +3528,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 bug_cleanup;
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* Re: [RFC] [PATCH] audit: log module name on init_module
  2017-01-26 19:50 ` Richard Guy Briggs
  (?)
@ 2017-01-26 20:14 ` Richard Guy Briggs
  -1 siblings, 0 replies; 26+ messages in thread
From: Richard Guy Briggs @ 2017-01-26 20:14 UTC (permalink / raw)
  To: linux-kernel, linux-audit; +Cc: Rusty Russell

On 2017-01-26 14:50, Richard Guy Briggs 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
> 
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>

Self-NACK.  I just realize this is on a crusty version.  I'll send an updated patch.

> ---
>  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 476bc12..6222042 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -358,6 +358,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)
>  {
> @@ -434,6 +435,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);
> +}
> +
>  extern int audit_n_rules;
>  extern int audit_signals;
>  #else /* CONFIG_AUDITSYSCALL */
> @@ -539,6 +546,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 843540c..513c930 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -110,6 +110,7 @@
>  #define AUDIT_SECCOMP		1326	/* Secure Computing event */
>  #define AUDIT_PROCTITLE		1327	/* Proctitle emit event */
>  #define AUDIT_FEATURE_CHANGE	1328	/* audit log listing feature changes */
> +#define AUDIT_MODULE_INIT	1329	/* 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 de6cbb7..cf86486 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -198,6 +198,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 b86cc04..93967b8 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1168,6 +1168,14 @@ static void audit_log_execve_info(struct audit_context *context,
>  	kfree(buf);
>  }
>  
> +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;
> @@ -1264,6 +1272,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;
>  	}
>  	audit_log_end(ab);
>  }
> @@ -2356,6 +2367,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 8f051a1..214ba85 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -59,6 +59,7 @@
>  #include <linux/jump_label.h>
>  #include <linux/pfn.h>
>  #include <linux/bsearch.h>
> +#include <linux/audit.h>
>  #include <uapi/linux/module.h>
>  #include "module-internal.h"
>  
> @@ -3441,6 +3442,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)
> @@ -3525,7 +3528,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 bug_cleanup;
> -- 
> 1.7.1
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [RFC] [PATCH] audit: log module name on init_module
  2017-01-26 19:50 ` Richard Guy Briggs
@ 2017-01-27  0:04   ` Rusty Russell
  -1 siblings, 0 replies; 26+ messages in thread
From: Rusty Russell @ 2017-01-27  0:04 UTC (permalink / raw)
  To: Richard Guy Briggs, linux-kernel, linux-audit
  Cc: Richard Guy Briggs, Eric Paris, Paul Moore, Steve Grubb, Jessica Yu

Hi RGB!

        This should get acked by the new module maintainer (and your RH
peer!) Jeyu, cc'd.

Cheers,
Rusty.
        
Richard Guy Briggs <rgb@redhat.com> writes:
> 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
>
> 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 476bc12..6222042 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -358,6 +358,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)
>  {
> @@ -434,6 +435,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);
> +}
> +
>  extern int audit_n_rules;
>  extern int audit_signals;
>  #else /* CONFIG_AUDITSYSCALL */
> @@ -539,6 +546,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 843540c..513c930 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -110,6 +110,7 @@
>  #define AUDIT_SECCOMP		1326	/* Secure Computing event */
>  #define AUDIT_PROCTITLE		1327	/* Proctitle emit event */
>  #define AUDIT_FEATURE_CHANGE	1328	/* audit log listing feature changes */
> +#define AUDIT_MODULE_INIT	1329	/* 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 de6cbb7..cf86486 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -198,6 +198,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 b86cc04..93967b8 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1168,6 +1168,14 @@ static void audit_log_execve_info(struct audit_context *context,
>  	kfree(buf);
>  }
>  
> +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;
> @@ -1264,6 +1272,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;
>  	}
>  	audit_log_end(ab);
>  }
> @@ -2356,6 +2367,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 8f051a1..214ba85 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -59,6 +59,7 @@
>  #include <linux/jump_label.h>
>  #include <linux/pfn.h>
>  #include <linux/bsearch.h>
> +#include <linux/audit.h>
>  #include <uapi/linux/module.h>
>  #include "module-internal.h"
>  
> @@ -3441,6 +3442,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)
> @@ -3525,7 +3528,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 bug_cleanup;
> -- 
> 1.7.1

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [RFC] [PATCH] audit: log module name on init_module
@ 2017-01-27  0:04   ` Rusty Russell
  0 siblings, 0 replies; 26+ messages in thread
From: Rusty Russell @ 2017-01-27  0:04 UTC (permalink / raw)
  To: linux-kernel, linux-audit
  Cc: Richard Guy Briggs, Eric Paris, Paul Moore, Steve Grubb, Jessica Yu

Hi RGB!

        This should get acked by the new module maintainer (and your RH
peer!) Jeyu, cc'd.

Cheers,
Rusty.
        
Richard Guy Briggs <rgb@redhat.com> writes:
> 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
>
> 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 476bc12..6222042 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -358,6 +358,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)
>  {
> @@ -434,6 +435,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);
> +}
> +
>  extern int audit_n_rules;
>  extern int audit_signals;
>  #else /* CONFIG_AUDITSYSCALL */
> @@ -539,6 +546,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 843540c..513c930 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -110,6 +110,7 @@
>  #define AUDIT_SECCOMP		1326	/* Secure Computing event */
>  #define AUDIT_PROCTITLE		1327	/* Proctitle emit event */
>  #define AUDIT_FEATURE_CHANGE	1328	/* audit log listing feature changes */
> +#define AUDIT_MODULE_INIT	1329	/* 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 de6cbb7..cf86486 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -198,6 +198,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 b86cc04..93967b8 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1168,6 +1168,14 @@ static void audit_log_execve_info(struct audit_context *context,
>  	kfree(buf);
>  }
>  
> +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;
> @@ -1264,6 +1272,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;
>  	}
>  	audit_log_end(ab);
>  }
> @@ -2356,6 +2367,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 8f051a1..214ba85 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -59,6 +59,7 @@
>  #include <linux/jump_label.h>
>  #include <linux/pfn.h>
>  #include <linux/bsearch.h>
> +#include <linux/audit.h>
>  #include <uapi/linux/module.h>
>  #include "module-internal.h"
>  
> @@ -3441,6 +3442,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)
> @@ -3525,7 +3528,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 bug_cleanup;
> -- 
> 1.7.1

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [RFC] [PATCH] audit: log module name on init_module
  2017-01-27  0:04   ` Rusty Russell
@ 2017-01-28 18:48     ` Richard Guy Briggs
  -1 siblings, 0 replies; 26+ messages in thread
From: Richard Guy Briggs @ 2017-01-28 18:48 UTC (permalink / raw)
  To: Rusty Russell
  Cc: linux-kernel, linux-audit, Eric Paris, Paul Moore, Steve Grubb,
	Jessica Yu

On 2017-01-27 10:34, Rusty Russell wrote:
> Hi RGB!

Hey Rusteeee!  Hi Jeyu,

>         This should get acked by the new module maintainer (and your RH
> peer!) Jeyu, cc'd.

Ah ok, that changed 4.9, this patch was based on 4.8 (audit/next).
Thanks!

> Cheers,
> Rusty.
>         
> Richard Guy Briggs <rgb@redhat.com> writes:
> > 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
> >
> > 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 476bc12..6222042 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -358,6 +358,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)
> >  {
> > @@ -434,6 +435,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);
> > +}
> > +
> >  extern int audit_n_rules;
> >  extern int audit_signals;
> >  #else /* CONFIG_AUDITSYSCALL */
> > @@ -539,6 +546,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 843540c..513c930 100644
> > --- a/include/uapi/linux/audit.h
> > +++ b/include/uapi/linux/audit.h
> > @@ -110,6 +110,7 @@
> >  #define AUDIT_SECCOMP		1326	/* Secure Computing event */
> >  #define AUDIT_PROCTITLE		1327	/* Proctitle emit event */
> >  #define AUDIT_FEATURE_CHANGE	1328	/* audit log listing feature changes */
> > +#define AUDIT_MODULE_INIT	1329	/* 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 de6cbb7..cf86486 100644
> > --- a/kernel/audit.h
> > +++ b/kernel/audit.h
> > @@ -198,6 +198,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 b86cc04..93967b8 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -1168,6 +1168,14 @@ static void audit_log_execve_info(struct audit_context *context,
> >  	kfree(buf);
> >  }
> >  
> > +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;
> > @@ -1264,6 +1272,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;
> >  	}
> >  	audit_log_end(ab);
> >  }
> > @@ -2356,6 +2367,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 8f051a1..214ba85 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -59,6 +59,7 @@
> >  #include <linux/jump_label.h>
> >  #include <linux/pfn.h>
> >  #include <linux/bsearch.h>
> > +#include <linux/audit.h>
> >  #include <uapi/linux/module.h>
> >  #include "module-internal.h"
> >  
> > @@ -3441,6 +3442,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)
> > @@ -3525,7 +3528,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 bug_cleanup;
> > -- 
> > 1.7.1

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [RFC] [PATCH] audit: log module name on init_module
@ 2017-01-28 18:48     ` Richard Guy Briggs
  0 siblings, 0 replies; 26+ messages in thread
From: Richard Guy Briggs @ 2017-01-28 18:48 UTC (permalink / raw)
  To: Rusty Russell; +Cc: Jessica Yu, linux-kernel, linux-audit

On 2017-01-27 10:34, Rusty Russell wrote:
> Hi RGB!

Hey Rusteeee!  Hi Jeyu,

>         This should get acked by the new module maintainer (and your RH
> peer!) Jeyu, cc'd.

Ah ok, that changed 4.9, this patch was based on 4.8 (audit/next).
Thanks!

> Cheers,
> Rusty.
>         
> Richard Guy Briggs <rgb@redhat.com> writes:
> > 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
> >
> > 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 476bc12..6222042 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -358,6 +358,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)
> >  {
> > @@ -434,6 +435,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);
> > +}
> > +
> >  extern int audit_n_rules;
> >  extern int audit_signals;
> >  #else /* CONFIG_AUDITSYSCALL */
> > @@ -539,6 +546,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 843540c..513c930 100644
> > --- a/include/uapi/linux/audit.h
> > +++ b/include/uapi/linux/audit.h
> > @@ -110,6 +110,7 @@
> >  #define AUDIT_SECCOMP		1326	/* Secure Computing event */
> >  #define AUDIT_PROCTITLE		1327	/* Proctitle emit event */
> >  #define AUDIT_FEATURE_CHANGE	1328	/* audit log listing feature changes */
> > +#define AUDIT_MODULE_INIT	1329	/* 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 de6cbb7..cf86486 100644
> > --- a/kernel/audit.h
> > +++ b/kernel/audit.h
> > @@ -198,6 +198,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 b86cc04..93967b8 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -1168,6 +1168,14 @@ static void audit_log_execve_info(struct audit_context *context,
> >  	kfree(buf);
> >  }
> >  
> > +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;
> > @@ -1264,6 +1272,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;
> >  	}
> >  	audit_log_end(ab);
> >  }
> > @@ -2356,6 +2367,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 8f051a1..214ba85 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -59,6 +59,7 @@
> >  #include <linux/jump_label.h>
> >  #include <linux/pfn.h>
> >  #include <linux/bsearch.h>
> > +#include <linux/audit.h>
> >  #include <uapi/linux/module.h>
> >  #include "module-internal.h"
> >  
> > @@ -3441,6 +3442,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)
> > @@ -3525,7 +3528,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 bug_cleanup;
> > -- 
> > 1.7.1

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [RFC] [PATCH] audit: log module name on init_module
  2017-01-26 19:50 ` Richard Guy Briggs
@ 2017-01-30 10:54   ` Steve Grubb
  -1 siblings, 0 replies; 26+ messages in thread
From: Steve Grubb @ 2017-01-30 10:54 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: linux-kernel, linux-audit, Eric Paris, Paul Moore, Rusty Russell

On Thu, 26 Jan 2017 14:50:07 -0500
Richard Guy Briggs <rgb@redhat.com> wrote:

> This adds a new auxiliary record MODULE_INIT to the SYSCALL event.

Thanks, this is definitely needed. Can you provide an example event
generated by this?

-Steve

> 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
> 
> 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 476bc12..6222042 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -358,6 +358,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)
>  {
> @@ -434,6 +435,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);
> +}
> +
>  extern int audit_n_rules;
>  extern int audit_signals;
>  #else /* CONFIG_AUDITSYSCALL */
> @@ -539,6 +546,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 843540c..513c930 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -110,6 +110,7 @@
>  #define AUDIT_SECCOMP		1326	/* Secure Computing
> event */ #define AUDIT_PROCTITLE		1327	/*
> Proctitle emit event */ #define AUDIT_FEATURE_CHANGE
> 1328	/* audit log listing feature changes */ +#define
> AUDIT_MODULE_INIT	1329	/* 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 de6cbb7..cf86486 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -198,6 +198,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 b86cc04..93967b8 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1168,6 +1168,14 @@ static void audit_log_execve_info(struct
> audit_context *context, kfree(buf);
>  }
>  
> +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;
> @@ -1264,6 +1272,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;
>  	}
>  	audit_log_end(ab);
>  }
> @@ -2356,6 +2367,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 8f051a1..214ba85 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -59,6 +59,7 @@
>  #include <linux/jump_label.h>
>  #include <linux/pfn.h>
>  #include <linux/bsearch.h>
> +#include <linux/audit.h>
>  #include <uapi/linux/module.h>
>  #include "module-internal.h"
>  
> @@ -3441,6 +3442,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)
> @@ -3525,7 +3528,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 bug_cleanup;

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [RFC] [PATCH] audit: log module name on init_module
@ 2017-01-30 10:54   ` Steve Grubb
  0 siblings, 0 replies; 26+ messages in thread
From: Steve Grubb @ 2017-01-30 10:54 UTC (permalink / raw)
  To: Richard Guy Briggs; +Cc: Rusty Russell, linux-audit, linux-kernel

On Thu, 26 Jan 2017 14:50:07 -0500
Richard Guy Briggs <rgb@redhat.com> wrote:

> This adds a new auxiliary record MODULE_INIT to the SYSCALL event.

Thanks, this is definitely needed. Can you provide an example event
generated by this?

-Steve

> 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
> 
> 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 476bc12..6222042 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -358,6 +358,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)
>  {
> @@ -434,6 +435,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);
> +}
> +
>  extern int audit_n_rules;
>  extern int audit_signals;
>  #else /* CONFIG_AUDITSYSCALL */
> @@ -539,6 +546,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 843540c..513c930 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -110,6 +110,7 @@
>  #define AUDIT_SECCOMP		1326	/* Secure Computing
> event */ #define AUDIT_PROCTITLE		1327	/*
> Proctitle emit event */ #define AUDIT_FEATURE_CHANGE
> 1328	/* audit log listing feature changes */ +#define
> AUDIT_MODULE_INIT	1329	/* 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 de6cbb7..cf86486 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -198,6 +198,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 b86cc04..93967b8 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1168,6 +1168,14 @@ static void audit_log_execve_info(struct
> audit_context *context, kfree(buf);
>  }
>  
> +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;
> @@ -1264,6 +1272,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;
>  	}
>  	audit_log_end(ab);
>  }
> @@ -2356,6 +2367,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 8f051a1..214ba85 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -59,6 +59,7 @@
>  #include <linux/jump_label.h>
>  #include <linux/pfn.h>
>  #include <linux/bsearch.h>
> +#include <linux/audit.h>
>  #include <uapi/linux/module.h>
>  #include "module-internal.h"
>  
> @@ -3441,6 +3442,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)
> @@ -3525,7 +3528,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 bug_cleanup;

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [RFC] [PATCH] audit: log module name on init_module
  2017-01-30 10:54   ` Steve Grubb
@ 2017-01-30 11:43     ` Richard Guy Briggs
  -1 siblings, 0 replies; 26+ messages in thread
From: Richard Guy Briggs @ 2017-01-30 11:43 UTC (permalink / raw)
  To: Steve Grubb
  Cc: linux-kernel, linux-audit, Eric Paris, Paul Moore, Rusty Russell

On 2017-01-30 11:54, Steve Grubb wrote:
> On Thu, 26 Jan 2017 14:50:07 -0500
> Richard Guy Briggs <rgb@redhat.com> wrote:
> 
> > This adds a new auxiliary record MODULE_INIT to the SYSCALL event.
> 
> Thanks, this is definitely needed. Can you provide an example event
> generated by this?

It's in the wiki RFE, the second link provided.

> -Steve
> 
> > 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
> > 
> > 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 476bc12..6222042 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -358,6 +358,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)
> >  {
> > @@ -434,6 +435,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);
> > +}
> > +
> >  extern int audit_n_rules;
> >  extern int audit_signals;
> >  #else /* CONFIG_AUDITSYSCALL */
> > @@ -539,6 +546,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 843540c..513c930 100644
> > --- a/include/uapi/linux/audit.h
> > +++ b/include/uapi/linux/audit.h
> > @@ -110,6 +110,7 @@
> >  #define AUDIT_SECCOMP		1326	/* Secure Computing
> > event */ #define AUDIT_PROCTITLE		1327	/*
> > Proctitle emit event */ #define AUDIT_FEATURE_CHANGE
> > 1328	/* audit log listing feature changes */ +#define
> > AUDIT_MODULE_INIT	1329	/* 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 de6cbb7..cf86486 100644
> > --- a/kernel/audit.h
> > +++ b/kernel/audit.h
> > @@ -198,6 +198,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 b86cc04..93967b8 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -1168,6 +1168,14 @@ static void audit_log_execve_info(struct
> > audit_context *context, kfree(buf);
> >  }
> >  
> > +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;
> > @@ -1264,6 +1272,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;
> >  	}
> >  	audit_log_end(ab);
> >  }
> > @@ -2356,6 +2367,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 8f051a1..214ba85 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -59,6 +59,7 @@
> >  #include <linux/jump_label.h>
> >  #include <linux/pfn.h>
> >  #include <linux/bsearch.h>
> > +#include <linux/audit.h>
> >  #include <uapi/linux/module.h>
> >  #include "module-internal.h"
> >  
> > @@ -3441,6 +3442,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)
> > @@ -3525,7 +3528,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 bug_cleanup;
> 

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [RFC] [PATCH] audit: log module name on init_module
@ 2017-01-30 11:43     ` Richard Guy Briggs
  0 siblings, 0 replies; 26+ messages in thread
From: Richard Guy Briggs @ 2017-01-30 11:43 UTC (permalink / raw)
  To: Steve Grubb; +Cc: Rusty Russell, linux-audit, linux-kernel

On 2017-01-30 11:54, Steve Grubb wrote:
> On Thu, 26 Jan 2017 14:50:07 -0500
> Richard Guy Briggs <rgb@redhat.com> wrote:
> 
> > This adds a new auxiliary record MODULE_INIT to the SYSCALL event.
> 
> Thanks, this is definitely needed. Can you provide an example event
> generated by this?

It's in the wiki RFE, the second link provided.

> -Steve
> 
> > 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
> > 
> > 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 476bc12..6222042 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -358,6 +358,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)
> >  {
> > @@ -434,6 +435,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);
> > +}
> > +
> >  extern int audit_n_rules;
> >  extern int audit_signals;
> >  #else /* CONFIG_AUDITSYSCALL */
> > @@ -539,6 +546,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 843540c..513c930 100644
> > --- a/include/uapi/linux/audit.h
> > +++ b/include/uapi/linux/audit.h
> > @@ -110,6 +110,7 @@
> >  #define AUDIT_SECCOMP		1326	/* Secure Computing
> > event */ #define AUDIT_PROCTITLE		1327	/*
> > Proctitle emit event */ #define AUDIT_FEATURE_CHANGE
> > 1328	/* audit log listing feature changes */ +#define
> > AUDIT_MODULE_INIT	1329	/* 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 de6cbb7..cf86486 100644
> > --- a/kernel/audit.h
> > +++ b/kernel/audit.h
> > @@ -198,6 +198,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 b86cc04..93967b8 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -1168,6 +1168,14 @@ static void audit_log_execve_info(struct
> > audit_context *context, kfree(buf);
> >  }
> >  
> > +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;
> > @@ -1264,6 +1272,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;
> >  	}
> >  	audit_log_end(ab);
> >  }
> > @@ -2356,6 +2367,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 8f051a1..214ba85 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -59,6 +59,7 @@
> >  #include <linux/jump_label.h>
> >  #include <linux/pfn.h>
> >  #include <linux/bsearch.h>
> > +#include <linux/audit.h>
> >  #include <uapi/linux/module.h>
> >  #include "module-internal.h"
> >  
> > @@ -3441,6 +3442,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)
> > @@ -3525,7 +3528,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 bug_cleanup;
> 

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [RFC] [PATCH] audit: log module name on init_module
  2017-02-04 13:27           ` Steve Grubb
@ 2017-02-04 17:20             ` Richard Guy Briggs
  0 siblings, 0 replies; 26+ messages in thread
From: Richard Guy Briggs @ 2017-02-04 17:20 UTC (permalink / raw)
  To: Steve Grubb
  Cc: Paul Moore, Rusty Russell, linux-audit, linux-kernel, Jessica Yu

On 2017-02-04 08:27, Steve Grubb wrote:
> On Friday, February 3, 2017 7:18:58 PM EST Paul Moore wrote:
> > On Tue, Jan 31, 2017 at 3:02 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > > On 2017-01-31 11:07, Paul Moore wrote:
> > >> On Tue, Jan 31, 2017 at 7:36 AM, Richard Guy Briggs <rgb@redhat.com> 
> wrote:
> > >> > On 2017-01-31 06:59, Paul Moore wrote:
> > >> >> 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-rec
> > >> >> > ord-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.
> > >> > 
> > >> > Ok fair enough, I had mis-understood your previous point.
> > >> 
> > >> I probably could have been more specific too.
> > >> 
> > >> > Any comment on the new record format?
> > >> 
> > >> Not really, it's just the single field so it's kinda hard to have
> > >> anything meaningful to say.  We obviously need to worry about the
> > >> field name, but I'll let Steve speak to that as that likely means more
> > >> to him than it does to me.  From my perspective, "name" seems
> > >> perfectly reasonable, especially since it is in the context of a
> > >> module specific record (no real worries about it being ambiguous).
> > > 
> > > Do you see a need to include module initialization arguments?  It sounds
> > > potentially useful to me, but also potentially bandwidth-consuming.  I
> > > have a prototype patch to add the args as one encoded field.  Along with
> > > the addition of this field is the concern about message lengths and
> > > buffer allocations since it is an encoded field that would need twice
> > > the length of the argment text to store in the message.
> > 
> > Argument filtering is surely going to be a mess, just look at the
> > related execve() stuff.  Unless there is a hard requirement I say skip
> > the argument logging for now, we can always add it later.
> 
> Its not a requirement at this point. Just cleanup the current patch and it 
> should be what we needed.

Good.  I've changed the record type from AUDIT_MODULE_INIT to
AUDIT_KERN_MODULE to be generally useful but to avoid confusion with
other types of modules.

> -Steve

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [RFC] [PATCH] audit: log module name on init_module
  2017-02-04  0:18           ` Paul Moore
  (?)
  (?)
@ 2017-02-04 17:19           ` Richard Guy Briggs
  -1 siblings, 0 replies; 26+ messages in thread
From: Richard Guy Briggs @ 2017-02-04 17:19 UTC (permalink / raw)
  To: Paul Moore
  Cc: Steve Grubb, Rusty Russell, linux-audit, linux-kernel, Jessica Yu

On 2017-02-03 19:18, Paul Moore wrote:
> On Tue, Jan 31, 2017 at 3:02 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2017-01-31 11:07, Paul Moore wrote:
> >> On Tue, Jan 31, 2017 at 7:36 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> >> > On 2017-01-31 06:59, Paul Moore wrote:
> >> >> 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.
> >> >
> >> > Ok fair enough, I had mis-understood your previous point.
> >>
> >> I probably could have been more specific too.
> >>
> >> > Any comment on the new record format?
> >>
> >> Not really, it's just the single field so it's kinda hard to have
> >> anything meaningful to say.  We obviously need to worry about the
> >> field name, but I'll let Steve speak to that as that likely means more
> >> to him than it does to me.  From my perspective, "name" seems
> >> perfectly reasonable, especially since it is in the context of a
> >> module specific record (no real worries about it being ambiguous).
> >
> > Do you see a need to include module initialization arguments?  It sounds
> > potentially useful to me, but also potentially bandwidth-consuming.  I
> > have a prototype patch to add the args as one encoded field.  Along with
> > the addition of this field is the concern about message lengths and
> > buffer allocations since it is an encoded field that would need twice
> > the length of the argment text to store in the message.
> 
> Argument filtering is surely going to be a mess, just look at the
> related execve() stuff.  Unless there is a hard requirement I say skip
> the argument logging for now, we can always add it later.

I *did* look at execve, which is why I was apprehensive.  :-/

> paul moore

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [RFC] [PATCH] audit: log module name on init_module
  2017-02-04  0:18           ` Paul Moore
  (?)
@ 2017-02-04 13:27           ` Steve Grubb
  2017-02-04 17:20             ` Richard Guy Briggs
  -1 siblings, 1 reply; 26+ messages in thread
From: Steve Grubb @ 2017-02-04 13:27 UTC (permalink / raw)
  To: Paul Moore
  Cc: Richard Guy Briggs, Rusty Russell, linux-audit, linux-kernel, Jessica Yu

On Friday, February 3, 2017 7:18:58 PM EST Paul Moore wrote:
> On Tue, Jan 31, 2017 at 3:02 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2017-01-31 11:07, Paul Moore wrote:
> >> On Tue, Jan 31, 2017 at 7:36 AM, Richard Guy Briggs <rgb@redhat.com> 
wrote:
> >> > On 2017-01-31 06:59, Paul Moore wrote:
> >> >> 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-rec
> >> >> > ord-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.
> >> > 
> >> > Ok fair enough, I had mis-understood your previous point.
> >> 
> >> I probably could have been more specific too.
> >> 
> >> > Any comment on the new record format?
> >> 
> >> Not really, it's just the single field so it's kinda hard to have
> >> anything meaningful to say.  We obviously need to worry about the
> >> field name, but I'll let Steve speak to that as that likely means more
> >> to him than it does to me.  From my perspective, "name" seems
> >> perfectly reasonable, especially since it is in the context of a
> >> module specific record (no real worries about it being ambiguous).
> > 
> > Do you see a need to include module initialization arguments?  It sounds
> > potentially useful to me, but also potentially bandwidth-consuming.  I
> > have a prototype patch to add the args as one encoded field.  Along with
> > the addition of this field is the concern about message lengths and
> > buffer allocations since it is an encoded field that would need twice
> > the length of the argment text to store in the message.
> 
> Argument filtering is surely going to be a mess, just look at the
> related execve() stuff.  Unless there is a hard requirement I say skip
> the argument logging for now, we can always add it later.

Its not a requirement at this point. Just cleanup the current patch and it 
should be what we needed.

-Steve

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [RFC] [PATCH] audit: log module name on init_module
  2017-01-31 20:02         ` Richard Guy Briggs
@ 2017-02-04  0:18           ` Paul Moore
  -1 siblings, 0 replies; 26+ messages in thread
From: Paul Moore @ 2017-02-04  0:18 UTC (permalink / raw)
  To: Richard Guy Briggs
  Cc: Steve Grubb, Rusty Russell, linux-audit, linux-kernel, Jessica Yu

On Tue, Jan 31, 2017 at 3:02 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2017-01-31 11:07, Paul Moore wrote:
>> On Tue, Jan 31, 2017 at 7:36 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
>> > On 2017-01-31 06:59, Paul Moore wrote:
>> >> 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.
>> >
>> > Ok fair enough, I had mis-understood your previous point.
>>
>> I probably could have been more specific too.
>>
>> > Any comment on the new record format?
>>
>> Not really, it's just the single field so it's kinda hard to have
>> anything meaningful to say.  We obviously need to worry about the
>> field name, but I'll let Steve speak to that as that likely means more
>> to him than it does to me.  From my perspective, "name" seems
>> perfectly reasonable, especially since it is in the context of a
>> module specific record (no real worries about it being ambiguous).
>
> Do you see a need to include module initialization arguments?  It sounds
> potentially useful to me, but also potentially bandwidth-consuming.  I
> have a prototype patch to add the args as one encoded field.  Along with
> the addition of this field is the concern about message lengths and
> buffer allocations since it is an encoded field that would need twice
> the length of the argment text to store in the message.

Argument filtering is surely going to be a mess, just look at the
related execve() stuff.  Unless there is a hard requirement I say skip
the argument logging for now, we can always add it later.

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [RFC] [PATCH] audit: log module name on init_module
@ 2017-02-04  0:18           ` Paul Moore
  0 siblings, 0 replies; 26+ messages in thread
From: Paul Moore @ 2017-02-04  0:18 UTC (permalink / raw)
  To: Richard Guy Briggs; +Cc: Jessica Yu, Rusty Russell, linux-kernel, linux-audit

On Tue, Jan 31, 2017 at 3:02 PM, Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2017-01-31 11:07, Paul Moore wrote:
>> On Tue, Jan 31, 2017 at 7:36 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
>> > On 2017-01-31 06:59, Paul Moore wrote:
>> >> 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.
>> >
>> > Ok fair enough, I had mis-understood your previous point.
>>
>> I probably could have been more specific too.
>>
>> > Any comment on the new record format?
>>
>> Not really, it's just the single field so it's kinda hard to have
>> anything meaningful to say.  We obviously need to worry about the
>> field name, but I'll let Steve speak to that as that likely means more
>> to him than it does to me.  From my perspective, "name" seems
>> perfectly reasonable, especially since it is in the context of a
>> module specific record (no real worries about it being ambiguous).
>
> Do you see a need to include module initialization arguments?  It sounds
> potentially useful to me, but also potentially bandwidth-consuming.  I
> have a prototype patch to add the args as one encoded field.  Along with
> the addition of this field is the concern about message lengths and
> buffer allocations since it is an encoded field that would need twice
> the length of the argment text to store in the message.

Argument filtering is surely going to be a mess, just look at the
related execve() stuff.  Unless there is a hard requirement I say skip
the argument logging for now, we can always add it later.

-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [RFC] [PATCH] audit: log module name on init_module
  2017-01-31 16:07     ` Paul Moore
  2017-01-31 20:02         ` Richard Guy Briggs
@ 2017-02-01  8:30       ` Steve Grubb
  1 sibling, 0 replies; 26+ messages in thread
From: Steve Grubb @ 2017-02-01  8:30 UTC (permalink / raw)
  To: Paul Moore
  Cc: Richard Guy Briggs, Jessica Yu, Rusty Russell, linux-kernel, linux-audit

On Tue, 31 Jan 2017 11:07:24 -0500
Paul Moore <paul@paul-moore.com> wrote:

> On Tue, Jan 31, 2017 at 7:36 AM, Richard Guy Briggs <rgb@redhat.com>
> wrote:
> > On 2017-01-31 06:59, Paul Moore wrote:  
> >> 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.  
> >
> > Ok fair enough, I had mis-understood your previous point.  
> 
> I probably could have been more specific too.
> 
> > Any comment on the new record format?  
> 
> Not really, it's just the single field so it's kinda hard to have
> anything meaningful to say.  We obviously need to worry about the
> field name, but I'll let Steve speak to that as that likely means more
> to him than it does to me.  From my perspective, "name" seems
> perfectly reasonable, especially since it is in the context of a
> module specific record (no real worries about it being ambiguous).

I'm fine with "name".

> I guess that does bring up the question of the record name, right now
> it is AUDIT_MODULE_INIT ... do we want it to be specific to init/load
> or do we want it more generic with an "op=" field (you mentioned some
> thoughts both ways in the GH issue).  I'm not sure I care too much
> either way, but once again I imagine Steve may have a preference for
> one over the other.

If this does not hook delete_module, then I see no reason for an op=
field. But even if it were to, the operation is implicit in the syscall
name. So, I don't think anything else is needed.

-Steve


> >> >  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;  
> >>
> >> paul moore  
> >
> > - RGB
> >
> > --
> > Richard Guy Briggs <rgb@redhat.com>
> > Kernel Security Engineering, Base Operating Systems, Red Hat
> > Remote, Ottawa, Canada
> > Voice: +1.647.777.2635, Internal: (81) 32635  
> 
> 
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [RFC] [PATCH] audit: log module name on init_module
  2017-01-31 16:07     ` Paul Moore
@ 2017-01-31 20:02         ` Richard Guy Briggs
  2017-02-01  8:30       ` Steve Grubb
  1 sibling, 0 replies; 26+ messages in thread
From: Richard Guy Briggs @ 2017-01-31 20:02 UTC (permalink / raw)
  To: Paul Moore, Steve Grubb
  Cc: Rusty Russell, linux-audit, linux-kernel, Jessica Yu

On 2017-01-31 11:07, Paul Moore wrote:
> On Tue, Jan 31, 2017 at 7:36 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2017-01-31 06:59, Paul Moore wrote:
> >> 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.
> >
> > Ok fair enough, I had mis-understood your previous point.
> 
> I probably could have been more specific too.
> 
> > Any comment on the new record format?
> 
> Not really, it's just the single field so it's kinda hard to have
> anything meaningful to say.  We obviously need to worry about the
> field name, but I'll let Steve speak to that as that likely means more
> to him than it does to me.  From my perspective, "name" seems
> perfectly reasonable, especially since it is in the context of a
> module specific record (no real worries about it being ambiguous).

Do you see a need to include module initialization arguments?  It sounds
potentially useful to me, but also potentially bandwidth-consuming.  I
have a prototype patch to add the args as one encoded field.  Along with
the addition of this field is the concern about message lengths and
buffer allocations since it is an encoded field that would need twice
the length of the argment text to store in the message.

> I guess that does bring up the question of the record name, right now
> it is AUDIT_MODULE_INIT ... do we want it to be specific to init/load
> or do we want it more generic with an "op=" field (you mentioned some
> thoughts both ways in the GH issue).  I'm not sure I care too much
> either way, but once again I imagine Steve may have a preference for
> one over the other.

Right, so more similar to SERVICE/DAEMON START/STOP, or using an op=
field...

> >> >  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;
> >>
> >> paul moore
> >
> > - RGB
> >
> > --
> > Richard Guy Briggs <rgb@redhat.com>
> > Kernel Security Engineering, Base Operating Systems, Red Hat
> > Remote, Ottawa, Canada
> > Voice: +1.647.777.2635, Internal: (81) 32635
> 
> 
> 
> -- 
> paul moore
> www.paul-moore.com

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [RFC] [PATCH] audit: log module name on init_module
@ 2017-01-31 20:02         ` Richard Guy Briggs
  0 siblings, 0 replies; 26+ messages in thread
From: Richard Guy Briggs @ 2017-01-31 20:02 UTC (permalink / raw)
  To: Paul Moore, Steve Grubb
  Cc: Jessica Yu, Rusty Russell, linux-kernel, linux-audit

On 2017-01-31 11:07, Paul Moore wrote:
> On Tue, Jan 31, 2017 at 7:36 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> > On 2017-01-31 06:59, Paul Moore wrote:
> >> 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.
> >
> > Ok fair enough, I had mis-understood your previous point.
> 
> I probably could have been more specific too.
> 
> > Any comment on the new record format?
> 
> Not really, it's just the single field so it's kinda hard to have
> anything meaningful to say.  We obviously need to worry about the
> field name, but I'll let Steve speak to that as that likely means more
> to him than it does to me.  From my perspective, "name" seems
> perfectly reasonable, especially since it is in the context of a
> module specific record (no real worries about it being ambiguous).

Do you see a need to include module initialization arguments?  It sounds
potentially useful to me, but also potentially bandwidth-consuming.  I
have a prototype patch to add the args as one encoded field.  Along with
the addition of this field is the concern about message lengths and
buffer allocations since it is an encoded field that would need twice
the length of the argment text to store in the message.

> I guess that does bring up the question of the record name, right now
> it is AUDIT_MODULE_INIT ... do we want it to be specific to init/load
> or do we want it more generic with an "op=" field (you mentioned some
> thoughts both ways in the GH issue).  I'm not sure I care too much
> either way, but once again I imagine Steve may have a preference for
> one over the other.

Right, so more similar to SERVICE/DAEMON START/STOP, or using an op=
field...

> >> >  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;
> >>
> >> paul moore
> >
> > - RGB
> >
> > --
> > Richard Guy Briggs <rgb@redhat.com>
> > Kernel Security Engineering, Base Operating Systems, Red Hat
> > Remote, Ottawa, Canada
> > Voice: +1.647.777.2635, Internal: (81) 32635
> 
> 
> 
> -- 
> paul moore
> www.paul-moore.com

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [RFC] [PATCH] audit: log module name on init_module
  2017-01-31 12:36     ` Richard Guy Briggs
  (?)
@ 2017-01-31 16:07     ` Paul Moore
  2017-01-31 20:02         ` Richard Guy Briggs
  2017-02-01  8:30       ` Steve Grubb
  -1 siblings, 2 replies; 26+ messages in thread
From: Paul Moore @ 2017-01-31 16:07 UTC (permalink / raw)
  To: Richard Guy Briggs; +Cc: Rusty Russell, linux-audit, linux-kernel, Jessica Yu

On Tue, Jan 31, 2017 at 7:36 AM, Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2017-01-31 06:59, Paul Moore wrote:
>> 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.
>
> Ok fair enough, I had mis-understood your previous point.

I probably could have been more specific too.

> Any comment on the new record format?

Not really, it's just the single field so it's kinda hard to have
anything meaningful to say.  We obviously need to worry about the
field name, but I'll let Steve speak to that as that likely means more
to him than it does to me.  From my perspective, "name" seems
perfectly reasonable, especially since it is in the context of a
module specific record (no real worries about it being ambiguous).

I guess that does bring up the question of the record name, right now
it is AUDIT_MODULE_INIT ... do we want it to be specific to init/load
or do we want it more generic with an "op=" field (you mentioned some
thoughts both ways in the GH issue).  I'm not sure I care too much
either way, but once again I imagine Steve may have a preference for
one over the other.

>> >  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;
>>
>> paul moore
>
> - RGB
>
> --
> Richard Guy Briggs <rgb@redhat.com>
> Kernel Security Engineering, Base Operating Systems, Red Hat
> Remote, Ottawa, Canada
> Voice: +1.647.777.2635, Internal: (81) 32635



-- 
paul moore
www.paul-moore.com

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [RFC] [PATCH] audit: log module name on init_module
  2017-01-31 11:59   ` Paul Moore
@ 2017-01-31 12:36     ` Richard Guy Briggs
  -1 siblings, 0 replies; 26+ messages in thread
From: Richard Guy Briggs @ 2017-01-31 12:36 UTC (permalink / raw)
  To: Paul Moore; +Cc: Rusty Russell, linux-audit, linux-kernel, Jessica Yu

On 2017-01-31 06:59, Paul Moore wrote:
> 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.

Ok fair enough, I had mis-understood your previous point.

Any comment on the new record format?

> >  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;
> 
> paul moore

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [RFC] [PATCH] audit: log module name on init_module
@ 2017-01-31 12:36     ` Richard Guy Briggs
  0 siblings, 0 replies; 26+ messages in thread
From: Richard Guy Briggs @ 2017-01-31 12:36 UTC (permalink / raw)
  To: Paul Moore; +Cc: Jessica Yu, Rusty Russell, linux-kernel, linux-audit

On 2017-01-31 06:59, Paul Moore wrote:
> 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.

Ok fair enough, I had mis-understood your previous point.

Any comment on the new record format?

> >  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;
> 
> paul moore

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Kernel Security Engineering, Base Operating Systems, Red Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [RFC] [PATCH] audit: log module name on init_module
  2017-01-26 21:21 ` Richard Guy Briggs
@ 2017-01-31 11:59   ` Paul Moore
  -1 siblings, 0 replies; 26+ messages in thread
From: Paul Moore @ 2017-01-31 11:59 UTC (permalink / raw)
  To: Richard Guy Briggs; +Cc: linux-kernel, linux-audit, Rusty Russell, Jessica Yu

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

^ permalink raw reply	[flat|nested] 26+ messages in thread
* Re: [RFC] [PATCH] audit: log module name on init_module
@ 2017-01-31 11:59   ` Paul Moore
  0 siblings, 0 replies; 26+ messages in thread
From: Paul Moore @ 2017-01-31 11:59 UTC (permalink / raw)
  To: Richard Guy Briggs; +Cc: Rusty Russell, linux-audit, linux-kernel, Jessica Yu

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

^ permalink raw reply	[flat|nested] 26+ messages in thread
* [RFC] [PATCH] audit: log module name on init_module
@ 2017-01-26 21:21 ` Richard Guy Briggs
  0 siblings, 0 replies; 26+ messages in thread
From: Richard Guy Briggs @ 2017-01-26 21:21 UTC (permalink / raw)
  To: linux-kernel, linux-audit
  Cc: Richard Guy Briggs, Eric Paris, Paul Moore, Steve Grubb, Rusty Russell

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

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);
+}
+
 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;
 	}
 	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

^ permalink raw reply related	[flat|nested] 26+ messages in thread
* [RFC] [PATCH] audit: log module name on init_module
@ 2017-01-26 21:21 ` Richard Guy Briggs
  0 siblings, 0 replies; 26+ messages in thread
From: Richard Guy Briggs @ 2017-01-26 21:21 UTC (permalink / raw)
  To: linux-kernel, linux-audit; +Cc: Richard Guy Briggs, Rusty Russell

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

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);
+}
+
 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;
 	}
 	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

^ permalink raw reply related	[flat|nested] 26+ messages in thread
end of thread, other threads:[~2017-02-04 17:21 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-26 19:50 [RFC] [PATCH] audit: log module name on init_module 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
2017-01-26 21:21 Richard Guy Briggs
2017-01-26 21:21 ` Richard Guy Briggs
2017-01-31 11:59 ` Paul Moore
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

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.