2015-08-05 20:20:09

by Richard Guy Briggs

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

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 <[email protected]>
---
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


2015-08-05 21:53:40

by Paul Moore

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

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 <[email protected]>
> ---
> 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

2015-08-06 03:40:39

by Richard Guy Briggs

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

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 <[email protected]>
> > ---
> > 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 <[email protected]>
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

2015-08-06 18:37:46

by Paul Moore

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

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