diff mbox series

[RFC,v1,2/7] audit: Fix inode numbers

Message ID 20241010152649.849254-2-mic@digikod.net (mailing list archive)
State New
Headers show
Series [RFC,v1,1/7] fs: Add inode_get_ino() and implement get_ino() for NFS | expand

Commit Message

Mickaël Salaün Oct. 10, 2024, 3:26 p.m. UTC
Use the new inode_get_ino() helper to log the user space's view of
inode's numbers instead of the private kernel values.

Cc: Paul Moore <paul@paul-moore.com>
Cc: Eric Paris <eparis@redhat.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
 security/lsm_audit.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Paul Moore Oct. 11, 2024, 1:20 a.m. UTC | #1
On Oct 10, 2024 =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= <mic@digikod.net> wrote:
> 
> Use the new inode_get_ino() helper to log the user space's view of
> inode's numbers instead of the private kernel values.
> 
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Eric Paris <eparis@redhat.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
>  security/lsm_audit.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

Acked-by: Paul Moore <paul@paul-moore.com>

--
paul-moore.com
Paul Moore Oct. 11, 2024, 1:38 a.m. UTC | #2
On Thu, Oct 10, 2024 at 9:20 PM Paul Moore <paul@paul-moore.com> wrote:
> On Oct 10, 2024 =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= <mic@digikod.net> wrote:
> >
> > Use the new inode_get_ino() helper to log the user space's view of
> > inode's numbers instead of the private kernel values.
> >
> > Cc: Paul Moore <paul@paul-moore.com>
> > Cc: Eric Paris <eparis@redhat.com>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > ---
> >  security/lsm_audit.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
>
> Acked-by: Paul Moore <paul@paul-moore.com>

It looks like patch 1/7 still needs some revisions, and an ACK from
the NFS/VFS folks, but once that's sorted I can send the patchset up
to Linus marked for stable.
Paul Moore Oct. 11, 2024, 9:34 p.m. UTC | #3
On Thu, Oct 10, 2024 at 11:26 AM Mickaël Salaün <mic@digikod.net> wrote:
>
> Use the new inode_get_ino() helper to log the user space's view of
> inode's numbers instead of the private kernel values.
>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: Eric Paris <eparis@redhat.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
>  security/lsm_audit.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

While answering some off-list questions regarding audit, I realized
we've got similar issues with audit_name->ino and audit_watch->ino.
It would be nice if you could also fix that in this patchset.
Mickaël Salaün Oct. 14, 2024, 1:30 p.m. UTC | #4
On Fri, Oct 11, 2024 at 05:34:21PM -0400, Paul Moore wrote:
> On Thu, Oct 10, 2024 at 11:26 AM Mickaël Salaün <mic@digikod.net> wrote:
> >
> > Use the new inode_get_ino() helper to log the user space's view of
> > inode's numbers instead of the private kernel values.
> >
> > Cc: Paul Moore <paul@paul-moore.com>
> > Cc: Eric Paris <eparis@redhat.com>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > ---
> >  security/lsm_audit.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> While answering some off-list questions regarding audit, I realized
> we've got similar issues with audit_name->ino and audit_watch->ino.
> It would be nice if you could also fix that in this patchset.

I can do that with the next version, but I'm wondering how it would fit
with the UAPI's struct audit_rule_data which has only 32-bit
fields/values.  Does 64-bit inode filtering currently work?
Paul Moore Oct. 14, 2024, 11:36 p.m. UTC | #5
On Mon, Oct 14, 2024 at 9:30 AM Mickaël Salaün <mic@digikod.net> wrote:
> On Fri, Oct 11, 2024 at 05:34:21PM -0400, Paul Moore wrote:
> > On Thu, Oct 10, 2024 at 11:26 AM Mickaël Salaün <mic@digikod.net> wrote:
> > >
> > > Use the new inode_get_ino() helper to log the user space's view of
> > > inode's numbers instead of the private kernel values.
> > >
> > > Cc: Paul Moore <paul@paul-moore.com>
> > > Cc: Eric Paris <eparis@redhat.com>
> > > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > > ---
> > >  security/lsm_audit.c | 10 +++++-----
> > >  1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > While answering some off-list questions regarding audit, I realized
> > we've got similar issues with audit_name->ino and audit_watch->ino.
> > It would be nice if you could also fix that in this patchset.
>
> I can do that with the next version, but I'm wondering how it would fit
> with the UAPI's struct audit_rule_data which has only 32-bit
> fields/values.

Don't worry about audit_rule_data for the moment, that's obviously
going to require a userspace update as well to supply 64-bit inode
numbers.  My guess is we'll probably want to introduce a new field
type, e.g. AUDIT_INODE64 or similar, that either carries the high
32-bits and is used in conjunction with AUDIT_INODE, or we create the
new AUDIT_INODE64 field as a "special" filter field which takes up two
of the u32 value spots.  Regardless, let's not worry about that for
this patchset and focus on ensuring the underlying kernel filtering
and reporting mechanisms work as expected so that when we do sort out
the UAPI issues everything *should* work.

> Does 64-bit inode filtering currently work?

Likely not :/
diff mbox series

Patch

diff --git a/security/lsm_audit.c b/security/lsm_audit.c
index 849e832719e2..c39a22b27cce 100644
--- a/security/lsm_audit.c
+++ b/security/lsm_audit.c
@@ -227,7 +227,7 @@  static void dump_common_audit_data(struct audit_buffer *ab,
 		if (inode) {
 			audit_log_format(ab, " dev=");
 			audit_log_untrustedstring(ab, inode->i_sb->s_id);
-			audit_log_format(ab, " ino=%lu", inode->i_ino);
+			audit_log_format(ab, " ino=%llu", inode_get_ino(inode));
 		}
 		break;
 	}
@@ -240,7 +240,7 @@  static void dump_common_audit_data(struct audit_buffer *ab,
 		if (inode) {
 			audit_log_format(ab, " dev=");
 			audit_log_untrustedstring(ab, inode->i_sb->s_id);
-			audit_log_format(ab, " ino=%lu", inode->i_ino);
+			audit_log_format(ab, " ino=%llu", inode_get_ino(inode));
 		}
 		break;
 	}
@@ -253,7 +253,7 @@  static void dump_common_audit_data(struct audit_buffer *ab,
 		if (inode) {
 			audit_log_format(ab, " dev=");
 			audit_log_untrustedstring(ab, inode->i_sb->s_id);
-			audit_log_format(ab, " ino=%lu", inode->i_ino);
+			audit_log_format(ab, " ino=%llu", inode_get_ino(inode));
 		}
 
 		audit_log_format(ab, " ioctlcmd=0x%hx", a->u.op->cmd);
@@ -271,7 +271,7 @@  static void dump_common_audit_data(struct audit_buffer *ab,
 		if (inode) {
 			audit_log_format(ab, " dev=");
 			audit_log_untrustedstring(ab, inode->i_sb->s_id);
-			audit_log_format(ab, " ino=%lu", inode->i_ino);
+			audit_log_format(ab, " ino=%llu", inode_get_ino(inode));
 		}
 		break;
 	}
@@ -290,7 +290,7 @@  static void dump_common_audit_data(struct audit_buffer *ab,
 		}
 		audit_log_format(ab, " dev=");
 		audit_log_untrustedstring(ab, inode->i_sb->s_id);
-		audit_log_format(ab, " ino=%lu", inode->i_ino);
+		audit_log_format(ab, " ino=%llu", inode_get_ino(inode));
 		rcu_read_unlock();
 		break;
 	}