Re: [PATCH V6 2/4] audit: clean simple fsnotify implementation
On 15/07/16, Paul Moore wrote:
> On Tuesday, July 14, 2015 11:50:24 AM Richard Guy Briggs wrote:
> > This is to be used to audit by executable rules, but audit watches
> > should be able to share this code eventually.
> >
> > At the moment the audit watch code is a lot more complex, that code only
> > creates one fsnotify watch per parent directory. That 'audit_parent' in
> > turn has a list of 'audit_watches' which contain the name, ino, dev of
> > the specific object we care about. This just creates one fsnotify watch
> > per object we care about. So if you watch 100 inodes in /etc this code
> > will create 100 fsnotify watches on /etc. The audit_watch code will
> > instead create 1 fsnotify watch on /etc (the audit_parent) and then 100
> > individual watches chained from that fsnotify mark.
> >
> > We should be able to convert the audit_watch code to do one fsnotify
> > mark per watch and simplify things/remove a whole lot of code. After
> > that conversion we should be able to convert the audit_fsnotify code to
> > support that hierarchy if the optimization is necessary.
> >
> > Signed-off-by: Eric Paris
> >
> > RGB: Move the access to the entry for audit_match_signal() to the beginning
> > of the function in case the entry found is the same one passed in. This
> > will enable it to be used by audit_autoremove_mark_rule().
> > RGB: Rename several "watch" references to "mark".
> > RGB: Rename audit_remove_rule() to audit_autoremove_mark_rule().
> > RGB: Put audit_alloc_mark() arguments in same order as watch, tree and
> > inode. RGB: Remove space from audit log value text in
> > audit_autoremove_mark_rule(). RGB: Explicitly declare prototypes as extern.
> > RGB: Rename audit_remove_mark_rule() called from audit_mark_handle_event()
> > to audit_autoremove_mark_rule() to avoid confusion with
> > audit_remove_{watch,tree}_rule() usage.
> > RGB: Add audit_remove_mark_rule() to provide similar interface as
> > audit_remove_{watch,tree}_rule().
> > RGB: Simplify stubs to defines.
> > RGB: Rename audit_free_fsnotify_mark() to audit_fsnotify_free_mark() in
> > keeping with the naming convention of inotify_free_mark(),
> > dnotify_free_mark(), fanotify_free_mark(), audit_watch_free_mark().
> > RGB: Return -ENOMEM rather than null in case of memory allocation failure
> > for audit_mark. RGB: Rename audit_free_mark() to audit_mark_free() to avoid
> > association with {i,d,fa}notify_free_mark() and audit_watch_free_mark().
>
> Definitely enough changes here to call this your own; credit Eric in the
> description and just stick with your sign off.
Done.
> > Based-on-code-by: Eric Paris
> > Signed-off-by: Richard Guy Briggs
>
> Based on the patch description, should this be patch 1/4 instead of 2/4?
Or 1/2 as you have suggested.
> > diff --git a/kernel/Makefile b/kernel/Makefile
> > index a7ea330..397109e 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 audit_exe.o
> > +obj-$(CONFIG_AUDIT_WATCH) += audit_watch.o audit_exe.o audit_fsnotify.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 3aca24f..491bd4b 100644
> > --- a/kernel/audit.h
> > +++ b/kernel/audit.h
> > @@ -286,6 +294,20 @@ extern int audit_exe_compare(struct task_struct *tsk,
> > struct audit_exe *exe); #define audit_watch_path(w) ""
> > #define audit_watch_compare(w, i, d) 0
> >
> > +#define audit_alloc_mark(k, p, l) (ERR_PTR(-EINVAL))
> > +static inline char *audit_mark_path(struct audit_fsnotify_mark *mark)
> > +{
> > + BUG();
> > + return "";
> > +}
> > +#define audit_remove_mark(m) BUG()
> > +#define audit_remove_mark_rule(k) BUG()
> > +static inline int audit_mark_compare(struct audit_fsnotify_mark *mark,
> > unsigned long ino, dev_t dev) +{
> > + BUG();
> > + return 0;
> > +}
>
> No BUG(), we've got enough of those things already.
Cleaned them out...
> > diff --git a/kernel/audit_fsnotify.c b/kernel/audit_fsnotify.c
> > new file mode 100644
> > index 000..a4e7b16
> > --- /dev/null
> > +++ b/kernel/audit_fsnotify.c
> > @@ -0,0 +1,253 @@
> > +/* audit_fsnotify.c -- tracking inodes
> > + *
> > + * Copyright 2003-2009,2014-2015 Red Hat, Inc.
> > + * Copyright 2005 Hewlett-Packard Development Company, L.P.
> > + * Copyright 2005 IBM Corporation
> > + *
> > + * 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
Re: [PATCH V6 2/4] audit: clean simple fsnotify implementation
On 15/07/16, Paul Moore wrote:
On Tuesday, July 14, 2015 11:50:24 AM Richard Guy Briggs wrote:
This is to be used to audit by executable rules, but audit watches
should be able to share this code eventually.
At the moment the audit watch code is a lot more complex, that code only
creates one fsnotify watch per parent directory. That 'audit_parent' in
turn has a list of 'audit_watches' which contain the name, ino, dev of
the specific object we care about. This just creates one fsnotify watch
per object we care about. So if you watch 100 inodes in /etc this code
will create 100 fsnotify watches on /etc. The audit_watch code will
instead create 1 fsnotify watch on /etc (the audit_parent) and then 100
individual watches chained from that fsnotify mark.
We should be able to convert the audit_watch code to do one fsnotify
mark per watch and simplify things/remove a whole lot of code. After
that conversion we should be able to convert the audit_fsnotify code to
support that hierarchy if the optimization is necessary.
Signed-off-by: Eric Paris epa...@redhat.com
RGB: Move the access to the entry for audit_match_signal() to the beginning
of the function in case the entry found is the same one passed in. This
will enable it to be used by audit_autoremove_mark_rule().
RGB: Rename several watch references to mark.
RGB: Rename audit_remove_rule() to audit_autoremove_mark_rule().
RGB: Put audit_alloc_mark() arguments in same order as watch, tree and
inode. RGB: Remove space from audit log value text in
audit_autoremove_mark_rule(). RGB: Explicitly declare prototypes as extern.
RGB: Rename audit_remove_mark_rule() called from audit_mark_handle_event()
to audit_autoremove_mark_rule() to avoid confusion with
audit_remove_{watch,tree}_rule() usage.
RGB: Add audit_remove_mark_rule() to provide similar interface as
audit_remove_{watch,tree}_rule().
RGB: Simplify stubs to defines.
RGB: Rename audit_free_fsnotify_mark() to audit_fsnotify_free_mark() in
keeping with the naming convention of inotify_free_mark(),
dnotify_free_mark(), fanotify_free_mark(), audit_watch_free_mark().
RGB: Return -ENOMEM rather than null in case of memory allocation failure
for audit_mark. RGB: Rename audit_free_mark() to audit_mark_free() to avoid
association with {i,d,fa}notify_free_mark() and audit_watch_free_mark().
Definitely enough changes here to call this your own; credit Eric in the
description and just stick with your sign off.
Done.
Based-on-code-by: Eric Paris epa...@redhat.com
Signed-off-by: Richard Guy Briggs r...@redhat.com
Based on the patch description, should this be patch 1/4 instead of 2/4?
Or 1/2 as you have suggested.
diff --git a/kernel/Makefile b/kernel/Makefile
index a7ea330..397109e 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 audit_exe.o
+obj-$(CONFIG_AUDIT_WATCH) += audit_watch.o audit_exe.o audit_fsnotify.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 3aca24f..491bd4b 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -286,6 +294,20 @@ extern int audit_exe_compare(struct task_struct *tsk,
struct audit_exe *exe); #define audit_watch_path(w)
#define audit_watch_compare(w, i, d) 0
+#define audit_alloc_mark(k, p, l) (ERR_PTR(-EINVAL))
+static inline char *audit_mark_path(struct audit_fsnotify_mark *mark)
+{
+ BUG();
+ return ;
+}
+#define audit_remove_mark(m) BUG()
+#define audit_remove_mark_rule(k) BUG()
+static inline int audit_mark_compare(struct audit_fsnotify_mark *mark,
unsigned long ino, dev_t dev) +{
+ BUG();
+ return 0;
+}
No BUG(), we've got enough of those things already.
Cleaned them out...
diff --git a/kernel/audit_fsnotify.c b/kernel/audit_fsnotify.c
new file mode 100644
index 000..a4e7b16
--- /dev/null
+++ b/kernel/audit_fsnotify.c
@@ -0,0 +1,253 @@
+/* audit_fsnotify.c -- tracking inodes
+ *
+ * Copyright 2003-2009,2014-2015 Red Hat, Inc.
+ * Copyright 2005 Hewlett-Packard Development Company, L.P.
+ * Copyright 2005 IBM Corporation
+ *
+ * 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
Re: [PATCH V6 2/4] audit: clean simple fsnotify implementation
On Tuesday, July 14, 2015 11:50:24 AM Richard Guy Briggs wrote:
> This is to be used to audit by executable rules, but audit watches
> should be able to share this code eventually.
>
> At the moment the audit watch code is a lot more complex, that code only
> creates one fsnotify watch per parent directory. That 'audit_parent' in
> turn has a list of 'audit_watches' which contain the name, ino, dev of
> the specific object we care about. This just creates one fsnotify watch
> per object we care about. So if you watch 100 inodes in /etc this code
> will create 100 fsnotify watches on /etc. The audit_watch code will
> instead create 1 fsnotify watch on /etc (the audit_parent) and then 100
> individual watches chained from that fsnotify mark.
>
> We should be able to convert the audit_watch code to do one fsnotify
> mark per watch and simplify things/remove a whole lot of code. After
> that conversion we should be able to convert the audit_fsnotify code to
> support that hierarchy if the optimization is necessary.
>
> Signed-off-by: Eric Paris
>
> RGB: Move the access to the entry for audit_match_signal() to the beginning
> of the function in case the entry found is the same one passed in. This
> will enable it to be used by audit_autoremove_mark_rule().
> RGB: Rename several "watch" references to "mark".
> RGB: Rename audit_remove_rule() to audit_autoremove_mark_rule().
> RGB: Put audit_alloc_mark() arguments in same order as watch, tree and
> inode. RGB: Remove space from audit log value text in
> audit_autoremove_mark_rule(). RGB: Explicitly declare prototypes as extern.
> RGB: Rename audit_remove_mark_rule() called from audit_mark_handle_event()
> to audit_autoremove_mark_rule() to avoid confusion with
> audit_remove_{watch,tree}_rule() usage.
> RGB: Add audit_remove_mark_rule() to provide similar interface as
> audit_remove_{watch,tree}_rule().
> RGB: Simplify stubs to defines.
> RGB: Rename audit_free_fsnotify_mark() to audit_fsnotify_free_mark() in
> keeping with the naming convention of inotify_free_mark(),
> dnotify_free_mark(), fanotify_free_mark(), audit_watch_free_mark().
> RGB: Return -ENOMEM rather than null in case of memory allocation failure
> for audit_mark. RGB: Rename audit_free_mark() to audit_mark_free() to avoid
> association with {i,d,fa}notify_free_mark() and audit_watch_free_mark().
Definitely enough changes here to call this your own; credit Eric in the
description and just stick with your sign off.
> Based-on-code-by: Eric Paris
> Signed-off-by: Richard Guy Briggs
Based on the patch description, should this be patch 1/4 instead of 2/4?
> diff --git a/kernel/Makefile b/kernel/Makefile
> index a7ea330..397109e 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 audit_exe.o
> +obj-$(CONFIG_AUDIT_WATCH) += audit_watch.o audit_exe.o audit_fsnotify.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 3aca24f..491bd4b 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -286,6 +294,20 @@ extern int audit_exe_compare(struct task_struct *tsk,
> struct audit_exe *exe); #define audit_watch_path(w) ""
> #define audit_watch_compare(w, i, d) 0
>
> +#define audit_alloc_mark(k, p, l) (ERR_PTR(-EINVAL))
> +static inline char *audit_mark_path(struct audit_fsnotify_mark *mark)
> +{
> + BUG();
> + return "";
> +}
> +#define audit_remove_mark(m) BUG()
> +#define audit_remove_mark_rule(k) BUG()
> +static inline int audit_mark_compare(struct audit_fsnotify_mark *mark,
> unsigned long ino, dev_t dev) +{
> + BUG();
> + return 0;
> +}
No BUG(), we've got enough of those things already.
> diff --git a/kernel/audit_fsnotify.c b/kernel/audit_fsnotify.c
> new file mode 100644
> index 000..a4e7b16
> --- /dev/null
> +++ b/kernel/audit_fsnotify.c
> @@ -0,0 +1,253 @@
> +/* audit_fsnotify.c -- tracking inodes
> + *
> + * Copyright 2003-2009,2014-2015 Red Hat, Inc.
> + * Copyright 2005 Hewlett-Packard Development Company, L.P.
> + * Copyright 2005 IBM Corporation
> + *
> + * 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.
> + */
...
> +static void audit_fsnotify_free_mark(struct fsnotify_mark *mark)
> +{
> + struct
Re: [PATCH V6 2/4] audit: clean simple fsnotify implementation
On Tuesday, July 14, 2015 11:50:24 AM Richard Guy Briggs wrote:
This is to be used to audit by executable rules, but audit watches
should be able to share this code eventually.
At the moment the audit watch code is a lot more complex, that code only
creates one fsnotify watch per parent directory. That 'audit_parent' in
turn has a list of 'audit_watches' which contain the name, ino, dev of
the specific object we care about. This just creates one fsnotify watch
per object we care about. So if you watch 100 inodes in /etc this code
will create 100 fsnotify watches on /etc. The audit_watch code will
instead create 1 fsnotify watch on /etc (the audit_parent) and then 100
individual watches chained from that fsnotify mark.
We should be able to convert the audit_watch code to do one fsnotify
mark per watch and simplify things/remove a whole lot of code. After
that conversion we should be able to convert the audit_fsnotify code to
support that hierarchy if the optimization is necessary.
Signed-off-by: Eric Paris epa...@redhat.com
RGB: Move the access to the entry for audit_match_signal() to the beginning
of the function in case the entry found is the same one passed in. This
will enable it to be used by audit_autoremove_mark_rule().
RGB: Rename several watch references to mark.
RGB: Rename audit_remove_rule() to audit_autoremove_mark_rule().
RGB: Put audit_alloc_mark() arguments in same order as watch, tree and
inode. RGB: Remove space from audit log value text in
audit_autoremove_mark_rule(). RGB: Explicitly declare prototypes as extern.
RGB: Rename audit_remove_mark_rule() called from audit_mark_handle_event()
to audit_autoremove_mark_rule() to avoid confusion with
audit_remove_{watch,tree}_rule() usage.
RGB: Add audit_remove_mark_rule() to provide similar interface as
audit_remove_{watch,tree}_rule().
RGB: Simplify stubs to defines.
RGB: Rename audit_free_fsnotify_mark() to audit_fsnotify_free_mark() in
keeping with the naming convention of inotify_free_mark(),
dnotify_free_mark(), fanotify_free_mark(), audit_watch_free_mark().
RGB: Return -ENOMEM rather than null in case of memory allocation failure
for audit_mark. RGB: Rename audit_free_mark() to audit_mark_free() to avoid
association with {i,d,fa}notify_free_mark() and audit_watch_free_mark().
Definitely enough changes here to call this your own; credit Eric in the
description and just stick with your sign off.
Based-on-code-by: Eric Paris epa...@redhat.com
Signed-off-by: Richard Guy Briggs r...@redhat.com
Based on the patch description, should this be patch 1/4 instead of 2/4?
diff --git a/kernel/Makefile b/kernel/Makefile
index a7ea330..397109e 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 audit_exe.o
+obj-$(CONFIG_AUDIT_WATCH) += audit_watch.o audit_exe.o audit_fsnotify.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 3aca24f..491bd4b 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -286,6 +294,20 @@ extern int audit_exe_compare(struct task_struct *tsk,
struct audit_exe *exe); #define audit_watch_path(w)
#define audit_watch_compare(w, i, d) 0
+#define audit_alloc_mark(k, p, l) (ERR_PTR(-EINVAL))
+static inline char *audit_mark_path(struct audit_fsnotify_mark *mark)
+{
+ BUG();
+ return ;
+}
+#define audit_remove_mark(m) BUG()
+#define audit_remove_mark_rule(k) BUG()
+static inline int audit_mark_compare(struct audit_fsnotify_mark *mark,
unsigned long ino, dev_t dev) +{
+ BUG();
+ return 0;
+}
No BUG(), we've got enough of those things already.
diff --git a/kernel/audit_fsnotify.c b/kernel/audit_fsnotify.c
new file mode 100644
index 000..a4e7b16
--- /dev/null
+++ b/kernel/audit_fsnotify.c
@@ -0,0 +1,253 @@
+/* audit_fsnotify.c -- tracking inodes
+ *
+ * Copyright 2003-2009,2014-2015 Red Hat, Inc.
+ * Copyright 2005 Hewlett-Packard Development Company, L.P.
+ * Copyright 2005 IBM Corporation
+ *
+ * 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.
+ */
...
+static void audit_fsnotify_free_mark(struct fsnotify_mark *mark)
+{
+ struct audit_fsnotify_mark *audit_mark;
+
+ audit_mark = container_of(mark,
