From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753550AbbHGChW (ORCPT ); Thu, 6 Aug 2015 22:37:22 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37455 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751541AbbHGChM (ORCPT ); Thu, 6 Aug 2015 22:37:12 -0400 From: Paul Moore To: Richard Guy Briggs Cc: linux-audit@redhat.com, linux-kernel@vger.kernel.org, sgrubb@redhat.com, eparis@redhat.com, peter@hda3.com Subject: Re: [PATCH V9 1/3] audit: clean simple fsnotify implementation Date: Thu, 06 Aug 2015 16:19:45 -0400 Message-ID: <3291294.XmtiFPecW4@sifl> Organization: Red Hat User-Agent: KMail/4.14.10 (Linux/4.1.2-gentoo; KDE/4.14.10; x86_64; ; ) In-Reply-To: <61a913a61d7790e8ada0e17dcc4b4af6e558f0ab.1438801342.git.rgb@redhat.com> References: <61a913a61d7790e8ada0e17dcc4b4af6e558f0ab.1438801342.git.rgb@redhat.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wednesday, August 05, 2015 04:29:36 PM Richard Guy Briggs wrote: > This is to be used to audit by executable path 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. > > Move the access to the entry for audit_match_signal() to the beginning of > the audit_del_rule() function in case the entry found is the same one passed > in. This will enable it to be used by audit_autoremove_mark_rule(), > kill_rules() and audit_remove_parent_watches(). > > This is a heavily modified and merged version of two patches originally > submitted by Eric Paris. > > Cc: Peter Moody > Cc: Eric Paris > Signed-off-by: Richard Guy Briggs > --- > kernel/Makefile | 2 +- > kernel/audit.h | 14 +++ > kernel/audit_fsnotify.c | 215 +++++++++++++++++++++++++++++++++++++++++++ > kernel/auditfilter.c | 2 +- > 4 files changed, 231 insertions(+), 2 deletions(-) > create mode 100644 kernel/audit_fsnotify.c Merged, although I had to add a line of whitespace to keep checkpatch happy. > diff --git a/kernel/Makefile b/kernel/Makefile > index 60c302c..58b5b52 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_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 d641f9b..46d10dd 100644 > --- a/kernel/audit.h > +++ b/kernel/audit.h > @@ -50,6 +50,7 @@ enum audit_state { > > /* Rule lists */ > struct audit_watch; > +struct audit_fsnotify_mark; > struct audit_tree; > struct audit_chunk; > > @@ -252,6 +253,7 @@ struct audit_net { > extern int selinux_audit_rule_update(void); > > extern struct mutex audit_filter_mutex; > +extern int audit_del_rule(struct audit_entry *); > extern void audit_free_rule_rcu(struct rcu_head *); > extern struct list_head audit_filter_list[]; > > @@ -269,6 +271,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 struct audit_fsnotify_mark *audit_alloc_mark(struct audit_krule > *krule, char *pathname, int len); +extern char *audit_mark_path(struct > audit_fsnotify_mark *mark); > +extern void audit_remove_mark(struct audit_fsnotify_mark *audit_mark); > +extern void audit_remove_mark_rule(struct audit_krule *krule); > +extern int audit_mark_compare(struct audit_fsnotify_mark *mark, unsigned > long ino, dev_t dev); + > #else > #define audit_put_watch(w) {} > #define audit_get_watch(w) {} > @@ -278,6 +287,11 @@ 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 > > +#define audit_alloc_mark(k, p, l) (ERR_PTR(-EINVAL)) > +#define audit_mark_path(m) "" > +#define audit_remove_mark(m) > +#define audit_remove_mark_rule(k) > +#define audit_mark_compare(m, i, d) 0 > #endif /* CONFIG_AUDIT_WATCH */ > > #ifdef CONFIG_AUDIT_TREE > diff --git a/kernel/audit_fsnotify.c b/kernel/audit_fsnotify.c > new file mode 100644 > index 0000000..654c6c7 > --- /dev/null > +++ b/kernel/audit_fsnotify.c > @@ -0,0 +1,215 @@ > +/* 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. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include "audit.h" > + > +/* > + * this mark lives on the parent directory of the inode in question. > + * but dev, ino, and path are about the child > + */ > +struct audit_fsnotify_mark { > + dev_t dev; /* associated superblock device */ > + unsigned long ino; /* associated inode number */ > + char *path; /* insertion path */ > + struct fsnotify_mark mark; /* fsnotify mark on the inode */ > + struct audit_krule *rule; > +}; > + > +/* fsnotify handle. */ > +static struct fsnotify_group *audit_fsnotify_group; > + > +/* fsnotify events we care about. */ > +#define AUDIT_FS_EVENTS (FS_MOVE | FS_CREATE | FS_DELETE | FS_DELETE_SELF > |\ + FS_MOVE_SELF | FS_EVENT_ON_CHILD) > + > +static void audit_fsnotify_mark_free(struct audit_fsnotify_mark > *audit_mark) +{ > + kfree(audit_mark->path); > + kfree(audit_mark); > +} > + > +static void audit_fsnotify_free_mark(struct fsnotify_mark *mark) > +{ > + struct audit_fsnotify_mark *audit_mark; > + > + audit_mark = container_of(mark, struct audit_fsnotify_mark, mark); > + audit_fsnotify_mark_free(audit_mark); > +} > + > +char *audit_mark_path(struct audit_fsnotify_mark *mark) > +{ > + return mark->path; > +} > + > +int audit_mark_compare(struct audit_fsnotify_mark *mark, unsigned long ino, > dev_t dev) +{ > + if (mark->ino == AUDIT_INO_UNSET) > + return 0; > + return (mark->ino == ino) && (mark->dev == dev); > +} > + > +static void audit_update_mark(struct audit_fsnotify_mark *audit_mark, > + struct inode *inode) > +{ > + audit_mark->dev = inode ? inode->i_sb->s_dev : AUDIT_DEV_UNSET; > + audit_mark->ino = inode ? inode->i_ino : AUDIT_INO_UNSET; > +} > + > +struct audit_fsnotify_mark *audit_alloc_mark(struct audit_krule *krule, > char *pathname, int len) +{ > + struct audit_fsnotify_mark *audit_mark; > + struct path path; > + struct dentry *dentry; > + struct inode *inode; > + int ret; > + > + if (pathname[0] != '/' || pathname[len-1] == '/') > + return ERR_PTR(-EINVAL); > + > + dentry = kern_path_locked(pathname, &path); > + if (IS_ERR(dentry)) > + return (void *)dentry; /* returning an error */ > + inode = path.dentry->d_inode; > + mutex_unlock(&inode->i_mutex); > + > + audit_mark = kzalloc(sizeof(*audit_mark), GFP_KERNEL); > + if (unlikely(!audit_mark)) { > + audit_mark = ERR_PTR(-ENOMEM); > + goto out; > + } > + > + fsnotify_init_mark(&audit_mark->mark, audit_fsnotify_free_mark); > + audit_mark->mark.mask = AUDIT_FS_EVENTS; > + audit_mark->path = pathname; > + audit_update_mark(audit_mark, dentry->d_inode); > + audit_mark->rule = krule; > + > + ret = fsnotify_add_mark(&audit_mark->mark, audit_fsnotify_group, inode, > NULL, true); + if (ret < 0) { > + audit_fsnotify_mark_free(audit_mark); > + audit_mark = ERR_PTR(ret); > + } > +out: > + dput(dentry); > + path_put(&path); > + return audit_mark; > +} > + > +static void audit_mark_log_rule_change(struct audit_fsnotify_mark > *audit_mark, char *op) +{ > + struct audit_buffer *ab; > + struct audit_krule *rule = audit_mark->rule; > + if (!audit_enabled) > + return; > + ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE); > + if (unlikely(!ab)) > + return; > + audit_log_format(ab, "auid=%u ses=%u op=", > + from_kuid(&init_user_ns, audit_get_loginuid(current)), > + audit_get_sessionid(current)); > + audit_log_string(ab, op); > + audit_log_format(ab, " path="); > + audit_log_untrustedstring(ab, audit_mark->path); > + audit_log_key(ab, rule->filterkey); > + audit_log_format(ab, " list=%d res=1", rule->listnr); > + audit_log_end(ab); > +} > + > +void audit_remove_mark(struct audit_fsnotify_mark *audit_mark) > +{ > + fsnotify_destroy_mark(&audit_mark->mark, audit_fsnotify_group); > + fsnotify_put_mark(&audit_mark->mark); > +} > + > +void audit_remove_mark_rule(struct audit_krule *krule) > +{ > + struct audit_fsnotify_mark *mark = krule->exe; > + > + audit_remove_mark(mark); > +} > + > +static void audit_autoremove_mark_rule(struct audit_fsnotify_mark > *audit_mark) +{ > + struct audit_krule *rule = audit_mark->rule; > + struct audit_entry *entry = container_of(rule, struct audit_entry, rule); > + > + audit_mark_log_rule_change(audit_mark, "autoremove_rule"); > + audit_del_rule(entry); > +} > + > +/* Update mark data in audit rules based on fsnotify events. */ > +static int audit_mark_handle_event(struct fsnotify_group *group, > + struct inode *to_tell, > + struct fsnotify_mark *inode_mark, > + struct fsnotify_mark *vfsmount_mark, > + u32 mask, void *data, int data_type, > + const unsigned char *dname, u32 cookie) > +{ > + struct audit_fsnotify_mark *audit_mark; > + struct inode *inode = NULL; > + > + audit_mark = container_of(inode_mark, struct audit_fsnotify_mark, mark); > + > + BUG_ON(group != audit_fsnotify_group); > + > + switch (data_type) { > + case (FSNOTIFY_EVENT_PATH): > + inode = ((struct path *)data)->dentry->d_inode; > + break; > + case (FSNOTIFY_EVENT_INODE): > + inode = (struct inode *)data; > + break; > + default: > + BUG(); > + return 0; > + }; > + > + if (mask & (FS_CREATE|FS_MOVED_TO|FS_DELETE|FS_MOVED_FROM)) { > + if (audit_compare_dname_path(dname, audit_mark->path, AUDIT_NAME_FULL)) > + return 0; > + audit_update_mark(audit_mark, inode); > + } else if (mask & (FS_DELETE_SELF|FS_UNMOUNT|FS_MOVE_SELF)) > + audit_autoremove_mark_rule(audit_mark); > + > + return 0; > +} > + > +static const struct fsnotify_ops audit_mark_fsnotify_ops = { > + .handle_event = audit_mark_handle_event, > +}; > + > +static int __init audit_fsnotify_init(void) > +{ > + audit_fsnotify_group = fsnotify_alloc_group(&audit_mark_fsnotify_ops); > + if (IS_ERR(audit_fsnotify_group)) { > + audit_fsnotify_group = NULL; > + audit_panic("cannot create audit fsnotify group"); > + } > + return 0; > +} > +device_initcall(audit_fsnotify_init); > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c > index 018719a..3d99196 100644 > --- a/kernel/auditfilter.c > +++ b/kernel/auditfilter.c > @@ -935,7 +935,7 @@ static inline int audit_add_rule(struct audit_entry > *entry) } > > /* Remove an existing rule from filterlist. */ > -static inline int audit_del_rule(struct audit_entry *entry) > +int audit_del_rule(struct audit_entry *entry) > { > struct audit_entry *e; > struct audit_tree *tree = entry->rule.tree; -- paul moore security @ redhat