2015-08-06 03:48:56

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]>
---
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


2015-08-07 02:37:42

by Paul Moore

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

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