Discussion:
[PATCH ghak21 V3 1/2] audit: remove path param from link denied function
(too old to reply)
Richard Guy Briggs
2018-03-14 05:42:59 UTC
Permalink
Raw Message
In commit 45b578fe4c3cade6f4ca1fc934ce199afd857edc
("audit: link denied should not directly generate PATH record")
the need for the struct path *link parameter was removed.
Remove the now useless struct path argument.

Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
fs/namei.c | 2 +-
include/linux/audit.h | 6 ++----
kernel/audit.c | 3 +--
3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 9cc91fb..50d2533 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1011,7 +1011,7 @@ static int may_linkat(struct path *link)
if (safe_hardlink_source(inode) || inode_owner_or_capable(inode))
return 0;

- audit_log_link_denied("linkat", link);
+ audit_log_link_denied("linkat");
return -EPERM;
}

diff --git a/include/linux/audit.h b/include/linux/audit.h
index af410d9..75d5b03 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -146,8 +146,7 @@ extern void audit_log_d_path(struct audit_buffer *ab,
const struct path *path);
extern void audit_log_key(struct audit_buffer *ab,
char *key);
-extern void audit_log_link_denied(const char *operation,
- const struct path *link);
+extern void audit_log_link_denied(const char *operation);
extern void audit_log_lost(const char *message);

extern int audit_log_task_context(struct audit_buffer *ab);
@@ -194,8 +193,7 @@ static inline void audit_log_d_path(struct audit_buffer *ab,
{ }
static inline void audit_log_key(struct audit_buffer *ab, char *key)
{ }
-static inline void audit_log_link_denied(const char *string,
- const struct path *link)
+static inline void audit_log_link_denied(const char *string)
{ }
static inline int audit_log_task_context(struct audit_buffer *ab)
{
diff --git a/kernel/audit.c b/kernel/audit.c
index 3f2f143..e8bf8d7 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -2308,9 +2308,8 @@ void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk)
/**
* audit_log_link_denied - report a link restriction denial
* @operation: specific link operation
- * @link: the path that triggered the restriction
*/
-void audit_log_link_denied(const char *operation, const struct path *link)
+void audit_log_link_denied(const char *operation)
{
struct audit_buffer *ab;
--
1.8.3.1
Richard Guy Briggs
2018-03-14 05:43:00 UTC
Permalink
Raw Message
Audit link denied events for symlinks had duplicate PATH records rather
than just updating the existing PATH record. Update the symlink's PATH
record with the current dentry and inode information.

See: https://github.com/linux-audit/audit-kernel/issues/21
Signed-off-by: Richard Guy Briggs <***@redhat.com>
---
fs/namei.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/fs/namei.c b/fs/namei.c
index 50d2533..00f5041 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -945,6 +945,7 @@ static inline int may_follow_link(struct nameidata *nd)
if (nd->flags & LOOKUP_RCU)
return -ECHILD;

+ audit_inode(nd->name, nd->stack[0].link.dentry, 0);
audit_log_link_denied("follow_link", &nd->stack[0].link);
return -EACCES;
}
--
1.8.3.1
Paul Moore
2018-03-20 20:11:59 UTC
Permalink
Raw Message
Post by Richard Guy Briggs
Audit link denied events for symlinks had duplicate PATH records rather
than just updating the existing PATH record. Update the symlink's PATH
record with the current dentry and inode information.
See: https://github.com/linux-audit/audit-kernel/issues/21
---
fs/namei.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/namei.c b/fs/namei.c
index 50d2533..00f5041 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -945,6 +945,7 @@ static inline int may_follow_link(struct nameidata *nd)
if (nd->flags & LOOKUP_RCU)
return -ECHILD;
+ audit_inode(nd->name, nd->stack[0].link.dentry, 0);
audit_log_link_denied("follow_link", &nd->stack[0].link);
That's the one, right above this comment ...

Be honest, this never compiled did it?

My confidence for these patches is really low right now, which is not
good for something asking to go in at this stage in the -rcX
development cycle. There have been some delays due to review, so I'm
willing to be a little flexible on timelines and accept stuff this
week, but for the v4 patchset please provide before and after audit
logs for both hard and soft links both where CWD is the same as the
parent as well as different (eight logs total if I did the math
right). You can include them in the 0/2 patch as it's probably a bit
much for the individual patches.
Post by Richard Guy Briggs
return -EACCES;
}
--
1.8.3.1
--
paul moore
www.paul-moore.com
Richard Guy Briggs
2018-03-21 08:47:58 UTC
Permalink
Raw Message
Post by Paul Moore
Post by Richard Guy Briggs
Audit link denied events for symlinks had duplicate PATH records rather
than just updating the existing PATH record. Update the symlink's PATH
record with the current dentry and inode information.
See: https://github.com/linux-audit/audit-kernel/issues/21
---
fs/namei.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/namei.c b/fs/namei.c
index 50d2533..00f5041 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -945,6 +945,7 @@ static inline int may_follow_link(struct nameidata *nd)
if (nd->flags & LOOKUP_RCU)
return -ECHILD;
+ audit_inode(nd->name, nd->stack[0].link.dentry, 0);
audit_log_link_denied("follow_link", &nd->stack[0].link);
That's the one, right above this comment ...
Be honest, this never compiled did it?
As far as your view is concerned it wasn't. It was compiled and tested,
but the fix ended up in the following patch in the branch that wasn't
included in this patchset so it effectively failed this bisection test.
The sample records were copied from the test and not cooked. git add -p
and/or git rebase -i fumble.
Post by Paul Moore
My confidence for these patches is really low right now, which is not
good for something asking to go in at this stage in the -rcX
development cycle. There have been some delays due to review, so I'm
willing to be a little flexible on timelines and accept stuff this
week, but for the v4 patchset please provide before and after audit
logs for both hard and soft links both where CWD is the same as the
parent as well as different (eight logs total if I did the math
right). You can include them in the 0/2 patch as it's probably a bit
much for the individual patches.
I'm very sorry to have wasted your time. I know it is precious right now.
Post by Paul Moore
Post by Richard Guy Briggs
return -EACCES;
}
paul moore
- RGB

