[PATCH 0/3] audit: minor audit cleanups

linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH 0/3] audit: minor audit cleanups
@ 2012-10-26 15:43 Jeff Layton
  2012-10-26 15:43 ` [PATCH 1/3] audit: eliminate optional dentry argument to audit_copy_inode Jeff Layton
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Jeff Layton @ 2012-10-26 15:43 UTC (permalink / raw)
  To: viro; +Cc: hch, eparis, linux-audit, linux-fsdevel, linux-kernel

These patches are some cleanups suggested by HCH when he was reviewing
my audit set that went into 3.7. They should introduce no behavioral
changes. They're just cleanups for clarity's sake.

I think the first patch in the series makes a lot of sense. The last two
patches don't seem to provide as much improvement in clarity to me. I'm
not opposed to including them if others think they do, so I'll post
them here so we can debate their merits.

These (or some subset of these) are probably appropriate for 3.8.

Jeff Layton (3):
  audit: eliminate optional dentry argument to audit_copy_inode
  audit: break the setup of the audit_name out of audit_inode and into
    separate function
  audit: break up __audit_inode into two functions

 fs/open.c             |  4 ++--
 fs/xattr.c            |  8 ++++----
 include/linux/audit.h | 11 +++++++++-
 ipc/mqueue.c          |  4 ++--
 kernel/auditsc.c      | 57 ++++++++++++++++++++++++++++++++++++---------------
 5 files changed, 59 insertions(+), 25 deletions(-)

-- 
1.7.11.7

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/3] audit: eliminate optional dentry argument to audit_copy_inode
  2012-10-26 15:43 [PATCH 0/3] audit: minor audit cleanups Jeff Layton
@ 2012-10-26 15:43 ` Jeff Layton
  2012-10-26 15:43 ` [PATCH 2/3] audit: break the setup of the audit_name out of audit_inode and into separate function Jeff Layton
  2012-10-26 15:43 ` [PATCH 3/3] audit: break up __audit_inode into two functions Jeff Layton
  2 siblings, 0 replies; 4+ messages in thread
From: Jeff Layton @ 2012-10-26 15:43 UTC (permalink / raw)
  To: viro; +Cc: hch, eparis, linux-audit, linux-fsdevel, linux-kernel

Just have callers that are passing in a dentry call audit_copy_fcaps
themselves after audit_copy_inode returns.

