2:42 p.m.
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(-)
2:42 p.m.
New subject: [PATCH V4 (was V6)] audit: use macros for unset inode and device values
Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
---
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(-)
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index d3475e1..971df22 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -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
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
5:34 p.m.
New subject: [PATCH V4 (was V6)] audit: use macros for unset inode and device values
On Saturday, August 01, 2015 03:42:23 PM Richard Guy Briggs wrote:
Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
---
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(-)
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.
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.
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index d3475e1..971df22 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -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
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
1:30 a.m.
New subject: [PATCH V4 (was V6)] audit: use macros for unset inode and device values
On 15/08/04, Paul Moore wrote:
On Saturday, August 01, 2015 03:42:23 PM Richard Guy Briggs wrote:
> Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
> ---
> 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(-)
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?
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.
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index d3475e1..971df22 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -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
> 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
- RGB
--
Richard Guy Briggs <rbriggs(a)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
2:16 p.m.
New subject: [PATCH V4 (was V6)] audit: use macros for unset inode and device values
On Wednesday, August 05, 2015 02:30:14 AM Richard Guy Briggs wrote:
On 15/08/04, Paul Moore wrote:
> On Saturday, August 01, 2015 03:42:23 PM Richard Guy Briggs wrote:
> > Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
> > ---
> >
> > 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(-)
>
> 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?
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.
--
paul moore
security @ redhat
3:08 p.m.
New subject: [PATCH V4 (was V6)] audit: use macros for unset inode and device values
On Wednesday, August 05, 2015 03:16:58 PM Paul Moore wrote:
On Wednesday, August 05, 2015 02:30:14 AM Richard Guy Briggs wrote:
> On 15/08/04, Paul Moore wrote:
> > On Saturday, August 01, 2015 03:42:23 PM Richard Guy Briggs wrote:
> > > Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
> > > ---
> > >
> > > 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(-)
> >
> > 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?
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
4:31 p.m.
New subject: [PATCH V4 (was V6)] audit: use macros for unset inode and device values
On 8/5/2015 1:08 PM, Steve Grubb wrote:
On Wednesday, August 05, 2015 03:16:58 PM Paul Moore wrote:
> On Wednesday, August 05, 2015 02:30:14 AM Richard Guy Briggs wrote:
>> On 15/08/04, Paul Moore wrote:
>>> On Saturday, August 01, 2015 03:42:23 PM Richard Guy Briggs wrote:
>>>> Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
>>>> ---
>>>>
>>>> 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(-)
>>> 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?
> 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.
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.
-Steve
--
Linux-audit mailing list
Linux-audit(a)redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
9:22 a.m.
New subject: [PATCH V4 (was V6)] audit: use macros for unset inode and device values
On Thursday, August 06, 2015 02:31:57 PM Casey Schaufler wrote:
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.
--
paul moore
security @ redhat
2:22 p.m.
New subject: [PATCH V4 (was V6)] audit: use macros for unset inode and device values
On Aug 1, 2015 12:44 PM, "Richard Guy Briggs" <rgb(a)redhat.com> wrote:
Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
---
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(-)
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index d3475e1..971df22 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -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?
/* audit_rule_data supports filter rules with both integer and string
* fields. It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and
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
--
Linux-audit mailing list
Linux-audit(a)redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
2:38 p.m.
New subject: [PATCH V4 (was V6)] audit: use macros for unset inode and device values
On 15/08/05, William Roberts wrote:
On Aug 1, 2015 12:44 PM, "Richard Guy Briggs"
<rgb(a)redhat.com> wrote:
>
> Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
> ---
> 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(-)
>
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index d3475e1..971df22 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -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?
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.
> /* audit_rule_data supports filter rules with both integer and
string
> * fields. It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and
> 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
>
> --
> Linux-audit mailing list
> Linux-audit(a)redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
--
Linux-audit mailing list
Linux-audit(a)redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
- RGB
--
Richard Guy Briggs <rbriggs(a)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
3:23 p.m.
New subject: [PATCH V4 (was V6)] audit: use macros for unset inode and device values
On Wed, Aug 5, 2015 at 3:38 PM, Richard Guy Briggs <rgb(a)redhat.com> wrote:
On 15/08/05, William Roberts wrote:
> On Aug 1, 2015 12:44 PM, "Richard Guy Briggs" <rgb(a)redhat.com>
wrote:
> >
> > Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com>
> > ---
> > 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(-)
> >
> > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > index d3475e1..971df22 100644
> > --- a/include/uapi/linux/audit.h
> > +++ b/include/uapi/linux/audit.h
> > @@ -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?
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.
--
paul moore
www.paul-moore.com
5:37 p.m.
On Saturday, August 01, 2015 03:42:22 PM Richard Guy Briggs wrote:
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(-)
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.
--
paul moore
security @ redhat
1:32 a.m.
On 15/08/04, Paul Moore wrote:
On Saturday, August 01, 2015 03:42:22 PM Richard Guy Briggs wrote:
> 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(-)
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.
paul moore
- RGB
--
Richard Guy Briggs <rbriggs(a)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
3373
days inactive
3379
days old
linux-audit@lists.linux-audit.osci.io
12 comments
6 participants
participants (6)
-
Casey Schaufler -
Paul Moore -
Paul Moore -
Richard Guy Briggs -
Steve Grubb -
William Roberts