audit: macros to replace unset inode and device values

PATCH In review Richard Guy Briggs 6 participants 13 messages Aug 1, 2015 – Aug 7, 2015 Audit

Share & tools

Richard Guy Briggs

author

2015-08-01 19:42 UTC #0 lore raw

PATCH v4 1/1 audit: macros to replace unset inode and device values

This is a patch to clean up a number of places were casted magic numbers are used to represent unset inode and device numbers inpreparation for the audit by executable path patch set.

Richard Guy Briggs (1):
audit: use macros for unset inode and device values

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

Richard Guy Briggs

author

2015-08-01 19:42 UTC #1 lore raw

PATCH v4 1/1 audit: use macros for unset inode and device values

Signed-off-by Richard Guy Briggs <[email protected]>

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

include/uapi/linux/audit.h +2 -0

index d3475e1..971df22 100644
@@ -440,6 +440,8 @@ struct audit_tty_status {
 };
 
 #define AUDIT_UID_UNSET (unsigned int)-1
+#define AUDIT_INO_UNSET (unsigned long)-1
+#define AUDIT_DEV_UNSET (unsigned)-1
 
 /* audit_rule_data supports filter rules with both integer and string
  * fields.  It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and

kernel/audit.c +1 -1

index 1c13e42..d546003 100644
@@ -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",

kernel/audit_watch.c +4 -4

index 8f123d7..c668bfc 100644
@@ -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);

kernel/auditsc.c +3 -3

index 9fb9d1c..701ea5c 100644
@@ -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);

Signature · 1 line

1.7.1

However, one question for you ... are we ever going to see a device or inode set to -1 in the userspace facing API? In other words, should the new #defines go in the uapi headers or simply in kernel/audit.h? Unless it is part of the API, let's leave it out of uapi as we have to be very careful about that stuff and I'd prefer to keep it minimal.

Also, if we can put the #defines in kernel/audit.h we can use the proper type for AUDIT_DEV_UNSET which would make me happy.

Quoted 89 lines

include/uapi/linux/audit.h +2 -0

index d3475e1..971df22 100644
@@ -440,6 +440,8 @@ struct audit_tty_status {
 };
 
 #define AUDIT_UID_UNSET (unsigned int)-1
+#define AUDIT_INO_UNSET (unsigned long)-1
+#define AUDIT_DEV_UNSET (unsigned)-1
 
 /* audit_rule_data supports filter rules with both integer and string
  * fields.  It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and

kernel/audit.c +0 -0

index 1c13e42..d546003 100644
@@ -1761,7 +1761,7 @@ void audit_log_name(struct audit_context *context,

struct audit_names *n, } else

kernel/audit.c +1 -1

 		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",

kernel/audit_watch.c +4 -4

index 8f123d7..c668bfc 100644
@@ -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);

kernel/auditsc.c +3 -3

index 9fb9d1c..701ea5c 100644
@@ -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);

Signature · 2 lines

paul moore
security @ redhat

Richard Guy Briggs

author

2015-08-05 06:30 UTC #3 lore raw

Re: [PATCH V4 (was V6)] audit: use macros for unset inode and device values

Quoted 9 lines · Paul Moore

Quoted 7 lines · Richard Guy Briggs

Signed-off-by Richard Guy Briggs <[email protected]>

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

Quoted 7 lines

Yipee, less magic numbers!
However, one question for you ... are we ever going to see a device or inode set to -1 in the userspace facing API? In other words, should the new #defines go in the uapi headers or simply in kernel/audit.h? Unless it is part of the API, let's leave it out of uapi as we have to be very careful about that stuff and I'd prefer to keep it minimal.

This is a good point. I did briefly thing about this at one point. Perhaps Steve can answer this. It would be trivial to move it back to uapi if needed. Would you be ok with it in include/linux/audit.h for now?

Quoted 84 lines

Also, if we can put the #defines in kernel/audit.h we can use the proper type for AUDIT_DEV_UNSET which would make me happy.

Quoted 81 lines

include/uapi/linux/audit.h +2 -0

index d3475e1..971df22 100644
@@ -440,6 +440,8 @@ struct audit_tty_status {
 };
 
 #define AUDIT_UID_UNSET (unsigned int)-1
+#define AUDIT_INO_UNSET (unsigned long)-1
+#define AUDIT_DEV_UNSET (unsigned)-1
 
 /* audit_rule_data supports filter rules with both integer and string
  * fields.  It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and

kernel/audit.c +0 -0

index 1c13e42..d546003 100644
@@ -1761,7 +1761,7 @@ void audit_log_name(struct audit_context *context,

struct audit_names *n, } else

kernel/audit.c +1 -1

 		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",

kernel/audit_watch.c +4 -4

index 8f123d7..c668bfc 100644
@@ -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);

kernel/auditsc.c +2 -2

index 9fb9d1c..701ea5c 100644
@@ -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);

Quoted 13 lines

Quoted 8 lines

kernel/auditsc.c +1 -1

@@ -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);

Signature · 2 lines

paul moore
security @ redhat

- 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

Paul Moore

2015-08-05 19:16 UTC #4 lore raw

Re: [PATCH V4 (was V6)] audit: use macros for unset inode and device values

Quoted 19 lines · Richard Guy Briggs

Quoted 10 lines · Paul Moore
Quoted 8 lines · Richard Guy Briggs
Signed-off-by Richard Guy Briggs <[email protected]>
---
 include/uapi/linux/audit.h |    2 ++
 kernel/audit.c             |    2 +-
 kernel/audit_watch.c       |    8 ++++----
 kernel/auditsc.c           |    6 +++---
 4 files changed, 10 insertions(+), 8 deletions(-)
Quoted 7 lines
Yipee, less magic numbers!
However, one question for you ... are we ever going to see a device or inode set to -1 in the userspace facing API? In other words, should the new #defines go in the uapi headers or simply in kernel/audit.h? Unless it is part of the API, let's leave it out of uapi as we have to be very careful about that stuff and I'd prefer to keep it minimal.

Quoted 4 lines

This is a good point. I did briefly thing about this at one point. Perhaps Steve can answer this. It would be trivial to move it back to uapi if needed. Would you be ok with it in include/linux/audit.h for now?

I have no problem with it in include/linux/audit.h, that is a kernel-only include that we can change at anytime. My concern is putting it into a uapi header which makes it very hard to change.

I'm thinking we should just go ahead and put it in include/linux/audit.h for now as I can't think of a reason why userspace should be passing in an invalid dev/inode value, it just doesn't make sense. If the invalid tokens prove to be valuable for userspace, we can always move the #defines.

Signature · 2 lines

paul moore
security @ redhat

Steve Grubb

2015-08-05 20:08 UTC #5 lore raw

Re: [PATCH V4 (was V6)] audit: use macros for unset inode and device values

Quoted 25 lines · Paul Moore

Quoted 19 lines · Richard Guy Briggs
Quoted 10 lines · Paul Moore
Quoted 8 lines · Richard Guy Briggs
Signed-off-by Richard Guy Briggs <[email protected]>
---
 include/uapi/linux/audit.h |    2 ++
 kernel/audit.c             |    2 +-
 kernel/audit_watch.c       |    8 ++++----
 kernel/auditsc.c           |    6 +++---
 4 files changed, 10 insertions(+), 8 deletions(-)
Quoted 7 lines
Yipee, less magic numbers!
However, one question for you ... are we ever going to see a device or inode set to -1 in the userspace facing API? In other words, should the new #defines go in the uapi headers or simply in kernel/audit.h? Unless it is part of the API, let's leave it out of uapi as we have to be very careful about that stuff and I'd prefer to keep it minimal.
Quoted 4 lines
This is a good point. I did briefly thing about this at one point. Perhaps Steve can answer this. It would be trivial to move it back to uapi if needed. Would you be ok with it in include/linux/audit.h for now?

Quoted 8 lines

I have no problem with it in include/linux/audit.h, that is a kernel-only include that we can change at anytime. My concern is putting it into a uapi header which makes it very hard to change.
I'm thinking we should just go ahead and put it in include/linux/audit.h for now as I can't think of a reason why userspace should be passing in an invalid dev/inode value, it just doesn't make sense. If the invalid tokens prove to be valuable for userspace, we can always move the #defines.

I can't imagine anyone auditing against a specific device or inode. Its like auditing a pid when you really want the program name. Its much more useful to audit by filename or directory and not inode/device. So, do whatever you want. The only unset value that people actually use is the auid because deamons have it unset.

-Steve

Casey Schaufler

2015-08-06 21:31 UTC #6 lore raw

Re: [PATCH V4 (was V6)] audit: use macros for unset inode and device values

Quoted 31 lines · Steve Grubb

Quoted 22 lines · Paul Moore
Quoted 17 lines · Richard Guy Briggs
Quoted 9 lines · Paul Moore
Quoted 8 lines · Richard Guy Briggs
Signed-off-by Richard Guy Briggs <[email protected]>
---
 include/uapi/linux/audit.h |    2 ++
 kernel/audit.c             |    2 +-
 kernel/audit_watch.c       |    8 ++++----
 kernel/auditsc.c           |    6 +++---
 4 files changed, 10 insertions(+), 8 deletions(-)
Quoted 7 lines
Yipee, less magic numbers!
However, one question for you ... are we ever going to see a device or inode set to -1 in the userspace facing API? In other words, should the new #defines go in the uapi headers or simply in kernel/audit.h? Unless it is part of the API, let's leave it out of uapi as we have to be very careful about that stuff and I'd prefer to keep it minimal.
Quoted 4 lines
This is a good point. I did briefly thing about this at one point. Perhaps Steve can answer this. It would be trivial to move it back to uapi if needed. Would you be ok with it in include/linux/audit.h for now?
Quoted 8 lines
I have no problem with it in include/linux/audit.h, that is a kernel-only include that we can change at anytime. My concern is putting it into a uapi header which makes it very hard to change.
I'm thinking we should just go ahead and put it in include/linux/audit.h for now as I can't think of a reason why userspace should be passing in an invalid dev/inode value, it just doesn't make sense. If the invalid tokens prove to be valuable for userspace, we can always move the #defines.

Quoted 5 lines

I can't imagine anyone auditing against a specific device or inode. Its like auditing a pid when you really want the program name. Its much more useful to audit by filename or directory and not inode/device. So, do whatever you want. The only unset value that people actually use is the auid because deamons have it unset.

I remember the Orange Book days when we were *required* to audit by dev/inode because it was the only true way to identify the object. Yes, it's analogous to auditing the pid, but we had to audit by that, too. The dev/indode and pid are the "true" names. Anything else is a hint at what you're looking at. I can easily imaging someone who really cares about the audit data supplying the dev/inode and pid.

Quoted 8 lines

-Steve
--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit

Paul Moore

2015-08-07 14:22 UTC #7 lore raw

Re: [PATCH V4 (was V6)] audit: use macros for unset inode and device values

Quoted 6 lines · Casey Schaufler

I remember the Orange Book days when we were *required* to audit by dev/inode because it was the only true way to identify the object. Yes, it's analogous to auditing the pid, but we had to audit by that, too. The dev/indode and pid are the "true" names. Anything else is a hint at what you're looking at. I can easily imaging someone who really cares about the audit data supplying the dev/inode and pid.

Just to add a bit of clarity, my original question was if there was any value in exposing the unset/invalid device and inode values, e.g. -1. While I agree that there is value in auditing by dev/inode, I can't think of a reasonable situation where the user would need to pass an unset/invalid device and/or inode value into the kernel as part of an audit configuration command.

Signature · 2 lines

paul moore
security @ redhat

William Roberts

2015-08-05 19:22 UTC #8 lore raw

Re: [PATCH V4 (was V6)] audit: use macros for unset inode and device values

In reply to Richard Guy Briggs

Quoted 14 lines · Richard Guy Briggs

Signed-off-by Richard Guy Briggs <[email protected]>

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

include/uapi/linux/audit.h +0 -0

index d3475e1..971df22 100644

include/uapi/linux/audit.h struct audit_tty_status { · line 444

Quoted 5 lines

include/uapi/linux/audit.h +2 -0

@@ -440,6 +440,8 @@ struct audit_tty_status {
 };
 
 #define AUDIT_UID_UNSET (unsigned int)-1
+#define AUDIT_INO_UNSET (unsigned long)-1
+#define AUDIT_DEV_UNSET (unsigned)-1

Why unsigned int in one but unsigned in the other?

kernel/audit.c void audit_log_name(struct audit_context *context,

Quoted 8 lines

include/uapi/linux/audit.h +0 -0

 /* audit_rule_data supports filter rules with both integer and string
  * fields.  It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and

kernel/audit.c +0 -0

index 1c13e42..d546003 100644
@@ -1761,7 +1761,7 @@ void audit_log_name(struct audit_context *context,

struct audit_names *n,

Quoted 13 lines

kernel/audit.c +1 -1

        } 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",

kernel/audit_watch.c +0 -0

index 8f123d7..c668bfc 100644

kernel/audit_watch.c char *audit_watch_path(struct audit_watch *watch) · line 139

Quoted 2 lines

kernel/audit_watch.c +0 -0

@@ -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)

kernel/audit_watch.c static struct audit_watch *audit_init_watch(char

Quoted 7 lines

kernel/audit_watch.c +1 -1

 {
-       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)

kernel/audit_watch.c static int audit_watch_handle_event(struct

Quoted 11 lines

kernel/audit_watch.c +2 -2

        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,

kernel/audit_watch.c static int audit_watch_handle_event(struct · line 494

Quoted 2 lines

kernel/audit_watch.c +0 -0

        if (mask & (FS_CREATE|FS_MOVED_TO) && inode)
                audit_update_watch(parent, dname, inode->i_sb->s_dev,

inode->i_ino, 0);

kernel/audit_watch.c static int audit_watch_handle_event(struct · line 496

Quoted 2 lines

kernel/audit_watch.c +0 -1

        else if (mask & (FS_DELETE|FS_MOVED_FROM))
-               audit_update_watch(parent, dname, (dev_t)-1, (unsigned

long)-1, 1);

kernel/audit_watch.c static int audit_watch_handle_event(struct · line 496

Quoted 1 line

kernel/audit_watch.c +1 -0

+               audit_update_watch(parent, dname, AUDIT_DEV_UNSET,

AUDIT_INO_UNSET, 1);

kernel/auditsc.c static int audit_match_filetype(struct audit_context

Quoted 8 lines

kernel/audit_watch.c +0 -0

        else if (mask & (FS_DELETE_SELF|FS_UNMOUNT|FS_MOVE_SELF))
                audit_remove_parent_watches(parent);

kernel/auditsc.c +0 -0

index 9fb9d1c..701ea5c 100644
@@ -180,7 +180,7 @@ static int audit_match_filetype(struct audit_context

*ctx, int val)

kernel/auditsc.c static struct audit_names *audit_alloc_name(struct

Quoted 9 lines

kernel/auditsc.c +1 -1

                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,

Quoted 24 lines

kernel/auditsc.c +2 -3

                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

--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit

Richard Guy Briggs

author

2015-08-05 19:38 UTC #9 lore raw

Re: [PATCH V4 (was V6)] audit: use macros for unset inode and device values

Quoted 17 lines · William Roberts

Quoted 16 lines · Richard Guy Briggs

Signed-off-by Richard Guy Briggs <[email protected]>

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

include/uapi/linux/audit.h +0 -0

index d3475e1..971df22 100644
@@ -440,6 +440,8 @@ struct audit_tty_status {
 };

Quoted 5 lines

include/uapi/linux/audit.h struct audit_tty_status { · line 444
Quoted 3 lines
include/uapi/linux/audit.h +2 -0
 #define AUDIT_UID_UNSET (unsigned int)-1
+#define AUDIT_INO_UNSET (unsigned long)-1
+#define AUDIT_DEV_UNSET (unsigned)-1
Why unsigned int in one but unsigned in the other?

It was based on one of the instances it was originally replacing (I can't find it now). It could be dev_t if it was moved out of uapi as Paul preferred, but I prefer unsigned int now that you point it out.

Quoted 91 lines

kernel/audit.c void audit_log_name(struct audit_context *context,

Quoted 7 lines

include/uapi/linux/audit.h +0 -0

 /* audit_rule_data supports filter rules with both integer and string
  * fields.  It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and

kernel/audit.c +0 -0

index 1c13e42..d546003 100644
@@ -1761,7 +1761,7 @@ void audit_log_name(struct audit_context *context,

struct audit_names *n,

Quoted 13 lines

kernel/audit.c +1 -1

        } 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",

kernel/audit_watch.c +0 -0

index 8f123d7..c668bfc 100644

kernel/audit_watch.c char *audit_watch_path(struct audit_watch *watch) · line 139

Quoted 2 lines

kernel/audit_watch.c +0 -0

@@ -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)

kernel/audit_watch.c static struct audit_watch *audit_init_watch(char

Quoted 7 lines

kernel/audit_watch.c +1 -1

 {
-       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)

kernel/audit_watch.c static int audit_watch_handle_event(struct

Quoted 11 lines

kernel/audit_watch.c +2 -2

        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,

kernel/audit_watch.c static int audit_watch_handle_event(struct · line 494

Quoted 2 lines

kernel/audit_watch.c +0 -0

        if (mask & (FS_CREATE|FS_MOVED_TO) && inode)
                audit_update_watch(parent, dname, inode->i_sb->s_dev,

inode->i_ino, 0);

kernel/audit_watch.c static int audit_watch_handle_event(struct · line 496

Quoted 2 lines

kernel/audit_watch.c +0 -1

        else if (mask & (FS_DELETE|FS_MOVED_FROM))
-               audit_update_watch(parent, dname, (dev_t)-1, (unsigned

long)-1, 1);

kernel/audit_watch.c static int audit_watch_handle_event(struct · line 496

Quoted 1 line

kernel/audit_watch.c +1 -0

+               audit_update_watch(parent, dname, AUDIT_DEV_UNSET,

AUDIT_INO_UNSET, 1);

kernel/auditsc.c static int audit_match_filetype(struct audit_context

Quoted 8 lines

kernel/audit_watch.c +0 -0

        else if (mask & (FS_DELETE_SELF|FS_UNMOUNT|FS_MOVE_SELF))
                audit_remove_parent_watches(parent);

kernel/auditsc.c +0 -0

index 9fb9d1c..701ea5c 100644
@@ -180,7 +180,7 @@ static int audit_match_filetype(struct audit_context

*ctx, int val)

kernel/auditsc.c static struct audit_names *audit_alloc_name(struct

Quoted 9 lines

kernel/auditsc.c +1 -1

                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,

Quoted 20 lines

kernel/auditsc.c +2 -3

                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

Quoted 9 lines

Quoted 4 lines
--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit
--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

Paul Moore

maintainer

2015-08-05 20:23 UTC #10 lore raw

Re: [PATCH V4 (was V6)] audit: use macros for unset inode and device values

Quoted 24 lines · Richard Guy Briggs

Quoted 17 lines · William Roberts
Quoted 16 lines · Richard Guy Briggs
Signed-off-by Richard Guy Briggs <[email protected]>
---
 include/uapi/linux/audit.h |    2 ++
 kernel/audit.c             |    2 +-
 kernel/audit_watch.c       |    8 ++++----
 kernel/auditsc.c           |    6 +++---
 4 files changed, 10 insertions(+), 8 deletions(-)
include/uapi/linux/audit.h +0 -0
index d3475e1..971df22 100644
@@ -440,6 +440,8 @@ struct audit_tty_status {
 };
 
Quoted 5 lines
include/uapi/linux/audit.h struct audit_tty_status { · line 444
Quoted 3 lines
include/uapi/linux/audit.h +2 -0
 #define AUDIT_UID_UNSET (unsigned int)-1
+#define AUDIT_INO_UNSET (unsigned long)-1
+#define AUDIT_DEV_UNSET (unsigned)-1
Why unsigned int in one but unsigned in the other?

Quoted 3 lines

It was based on one of the instances it was originally replacing (I can't find it now). It could be dev_t if it was moved out of uapi as Paul preferred, but I prefer unsigned int now that you point it out.

Once we move it out of the uapi header it should probably be (dev_t)-1.

Signature · 2 lines

paul moore
www.paul-moore.com

Paul Moore

2015-08-04 22:37 UTC #11 lore raw

In reply to Richard Guy Briggs

Quoted 4 lines · Richard Guy Briggs

This is a patch to clean up a number of places were casted magic numbers are used to represent unset inode and device numbers inpreparation for the audit by executable path patch set.

Quoted 8 lines

Richard Guy Briggs (1):
audit: use macros for unset inode and device values

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

Also, when there is only one patch in the patchset, no need to send a cover email, e.g. patch 0/1, just put the text in the patch description itself.

Signature · 2 lines

paul moore
security @ redhat

Richard Guy Briggs

author

2015-08-05 06:32 UTC #12 lore raw

Quoted 8 lines · Paul Moore

Quoted 7 lines · Richard Guy Briggs
This is a patch to clean up a number of places were casted magic numbers are used to represent unset inode and device numbers inpreparation for the audit by executable path patch set.
Richard Guy Briggs (1):
audit: use macros for unset inode and device values

Quoted 8 lines

Quoted 5 lines
 include/uapi/linux/audit.h |    2 ++
 kernel/audit.c             |    2 +-
 kernel/audit_watch.c       |    8 ++++----
 kernel/auditsc.c           |    6 +++---
 4 files changed, 10 insertions(+), 8 deletions(-)
Also, when there is only one patch in the patchset, no need to send a cover email, e.g. patch 0/1, just put the text in the patch description itself.

Or drop it into a comment after the "---" demarcation... Which would be better for questions that turn out to need no further followup.

Quoted 1 line

paul moore

- RGB