Re: [PATCH V6 1/4] audit: implement audit by executable

2015-07-20 Thread Paul Moore
On Friday, July 17, 2015 04:46:18 PM Richard Guy Briggs wrote:
> On 15/07/17, Paul Moore wrote:
> > You could do a "based on" or similar tag if you want.  I'm honestly not
> > sure what the official tags are beyond signed-off, acked, and reviewed. 
> > Those are the only ones I really care about anyway ;)
> 
> From Documentation/SubmittingPatches:
>   11) Signed-off-by:
>   12) Acked-by: and Cc:
>   13) Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:

... you would think after referring people to this file I would have a better 
idea of what currently lives in there ;)

Thanks for the info.

-- 
paul moore
security @ redhat

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V6 1/4] audit: implement audit by executable

2015-07-17 Thread Richard Guy Briggs
On 15/07/17, Paul Moore wrote:
> On Friday, July 17, 2015 11:33:17 AM Richard Guy Briggs wrote:
> > On 15/07/16, Paul Moore wrote:
> > > On Tuesday, July 14, 2015 11:50:23 AM Richard Guy Briggs wrote:
> > > > From: Eric Paris 
> > > > 
> > > > This patch implements the ability to filter on the executable.  It is
> > > > clearly incomplete!  This patch adds the inode/dev of the executable at
> > > > the moment the rule is loaded.  It does not update if the executable is
> > > > updated/moved/whatever.  That should be added.  But at this moment, this
> > > > patch works.
> > > 
> > > This needs work.  Either this patch is incomplete as the description says,
> > > in which case I'm not going to merge it, or the description is out of
> > > date and needs to be updated.
> > 
> > It is pretty close to the original, so the description is valid, however
> > combining this patch with 3/4 and 4/4 triggers a rewrite.
> > 
> > > If later patches in the series fix deficiencies in this patch (I haven't
> > > gotten past this description yet) then we should consider merging some of
> > > the patches together so they are more useful.
> > 
> > Ok, no problem.  (More below...)
> > 
> > > > RGB: Explicitly declare prototypes as extern.
> > > > RGB: Rename audit_dup_exe() to audit_dupe_exe() consistent with rule,
> > > > watch, lsm_field.
> > > > 
> > > > Based-on-user-interface-by: Richard Guy Briggs 
> > > > Based-on-idea-by: Peter Moody 
> > > 
> > > I'm not fully up on the different patch metadata the cool kids are using
> > > these days, but I think it is okay to simply credit Peter in the patch
> > > description and omit the two lines above.  Giving credit is important,
> > > but these metadata tags are a bit silly in my opinion.
> > 
> > Fair enough.  If it doesn't need to be machine-readable, I'll merge it
> > into the patch description prose.
> 
> You could do a "based on" or similar tag if you want.  I'm honestly not sure 
> what the official tags are beyond signed-off, acked, and reviewed.  Those are 
> the only ones I really care about anyway ;)

>From Documentation/SubmittingPatches:
11) Signed-off-by:
12) Acked-by: and Cc:
13) Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes:

> It isn't something I care enough about to reject a patch over, I figured you 
> were going to need to do some respin work anyway so I wanted to mention it.

The description will need a complete overhaul, so it'll get fixed.

