[PATCH V5] audit: use macros for unset inode and device values

[PATCH V5] audit: use macros for unset inode and device values

[Richard Guy Briggs]

Clean up a number of places were casted magic numbers are used to represent
unset inode and device numbers in preparation for the audit by executable path
patch set.

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
v6: Change dev macro cast from unsigned int to dev_t.

v5: Move macros from include/uapi/linux/audit.h to include/linux/audit.h
    Use "unsigned int" rather than bare "unsigned".

 include/linux/audit.h |    3 +++
 kernel/audit.c        |    2 +-
 kernel/audit_watch.c  |    8 ++++----
 kernel/auditsc.c      |    6 +++---
 4 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index c2e7e3a..1514412 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -27,6 +27,9 @@
 #include <linux/ptrace.h>
 #include <uapi/linux/audit.h>
 
+#define AUDIT_INO_UNSET (unsigned long)-1
+#define AUDIT_DEV_UNSET (dev_t)-1
+
 struct audit_sig_info {
 	uid_t		uid;
 	pid_t		pid;
diff --git a/kernel/audit.c b/kernel/audit.c
index 1c13e42..d546003 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1761,7 +1761,7 @@ void audit_log_name(struct audit_context *context, struct audit_names *n,
 	} else
 		audit_log_format(ab, " name=(null)");
 
-	if (n->ino != (unsigned long)-1)
+	if (n->ino != AUDIT_INO_UNSET)
 		audit_log_format(ab, " inode=%lu"
 				 " dev=%02x:%02x mode=%#ho"
 				 " ouid=%u ogid=%u rdev=%02x:%02x",
diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index 8f123d7..c668bfc 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -138,7 +138,7 @@ char *audit_watch_path(struct audit_watch *watch)
 
 int audit_watch_compare(struct audit_watch *watch, unsigned long ino, dev_t dev)
 {
-	return (watch->ino != (unsigned long)-1) &&
+	return (watch->ino != AUDIT_INO_UNSET) &&
 		(watch->ino == ino) &&
 		(watch->dev == dev);
 }
@@ -179,8 +179,8 @@ static struct audit_watch *audit_init_watch(char *path)
 	INIT_LIST_HEAD(&watch->rules);
 	atomic_set(&watch->count, 1);
 	watch->path = path;
-	watch->dev = (dev_t)-1;
-	watch->ino = (unsigned long)-1;
+	watch->dev = AUDIT_DEV_UNSET;
+	watch->ino = AUDIT_INO_UNSET;
 
 	return watch;
 }
@@ -493,7 +493,7 @@ static int audit_watch_handle_event(struct fsnotify_group *group,
 	if (mask & (FS_CREATE|FS_MOVED_TO) && inode)
 		audit_update_watch(parent, dname, inode->i_sb->s_dev, inode->i_ino, 0);
 	else if (mask & (FS_DELETE|FS_MOVED_FROM))
-		audit_update_watch(parent, dname, (dev_t)-1, (unsigned long)-1, 1);
+		audit_update_watch(parent, dname, AUDIT_DEV_UNSET, AUDIT_INO_UNSET, 1);
 	else if (mask & (FS_DELETE_SELF|FS_UNMOUNT|FS_MOVE_SELF))
 		audit_remove_parent_watches(parent);
 
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 9fb9d1c..701ea5c 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -180,7 +180,7 @@ static int audit_match_filetype(struct audit_context *ctx, int val)
 		return 0;
 
 	list_for_each_entry(n, &ctx->names_list, list) {
-		if ((n->ino != -1) &&
+		if ((n->ino != AUDIT_INO_UNSET) &&
 		    ((n->mode & S_IFMT) == mode))
 			return 1;
 	}
@@ -1683,7 +1683,7 @@ static struct audit_names *audit_alloc_name(struct audit_context *context,
 		aname->should_free = true;
 	}
 
-	aname->ino = (unsigned long)-1;
+	aname->ino = AUDIT_INO_UNSET;
 	aname->type = type;
 	list_add_tail(&aname->list, &context->names_list);
 
@@ -1925,7 +1925,7 @@ void __audit_inode_child(const struct inode *parent,
 	if (inode)
 		audit_copy_inode(found_child, dentry, inode);
 	else
-		found_child->ino = (unsigned long)-1;
+		found_child->ino = AUDIT_INO_UNSET;
 }
 EXPORT_SYMBOL_GPL(__audit_inode_child);
 
-- 
1.7.1

Re: [PATCH V5] audit: use macros for unset inode and device values

[Paul Moore]

On Wednesday, August 05, 2015 11:48:20 PM Richard Guy Briggs wrote:
> Clean up a number of places were casted magic numbers are used to represent
> unset inode and device numbers in preparation for the audit by executable
> path patch set.
> 
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> v6: Change dev macro cast from unsigned int to dev_t.
> 
> v5: Move macros from include/uapi/linux/audit.h to include/linux/audit.h
>     Use "unsigned int" rather than bare "unsigned".
> 
>  include/linux/audit.h |    3 +++
>  kernel/audit.c        |    2 +-
>  kernel/audit_watch.c  |    8 ++++----
>  kernel/auditsc.c      |    6 +++---
>  4 files changed, 11 insertions(+), 8 deletions(-)

Great, thank you.

... and since we were just talking about modifications while I apply the 
patches, here are two examples of changes I'm "OK" making:

* There was a minor conflict in audit_log_name() that caused that 
AUDIT_INO_UNSET to be dropped so I restored it.  This is just par for the 
course when you're applying patches to a tree; I don't think twice about these 
changes so long as the merge is trivially understood.

* The AUDIT_{INO,DEV}_UNSET macros needed to be wrapped in parentheses to keep 
./scripts/checkpatch happy.  I consider that trivial and not a functional 
change so I made that change after noting it in the description metadata.

> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index c2e7e3a..1514412 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -27,6 +27,9 @@
>  #include <linux/ptrace.h>
>  #include <uapi/linux/audit.h>
> 
> +#define AUDIT_INO_UNSET (unsigned long)-1
> +#define AUDIT_DEV_UNSET (dev_t)-1
> +
>  struct audit_sig_info {
>  	uid_t		uid;
>  	pid_t		pid;
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 1c13e42..d546003 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1761,7 +1761,7 @@ void audit_log_name(struct audit_context *context,
> struct audit_names *n, } else
>  		audit_log_format(ab, " name=(null)");
> 
> -	if (n->ino != (unsigned long)-1)
> +	if (n->ino != AUDIT_INO_UNSET)
>  		audit_log_format(ab, " inode=%lu"
>  				 " dev=%02x:%02x mode=%#ho"
>  				 " ouid=%u ogid=%u rdev=%02x:%02x",
> diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
> index 8f123d7..c668bfc 100644
> --- a/kernel/audit_watch.c
> +++ b/kernel/audit_watch.c
> @@ -138,7 +138,7 @@ char *audit_watch_path(struct audit_watch *watch)
> 
>  int audit_watch_compare(struct audit_watch *watch, unsigned long ino, dev_t
> dev) {
> -	return (watch->ino != (unsigned long)-1) &&
> +	return (watch->ino != AUDIT_INO_UNSET) &&
>  		(watch->ino == ino) &&
>  		(watch->dev == dev);
>  }
> @@ -179,8 +179,8 @@ static struct audit_watch *audit_init_watch(char *path)
>  	INIT_LIST_HEAD(&watch->rules);
>  	atomic_set(&watch->count, 1);
>  	watch->path = path;
> -	watch->dev = (dev_t)-1;
> -	watch->ino = (unsigned long)-1;
> +	watch->dev = AUDIT_DEV_UNSET;
> +	watch->ino = AUDIT_INO_UNSET;
> 
>  	return watch;
>  }
> @@ -493,7 +493,7 @@ static int audit_watch_handle_event(struct
> fsnotify_group *group, if (mask & (FS_CREATE|FS_MOVED_TO) && inode)
>  		audit_update_watch(parent, dname, inode->i_sb->s_dev, inode->i_ino, 0);
>  	else if (mask & (FS_DELETE|FS_MOVED_FROM))
> -		audit_update_watch(parent, dname, (dev_t)-1, (unsigned long)-1, 1);
> +		audit_update_watch(parent, dname, AUDIT_DEV_UNSET, AUDIT_INO_UNSET, 1);
>  	else if (mask & (FS_DELETE_SELF|FS_UNMOUNT|FS_MOVE_SELF))
>  		audit_remove_parent_watches(parent);
> 
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 9fb9d1c..701ea5c 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -180,7 +180,7 @@ static int audit_match_filetype(struct audit_context
> *ctx, int val) return 0;
> 
>  	list_for_each_entry(n, &ctx->names_list, list) {
> -		if ((n->ino != -1) &&
> +		if ((n->ino != AUDIT_INO_UNSET) &&
>  		    ((n->mode & S_IFMT) == mode))
>  			return 1;
>  	}
> @@ -1683,7 +1683,7 @@ static struct audit_names *audit_alloc_name(struct
> audit_context *context, aname->should_free = true;
>  	}
> 
> -	aname->ino = (unsigned long)-1;
> +	aname->ino = AUDIT_INO_UNSET;
>  	aname->type = type;
>  	list_add_tail(&aname->list, &context->names_list);
> 
> @@ -1925,7 +1925,7 @@ void __audit_inode_child(const struct inode *parent,
>  	if (inode)
>  		audit_copy_inode(found_child, dentry, inode);
>  	else
> -		found_child->ino = (unsigned long)-1;
> +		found_child->ino = AUDIT_INO_UNSET;
>  }
>  EXPORT_SYMBOL_GPL(__audit_inode_child);

-- 
paul moore
security @ redhat

[PATCH V5] audit: use macros for unset inode and device values

[Richard Guy Briggs]

Clean up a number of places were casted magic numbers are used to represent
unset inode and device numbers in preparation for the audit by executable path
patch set.

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
v5: Move macros from include/uapi/linux/audit.h to include/linux/audit.h
    Use "unsigned int" rather than bare "unsigned".

 include/linux/audit.h |    3 +++
 kernel/audit.c        |    2 +-
 kernel/audit_watch.c  |    8 ++++----
 kernel/auditsc.c      |    6 +++---
 4 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index c2e7e3a..48ae90c 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -27,6 +27,9 @@
 #include <linux/ptrace.h>
 #include <uapi/linux/audit.h>
 
+#define AUDIT_INO_UNSET (unsigned long)-1
+#define AUDIT_DEV_UNSET (unsigned int)-1
+
 struct audit_sig_info {
 	uid_t		uid;
 	pid_t		pid;
diff --git a/kernel/audit.c b/kernel/audit.c
index 1c13e42..d546003 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1761,7 +1761,7 @@ void audit_log_name(struct audit_context *context, struct audit_names *n,
 	} else
 		audit_log_format(ab, " name=(null)");
 
-	if (n->ino != (unsigned long)-1)
+	if (n->ino != AUDIT_INO_UNSET)
 		audit_log_format(ab, " inode=%lu"
 				 " dev=%02x:%02x mode=%#ho"
 				 " ouid=%u ogid=%u rdev=%02x:%02x",
diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index 8f123d7..c668bfc 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -138,7 +138,7 @@ char *audit_watch_path(struct audit_watch *watch)
 
 int audit_watch_compare(struct audit_watch *watch, unsigned long ino, dev_t dev)
 {
-	return (watch->ino != (unsigned long)-1) &&
+	return (watch->ino != AUDIT_INO_UNSET) &&
 		(watch->ino == ino) &&
 		(watch->dev == dev);
 }
@@ -179,8 +179,8 @@ static struct audit_watch *audit_init_watch(char *path)
 	INIT_LIST_HEAD(&watch->rules);
 	atomic_set(&watch->count, 1);
 	watch->path = path;
-	watch->dev = (dev_t)-1;
-	watch->ino = (unsigned long)-1;
+	watch->dev = AUDIT_DEV_UNSET;
+	watch->ino = AUDIT_INO_UNSET;
 
 	return watch;
 }
@@ -493,7 +493,7 @@ static int audit_watch_handle_event(struct fsnotify_group *group,
 	if (mask & (FS_CREATE|FS_MOVED_TO) && inode)
 		audit_update_watch(parent, dname, inode->i_sb->s_dev, inode->i_ino, 0);
 	else if (mask & (FS_DELETE|FS_MOVED_FROM))
-		audit_update_watch(parent, dname, (dev_t)-1, (unsigned long)-1, 1);
+		audit_update_watch(parent, dname, AUDIT_DEV_UNSET, AUDIT_INO_UNSET, 1);
 	else if (mask & (FS_DELETE_SELF|FS_UNMOUNT|FS_MOVE_SELF))
 		audit_remove_parent_watches(parent);
 
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 9fb9d1c..701ea5c 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -180,7 +180,7 @@ static int audit_match_filetype(struct audit_context *ctx, int val)
 		return 0;
 
 	list_for_each_entry(n, &ctx->names_list, list) {
-		if ((n->ino != -1) &&
+		if ((n->ino != AUDIT_INO_UNSET) &&
 		    ((n->mode & S_IFMT) == mode))
 			return 1;
 	}
@@ -1683,7 +1683,7 @@ static struct audit_names *audit_alloc_name(struct audit_context *context,
 		aname->should_free = true;
 	}
 
-	aname->ino = (unsigned long)-1;
+	aname->ino = AUDIT_INO_UNSET;
 	aname->type = type;
 	list_add_tail(&aname->list, &context->names_list);
 
@@ -1925,7 +1925,7 @@ void __audit_inode_child(const struct inode *parent,
 	if (inode)
 		audit_copy_inode(found_child, dentry, inode);
 	else
-		found_child->ino = (unsigned long)-1;
+		found_child->ino = AUDIT_INO_UNSET;
 }
 EXPORT_SYMBOL_GPL(__audit_inode_child);
 
-- 
1.7.1

Re: [PATCH V5] audit: use macros for unset inode and device values

[Paul Moore]

On Wednesday, August 05, 2015 04:19:09 PM Richard Guy Briggs wrote:
> Clean up a number of places were casted magic numbers are used to represent
> unset inode and device numbers in preparation for the audit by executable
> path patch set.
> 
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
> v5: Move macros from include/uapi/linux/audit.h to include/linux/audit.h
>     Use "unsigned int" rather than bare "unsigned".
> 
>  include/linux/audit.h |    3 +++
>  kernel/audit.c        |    2 +-
>  kernel/audit_watch.c  |    8 ++++----
>  kernel/auditsc.c      |    6 +++---
>  4 files changed, 11 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index c2e7e3a..48ae90c 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -27,6 +27,9 @@
>  #include <linux/ptrace.h>
>  #include <uapi/linux/audit.h>
> 
> +#define AUDIT_INO_UNSET (unsigned long)-1
> +#define AUDIT_DEV_UNSET (unsigned int)-1

I suspect it was lost in the noise when I mentioned it on v4, but how about 
changing AUDIT_DEV_UNSET to "(dev_t)-1"?

-- 
paul moore
security @ redhat

Re: [PATCH V5] audit: use macros for unset inode and device values

[Richard Guy Briggs]

On 15/08/05, Paul Moore wrote:
> On Wednesday, August 05, 2015 04:19:09 PM Richard Guy Briggs wrote:
> > Clean up a number of places were casted magic numbers are used to represent
> > unset inode and device numbers in preparation for the audit by executable
> > path patch set.
> > 
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> > v5: Move macros from include/uapi/linux/audit.h to include/linux/audit.h
> >     Use "unsigned int" rather than bare "unsigned".
> > 
> >  include/linux/audit.h |    3 +++
> >  kernel/audit.c        |    2 +-
> >  kernel/audit_watch.c  |    8 ++++----
> >  kernel/auditsc.c      |    6 +++---
> >  4 files changed, 11 insertions(+), 8 deletions(-)
> > 
> > diff --git a/include/linux/audit.h b/include/linux/audit.h
> > index c2e7e3a..48ae90c 100644
> > --- a/include/linux/audit.h
> > +++ b/include/linux/audit.h
> > @@ -27,6 +27,9 @@
> >  #include <linux/ptrace.h>
> >  #include <uapi/linux/audit.h>
> > 
> > +#define AUDIT_INO_UNSET (unsigned long)-1
> > +#define AUDIT_DEV_UNSET (unsigned int)-1
> 
> I suspect it was lost in the noise when I mentioned it on v4, but how about 
> changing AUDIT_DEV_UNSET to "(dev_t)-1"?

I saw your comment only after resubmitting.  I'm fine either way.  If it
is needed for uapi later it can be changed then.  Is it easy to change
in your workflow, or should I resubmit?  I know you routinely change the
patch description, but could not remember if you have actually changed
the patch itself...

> paul moore

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
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

Re: [PATCH V5] audit: use macros for unset inode and device values

[Paul Moore]

On Wednesday, August 05, 2015 11:40:34 PM Richard Guy Briggs wrote:
> On 15/08/05, Paul Moore wrote:
> > I suspect it was lost in the noise when I mentioned it on v4, but how
> > about changing AUDIT_DEV_UNSET to "(dev_t)-1"?
> 
> I saw your comment only after resubmitting.  I'm fine either way.  If it
> is needed for uapi later it can be changed then.  Is it easy to change
> in your workflow, or should I resubmit?  I know you routinely change the
> patch description, but could not remember if you have actually changed
> the patch itself...

With the exception of trivial merge conflicts, in general I like to avoid 
changing the body of the patches when I apply them; there are always going to 
be exceptions, but when possible I try to avoid it.  I consider tweaking the 
patch subject lines pretty trivial, especially in the last case where the 
subject line had become wrong/invalid due to changes in the patch itself.

-- 
paul moore
security @ redhat