11:35 a.m.
I recently posted a set of patches to have the kernel retry the lookup
and call when path-based syscalls would ordinarily return ESTALE. Al
took a look and pointed out that this would break the fragile logic that
handles the audit_names for syscall auditing.
This patchset comprises a number of incremental changes that should make
it ok to retry on a path-based syscall. The main caveat is that the retry
mustn't redo the getname() on the strings involved.
Unfortunately, we don't have anything that really describes what the
correct behavior is for this stuff, so I'm shooting here for "no
discernable difference" on a retry.
This seems to do the right thing in the cases that I've tested; mostly
the normal case where things succeed or fail for some reason and where
the syscall is retried after an ESTALE error.
Review is of course appreciated. There are some fixes in here too for
some subtle bugs in the existing code. Some of these patches may also
help performance in some cases, but I haven't measured it for that.
I'd like to see this patchset go into 3.6 if at all possible.
Eric Paris (1):
audit: make audit_compare_dname_path use parent_len helper
Jeff Layton (8):
audit: remove unnecessary NULL ptr checks from do_path_lookup
audit: pass in dentry to audit_copy_inode wherever possible
audit: reverse arguments to audit_inode_child
audit: add a new "type" field to audit_names struct
audit: set the name_len in audit_inode for parent lookups
audit: remove dirlen argument to audit_compare_dname_path
audit: optimize audit_compare_dname_path
audit: overhaul __audit_inode_child to accomodate retrying
fs/btrfs/ioctl.c | 2 +-
fs/namei.c | 20 +++-----
fs/open.c | 4 +-
fs/xattr.c | 8 ++--
include/linux/audit.h | 36 ++++++++++-----
include/linux/fsnotify.h | 8 ++--
ipc/mqueue.c | 8 ++--
kernel/audit.h | 7 ++-
kernel/audit_watch.c | 3 +-
kernel/auditfilter.c | 65 +++++++++++++++++---------
kernel/auditsc.c | 115 +++++++++++++++++++++++++++------------------
11 files changed, 165 insertions(+), 111 deletions(-)
--
1.7.7.6
11:35 a.m.
New subject: [PATCH v4 1/9] audit: remove unnecessary NULL ptr checks from do_path_lookup
As best I can tell, whenever retval == 0, nd->path.dentry and nd->inode
are also non-NULL. Eliminate those checks and the superfluous
audit_context check.
Signed-off-by: Eric Paris <eparis(a)redhat.com>
Signed-off-by: Jeff Layton <jlayton(a)redhat.com>
---
fs/namei.c | 8 ++------
1 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 7d69419..5f6a34d 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1812,12 +1812,8 @@ static int do_path_lookup(int dfd, const char *name,
if (unlikely(retval == -ESTALE))
retval = path_lookupat(dfd, name, flags | LOOKUP_REVAL, nd);
- if (likely(!retval)) {
- if (unlikely(!audit_dummy_context())) {
- if (nd->path.dentry && nd->inode)
- audit_inode(name, nd->path.dentry);
- }
- }
+ if (likely(!retval))
+ audit_inode(name, nd->path.dentry);
return retval;
}
--
1.7.7.6
11:35 a.m.
New subject: [PATCH v4 2/9] audit: pass in dentry to audit_copy_inode wherever possible
In some cases, we were passing in NULL even when we have a dentry.
Reported-by: Eric Paris <eparis(a)redhat.com>
Signed-off-by: Jeff Layton <jlayton(a)redhat.com>
---
kernel/auditsc.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 4b96415..5c45b9b 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2226,7 +2226,7 @@ void __audit_inode_child(const struct dentry *dentry,
if (!strcmp(dname, n->name) ||
!audit_compare_dname_path(dname, n->name, &dirlen)) {
if (inode)
- audit_copy_inode(n, NULL, inode);
+ audit_copy_inode(n, dentry, inode);
else
n->ino = (unsigned long)-1;
found_child = n->name;
@@ -2258,7 +2258,7 @@ add_names:
}
if (inode)
- audit_copy_inode(n, NULL, inode);
+ audit_copy_inode(n, dentry, inode);
}
}
EXPORT_SYMBOL_GPL(__audit_inode_child);
--
1.7.7.6
11:35 a.m.
New subject: [PATCH v4 3/9] audit: reverse arguments to audit_inode_child
Most of the callers get called with an inode and dentry in the reverse
order. The compiler then has to reshuffle the arg registers and/or
stack in order to pass them on to audit_inode_child.
Reverse those arguments for a micro-optimization.
Reported-by: Eric Paris <eparis(a)redhat.com>
Signed-off-by: Jeff Layton <jlayton(a)redhat.com>
---
fs/btrfs/ioctl.c | 2 +-
fs/namei.c | 2 +-
include/linux/audit.h | 14 +++++++-------
include/linux/fsnotify.h | 8 ++++----
kernel/auditsc.c | 8 ++++----
5 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 0e92e57..90c67b0 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -592,7 +592,7 @@ static int btrfs_may_delete(struct inode *dir,struct dentry
*victim,int isdir)
return -ENOENT;
BUG_ON(victim->d_parent->d_inode != dir);
- audit_inode_child(victim, dir);
+ audit_inode_child(dir, victim);
error = inode_permission(dir, MAY_WRITE | MAY_EXEC);
if (error)
diff --git a/fs/namei.c b/fs/namei.c
index 5f6a34d..8539f8f 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1998,7 +1998,7 @@ static int may_delete(struct inode *dir,struct dentry *victim,int
isdir)
return -ENOENT;
BUG_ON(victim->d_parent->d_inode != dir);
- audit_inode_child(victim, dir);
+ audit_inode_child(dir, victim);
error = inode_permission(dir, MAY_WRITE | MAY_EXEC);
if (error)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 22f292a..51eca54 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -461,8 +461,8 @@ extern void __audit_syscall_exit(int ret_success, long ret_value);
extern void __audit_getname(const char *name);
extern void audit_putname(const char *name);
extern void __audit_inode(const char *name, const struct dentry *dentry);
-extern void __audit_inode_child(const struct dentry *dentry,
- const struct inode *parent);
+extern void __audit_inode_child(const struct inode *parent,
+ const struct dentry *dentry);
extern void __audit_seccomp(unsigned long syscall, long signr, int code);
extern void __audit_ptrace(struct task_struct *t);
@@ -501,10 +501,10 @@ static inline void audit_inode(const char *name, const struct dentry
*dentry) {
if (unlikely(!audit_dummy_context()))
__audit_inode(name, dentry);
}
-static inline void audit_inode_child(const struct dentry *dentry,
- const struct inode *parent) {
+static inline void audit_inode_child(const struct inode *parent,
+ const struct dentry *dentry) {
if (unlikely(!audit_dummy_context()))
- __audit_inode_child(dentry, parent);
+ __audit_inode_child(parent, dentry);
}
void audit_core_dumps(long signr);
@@ -630,9 +630,9 @@ extern int audit_signals;
#define audit_getname(n) do { ; } while (0)
#define audit_putname(n) do { ; } while (0)
#define __audit_inode(n,d) do { ; } while (0)
-#define __audit_inode_child(i,p) do { ; } while (0)
+#define __audit_inode_child(p,d) do { ; } while (0)
#define audit_inode(n,d) do { (void)(d); } while (0)
-#define audit_inode_child(i,p) do { ; } while (0)
+#define audit_inode_child(p,d) do { ; } while (0)
#define audit_core_dumps(i) do { ; } while (0)
#define audit_seccomp(i,s,c) do { ; } while (0)
#define auditsc_get_stamp(c,t,s) (0)
diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
index a6dfe69..9c28471 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -109,7 +109,7 @@ static inline void fsnotify_move(struct inode *old_dir, struct inode
*new_dir,
if (source)
fsnotify(source, FS_MOVE_SELF, moved->d_inode, FSNOTIFY_EVENT_INODE, NULL, 0);
- audit_inode_child(moved, new_dir);
+ audit_inode_child(new_dir, moved);
}
/*
@@ -155,7 +155,7 @@ static inline void fsnotify_inoderemove(struct inode *inode)
*/
static inline void fsnotify_create(struct inode *inode, struct dentry *dentry)
{
- audit_inode_child(dentry, inode);
+ audit_inode_child(inode, dentry);
fsnotify(inode, FS_CREATE, dentry->d_inode, FSNOTIFY_EVENT_INODE,
dentry->d_name.name, 0);
}
@@ -168,7 +168,7 @@ static inline void fsnotify_create(struct inode *inode, struct dentry
*dentry)
static inline void fsnotify_link(struct inode *dir, struct inode *inode, struct dentry
*new_dentry)
{
fsnotify_link_count(inode);
- audit_inode_child(new_dentry, dir);
+ audit_inode_child(dir, new_dentry);
fsnotify(dir, FS_CREATE, inode, FSNOTIFY_EVENT_INODE, new_dentry->d_name.name, 0);
}
@@ -181,7 +181,7 @@ static inline void fsnotify_mkdir(struct inode *inode, struct dentry
*dentry)
__u32 mask = (FS_CREATE | FS_ISDIR);
struct inode *d_inode = dentry->d_inode;
- audit_inode_child(dentry, inode);
+ audit_inode_child(inode, dentry);
fsnotify(inode, mask, d_inode, FSNOTIFY_EVENT_INODE, dentry->d_name.name, 0);
}
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 5c45b9b..12b007b 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2176,9 +2176,9 @@ out:
}
/**
- * audit_inode_child - collect inode info for created/removed objects
- * @dentry: dentry being audited
+ * __audit_inode_child - collect inode info for created/removed objects
* @parent: inode of dentry parent
+ * @dentry: dentry being audited
*
* For syscalls that create or remove filesystem objects, audit_inode
* can only collect information for the filesystem object's parent.
@@ -2188,8 +2188,8 @@ out:
* must be hooked prior, in order to capture the target inode during
* unsuccessful attempts.
*/
-void __audit_inode_child(const struct dentry *dentry,
- const struct inode *parent)
+void __audit_inode_child(const struct inode *parent,
+ const struct dentry *dentry)
{
struct audit_context *context = current->audit_context;
const char *found_parent = NULL, *found_child = NULL;
--
1.7.7.6
11:35 a.m.
New subject: [PATCH v4 4/9] audit: add a new "type" field to audit_names struct
For now, we just have two possibilities:
UNKNOWN: for a new audit_names record that we don't know anything about yet
NORMAL: for everything else
In later patches, we'll add other types so we can distinguish and update
records created under different circumstances.
Signed-off-by: Jeff Layton <jlayton(a)redhat.com>
---
include/linux/audit.h | 5 +++++
kernel/auditsc.c | 15 ++++++++++-----
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 51eca54..7acbfc8 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -449,6 +449,11 @@ struct audit_field {
extern int __init audit_register_class(int class, unsigned *list);
extern int audit_classify_syscall(int abi, unsigned syscall);
extern int audit_classify_arch(int arch);
+
+/* audit_names->type values */
+#define AUDIT_TYPE_UNKNOWN 0 /* we don't know yet */
+#define AUDIT_TYPE_NORMAL 1 /* a "normal" audit record */
+
#ifdef CONFIG_AUDITSYSCALL
/* These are defined in auditsc.c */
/* Public API */
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 12b007b..21c4223 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -120,6 +120,7 @@ struct audit_names {
struct audit_cap_data fcap;
unsigned int fcap_ver;
int name_len; /* number of name's characters to log */
+ unsigned char type; /* record type */
bool name_put; /* call __putname() for this name */
/*
* This was an allocated audit_names and not from the array of
@@ -2009,7 +2010,8 @@ retry:
#endif
}
-static struct audit_names *audit_alloc_name(struct audit_context *context)
+static struct audit_names *audit_alloc_name(struct audit_context *context,
+ unsigned char type)
{
struct audit_names *aname;
@@ -2024,6 +2026,7 @@ static struct audit_names *audit_alloc_name(struct audit_context
*context)
}
aname->ino = (unsigned long)-1;
+ aname->type = type;
list_add_tail(&aname->list, &context->names_list);
context->name_count++;
@@ -2054,7 +2057,7 @@ void __audit_getname(const char *name)
return;
}
- n = audit_alloc_name(context);
+ n = audit_alloc_name(context, AUDIT_TYPE_UNKNOWN);
if (!n)
return;
@@ -2167,12 +2170,13 @@ void __audit_inode(const char *name, const struct dentry *dentry)
}
/* unable to find the name from a previous getname() */
- n = audit_alloc_name(context);
+ n = audit_alloc_name(context, AUDIT_TYPE_NORMAL);
if (!n)
return;
out:
handle_path(dentry);
audit_copy_inode(n, dentry, inode);
+ n->type = AUDIT_TYPE_NORMAL;
}
/**
@@ -2229,6 +2233,7 @@ void __audit_inode_child(const struct inode *parent,
audit_copy_inode(n, dentry, inode);
else
n->ino = (unsigned long)-1;
+ n->type = AUDIT_TYPE_NORMAL;
found_child = n->name;
goto add_names;
}
@@ -2236,14 +2241,14 @@ void __audit_inode_child(const struct inode *parent,
add_names:
if (!found_parent) {
- n = audit_alloc_name(context);
+ n = audit_alloc_name(context, AUDIT_TYPE_NORMAL);
if (!n)
return;
audit_copy_inode(n, NULL, parent);
}
if (!found_child) {
- n = audit_alloc_name(context);
+ n = audit_alloc_name(context, AUDIT_TYPE_NORMAL);
if (!n)
return;
--
1.7.7.6
11:35 a.m.
New subject: [PATCH v4 5/9] audit: set the name_len in audit_inode for parent lookups
Currently, this gets set mostly by happenstance when we call into
audit_inode_child. While that might be a little more efficient, it seems
wrong. If the syscall ends up failing before audit_inode_child ever gets
called, then you'll have an audit_names record that shows the full path
but has the parent inode info attached.
Fix this by passing in a parent flag when we call audit_inode that gets
set to the value of LOOKUP_PARENT. We can then fix up the pathname for
the audit entry correctly from the get-go.
Signed-off-by: Jeff Layton <jlayton(a)redhat.com>
---
fs/namei.c | 12 ++++++------
fs/open.c | 4 ++--
fs/xattr.c | 8 ++++----
include/linux/audit.h | 13 ++++++++-----
ipc/mqueue.c | 8 ++++----
kernel/audit.h | 1 +
kernel/auditfilter.c | 30 ++++++++++++++++++++++++++++++
kernel/auditsc.c | 41 +++++++++++++++++++++++++++++------------
8 files changed, 84 insertions(+), 33 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 8539f8f..8c1b3cd 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1813,7 +1813,7 @@ static int do_path_lookup(int dfd, const char *name,
retval = path_lookupat(dfd, name, flags | LOOKUP_REVAL, nd);
if (likely(!retval))
- audit_inode(name, nd->path.dentry);
+ audit_inode(name, nd->path.dentry, flags & LOOKUP_PARENT);
return retval;
}
@@ -2216,7 +2216,7 @@ static struct file *do_last(struct nameidata *nd, struct path
*path,
error = complete_walk(nd);
if (error)
return ERR_PTR(error);
- audit_inode(pathname, nd->path.dentry);
+ audit_inode(pathname, nd->path.dentry, 0);
if (open_flag & O_CREAT) {
error = -EISDIR;
goto exit;
@@ -2226,7 +2226,7 @@ static struct file *do_last(struct nameidata *nd, struct path
*path,
error = complete_walk(nd);
if (error)
return ERR_PTR(error);
- audit_inode(pathname, dir);
+ audit_inode(pathname, dir, 0);
goto ok;
}
@@ -2259,7 +2259,7 @@ static struct file *do_last(struct nameidata *nd, struct path
*path,
if (error)
return ERR_PTR(error);
- audit_inode(pathname, dir);
+ audit_inode(pathname, dir, 0);
error = -EISDIR;
/* trailing slashes? */
if (nd->last.name[nd->last.len])
@@ -2314,7 +2314,7 @@ retry_lookup:
* It already exists.
*/
mutex_unlock(&dir->d_inode->i_mutex);
- audit_inode(pathname, path->dentry);
+ audit_inode(pathname, path->dentry, 0);
error = -EEXIST;
if (open_flag & O_EXCL)
@@ -2369,7 +2369,7 @@ finish_lookup:
error = -ENOTDIR;
if ((nd->flags & LOOKUP_DIRECTORY) && !nd->inode->i_op->lookup)
goto exit;
- audit_inode(pathname, nd->path.dentry);
+ audit_inode(pathname, nd->path.dentry, 0);
ok:
if (!S_ISREG(nd->inode->i_mode))
will_truncate = 0;
diff --git a/fs/open.c b/fs/open.c
index d6c79a0..5af0872 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -476,7 +476,7 @@ SYSCALL_DEFINE2(fchmod, unsigned int, fd, umode_t, mode)
file = fget(fd);
if (file) {
- audit_inode(NULL, file->f_path.dentry);
+ audit_inode(NULL, file->f_path.dentry, 0);
err = chmod_common(&file->f_path, mode);
fput(file);
}
@@ -616,7 +616,7 @@ SYSCALL_DEFINE3(fchown, unsigned int, fd, uid_t, user, gid_t, group)
if (error)
goto out_fput;
dentry = file->f_path.dentry;
- audit_inode(NULL, dentry);
+ audit_inode(NULL, dentry, 0);
error = chown_common(&file->f_path, user, group);
mnt_drop_write_file(file);
out_fput:
diff --git a/fs/xattr.c b/fs/xattr.c
index 1d7ac37..9d25532 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -408,7 +408,7 @@ SYSCALL_DEFINE5(fsetxattr, int, fd, const char __user *, name,
if (!f)
return error;
dentry = f->f_path.dentry;
- audit_inode(NULL, dentry);
+ audit_inode(NULL, dentry, 0);
error = mnt_want_write_file(f);
if (!error) {
error = setxattr(dentry, name, value, size, flags);
@@ -494,7 +494,7 @@ SYSCALL_DEFINE4(fgetxattr, int, fd, const char __user *, name,
f = fget_light(fd, &fput_needed);
if (!f)
return error;
- audit_inode(NULL, f->f_path.dentry);
+ audit_inode(NULL, f->f_path.dentry, 0);
error = getxattr(f->f_path.dentry, name, value, size);
fput_light(f, fput_needed);
return error;
@@ -575,7 +575,7 @@ SYSCALL_DEFINE3(flistxattr, int, fd, char __user *, list, size_t,
size)
f = fget_light(fd, &fput_needed);
if (!f)
return error;
- audit_inode(NULL, f->f_path.dentry);
+ audit_inode(NULL, f->f_path.dentry, 0);
error = listxattr(f->f_path.dentry, list, size);
fput_light(f, fput_needed);
return error;
@@ -646,7 +646,7 @@ SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
if (!f)
return error;
dentry = f->f_path.dentry;
- audit_inode(NULL, dentry);
+ audit_inode(NULL, dentry, 0);
error = mnt_want_write_file(f);
if (!error) {
error = removexattr(dentry, name);
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 7acbfc8..2fc6831 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -453,6 +453,7 @@ extern int audit_classify_arch(int arch);
/* audit_names->type values */
#define AUDIT_TYPE_UNKNOWN 0 /* we don't know yet */
#define AUDIT_TYPE_NORMAL 1 /* a "normal" audit record */
+#define AUDIT_TYPE_PARENT 2 /* a parent audit record */
#ifdef CONFIG_AUDITSYSCALL
/* These are defined in auditsc.c */
@@ -465,7 +466,8 @@ extern void __audit_syscall_entry(int arch,
extern void __audit_syscall_exit(int ret_success, long ret_value);
extern void __audit_getname(const char *name);
extern void audit_putname(const char *name);
-extern void __audit_inode(const char *name, const struct dentry *dentry);
+extern void __audit_inode(const char *name, const struct dentry *dentry,
+ unsigned int parent);
extern void __audit_inode_child(const struct inode *parent,
const struct dentry *dentry);
extern void __audit_seccomp(unsigned long syscall, long signr, int code);
@@ -502,9 +504,10 @@ static inline void audit_getname(const char *name)
if (unlikely(!audit_dummy_context()))
__audit_getname(name);
}
-static inline void audit_inode(const char *name, const struct dentry *dentry) {
+static inline void audit_inode(const char *name, const struct dentry *dentry,
+ unsigned int parent) {
if (unlikely(!audit_dummy_context()))
- __audit_inode(name, dentry);
+ __audit_inode(name, dentry, parent);
}
static inline void audit_inode_child(const struct inode *parent,
const struct dentry *dentry) {
@@ -634,9 +637,9 @@ extern int audit_signals;
#define audit_dummy_context() 1
#define audit_getname(n) do { ; } while (0)
#define audit_putname(n) do { ; } while (0)
-#define __audit_inode(n,d) do { ; } while (0)
+#define __audit_inode(n,d,p) do { ; } while (0)
#define __audit_inode_child(p,d) do { ; } while (0)
-#define audit_inode(n,d) do { (void)(d); } while (0)
+#define audit_inode(n,d,p) do { (void)(d); } while (0)
#define audit_inode_child(p,d) do { ; } while (0)
#define audit_core_dumps(i) do { ; } while (0)
#define audit_seccomp(i,s,c) do { ; } while (0)
diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index 8ce5769..8cb676c 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -832,7 +832,7 @@ SYSCALL_DEFINE4(mq_open, const char __user *, u_name, int, oflag,
umode_t, mode,
if (oflag & O_CREAT) {
if (dentry->d_inode) { /* entry already exists */
- audit_inode(name, dentry);
+ audit_inode(name, dentry, 0);
if (oflag & O_EXCL) {
error = -EEXIST;
goto out;
@@ -848,7 +848,7 @@ SYSCALL_DEFINE4(mq_open, const char __user *, u_name, int, oflag,
umode_t, mode,
error = -ENOENT;
goto out;
}
- audit_inode(name, dentry);
+ audit_inode(name, dentry, 0);
filp = do_open(ipc_ns, dentry, oflag);
}
@@ -1007,7 +1007,7 @@ SYSCALL_DEFINE5(mq_timedsend, mqd_t, mqdes, const char __user *,
u_msg_ptr,
goto out_fput;
}
info = MQUEUE_I(inode);
- audit_inode(NULL, filp->f_path.dentry);
+ audit_inode(NULL, filp->f_path.dentry, 0);
if (unlikely(!(filp->f_mode & FMODE_WRITE))) {
ret = -EBADF;
@@ -1124,7 +1124,7 @@ SYSCALL_DEFINE5(mq_timedreceive, mqd_t, mqdes, char __user *,
u_msg_ptr,
goto out_fput;
}
info = MQUEUE_I(inode);
- audit_inode(NULL, filp->f_path.dentry);
+ audit_inode(NULL, filp->f_path.dentry, 0);
if (unlikely(!(filp->f_mode & FMODE_READ))) {
ret = -EBADF;
diff --git a/kernel/audit.h b/kernel/audit.h
index 8167668..276ca88 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -76,6 +76,7 @@ static inline int audit_hash_ino(u32 ino)
extern int audit_match_class(int class, unsigned syscall);
extern int audit_comparator(const u32 left, const u32 op, const u32 right);
+extern int parent_len(const char *path);
extern int audit_compare_dname_path(const char *dname, const char *path,
int *dirlen);
extern struct sk_buff * audit_make_reply(int pid, int seq, int type,
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index a6c3f1a..29b167b 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -1198,6 +1198,36 @@ int audit_comparator(u32 left, u32 op, u32 right)
}
}
+/**
+ * parent_len - find the length of the parent portion of a pathname
+ * @path: pathname of which to determine length
+ */
+int parent_len(const char *path)
+{
+ int plen;
+ const char *p;
+
+ plen = strlen(path);
+
+ if (plen == 0)
+ return plen;
+
+ /* disregard trailing slashes */
+ p = path + plen - 1;
+ while ((*p == '/') && (p > path))
+ p--;
+
+ /* walk backward until we find the next slash or hit beginning */
+ while ((*p != '/') && (p > path))
+ p--;
+
+ /* did we find a slash? Then increment to include it in path */
+ if (*p == '/')
+ p++;
+
+ return p - path;
+}
+
/* Compare given dentry name with last component in given path,
* return of 0 indicates a match. */
int audit_compare_dname_path(const char *dname, const char *path,
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 21c4223..e9f8b60 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2149,13 +2149,13 @@ static void audit_copy_inode(struct audit_names *name, const
struct dentry *dent
}
/**
- * audit_inode - store the inode and device from a lookup
+ * __audit_inode - store the inode and device from a lookup
* @name: name being audited
* @dentry: dentry being audited
- *
- * Called from fs/namei.c:path_lookup().
+ * @parent: does this dentry represent the parent?
*/
-void __audit_inode(const char *name, const struct dentry *dentry)
+void __audit_inode(const char *name, const struct dentry *dentry,
+ unsigned int parent)
{
struct audit_context *context = current->audit_context;
const struct inode *inode = dentry->d_inode;
@@ -2165,18 +2165,37 @@ void __audit_inode(const char *name, const struct dentry *dentry)
return;
list_for_each_entry_reverse(n, &context->names_list, list) {
- if (n->name && (n->name == name))
- goto out;
+ /* does the name pointer match? */
+ if (!n->name || n->name != name)
+ continue;
+
+ /* match the correct record type */
+ if (parent) {
+ if (n->type == AUDIT_TYPE_PARENT ||
+ n->type == AUDIT_TYPE_UNKNOWN)
+ goto out;
+ } else {
+ if (n->type != AUDIT_TYPE_PARENT)
+ goto out;
+ }
}
- /* unable to find the name from a previous getname() */
+ /* unable to find the name from a previous getname(). Allocate a new
+ * anonymous entry.
+ */
n = audit_alloc_name(context, AUDIT_TYPE_NORMAL);
if (!n)
return;
out:
+ if (parent) {
+ n->name_len = n->name ? parent_len(n->name) : AUDIT_NAME_FULL;
+ n->type = AUDIT_TYPE_PARENT;
+ } else {
+ n->name_len = AUDIT_NAME_FULL;
+ n->type = AUDIT_TYPE_NORMAL;
+ }
handle_path(dentry);
audit_copy_inode(n, dentry, inode);
- n->type = AUDIT_TYPE_NORMAL;
}
/**
@@ -2200,7 +2219,6 @@ void __audit_inode_child(const struct inode *parent,
const struct inode *inode = dentry->d_inode;
const char *dname = dentry->d_name.name;
struct audit_names *n;
- int dirlen = 0;
if (!context->in_syscall)
return;
@@ -2214,8 +2232,7 @@ void __audit_inode_child(const struct inode *parent,
continue;
if (n->ino == parent->i_ino &&
- !audit_compare_dname_path(dname, n->name, &dirlen)) {
- n->name_len = dirlen; /* update parent data in place */
+ !audit_compare_dname_path(dname, n->name, NULL)) {
found_parent = n->name;
goto add_names;
}
@@ -2228,7 +2245,7 @@ void __audit_inode_child(const struct inode *parent,
/* strcmp() is the more likely scenario */
if (!strcmp(dname, n->name) ||
- !audit_compare_dname_path(dname, n->name, &dirlen)) {
+ !audit_compare_dname_path(dname, n->name, NULL)) {
if (inode)
audit_copy_inode(n, dentry, inode);
else
--
1.7.7.6
11:35 a.m.
New subject: [PATCH v4 6/9] audit: remove dirlen argument to audit_compare_dname_path
All the callers set this to NULL now.
Signed-off-by: Jeff Layton <jlayton(a)redhat.com>
---
kernel/audit.h | 3 +--
kernel/audit_watch.c | 2 +-
kernel/auditfilter.c | 6 +-----
kernel/auditsc.c | 4 ++--
4 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/kernel/audit.h b/kernel/audit.h
index 276ca88..ee31316 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -77,8 +77,7 @@ static inline int audit_hash_ino(u32 ino)
extern int audit_match_class(int class, unsigned syscall);
extern int audit_comparator(const u32 left, const u32 op, const u32 right);
extern int parent_len(const char *path);
-extern int audit_compare_dname_path(const char *dname, const char *path,
- int *dirlen);
+extern int audit_compare_dname_path(const char *dname, const char *path);
extern struct sk_buff * audit_make_reply(int pid, int seq, int type,
int done, int multi,
const void *payload, int size);
diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index e683869..44f3b31 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -265,7 +265,7 @@ static void audit_update_watch(struct audit_parent *parent,
/* Run all of the watches on this parent looking for the one that
* matches the given dname */
list_for_each_entry_safe(owatch, nextw, &parent->watches, wlist) {
- if (audit_compare_dname_path(dname, owatch->path, NULL))
+ if (audit_compare_dname_path(dname, owatch->path))
continue;
/* If the update involves invalidating rules, do the inode-based
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 29b167b..f9c48d0 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -1230,8 +1230,7 @@ int parent_len(const char *path)
/* Compare given dentry name with last component in given path,
* return of 0 indicates a match. */
-int audit_compare_dname_path(const char *dname, const char *path,
- int *dirlen)
+int audit_compare_dname_path(const char *dname, const char *path)
{
int dlen, plen;
const char *p;
@@ -1260,9 +1259,6 @@ int audit_compare_dname_path(const char *dname, const char *path,
p++;
}
- /* return length of path's directory component */
- if (dirlen)
- *dirlen = p - path;
return strncmp(p, dname, dlen);
}
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index e9f8b60..34d7ab2 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2232,7 +2232,7 @@ void __audit_inode_child(const struct inode *parent,
continue;
if (n->ino == parent->i_ino &&
- !audit_compare_dname_path(dname, n->name, NULL)) {
+ !audit_compare_dname_path(dname, n->name)) {
found_parent = n->name;
goto add_names;
}
@@ -2245,7 +2245,7 @@ void __audit_inode_child(const struct inode *parent,
/* strcmp() is the more likely scenario */
if (!strcmp(dname, n->name) ||
- !audit_compare_dname_path(dname, n->name, NULL)) {
+ !audit_compare_dname_path(dname, n->name)) {
if (inode)
audit_copy_inode(n, dentry, inode);
else
--
1.7.7.6
11:35 a.m.
New subject: [PATCH v4 7/9] audit: make audit_compare_dname_path use parent_len helper
From: Eric Paris <eparis(a)redhat.com>
Signed-off-by: Eric Paris <eparis(a)redhat.com>
---
kernel/auditfilter.c | 27 +++++++--------------------
1 files changed, 7 insertions(+), 20 deletions(-)
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index f9c48d0..f47ba18 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -1232,32 +1232,19 @@ int parent_len(const char *path)
* return of 0 indicates a match. */
int audit_compare_dname_path(const char *dname, const char *path)
{
- int dlen, plen;
+ int dlen, pathlen, parentlen;
const char *p;
- if (!dname || !path)
- return 1;
-
dlen = strlen(dname);
- plen = strlen(path);
- if (plen < dlen)
+ pathlen = strlen(path);
+ if (pathlen < dlen)
return 1;
- /* disregard trailing slashes */
- p = path + plen - 1;
- while ((*p == '/') && (p > path))
- p--;
-
- /* find last path component */
- p = p - dlen + 1;
- if (p < path)
+ parentlen = parent_len(path);
+ if (pathlen - parentlen != dlen)
return 1;
- else if (p > path) {
- if (*--p != '/')
- return 1;
- else
- p++;
- }
+
+ p = path + parentlen;
return strncmp(p, dname, dlen);
}
--
1.7.7.6
11:35 a.m.
New subject: [PATCH v4 8/9] audit: optimize audit_compare_dname_path
In the cases where we already know the length of the parent, pass it as
a parm so we don't need to recompute it. In the cases where we don't
know the length, pass in AUDIT_NAME_FULL (-1) to indicate that it should
be determined.
Signed-off-by: Jeff Layton <jlayton(a)redhat.com>
---
kernel/audit.h | 5 ++++-
kernel/audit_watch.c | 3 ++-
kernel/auditfilter.c | 16 +++++++++++-----
kernel/auditsc.c | 8 +++-----
4 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/kernel/audit.h b/kernel/audit.h
index ee31316..34af33c 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -74,10 +74,13 @@ static inline int audit_hash_ino(u32 ino)
return (ino & (AUDIT_INODE_BUCKETS-1));
}
+/* Indicates that audit should log the full pathname. */
+#define AUDIT_NAME_FULL -1
+
extern int audit_match_class(int class, unsigned syscall);
extern int audit_comparator(const u32 left, const u32 op, const u32 right);
extern int parent_len(const char *path);
-extern int audit_compare_dname_path(const char *dname, const char *path);
+extern int audit_compare_dname_path(const char *dname, const char *path, int plen);
extern struct sk_buff * audit_make_reply(int pid, int seq, int type,
int done, int multi,
const void *payload, int size);
diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index 44f3b31..793d0c8 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -265,7 +265,8 @@ static void audit_update_watch(struct audit_parent *parent,
/* Run all of the watches on this parent looking for the one that
* matches the given dname */
list_for_each_entry_safe(owatch, nextw, &parent->watches, wlist) {
- if (audit_compare_dname_path(dname, owatch->path))
+ if (audit_compare_dname_path(dname, owatch->path,
+ AUDIT_NAME_FULL))
continue;
/* If the update involves invalidating rules, do the inode-based
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index f47ba18..1e0899d 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -1228,11 +1228,17 @@ int parent_len(const char *path)
return p - path;
}
-/* Compare given dentry name with last component in given path,
- * return of 0 indicates a match. */
-int audit_compare_dname_path(const char *dname, const char *path)
+/**
+ * audit_compare_dname_path - compare given dentry name with last component in
+ * given path. Return of 0 indicates a match.
+ * @dname: dentry name that we're comparing
+ * @path: full pathname that we're comparing
+ * @parentlen: length of the parent if known. Passing in AUDIT_NAME_FULL
+ * here indicates that we must compute this value.
+ */
+int audit_compare_dname_path(const char *dname, const char *path, int parentlen)
{
- int dlen, pathlen, parentlen;
+ int dlen, pathlen;
const char *p;
dlen = strlen(dname);
@@ -1240,7 +1246,7 @@ int audit_compare_dname_path(const char *dname, const char *path)
if (pathlen < dlen)
return 1;
- parentlen = parent_len(path);
+ parentlen = parentlen == AUDIT_NAME_FULL ? parent_len(path) : parentlen;
if (pathlen - parentlen != dlen)
return 1;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 34d7ab2..f4cdefe 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -81,9 +81,6 @@
* a name dynamically and also add those to the list anchored by names_list. */
#define AUDIT_NAMES 5
-/* Indicates that audit should log the full pathname. */
-#define AUDIT_NAME_FULL -1
-
/* no execve audit message should be longer than this (userspace limits) */
#define MAX_EXECVE_AUDIT_LEN 7500
@@ -2232,7 +2229,7 @@ void __audit_inode_child(const struct inode *parent,
continue;
if (n->ino == parent->i_ino &&
- !audit_compare_dname_path(dname, n->name)) {
+ !audit_compare_dname_path(dname, n->name, n->name_len)) {
found_parent = n->name;
goto add_names;
}
@@ -2245,7 +2242,8 @@ void __audit_inode_child(const struct inode *parent,
/* strcmp() is the more likely scenario */
if (!strcmp(dname, n->name) ||
- !audit_compare_dname_path(dname, n->name)) {
+ !audit_compare_dname_path(dname, n->name,
+ AUDIT_NAME_FULL)) {
if (inode)
audit_copy_inode(n, dentry, inode);
else
--
1.7.7.6
11:35 a.m.
New subject: [PATCH v4 9/9] audit: overhaul __audit_inode_child to accomodate retrying
In order to accomodate retrying path-based syscalls, we need to add a
new "type" argument to audit_inode_child. This will tell us whether
we're looking for a child entry that represents a create or a delete.
If we find a parent, don't automatically assume that we need to create a
new entry. Instead, use the information we have to try to find an
existing entry first. Update it if one is found and create a new one if
not.
Signed-off-by: Jeff Layton <jlayton(a)redhat.com>
---
fs/btrfs/ioctl.c | 2 +-
fs/namei.c | 2 +-
include/linux/audit.h | 12 ++++++---
include/linux/fsnotify.h | 8 +++---
kernel/auditsc.c | 57 ++++++++++++++++++++++++---------------------
5 files changed, 44 insertions(+), 37 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 90c67b0..b638ac5 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -592,7 +592,7 @@ static int btrfs_may_delete(struct inode *dir,struct dentry
*victim,int isdir)
return -ENOENT;
BUG_ON(victim->d_parent->d_inode != dir);
- audit_inode_child(dir, victim);
+ audit_inode_child(dir, victim, AUDIT_TYPE_CHILD_DELETE);
error = inode_permission(dir, MAY_WRITE | MAY_EXEC);
if (error)
diff --git a/fs/namei.c b/fs/namei.c
index 8c1b3cd..196f039 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1998,7 +1998,7 @@ static int may_delete(struct inode *dir,struct dentry *victim,int
isdir)
return -ENOENT;
BUG_ON(victim->d_parent->d_inode != dir);
- audit_inode_child(dir, victim);
+ audit_inode_child(dir, victim, AUDIT_TYPE_CHILD_DELETE);
error = inode_permission(dir, MAY_WRITE | MAY_EXEC);
if (error)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 2fc6831..7a15359 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -454,6 +454,8 @@ extern int audit_classify_arch(int arch);
#define AUDIT_TYPE_UNKNOWN 0 /* we don't know yet */
#define AUDIT_TYPE_NORMAL 1 /* a "normal" audit record */
#define AUDIT_TYPE_PARENT 2 /* a parent audit record */
+#define AUDIT_TYPE_CHILD_DELETE 3 /* a child being deleted */
+#define AUDIT_TYPE_CHILD_CREATE 4 /* a child being created */
#ifdef CONFIG_AUDITSYSCALL
/* These are defined in auditsc.c */
@@ -469,7 +471,8 @@ extern void audit_putname(const char *name);
extern void __audit_inode(const char *name, const struct dentry *dentry,
unsigned int parent);
extern void __audit_inode_child(const struct inode *parent,
- const struct dentry *dentry);
+ const struct dentry *dentry,
+ const unsigned char type);
extern void __audit_seccomp(unsigned long syscall, long signr, int code);
extern void __audit_ptrace(struct task_struct *t);
@@ -510,9 +513,10 @@ static inline void audit_inode(const char *name, const struct dentry
*dentry,
__audit_inode(name, dentry, parent);
}
static inline void audit_inode_child(const struct inode *parent,
- const struct dentry *dentry) {
+ const struct dentry *dentry,
+ const unsigned char type) {
if (unlikely(!audit_dummy_context()))
- __audit_inode_child(parent, dentry);
+ __audit_inode_child(parent, dentry, type);
}
void audit_core_dumps(long signr);
@@ -640,7 +644,7 @@ extern int audit_signals;
#define __audit_inode(n,d,p) do { ; } while (0)
#define __audit_inode_child(p,d) do { ; } while (0)
#define audit_inode(n,d,p) do { (void)(d); } while (0)
-#define audit_inode_child(p,d) do { ; } while (0)
+#define audit_inode_child(p,d,t) do { ; } while (0)
#define audit_core_dumps(i) do { ; } while (0)
#define audit_seccomp(i,s,c) do { ; } while (0)
#define auditsc_get_stamp(c,t,s) (0)
diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
index 9c28471..0fbfb46 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -109,7 +109,7 @@ static inline void fsnotify_move(struct inode *old_dir, struct inode
*new_dir,
if (source)
fsnotify(source, FS_MOVE_SELF, moved->d_inode, FSNOTIFY_EVENT_INODE, NULL, 0);
- audit_inode_child(new_dir, moved);
+ audit_inode_child(new_dir, moved, AUDIT_TYPE_CHILD_CREATE);
}
/*
@@ -155,7 +155,7 @@ static inline void fsnotify_inoderemove(struct inode *inode)
*/
static inline void fsnotify_create(struct inode *inode, struct dentry *dentry)
{
- audit_inode_child(inode, dentry);
+ audit_inode_child(inode, dentry, AUDIT_TYPE_CHILD_CREATE);
fsnotify(inode, FS_CREATE, dentry->d_inode, FSNOTIFY_EVENT_INODE,
dentry->d_name.name, 0);
}
@@ -168,7 +168,7 @@ static inline void fsnotify_create(struct inode *inode, struct dentry
*dentry)
static inline void fsnotify_link(struct inode *dir, struct inode *inode, struct dentry
*new_dentry)
{
fsnotify_link_count(inode);
- audit_inode_child(dir, new_dentry);
+ audit_inode_child(dir, new_dentry, AUDIT_TYPE_CHILD_CREATE);
fsnotify(dir, FS_CREATE, inode, FSNOTIFY_EVENT_INODE, new_dentry->d_name.name, 0);
}
@@ -181,7 +181,7 @@ static inline void fsnotify_mkdir(struct inode *inode, struct dentry
*dentry)
__u32 mask = (FS_CREATE | FS_ISDIR);
struct inode *d_inode = dentry->d_inode;
- audit_inode_child(inode, dentry);
+ audit_inode_child(inode, dentry, AUDIT_TYPE_CHILD_CREATE);
fsnotify(inode, mask, d_inode, FSNOTIFY_EVENT_INODE, dentry->d_name.name, 0);
}
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index f4cdefe..bafb618 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2199,6 +2199,7 @@ out:
* __audit_inode_child - collect inode info for created/removed objects
* @parent: inode of dentry parent
* @dentry: dentry being audited
+ * @type: AUDIT_TYPE_* value that we're looking for
*
* For syscalls that create or remove filesystem objects, audit_inode
* can only collect information for the filesystem object's parent.
@@ -2209,13 +2210,13 @@ out:
* unsuccessful attempts.
*/
void __audit_inode_child(const struct inode *parent,
- const struct dentry *dentry)
+ const struct dentry *dentry,
+ const unsigned char type)
{
struct audit_context *context = current->audit_context;
- const char *found_parent = NULL, *found_child = NULL;
const struct inode *inode = dentry->d_inode;
const char *dname = dentry->d_name.name;
- struct audit_names *n;
+ struct audit_names *n, *found_parent = NULL, *found_child = NULL;
if (!context->in_syscall)
return;
@@ -2223,63 +2224,65 @@ void __audit_inode_child(const struct inode *parent,
if (inode)
handle_one(inode);
- /* parent is more likely, look for it first */
+ /* look for a parent entry first */
list_for_each_entry(n, &context->names_list, list) {
- if (!n->name)
+ if (!n->name || n->type != AUDIT_TYPE_PARENT)
continue;
if (n->ino == parent->i_ino &&
!audit_compare_dname_path(dname, n->name, n->name_len)) {
- found_parent = n->name;
- goto add_names;
+ found_parent = n;
+ break;
}
}
- /* no matching parent, look for matching child */
+ /* is there a matching child entry? */
list_for_each_entry(n, &context->names_list, list) {
- if (!n->name)
+ /* can only match entries that have a name */
+ if (!n->name || n->type != type)
+ continue;
+
+ /* if we found a parent, make sure this one is a child of it */
+ if (found_parent && (n->name != found_parent->name))
continue;
- /* strcmp() is the more likely scenario */
if (!strcmp(dname, n->name) ||
!audit_compare_dname_path(dname, n->name,
+ found_parent ?
+ found_parent->name_len :
AUDIT_NAME_FULL)) {
- if (inode)
- audit_copy_inode(n, dentry, inode);
- else
- n->ino = (unsigned long)-1;
- n->type = AUDIT_TYPE_NORMAL;
- found_child = n->name;
- goto add_names;
+ found_child = n;
+ break;
}
}
-add_names:
if (!found_parent) {
- n = audit_alloc_name(context, AUDIT_TYPE_NORMAL);
+ /* create a new, "anonymous" parent record */
+ n = audit_alloc_name(context, AUDIT_TYPE_PARENT);
if (!n)
return;
audit_copy_inode(n, NULL, parent);
}
if (!found_child) {
- n = audit_alloc_name(context, AUDIT_TYPE_NORMAL);
- if (!n)
+ found_child = audit_alloc_name(context, type);
+ if (!found_child)
return;
/* Re-use the name belonging to the slot for a matching parent
* directory. All names for this context are relinquished in
* audit_free_names() */
if (found_parent) {
- n->name = found_parent;
- n->name_len = AUDIT_NAME_FULL;
+ found_child->name = found_parent->name;
+ found_child->name_len = AUDIT_NAME_FULL;
/* don't call __putname() */
- n->name_put = false;
+ found_child->name_put = false;
}
-
- if (inode)
- audit_copy_inode(n, dentry, inode);
}
+ if (inode)
+ audit_copy_inode(found_child, dentry, inode);
+ else
+ found_child->ino = (unsigned long)-1;
}
EXPORT_SYMBOL_GPL(__audit_inode_child);
--
1.7.7.6
4570
days inactive
4570
days old
linux-audit@lists.linux-audit.osci.io
9 comments
1 participants
participants (1)
-
Jeff Layton