> > > > diff --git a/kernel/audit.h b/kernel/audit.h
> > > > index d641f9b..3aca24f 100644
> > > > --- a/kernel/audit.h
> > > > +++ b/kernel/audit.h
> > > > @@ -278,6 +286,30 @@ extern int audit_watch_compare(struct audit_watch
> > > > *watch, unsigned long ino, dev #define audit_watch_path(w) ""
> > > > 
> > > >  #define audit_watch_compare(w, i, d) 0
> > > > 
> > > > +static inline int audit_make_exe_rule(struct audit_krule *krule, char
> > > > *pathname, int len, u32 op)
> > > > +{
> > > > +   return -EINVAL;
> > > > +}
> > > > +static inline void audit_remove_exe_rule(struct audit_krule *krule)
> > > > +{
> > > > +   BUG();
> > > > +   return 0;
> > > > +}
> > > > +static inline char *audit_exe_path(struct audit_exe *exe)
> > > > +{
> > > > +   BUG();
> > > > +   return "";
> > > > +}
> > > > +static inline int audit_dupe_exe(struct audit_krule *new, struct
> > > > audit_krule *old) +{
> > > > +   BUG();
> > > > +   return -EINVAL
> > > > +}
> > > > +static inline int audit_exe_compare(struct task_struct *tsk, struct
> > > > audit_exe *exe) +{
> > > > +   BUG();
> > > > +   return 0;
> > > > +}
> > > > 
> > > >  #endif /* CONFIG_AUDIT_WATCH */
> > > 
> > > Not a big fan of the BUG() calls in the stubs above, let's get rid of
> > > them.
> > 
> > Ok, that way I can more easily convert them to #defines.
> > 
> > > Yes, I know some other audit stubs are defined as BUG(), or similar, those
> > > aren't a good idea either, but they are already there ...
> > 
> > Ok, cleanup patch later...
> 
> Much later.  It isn't something I worry much about, I just don't want to see 
> us going crazy with BUG assertions in new code.
> 
> > > > diff --git a/kernel/audit_exe.c b/kernel/audit_exe.c
> > > > new file mode 100644
> > > > index 000..d4cc8b5
> > > > --- /dev/null
> > > > +++ b/kernel/audit_exe.c
> > > > @@ -0,0 +1,109 @@
> > > > +/* audit_exe.c -- filtering of audit events
> > > > + *
> > > > + * Copyright 2014-2015 Red Hat, Inc.
> > > > + *
> > > > + * This program is free software; you can redistribute it and/or modify
> > > > + * it under the terms of the GNU General Public License as published by
> > > > + * the Free Software Foundation; either version 2 of the License, or
> > > > + * (at your option) any later version.
> > > > + *
> > > > + * This program is distributed in the hope that it will be useful,
> > > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOS

Re: [PATCH V6 1/4] audit: implement audit by executable

2015-07-17 Thread Richard Guy Briggs
On 15/07/17, Richard Guy Briggs wrote:
> On 15/07/16, Paul Moore wrote:
> > On Tuesday, July 14, 2015 11:50:23 AM Richard Guy Briggs wrote:
> > > From: Eric Paris 
> > > 
> > > This patch implements the ability to filter on the executable.  It is
> > > clearly incomplete!  This patch adds the inode/dev of the executable at
> > > the moment the rule is loaded.  It does not update if the executable is
> > > updated/moved/whatever.  That should be added.  But at this moment, this
> > > patch works.



> > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > > index 9fb9d1c..bf745c7 100644
> > > --- a/kernel/auditsc.c
> > > +++ b/kernel/auditsc.c
> > > @@ -71,6 +72,7 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > >  #include 
> > >  #include 
> > >  #include 
> > > @@ -466,6 +468,20 @@ static int audit_filter_rules(struct task_struct 
> > > *tsk,
> > >   result = audit_comparator(ctx->ppid, f->op, 
> > > f->val);
> > >   }
> > >   break;
> > > + case AUDIT_EXE:
> > > + result = audit_exe_compare(tsk, rule->exe);
> > > + break;
> > > + case AUDIT_EXE_CHILDREN:
> > > + {
> > > + struct task_struct *ptsk;
> > > + for (ptsk = tsk; ptsk->parent->pid > 0; ptsk =
> > >  find_task_by_vpid(ptsk->parent->pid)) {
> > > + if (audit_exe_compare(ptsk, rule->exe)) {
> > > + ++result;
> > > + break;
> > > + }
> > > + }
> > > + }
> > > + break;
> > 
> > I don't completely understand the point of AUDIT_EXE_CHILDREN filter, what 
> > problem are we trying to solve?  It checks to see if there is an executable 
> > match starting with the current process and walking up the process' parents 
> > in 
> > the current pid namespace?
> 
> Say we want to monitor /usr/sbin/apache2 and all its spawned processes.
> Set up a rule that uses AUDIT_EXE_CHILDREN with /usr/sbin/apache2, then
> when it spawns a cgi running perl or php, those actions will be caught.
> 
> > Help me understand what this accomplishes, I'm a little tried right now and 
> > I 
> > just don't get it.
> 
> This was Peter Moody's idea and it made sense, so we kept it.

Peter, do you have anything to add to justify keeping
AUDIT_EXE_CHILDREN?

> > paul moore
> 
> - RGB

- RGB

--
Richard Guy Briggs 
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red 
Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH V6 1/4] audit: implement audit by executable

2015-07-17 Thread Paul Moore
On Friday, July 17, 2015 11:33:17 AM Richard Guy Briggs wrote:
> On 15/07/16, Paul Moore wrote:
> > On Tuesday, July 14, 2015 11:50:23 AM Richard Guy Briggs wrote:
> > > From: Eric Paris 
> > > 
> > > This patch implements the ability to filter on the executable.  It is
> > > clearly incomplete!  This patch adds the inode/dev of the executable at
> > > the moment the rule is loaded.  It does not update if the executable is
> > > updated/moved/whatever.  That should be added.  But at this moment, this
> > > patch works.
> > 
> > This needs work.  Either this patch is incomplete as the description says,
> > in which case I'm not going to merge it, or the description is out of
> > date and needs to be updated.
> 
> It is pretty close to the original, so the description is valid, however
> combining this patch with 3/4 and 4/4 triggers a rewrite.
> 
> > If later patches in the series fix deficiencies in this patch (I haven't
> > gotten past this description yet) then we should consider merging some of
> > the patches together so they are more useful.
> 
> Ok, no problem.  (More below...)
> 
> > > RGB: Explicitly declare prototypes as extern.
> > > RGB: Rename audit_dup_exe() to audit_dupe_exe() consistent with rule,
> > > watch, lsm_field.
> > > 
> > > Based-on-user-interface-by: Richard Guy Briggs 
> > > Based-on-idea-by: Peter Moody 
> > 
> > I'm not fully up on the different patch metadata the cool kids are using
> > these days, but I think it is okay to simply credit Peter in the patch
> > description and omit the two lines above.  Giving credit is important,
> > but these metadata tags are a bit silly in my opinion.
> 
> Fair enough.  If it doesn't need to be machine-readable, I'll merge it
> into the patch description prose.

You could do a "based on" or similar tag if you want.  I'm honestly not sure 
what the official tags are beyond signed-off, acked, and reviewed.  Those are 
the only ones I really care about anyway ;)

It isn't something I care enough about to reject a patch over, I figured you 
were going to need to do some respin work anyway so I wanted to mention it.

> > > diff --git a/kernel/audit.h b/kernel/audit.h
> > > index d641f9b..3aca24f 100644
> > > --- a/kernel/audit.h
> > > +++ b/kernel/audit.h
> > > @@ -278,6 +286,30 @@ extern int audit_watch_compare(struct audit_watch
> > > *watch, unsigned long ino, dev #define audit_watch_path(w) ""
> > > 
> > >  #define audit_watch_compare(w, i, d) 0
> > > 
> > > +static inline int audit_make_exe_rule(struct audit_krule *krule, char
> > > *pathname, int len, u32 op)
> > > +{
> > > + return -EINVAL;
> > > +}
> > > +static inline void audit_remove_exe_rule(struct audit_krule *krule)
> > > +{
> > > + BUG();
> > > + return 0;
> > > +}
> > > +static inline char *audit_exe_path(struct audit_exe *exe)
> > > +{
> > > + BUG();
> > > + return "";
> > > +}
> > > +static inline int audit_dupe_exe(struct audit_krule *new, struct
> > > audit_krule *old) +{
> > > + BUG();
> > > + return -EINVAL
> > > +}
> > > +static inline int audit_exe_compare(struct task_struct *tsk, struct
> > > audit_exe *exe) +{
> > > + BUG();
> > > + return 0;
> > > +}
> > > 
> > >  #endif /* CONFIG_AUDIT_WATCH */
> > 
> > Not a big fan of the BUG() calls in the stubs above, let's get rid of
> > them.
> 
> Ok, that way I can more easily convert them to #defines.
> 
> > Yes, I know some other audit stubs are defined as BUG(), or similar, those
> > aren't a good idea either, but they are already there ...
> 
> Ok, cleanup patch later...

Much later.  It isn't something I worry much about, I just don't want to see 
us going crazy with BUG assertions in new code.

> > > diff --git a/kernel/audit_exe.c b/kernel/audit_exe.c
> > > new file mode 100644
> > > index 000..d4cc8b5
> > > --- /dev/null
> > > +++ b/kernel/audit_exe.c
> > > @@ -0,0 +1,109 @@
> > > +/* audit_exe.c -- filtering of audit events
> > > + *
> > > + * Copyright 2014-2015 Red Hat, Inc.
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License as published by
> > > + * the Free Software Foundation; either version 2 of the License, or
> > > + * (at your option) any later version.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > + * GNU General Public License for more details.
> > > + */
> > > +
> > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > +
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include 
> > > +#include "audit.h"
> > > +
> > > +struct audit_exe {
> > > + char *pathname;
> > > + unsigned long ino;
> > > + dev_t dev;
> > > +};
> > > +
> > > +/* Translate a watch string to kernel respresentation. */
> > > +int audit_make_exe_rule(struct audit_krule *krule, char *pathname, int
> > > len, u3

Re: [PATCH V6 1/4] audit: implement audit by executable

2015-07-17 Thread Richard Guy Briggs
On 15/07/16, Paul Moore wrote:
> On Tuesday, July 14, 2015 11:50:23 AM Richard Guy Briggs wrote:
> > From: Eric Paris 
> > 
> > This patch implements the ability to filter on the executable.  It is
> > clearly incomplete!  This patch adds the inode/dev of the executable at
> > the moment the rule is loaded.  It does not update if the executable is
> > updated/moved/whatever.  That should be added.  But at this moment, this
> > patch works.
> 
> This needs work.  Either this patch is incomplete as the description says, in 
> which case I'm not going to merge it, or the description is out of date and 
> needs to be updated.

It is pretty close to the original, so the description is valid, however
combining this patch with 3/4 and 4/4 triggers a rewrite.

> If later patches in the series fix deficiencies in this patch (I haven't 
> gotten past this description yet) then we should consider merging some of the 
> patches together so they are more useful.

Ok, no problem.  (More below...)

> > RGB: Explicitly declare prototypes as extern.
> > RGB: Rename audit_dup_exe() to audit_dupe_exe() consistent with rule, watch,
> > lsm_field.
> > 
> > Based-on-user-interface-by: Richard Guy Briggs 
> > Based-on-idea-by: Peter Moody 
> 
> I'm not fully up on the different patch metadata the cool kids are using 
> these 
> days, but I think it is okay to simply credit Peter in the patch description 
> and omit the two lines above.  Giving credit is important, but these metadata 
> tags are a bit silly in my opinion.

Fair enough.  If it doesn't need to be machine-readable, I'll merge it
into the patch description prose.

> > Cc: pmo...@google.com
> > Signed-off-by: Eric Paris 
> > Signed-off-by: Richard Guy Briggs 
> 
> It has been a while since I've looked at Eric's original patch, but 
> considering considering some of the work involved, I think it is reasonable 
> to 
> claim this patch as your own and credit Eric in the patch description.

I left it in his authorship since all I did was declare the prototypes
explicitly as external and renamed one funciton by one letter.  I didn't
think that meritted claiming authorhship, but if I merge it as you
recommend and makes sense to me, there's no reason not to assume
authorship.

> > diff --git a/kernel/audit.h b/kernel/audit.h
> > index d641f9b..3aca24f 100644
> > --- a/kernel/audit.h
> > +++ b/kernel/audit.h
> > @@ -278,6 +286,30 @@ extern int audit_watch_compare(struct audit_watch
> > *watch, unsigned long ino, dev #define audit_watch_path(w) ""
> >  #define audit_watch_compare(w, i, d) 0
> > 
> > +static inline int audit_make_exe_rule(struct audit_krule *krule, char
> > *pathname, int len, u32 op)
> > +{
> > +   return -EINVAL;
> > +}
> > +static inline void audit_remove_exe_rule(struct audit_krule *krule)
> > +{
> > +   BUG();
> > +   return 0;
> > +}
> > +static inline char *audit_exe_path(struct audit_exe *exe)
> > +{
> > +   BUG();
> > +   return "";
> > +}
> > +static inline int audit_dupe_exe(struct audit_krule *new, struct
> > audit_krule *old) +{
> > +   BUG();
> > +   return -EINVAL
> > +}
> > +static inline int audit_exe_compare(struct task_struct *tsk, struct
> > audit_exe *exe) +{
> > +   BUG();
> > +   return 0;
> > +}
> >  #endif /* CONFIG_AUDIT_WATCH */
> 
> Not a big fan of the BUG() calls in the stubs above, let's get rid of them.  

Ok, that way I can more easily convert them to #defines.

> Yes, I know some other audit stubs are defined as BUG(), or similar, those 
> aren't a good idea either, but they are already there ...

Ok, cleanup patch later...

> > diff --git a/kernel/audit_exe.c b/kernel/audit_exe.c
> > new file mode 100644
> > index 000..d4cc8b5
> > --- /dev/null
> > +++ b/kernel/audit_exe.c
> > @@ -0,0 +1,109 @@
> > +/* audit_exe.c -- filtering of audit events
> > + *
> > + * Copyright 2014-2015 Red Hat, Inc.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +#include "audit.h"
> > +
> > +struct audit_exe {
> > +   char *pathname;
> > +   unsigned long ino;
> > +   dev_t dev;
> > +};
> > +
> > +/* Translate a watch string to kernel respresentation. */
> > +int audit_make_exe_rule(struct audit_krule *krule, char *pathname, int len,
> > u32 op)
> > +{
> > +   struct audit_exe *exe;
> > +   struct path path;
> > +   struct dentry *dentry;
> > +   unsigned long ino;
> > +   dev_t dev;
> >

Re: [PATCH V6 1/4] audit: implement audit by executable

2015-07-16 Thread Paul Moore
On Tuesday, July 14, 2015 11:50:23 AM Richard Guy Briggs wrote:
> From: Eric Paris 
> 
> This patch implements the ability to filter on the executable.  It is
> clearly incomplete!  This patch adds the inode/dev of the executable at
> the moment the rule is loaded.  It does not update if the executable is
> updated/moved/whatever.  That should be added.  But at this moment, this
> patch works.

This needs work.  Either this patch is incomplete as the description says, in 
which case I'm not going to merge it, or the description is out of date and 
needs to be updated.

If later patches in the series fix deficiencies in this patch (I haven't 
gotten past this description yet) then we should consider merging some of the 
patches together so they are more useful.

> RGB: Explicitly declare prototypes as extern.
> RGB: Rename audit_dup_exe() to audit_dupe_exe() consistent with rule, watch,
> lsm_field.
> 
> Based-on-user-interface-by: Richard Guy Briggs 
> Based-on-idea-by: Peter Moody 

I'm not fully up on the different patch metadata the cool kids are using these 
days, but I think it is okay to simply credit Peter in the patch description 
and omit the two lines above.  Giving credit is important, but these metadata 
tags are a bit silly in my opinion.

> Cc: pmo...@google.com
> Signed-off-by: Eric Paris 
> Signed-off-by: Richard Guy Briggs 

It has been a while since I've looked at Eric's original patch, but 
considering considering some of the work involved, I think it is reasonable to 
claim this patch as your own and credit Eric in the patch description.
 
> diff --git a/kernel/audit.h b/kernel/audit.h
> index d641f9b..3aca24f 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -278,6 +286,30 @@ extern int audit_watch_compare(struct audit_watch
> *watch, unsigned long ino, dev #define audit_watch_path(w) ""
>  #define audit_watch_compare(w, i, d) 0
> 
> +static inline int audit_make_exe_rule(struct audit_krule *krule, char
> *pathname, int len, u32 op)
> +{
> + return -EINVAL;
> +}
> +static inline void audit_remove_exe_rule(struct audit_krule *krule)
> +{
> + BUG();
> + return 0;
> +}
> +static inline char *audit_exe_path(struct audit_exe *exe)
> +{
> + BUG();
> + return "";
> +}
> +static inline int audit_dupe_exe(struct audit_krule *new, struct
> audit_krule *old) +{
> + BUG();
> + return -EINVAL
> +}
> +static inline int audit_exe_compare(struct task_struct *tsk, struct
> audit_exe *exe) +{
> + BUG();
> + return 0;
> +}
>  #endif /* CONFIG_AUDIT_WATCH */

Not a big fan of the BUG() calls in the stubs above, let's get rid of them.  
Yes, I know some other audit stubs are defined as BUG(), or similar, those 
aren't a good idea either, but they are already there ...
 
> diff --git a/kernel/audit_exe.c b/kernel/audit_exe.c
> new file mode 100644
> index 000..d4cc8b5
> --- /dev/null
> +++ b/kernel/audit_exe.c
> @@ -0,0 +1,109 @@
> +/* audit_exe.c -- filtering of audit events
> + *
> + * Copyright 2014-2015 Red Hat, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "audit.h"
> +
> +struct audit_exe {
> + char *pathname;
> + unsigned long ino;
> + dev_t dev;
> +};
> +
> +/* Translate a watch string to kernel respresentation. */
> +int audit_make_exe_rule(struct audit_krule *krule, char *pathname, int len,
> u32 op)
> +{
> + struct audit_exe *exe;
> + struct path path;
> + struct dentry *dentry;
> + unsigned long ino;
> + dev_t dev;
> +
> + if (pathname[0] != '/' || pathname[len-1] == '/')
> + return -EINVAL;
> +
> + dentry = kern_path_locked(pathname, &path);
> + if (IS_ERR(dentry))
> + return PTR_ERR(dentry);
> + mutex_unlock(&path.dentry->d_inode->i_mutex);
> +
> + if (!dentry->d_inode)
> + return -ENOENT;
> + dev = dentry->d_inode->i_sb->s_dev;
> + ino = dentry->d_inode->i_ino;
> + dput(dentry);
> +
> + exe = kmalloc(sizeof(*exe), GFP_KERNEL);
> + if (!exe)
> + return -ENOMEM;
> + exe->ino = ino;
> + exe->dev = dev;
> + exe->pathname = pathname;
> + krule->exe = exe;

You don't need the dev and ino variables here, just move the kmalloc() to just 
after the dentry->d_inode check ... or put it after the sanity checks, 
although you'll have to be careful to free it on error.

> + return 0;
> +}

[PATCH V6 1/4] audit: implement audit by executable

2015-07-14 Thread Richard Guy Briggs
From: Eric Paris 

This patch implements the ability to filter on the executable.  It is
clearly incomplete!  This patch adds the inode/dev of the executable at
the moment the rule is loaded.  It does not update if the executable is
updated/moved/whatever.  That should be added.  But at this moment, this
patch works.

RGB: Explicitly declare prototypes as extern.
RGB: Rename audit_dup_exe() to audit_dupe_exe() consistent with rule, watch, 
lsm_field.

Based-on-user-interface-by: Richard Guy Briggs 
Based-on-idea-by: Peter Moody 
Cc: pmo...@google.com
Signed-off-by: Eric Paris 
Signed-off-by: Richard Guy Briggs 
---
 include/linux/audit.h  |1 +
 include/uapi/linux/audit.h |2 +
 kernel/Makefile|2 +-
 kernel/audit.h |   32 +
 kernel/audit_exe.c |  109 
 kernel/auditfilter.c   |   44 ++
 kernel/auditsc.c   |   16 ++
 7 files changed, 205 insertions(+), 1 deletions(-)
 create mode 100644 kernel/audit_exe.c

diff --git a/include/linux/audit.h b/include/linux/audit.h
index c2e7e3a..95463a2 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -59,6 +59,7 @@ struct audit_krule {
struct audit_field  *inode_f; /* quick access to an inode field */
struct audit_watch  *watch; /* associated watch */
struct audit_tree   *tree;  /* associated watched tree */
+   struct audit_exe*exe;
struct list_headrlist;  /* entry in audit_{watch,tree}.rules 
list */
struct list_headlist;   /* for AUDIT_LIST* purposes only */
u64 prio;
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index d3475e1..489aebc 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -266,6 +266,8 @@
 #define AUDIT_OBJ_UID  109
 #define AUDIT_OBJ_GID  110
 #define AUDIT_FIELD_COMPARE111
+#define AUDIT_EXE  112
+#define AUDIT_EXE_CHILDREN 113
 
 #define AUDIT_ARG0  200
 #define AUDIT_ARG1  (AUDIT_ARG0+1)
diff --git a/kernel/Makefile b/kernel/Makefile
index 60c302c..a7ea330 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -64,7 +64,7 @@ obj-$(CONFIG_SMP) += stop_machine.o
 obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o
 obj-$(CONFIG_AUDIT) += audit.o auditfilter.o
 obj-$(CONFIG_AUDITSYSCALL) += auditsc.o
-obj-$(CONFIG_AUDIT_WATCH) += audit_watch.o
+obj-$(CONFIG_AUDIT_WATCH) += audit_watch.o audit_exe.o
 obj-$(CONFIG_AUDIT_TREE) += audit_tree.o
 obj-$(CONFIG_GCOV_KERNEL) += gcov/
 obj-$(CONFIG_KPROBES) += kprobes.o
diff --git a/kernel/audit.h b/kernel/audit.h
index d641f9b..3aca24f 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -50,6 +50,7 @@ enum audit_state {
 
 /* Rule lists */
 struct audit_watch;
+struct audit_exe;
 struct audit_tree;
 struct audit_chunk;
 
@@ -269,6 +270,13 @@ extern int audit_add_watch(struct audit_krule *krule, 
struct list_head **list);
 extern void audit_remove_watch_rule(struct audit_krule *krule);
 extern char *audit_watch_path(struct audit_watch *watch);
 extern int audit_watch_compare(struct audit_watch *watch, unsigned long ino, 
dev_t dev);
+
+extern int audit_make_exe_rule(struct audit_krule *krule, char *pathname, int 
len, u32 op);
+extern void audit_remove_exe_rule(struct audit_krule *krule);
+extern char *audit_exe_path(struct audit_exe *exe);
+extern int audit_dupe_exe(struct audit_krule *new, struct audit_krule *old);
+extern int audit_exe_compare(struct task_struct *tsk, struct audit_exe *exe);
+
 #else
 #define audit_put_watch(w) {}
 #define audit_get_watch(w) {}
@@ -278,6 +286,30 @@ extern int audit_watch_compare(struct audit_watch *watch, 
unsigned long ino, dev
 #define audit_watch_path(w) ""
 #define audit_watch_compare(w, i, d) 0
 
+static inline int audit_make_exe_rule(struct audit_krule *krule, char 
*pathname, int len, u32 op)
+{
+   return -EINVAL;
+}
+static inline void audit_remove_exe_rule(struct audit_krule *krule)
+{
+   BUG();
+   return 0;
+}
+static inline char *audit_exe_path(struct audit_exe *exe)
+{
+   BUG();
+   return "";
+}
+static inline int audit_dupe_exe(struct audit_krule *new, struct audit_krule 
*old)
+{
+   BUG();
+   return -EINVAL
+}
+static inline int audit_exe_compare(struct task_struct *tsk, struct audit_exe 
*exe)
+{
+   BUG();
+   return 0;
+}
 #endif /* CONFIG_AUDIT_WATCH */
 
 #ifdef CONFIG_AUDIT_TREE
diff --git a/kernel/audit_exe.c b/kernel/audit_exe.c
new file mode 100644
index 000..d4cc8b5
--- /dev/null
+++ b/kernel/audit_exe.c
@@ -0,0 +1,109 @@
+/* audit_exe.c -- filtering of audit events
+ *
+ * Copyright 2014-2015 Red Hat, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is dist