Reported-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 kernel/auditsc.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 2f186ed..6a6d95a 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2148,7 +2148,7 @@ static inline int audit_copy_fcaps(struct audit_names *name, const struct dentry
 
 
 /* Copy inode data into an audit_names. */
-static void audit_copy_inode(struct audit_names *name, const struct dentry *dentry,
+static void audit_copy_inode(struct audit_names *name,
 			     const struct inode *inode)
 {
 	name->ino   = inode->i_ino;
@@ -2158,7 +2158,6 @@ static void audit_copy_inode(struct audit_names *name, const struct dentry *dent
 	name->gid   = inode->i_gid;
 	name->rdev  = inode->i_rdev;
 	security_inode_getsecid(inode, &name->osid);
-	audit_copy_fcaps(name, dentry);
 }
 
 /**
@@ -2232,7 +2231,8 @@ out:
 		n->type = AUDIT_TYPE_NORMAL;
 	}
 	handle_path(dentry);
-	audit_copy_inode(n, dentry, inode);
+	audit_copy_inode(n, inode);
+	audit_copy_fcaps(n, dentry);
 }
 
 /**
@@ -2301,7 +2301,7 @@ void __audit_inode_child(const struct inode *parent,
 		n = audit_alloc_name(context, AUDIT_TYPE_PARENT);
 		if (!n)
 			return;
-		audit_copy_inode(n, NULL, parent);
+		audit_copy_inode(n, parent);
 	}
 
 	if (!found_child) {
@@ -2319,10 +2319,13 @@ void __audit_inode_child(const struct inode *parent,
 			found_child->name_put = false;
 		}
 	}
-	if (inode)
-		audit_copy_inode(found_child, dentry, inode);
-	else
+
+	if (inode) {
+		audit_copy_inode(found_child, inode);
+		audit_copy_fcaps(found_child, dentry);
+	} else {
 		found_child->ino = (unsigned long)-1;
+	}
 }
 EXPORT_SYMBOL_GPL(__audit_inode_child);
 
-- 
1.7.11.7


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/3] audit: break the setup of the audit_name out of audit_inode and into separate function
  2012-10-26 15:43 [PATCH 0/3] audit: minor audit cleanups Jeff Layton
  2012-10-26 15:43 ` [PATCH 1/3] audit: eliminate optional dentry argument to audit_copy_inode Jeff Layton
@ 2012-10-26 15:43 ` Jeff Layton
  2012-10-26 15:43 ` [PATCH 3/3] audit: break up __audit_inode into two functions Jeff Layton
  2 siblings, 0 replies; 4+ messages in thread
From: Jeff Layton @ 2012-10-26 15:43 UTC (permalink / raw)
  To: viro; +Cc: hch, eparis, linux-audit, linux-fsdevel, linux-kernel

When we have 2 variants of __audit_inode then they can each call this
function to do the setup.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 kernel/auditsc.c | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 6a6d95a..9a65af0 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2160,6 +2160,21 @@ static void audit_copy_inode(struct audit_names *name,
 	security_inode_getsecid(inode, &name->osid);
 }
 
+static void audit_names_setup(struct audit_names *n,
+			const struct dentry *dentry, unsigned int parent)
+{
+	if (parent) {
+		n->name_len = n->name ? parent_len(n->name->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->d_inode);
+	audit_copy_fcaps(n, dentry);
+}
+
 /**
  * __audit_inode - store the inode and device from a lookup
  * @name: name being audited
@@ -2170,7 +2185,6 @@ void __audit_inode(struct filename *name, const struct dentry *dentry,
 		   unsigned int parent)
 {
 	struct audit_context *context = current->audit_context;
-	const struct inode *inode = dentry->d_inode;
 	struct audit_names *n;
 
 	if (!context->in_syscall)
@@ -2223,16 +2237,7 @@ out_alloc:
 	if (!n)
 		return;
 out:
-	if (parent) {
-		n->name_len = n->name ? parent_len(n->name->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, inode);
-	audit_copy_fcaps(n, dentry);
+	audit_names_setup(n, dentry, parent);
 }
 
 /**
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 3/3] audit: break up __audit_inode into two functions
  2012-10-26 15:43 [PATCH 0/3] audit: minor audit cleanups Jeff Layton
  2012-10-26 15:43 ` [PATCH 1/3] audit: eliminate optional dentry argument to audit_copy_inode Jeff Layton
  2012-10-26 15:43 ` [PATCH 2/3] audit: break the setup of the audit_name out of audit_inode and into separate function Jeff Layton
@ 2012-10-26 15:43 ` Jeff Layton
  2 siblings, 0 replies; 4+ messages in thread
From: Jeff Layton @ 2012-10-26 15:43 UTC (permalink / raw)
  To: viro; +Cc: hch, eparis, linux-audit, linux-fsdevel, linux-kernel

One to handle the case where we have a ginfo and a parent flag. Another to
handle the trivial case where we have no ginfo and no parent flag.

Reported-by: Christoph Hellwig <hch@infradead.org>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/open.c             |  4 ++--
 fs/xattr.c            |  8 ++++----
 include/linux/audit.h | 11 ++++++++++-
 ipc/mqueue.c          |  4 ++--
 kernel/auditsc.c      | 17 +++++++++++++++++
 5 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/fs/open.c b/fs/open.c
index 59071f5..94d5649 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -478,7 +478,7 @@ SYSCALL_DEFINE2(fchmod, unsigned int, fd, umode_t, mode)
 
 	file = fget(fd);
 	if (file) {
-		audit_inode(NULL, file->f_path.dentry, 0);
+		audit_anonymous(file->f_path.dentry);
 		err = chmod_common(&file->f_path, mode);
 		fput(file);
 	}
@@ -588,7 +588,7 @@ SYSCALL_DEFINE3(fchown, unsigned int, fd, uid_t, user, gid_t, group)
 	error = mnt_want_write_file(f.file);
 	if (error)
 		goto out_fput;
-	audit_inode(NULL, f.file->f_path.dentry, 0);
+	audit_anonymous(f.file->f_path.dentry);
 	error = chown_common(&f.file->f_path, user, group);
 	mnt_drop_write_file(f.file);
 out_fput:
diff --git a/fs/xattr.c b/fs/xattr.c
index e21c119..f3a2ffa 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -412,7 +412,7 @@ SYSCALL_DEFINE5(fsetxattr, int, fd, const char __user *, name,
 	if (!f.file)
 		return error;
 	dentry = f.file->f_path.dentry;
-	audit_inode(NULL, dentry, 0);
+	audit_anonymous(dentry);
 	error = mnt_want_write_file(f.file);
 	if (!error) {
 		error = setxattr(dentry, name, value, size, flags);
@@ -507,7 +507,7 @@ SYSCALL_DEFINE4(fgetxattr, int, fd, const char __user *, name,
 
 	if (!f.file)
 		return error;
-	audit_inode(NULL, f.file->f_path.dentry, 0);
+	audit_anonymous(f.file->f_path.dentry);
 	error = getxattr(f.file->f_path.dentry, name, value, size);
 	fdput(f);
 	return error;
@@ -586,7 +586,7 @@ SYSCALL_DEFINE3(flistxattr, int, fd, char __user *, list, size_t, size)
 
 	if (!f.file)
 		return error;
-	audit_inode(NULL, f.file->f_path.dentry, 0);
+	audit_anonymous(f.file->f_path.dentry);
 	error = listxattr(f.file->f_path.dentry, list, size);
 	fdput(f);
 	return error;
@@ -655,7 +655,7 @@ SYSCALL_DEFINE2(fremovexattr, int, fd, const char __user *, name)
 	if (!f.file)
 		return error;
 	dentry = f.file->f_path.dentry;
-	audit_inode(NULL, dentry, 0);
+	audit_anonymous(dentry);
 	error = mnt_want_write_file(f.file);
 	if (!error) {
 		error = removexattr(dentry, name);
diff --git a/include/linux/audit.h b/include/linux/audit.h
index bce729a..2214478 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -97,6 +97,7 @@ extern void __audit_syscall_exit(int ret_success, long ret_value);
 extern struct filename *__audit_reusename(const __user char *uptr);
 extern void __audit_getname(struct filename *name);
 extern void audit_putname(struct filename *name);
+extern void __audit_anonymous(const struct dentry *dentry);
 extern void __audit_inode(struct filename *name, const struct dentry *dentry,
 				unsigned int parent);
 extern void __audit_inode_child(const struct inode *parent,
@@ -142,6 +143,10 @@ static inline void audit_getname(struct filename *name)
 	if (unlikely(!audit_dummy_context()))
 		__audit_getname(name);
 }
+static inline void audit_anonymous(const struct dentry *dentry) {
+	if (unlikely(!audit_dummy_context()))
+		__audit_anonymous(dentry);
+}
 static inline void audit_inode(struct filename *name, const struct dentry *dentry,
 				unsigned int parent) {
 	if (unlikely(!audit_dummy_context()))
@@ -303,7 +308,9 @@ static inline void audit_getname(struct filename *name)
 { }
 static inline void audit_putname(struct filename *name)
 { }
-static inline void __audit_inode(struct filename *name,
+static inline void __audit_anonymous(const struct dentry *dentry)
+{ }
+static inline void __audit_inode(struct getname_info *ginfo,
 					const struct dentry *dentry,
 					unsigned int parent)
 { }
@@ -311,6 +318,8 @@ static inline void __audit_inode_child(const struct inode *parent,
 					const struct dentry *dentry,
 					const unsigned char type)
 { }
+static inline void audit_anonymous(const struct dentry *dentry)
+{ }
 static inline void audit_inode(struct filename *name,
 				const struct dentry *dentry,
 				unsigned int parent)
diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index 71a3ca1..2967a09 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -979,7 +979,7 @@ SYSCALL_DEFINE5(mq_timedsend, mqd_t, mqdes, const char __user *, u_msg_ptr,
 		goto out_fput;
 	}
 	info = MQUEUE_I(inode);
-	audit_inode(NULL, f.file->f_path.dentry, 0);
+	audit_anonymous(f.file->f_path.dentry);
 
 	if (unlikely(!(f.file->f_mode & FMODE_WRITE))) {
 		ret = -EBADF;
@@ -1095,7 +1095,7 @@ SYSCALL_DEFINE5(mq_timedreceive, mqd_t, mqdes, char __user *, u_msg_ptr,
 		goto out_fput;
 	}
 	info = MQUEUE_I(inode);
-	audit_inode(NULL, f.file->f_path.dentry, 0);
+	audit_anonymous(f.file->f_path.dentry);
 
 	if (unlikely(!(f.file->f_mode & FMODE_READ))) {
 		ret = -EBADF;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 9a65af0..e5495f2 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -2175,6 +2175,23 @@ static void audit_names_setup(struct audit_names *n,
 	audit_copy_fcaps(n, dentry);
 }
 
+/*
+ * __audit_anonymous - store a new audit_names record for an
+ *		       dentry with no pathname
+ * @dentry: dentry being audited
+ */
+void __audit_anonymous(const struct dentry *dentry)
+{
+	struct audit_context *context = current->audit_context;
+	struct audit_names *n;
+
+	n = audit_alloc_name(context, AUDIT_TYPE_NORMAL);
+	if (!n)
+		return;
+
+	audit_names_setup(n, dentry, 0);
+}
+
 /**
  * __audit_inode - store the inode and device from a lookup
  * @name: name being audited
-- 
1.7.11.7

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-10-26 15:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-26 15:43 [PATCH 0/3] audit: minor audit cleanups Jeff Layton
2012-10-26 15:43 ` [PATCH 1/3] audit: eliminate optional dentry argument to audit_copy_inode Jeff Layton
2012-10-26 15:43 ` [PATCH 2/3] audit: break the setup of the audit_name out of audit_inode and into separate function Jeff Layton
2012-10-26 15:43 ` [PATCH 3/3] audit: break up __audit_inode into two functions Jeff Layton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).