--
Richard Guy Briggs <***@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

Paul Moore
2018-03-20 20:05:39 UTC
Permalink
Raw Message
Post by Richard Guy Briggs
In commit 45b578fe4c3cade6f4ca1fc934ce199afd857edc
("audit: link denied should not directly generate PATH record")
the need for the struct path *link parameter was removed.
Remove the now useless struct path argument.
---
fs/namei.c | 2 +-
include/linux/audit.h | 6 ++----
kernel/audit.c | 3 +--
3 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 9cc91fb..50d2533 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1011,7 +1011,7 @@ static int may_linkat(struct path *link)
if (safe_hardlink_source(inode) || inode_owner_or_capable(inode))
return 0;
- audit_log_link_denied("linkat", link);
+ audit_log_link_denied("linkat");
return -EPERM;
}
diff --git a/include/linux/audit.h b/include/linux/audit.h
index af410d9..75d5b03 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -146,8 +146,7 @@ extern void audit_log_d_path(struct audit_buffer *ab,
const struct path *path);
extern void audit_log_key(struct audit_buffer *ab,
char *key);
-extern void audit_log_link_denied(const char *operation,
- const struct path *link);
+extern void audit_log_link_denied(const char *operation);
extern void audit_log_lost(const char *message);
extern int audit_log_task_context(struct audit_buffer *ab);
@@ -194,8 +193,7 @@ static inline void audit_log_d_path(struct audit_buffer *ab,
{ }
static inline void audit_log_key(struct audit_buffer *ab, char *key)
{ }
-static inline void audit_log_link_denied(const char *string,
- const struct path *link)
+static inline void audit_log_link_denied(const char *string)
{ }
static inline int audit_log_task_context(struct audit_buffer *ab)
{
diff --git a/kernel/audit.c b/kernel/audit.c
index 3f2f143..e8bf8d7 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -2308,9 +2308,8 @@ void audit_log_task_info(struct audit_buffer *ab, struct task_struct *tsk)
/**
* audit_log_link_denied - report a link restriction denial
*/
-void audit_log_link_denied(const char *operation, const struct path *link)
+void audit_log_link_denied(const char *operation)
{
struct audit_buffer *ab;
*sigh*

You forgot to update may_follow_link().
--
paul moore
www.paul-moore.com
Loading...