2021-03-18 01:54:33

by Richard Guy Briggs

[permalink] [raw]

Subject: [PATCH 0/2] audit: add support for openat2

---

The openat2(2) syscall was added in v5.6. Add support for openat2 to the
audit syscall classifier and for recording openat2 parameters that cannot
be captured in the syscall parameters of the SYSCALL record.

Supporting userspace code can be found in
https://github.com/rgbriggs/audit-userspace/tree/ghau-openat2

Supporting test case can be found in
https://github.com/linux-audit/audit-testsuite/pull/103

Richard Guy Briggs (2):
audit: add support for the openat2 syscall
audit: add OPENAT2 record to list how

arch/alpha/kernel/audit.c | 2 ++
arch/ia64/kernel/audit.c | 2 ++
arch/parisc/kernel/audit.c | 2 ++
arch/parisc/kernel/compat_audit.c | 2 ++
arch/powerpc/kernel/audit.c | 2 ++
arch/powerpc/kernel/compat_audit.c | 2 ++
arch/s390/kernel/audit.c | 2 ++
arch/s390/kernel/compat_audit.c | 2 ++
arch/sparc/kernel/audit.c | 2 ++
arch/sparc/kernel/compat_audit.c | 2 ++
arch/x86/ia32/audit.c | 2 ++
arch/x86/kernel/audit_64.c | 2 ++
fs/open.c | 2 ++
include/linux/audit.h | 10 ++++++++++
include/uapi/linux/audit.h | 1 +
kernel/audit.h | 2 ++
kernel/auditsc.c | 19 +++++++++++++++++++
lib/audit.c | 4 ++++
lib/compat_audit.c | 4 ++++
19 files changed, 66 insertions(+)

--
2.27.0

2021-03-18 01:55:49

by Richard Guy Briggs

[permalink] [raw]

Subject: [PATCH 1/2] audit: add support for the openat2 syscall

---

The openat2(2) syscall was added in kernel v5.6 with commit fddb5d430ad9
("open: introduce openat2(2) syscall")

Add the openat2(2) syscall to the audit syscall classifier.

See the github issue
https://github.com/linux-audit/audit-kernel/issues/67

Signed-off-by: Richard Guy Briggs <[email protected]>
---
arch/alpha/kernel/audit.c | 2 ++
arch/ia64/kernel/audit.c | 2 ++
arch/parisc/kernel/audit.c | 2 ++
arch/parisc/kernel/compat_audit.c | 2 ++
arch/powerpc/kernel/audit.c | 2 ++
arch/powerpc/kernel/compat_audit.c | 2 ++
arch/s390/kernel/audit.c | 2 ++
arch/s390/kernel/compat_audit.c | 2 ++
arch/sparc/kernel/audit.c | 2 ++
arch/sparc/kernel/compat_audit.c | 2 ++
arch/x86/ia32/audit.c | 2 ++
arch/x86/kernel/audit_64.c | 2 ++
kernel/auditsc.c | 3 +++
lib/audit.c | 4 ++++
lib/compat_audit.c | 4 ++++
15 files changed, 35 insertions(+)

diff --git a/arch/alpha/kernel/audit.c b/arch/alpha/kernel/audit.c
index 96a9d18ff4c4..06a911b685d1 100644
--- a/arch/alpha/kernel/audit.c
+++ b/arch/alpha/kernel/audit.c
@@ -42,6 +42,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
return 3;
case __NR_execve:
return 5;
+ case __NR_openat2:
+ return 6;
default:
return 0;
}
diff --git a/arch/ia64/kernel/audit.c b/arch/ia64/kernel/audit.c
index 5192ca899fe6..5eaa888c8fd3 100644
--- a/arch/ia64/kernel/audit.c
+++ b/arch/ia64/kernel/audit.c
@@ -43,6 +43,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
return 3;
case __NR_execve:
return 5;
+ case __NR_openat2:
+ return 6;
default:
return 0;
}
diff --git a/arch/parisc/kernel/audit.c b/arch/parisc/kernel/audit.c
index 9eb47b2225d2..fc721a7727ba 100644
--- a/arch/parisc/kernel/audit.c
+++ b/arch/parisc/kernel/audit.c
@@ -52,6 +52,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
return 3;
case __NR_execve:
return 5;
+ case __NR_openat2:
+ return 6;
default:
return 0;
}
diff --git a/arch/parisc/kernel/compat_audit.c b/arch/parisc/kernel/compat_audit.c
index 20c39c9d86a9..fc6d35918c44 100644
--- a/arch/parisc/kernel/compat_audit.c
+++ b/arch/parisc/kernel/compat_audit.c
@@ -35,6 +35,8 @@ int parisc32_classify_syscall(unsigned syscall)
return 3;
case __NR_execve:
return 5;
+ case __NR_openat2:
+ return 6;
default:
return 1;
}
diff --git a/arch/powerpc/kernel/audit.c b/arch/powerpc/kernel/audit.c
index a2dddd7f3d09..8f32700b0baa 100644
--- a/arch/powerpc/kernel/audit.c
+++ b/arch/powerpc/kernel/audit.c
@@ -54,6 +54,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
return 4;
case __NR_execve:
return 5;
+ case __NR_openat2:
+ return 6;
default:
return 0;
}
diff --git a/arch/powerpc/kernel/compat_audit.c b/arch/powerpc/kernel/compat_audit.c
index 55c6ccda0a85..ebe45534b1c9 100644
--- a/arch/powerpc/kernel/compat_audit.c
+++ b/arch/powerpc/kernel/compat_audit.c
@@ -38,6 +38,8 @@ int ppc32_classify_syscall(unsigned syscall)
return 4;
case __NR_execve:
return 5;
+ case __NR_openat2:
+ return 6;
default:
return 1;
}
diff --git a/arch/s390/kernel/audit.c b/arch/s390/kernel/audit.c
index d395c6c9944c..d964cb94cfaf 100644
--- a/arch/s390/kernel/audit.c
+++ b/arch/s390/kernel/audit.c
@@ -54,6 +54,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
return 4;
case __NR_execve:
return 5;
+ case __NR_openat2:
+ return 6;
default:
return 0;
}
diff --git a/arch/s390/kernel/compat_audit.c b/arch/s390/kernel/compat_audit.c
index 444fb1f66944..f7b32933ce0e 100644
--- a/arch/s390/kernel/compat_audit.c
+++ b/arch/s390/kernel/compat_audit.c
@@ -39,6 +39,8 @@ int s390_classify_syscall(unsigned syscall)
return 4;
case __NR_execve:
return 5;
+ case __NR_openat2:
+ return 6;
default:
return 1;
}
diff --git a/arch/sparc/kernel/audit.c b/arch/sparc/kernel/audit.c
index a6e91bf34d48..b6dcca9c6520 100644
--- a/arch/sparc/kernel/audit.c
+++ b/arch/sparc/kernel/audit.c
@@ -55,6 +55,8 @@ int audit_classify_syscall(int abi, unsigned int syscall)
return 4;
case __NR_execve:
return 5;
+ case __NR_openat2:
+ return 6;
default:
return 0;
}
diff --git a/arch/sparc/kernel/compat_audit.c b/arch/sparc/kernel/compat_audit.c
index 10eeb4f15b20..d2652a1083ad 100644
--- a/arch/sparc/kernel/compat_audit.c
+++ b/arch/sparc/kernel/compat_audit.c
@@ -39,6 +39,8 @@ int sparc32_classify_syscall(unsigned int syscall)
return 4;
case __NR_execve:
return 5;
+ case __NR_openat2:
+ return 6;
default:
return 1;
}
diff --git a/arch/x86/ia32/audit.c b/arch/x86/ia32/audit.c
index 6efe6cb3768a..57a02ade5503 100644
--- a/arch/x86/ia32/audit.c
+++ b/arch/x86/ia32/audit.c
@@ -39,6 +39,8 @@ int ia32_classify_syscall(unsigned syscall)
case __NR_execve:
case __NR_execveat:
return 5;
+ case __NR_openat2:
+ return 6;
default:
return 1;
}
diff --git a/arch/x86/kernel/audit_64.c b/arch/x86/kernel/audit_64.c
index 83d9cad4e68b..39de1e021258 100644
--- a/arch/x86/kernel/audit_64.c
+++ b/arch/x86/kernel/audit_64.c
@@ -53,6 +53,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
case __NR_execve:
case __NR_execveat:
return 5;
+ case __NR_openat2:
+ return 6;
default:
return 0;
}
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 8bb9ac84d2fb..f5616e70d129 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -76,6 +76,7 @@
#include <linux/fsnotify_backend.h>
#include <uapi/linux/limits.h>
#include <uapi/linux/netfilter/nf_tables.h>
+#include <uapi/linux/openat2.h>

#include "audit.h"

@@ -195,6 +196,8 @@ static int audit_match_perm(struct audit_context *ctx, int mask)
return ((mask & AUDIT_PERM_WRITE) && ctx->argv[0] == SYS_BIND);
case 5: /* execve */
return mask & AUDIT_PERM_EXEC;
+ case 6: /* openat2 */
+ return mask & ACC_MODE((u32)((struct open_how *)ctx->argv[2])->flags);
default:
return 0;
}
diff --git a/lib/audit.c b/lib/audit.c
index 5004bff928a7..8f030b9a2d10 100644
--- a/lib/audit.c
+++ b/lib/audit.c
@@ -60,6 +60,10 @@ int audit_classify_syscall(int abi, unsigned syscall)
#endif
case __NR_execve:
return 5;
+#ifdef __NR_openat2
+ case __NR_openat2:
+ return 6;
+#endif
default:
return 0;
}
diff --git a/lib/compat_audit.c b/lib/compat_audit.c
index 77eabad69b4a..8aff0d0d9ba0 100644
--- a/lib/compat_audit.c
+++ b/lib/compat_audit.c
@@ -45,6 +45,10 @@ int audit_classify_compat_syscall(int abi, unsigned syscall)
#endif
case __NR_execve:
return 5;
+#ifdef __NR_openat2
+ case __NR_openat2:
+ return 6;
+#endif
default:
return 1;
}
--
2.27.0

2021-03-18 01:57:00

by Richard Guy Briggs

[permalink] [raw]

Subject: [PATCH 2/2] audit: add OPENAT2 record to list how

---

Since the openat2(2) syscall uses a struct open_how pointer to communicate
its parameters they are not usefully recorded by the audit SYSCALL record's
four existing arguments.

Add a new audit record type OPENAT2 that reports the parameters in its
third argument, struct open_how with fields oflag, mode and resolve.

The new record in the context of an event would look like:
time->Wed Mar 17 16:28:53 2021
type=PROCTITLE msg=audit(1616012933.531:184): proctitle=73797363616C6C735F66696C652F6F70656E617432002F746D702F61756469742D7465737473756974652D737641440066696C652D6F70656E617432
type=PATH msg=audit(1616012933.531:184): item=1 name="file-openat2" inode=29 dev=00:1f mode=0100600 ouid=0 ogid=0 rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0 nametype=CREATE cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0
type=PATH msg=audit(1616012933.531:184): item=0 name="/root/rgb/git/audit-testsuite/tests" inode=25 dev=00:1f mode=040700 ouid=0 ogid=0 rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0 nametype=PARENT cap_fp=0 cap_fi=0 cap_fe=0 cap_fver=0 cap_frootid=0
type=CWD msg=audit(1616012933.531:184): cwd="/root/rgb/git/audit-testsuite/tests"
type=OPENAT2 msg=audit(1616012933.531:184): oflag=0100302 mode=0600 resolve=0xa
type=SYSCALL msg=audit(1616012933.531:184): arch=c000003e syscall=437 success=yes exit=4 a0=3 a1=7ffe315f1c53 a2=7ffe315f1550 a3=18 items=2 ppid=528 pid=540 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=ttyS0 ses=1 comm="openat2" exe="/root/rgb/git/audit-testsuite/tests/syscalls_file/openat2" subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key="testsuite-1616012933-bjAUcEPO"

Signed-off-by: Richard Guy Briggs <[email protected]>
---
fs/open.c | 2 ++
include/linux/audit.h | 10 ++++++++++
include/uapi/linux/audit.h | 1 +
kernel/audit.h | 2 ++
kernel/auditsc.c | 18 +++++++++++++++++-
5 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/fs/open.c b/fs/open.c
index e53af13b5835..2a15bec0cf6d 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1235,6 +1235,8 @@ SYSCALL_DEFINE4(openat2, int, dfd, const char __user *, filename,
if (err)
return err;

+ audit_openat2_how(&tmp);
+
/* O_LARGEFILE is only allowed for non-O_PATH. */
if (!(tmp.flags & O_PATH) && force_o_largefile())
tmp.flags |= O_LARGEFILE;
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 82b7c1116a85..4c9bc387f7b3 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -398,6 +398,7 @@ extern int __audit_log_bprm_fcaps(struct linux_binprm *bprm,
const struct cred *old);
extern void __audit_log_capset(const struct cred *new, const struct cred *old);
extern void __audit_mmap_fd(int fd, int flags);
+extern void __audit_openat2_how(struct open_how *how);
extern void __audit_log_kern_module(char *name);
extern void __audit_fanotify(unsigned int response);
extern void __audit_tk_injoffset(struct timespec64 offset);
@@ -494,6 +495,12 @@ static inline void audit_mmap_fd(int fd, int flags)
__audit_mmap_fd(fd, flags);
}

+static inline void audit_openat2_how(struct open_how *how)
+{
+ if (unlikely(!audit_dummy_context()))
+ __audit_openat2_how(how);
+}
+
static inline void audit_log_kern_module(char *name)
{
if (!audit_dummy_context())
@@ -645,6 +652,9 @@ static inline void audit_log_capset(const struct cred *new,
static inline void audit_mmap_fd(int fd, int flags)
{ }

+static inline void audit_openat2_how(struct open_how *how)
+{ }
+
static inline void audit_log_kern_module(char *name)
{
}
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index cd2d8279a5e4..67aea2370c6d 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -118,6 +118,7 @@
#define AUDIT_TIME_ADJNTPVAL 1333 /* NTP value adjustment */
#define AUDIT_BPF 1334 /* BPF subsystem */
#define AUDIT_EVENT_LISTENER 1335 /* Task joined multicast read socket */
+#define AUDIT_OPENAT2 1336 /* Record showing openat2 how args */

#define AUDIT_AVC 1400 /* SE Linux avc denial or grant */
#define AUDIT_SELINUX_ERR 1401 /* Internal SE Linux Errors */
diff --git a/kernel/audit.h b/kernel/audit.h
index 3b9c0945225a..97db994155e0 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -11,6 +11,7 @@
#include <linux/skbuff.h>
#include <uapi/linux/mqueue.h>
#include <linux/tty.h>
+#include <uapi/linux/openat2.h> // struct open_how

/* AUDIT_NAMES is the number of slots we reserve in the audit_context
* for saving names from getname(). If we get more names we will allocate
@@ -185,6 +186,7 @@ struct audit_context {
int fd;
int flags;
} mmap;
+ struct open_how openat2;
struct {
int argc;
} execve;
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index f5616e70d129..9f6fa2574ecc 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -76,7 +76,7 @@
#include <linux/fsnotify_backend.h>
#include <uapi/linux/limits.h>
#include <uapi/linux/netfilter/nf_tables.h>
-#include <uapi/linux/openat2.h>
+#include <uapi/linux/openat2.h> // struct open_how

#include "audit.h"

@@ -1310,6 +1310,12 @@ static void show_special(struct audit_context *context, int *call_panic)
audit_log_format(ab, "fd=%d flags=0x%x", context->mmap.fd,
context->mmap.flags);
break;
+ case AUDIT_OPENAT2:
+ audit_log_format(ab, "oflag=0%llo mode=0%llo resolve=0x%llx",
+ context->openat2.flags,
+ context->openat2.mode,
+ context->openat2.resolve);
+ break;
case AUDIT_EXECVE:
audit_log_execve_info(context, &ab);
break;
@@ -2529,6 +2535,16 @@ void __audit_mmap_fd(int fd, int flags)
context->type = AUDIT_MMAP;
}

+void __audit_openat2_how(struct open_how *how)
+{
+ struct audit_context *context = audit_context();
+
+ context->openat2.flags = how->flags;
+ context->openat2.mode = how->mode;
+ context->openat2.resolve = how->resolve;
+ context->type = AUDIT_OPENAT2;
+}
+
void __audit_log_kern_module(char *name)
{
struct audit_context *context = audit_context();
--
2.27.0

2021-03-18 10:52:27

by Christian Brauner

[permalink] [raw]

Subject: Re: [PATCH 1/2] audit: add support for the openat2 syscall

---

[+Cc Aleksa, the author of openat2()]

and a comment below. :)

On Wed, Mar 17, 2021 at 09:47:17PM -0400, Richard Guy Briggs wrote:
> The openat2(2) syscall was added in kernel v5.6 with commit fddb5d430ad9
> ("open: introduce openat2(2) syscall")
>
> Add the openat2(2) syscall to the audit syscall classifier.
>
> See the github issue
> https://github.com/linux-audit/audit-kernel/issues/67
>
> Signed-off-by: Richard Guy Briggs <[email protected]>
> ---
> arch/alpha/kernel/audit.c | 2 ++
> arch/ia64/kernel/audit.c | 2 ++
> arch/parisc/kernel/audit.c | 2 ++
> arch/parisc/kernel/compat_audit.c | 2 ++
> arch/powerpc/kernel/audit.c | 2 ++
> arch/powerpc/kernel/compat_audit.c | 2 ++
> arch/s390/kernel/audit.c | 2 ++
> arch/s390/kernel/compat_audit.c | 2 ++
> arch/sparc/kernel/audit.c | 2 ++
> arch/sparc/kernel/compat_audit.c | 2 ++
> arch/x86/ia32/audit.c | 2 ++
> arch/x86/kernel/audit_64.c | 2 ++
> kernel/auditsc.c | 3 +++
> lib/audit.c | 4 ++++
> lib/compat_audit.c | 4 ++++
> 15 files changed, 35 insertions(+)
>
> diff --git a/arch/alpha/kernel/audit.c b/arch/alpha/kernel/audit.c
> index 96a9d18ff4c4..06a911b685d1 100644
> --- a/arch/alpha/kernel/audit.c
> +++ b/arch/alpha/kernel/audit.c
> @@ -42,6 +42,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> return 3;
> case __NR_execve:
> return 5;
> + case __NR_openat2:
> + return 6;
> default:
> return 0;
> }
> diff --git a/arch/ia64/kernel/audit.c b/arch/ia64/kernel/audit.c
> index 5192ca899fe6..5eaa888c8fd3 100644
> --- a/arch/ia64/kernel/audit.c
> +++ b/arch/ia64/kernel/audit.c
> @@ -43,6 +43,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> return 3;
> case __NR_execve:
> return 5;
> + case __NR_openat2:
> + return 6;
> default:
> return 0;
> }
> diff --git a/arch/parisc/kernel/audit.c b/arch/parisc/kernel/audit.c
> index 9eb47b2225d2..fc721a7727ba 100644
> --- a/arch/parisc/kernel/audit.c
> +++ b/arch/parisc/kernel/audit.c
> @@ -52,6 +52,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> return 3;
> case __NR_execve:
> return 5;
> + case __NR_openat2:
> + return 6;
> default:
> return 0;
> }
> diff --git a/arch/parisc/kernel/compat_audit.c b/arch/parisc/kernel/compat_audit.c
> index 20c39c9d86a9..fc6d35918c44 100644
> --- a/arch/parisc/kernel/compat_audit.c
> +++ b/arch/parisc/kernel/compat_audit.c
> @@ -35,6 +35,8 @@ int parisc32_classify_syscall(unsigned syscall)
> return 3;
> case __NR_execve:
> return 5;
> + case __NR_openat2:
> + return 6;
> default:
> return 1;
> }
> diff --git a/arch/powerpc/kernel/audit.c b/arch/powerpc/kernel/audit.c
> index a2dddd7f3d09..8f32700b0baa 100644
> --- a/arch/powerpc/kernel/audit.c
> +++ b/arch/powerpc/kernel/audit.c
> @@ -54,6 +54,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> return 4;
> case __NR_execve:
> return 5;
> + case __NR_openat2:
> + return 6;
> default:
> return 0;
> }
> diff --git a/arch/powerpc/kernel/compat_audit.c b/arch/powerpc/kernel/compat_audit.c
> index 55c6ccda0a85..ebe45534b1c9 100644
> --- a/arch/powerpc/kernel/compat_audit.c
> +++ b/arch/powerpc/kernel/compat_audit.c
> @@ -38,6 +38,8 @@ int ppc32_classify_syscall(unsigned syscall)
> return 4;
> case __NR_execve:
> return 5;
> + case __NR_openat2:
> + return 6;
> default:
> return 1;
> }
> diff --git a/arch/s390/kernel/audit.c b/arch/s390/kernel/audit.c
> index d395c6c9944c..d964cb94cfaf 100644
> --- a/arch/s390/kernel/audit.c
> +++ b/arch/s390/kernel/audit.c
> @@ -54,6 +54,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> return 4;
> case __NR_execve:
> return 5;
> + case __NR_openat2:
> + return 6;
> default:
> return 0;
> }
> diff --git a/arch/s390/kernel/compat_audit.c b/arch/s390/kernel/compat_audit.c
> index 444fb1f66944..f7b32933ce0e 100644
> --- a/arch/s390/kernel/compat_audit.c
> +++ b/arch/s390/kernel/compat_audit.c
> @@ -39,6 +39,8 @@ int s390_classify_syscall(unsigned syscall)
> return 4;
> case __NR_execve:
> return 5;
> + case __NR_openat2:
> + return 6;
> default:
> return 1;
> }
> diff --git a/arch/sparc/kernel/audit.c b/arch/sparc/kernel/audit.c
> index a6e91bf34d48..b6dcca9c6520 100644
> --- a/arch/sparc/kernel/audit.c
> +++ b/arch/sparc/kernel/audit.c
> @@ -55,6 +55,8 @@ int audit_classify_syscall(int abi, unsigned int syscall)
> return 4;
> case __NR_execve:
> return 5;
> + case __NR_openat2:
> + return 6;
> default:
> return 0;
> }
> diff --git a/arch/sparc/kernel/compat_audit.c b/arch/sparc/kernel/compat_audit.c
> index 10eeb4f15b20..d2652a1083ad 100644
> --- a/arch/sparc/kernel/compat_audit.c
> +++ b/arch/sparc/kernel/compat_audit.c
> @@ -39,6 +39,8 @@ int sparc32_classify_syscall(unsigned int syscall)
> return 4;
> case __NR_execve:
> return 5;
> + case __NR_openat2:
> + return 6;
> default:
> return 1;
> }
> diff --git a/arch/x86/ia32/audit.c b/arch/x86/ia32/audit.c
> index 6efe6cb3768a..57a02ade5503 100644
> --- a/arch/x86/ia32/audit.c
> +++ b/arch/x86/ia32/audit.c
> @@ -39,6 +39,8 @@ int ia32_classify_syscall(unsigned syscall)
> case __NR_execve:
> case __NR_execveat:
> return 5;
> + case __NR_openat2:
> + return 6;
> default:
> return 1;
> }
> diff --git a/arch/x86/kernel/audit_64.c b/arch/x86/kernel/audit_64.c
> index 83d9cad4e68b..39de1e021258 100644
> --- a/arch/x86/kernel/audit_64.c
> +++ b/arch/x86/kernel/audit_64.c
> @@ -53,6 +53,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> case __NR_execve:
> case __NR_execveat:
> return 5;
> + case __NR_openat2:
> + return 6;
> default:
> return 0;
> }
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 8bb9ac84d2fb..f5616e70d129 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -76,6 +76,7 @@
> #include <linux/fsnotify_backend.h>
> #include <uapi/linux/limits.h>
> #include <uapi/linux/netfilter/nf_tables.h>
> +#include <uapi/linux/openat2.h>
>
> #include "audit.h"
>
> @@ -195,6 +196,8 @@ static int audit_match_perm(struct audit_context *ctx, int mask)
> return ((mask & AUDIT_PERM_WRITE) && ctx->argv[0] == SYS_BIND);
> case 5: /* execve */
> return mask & AUDIT_PERM_EXEC;
> + case 6: /* openat2 */
> + return mask & ACC_MODE((u32)((struct open_how *)ctx->argv[2])->flags);

That looks a bit dodgy. Maybe sm like the below would be a bit better?

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 47fb48f42c93..531e882a5096 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -159,6 +159,7 @@ static const struct audit_nfcfgop_tab audit_nfcfgs[] = {

static int audit_match_perm(struct audit_context *ctx, int mask)
{
+ struct open_how *openat2;
unsigned n;
if (unlikely(!ctx))
return 0;
@@ -195,6 +196,12 @@ static int audit_match_perm(struct audit_context *ctx, int mask)
return ((mask & AUDIT_PERM_WRITE) && ctx->argv[0] == SYS_BIND);
case 5: /* execve */
return mask & AUDIT_PERM_EXEC;
+ case 6: /* openat2 */
+ openat2 = ctx->argv[2];
+ if (upper_32_bits(openat2->flags))
+ pr_warn("Some sensible warning about unknown flags");
+
+ return mask & ACC_MODE(lower_32_bits(openat2->flags));
default:
return 0;
}

(Ideally we'd probably notice at build-time that we've got flags
exceeding 32bits. Could probably easily been done by exposing an all
flags macro somewhere and then we can place a BUILD_BUG_ON() or sm into
such places.)

Christian

2021-03-18 10:53:31

by Christian Brauner

[permalink] [raw]

Subject: Re: [PATCH 0/2] audit: add support for openat2

---

On Wed, Mar 17, 2021 at 09:47:16PM -0400, Richard Guy Briggs wrote:
> The openat2(2) syscall was added in v5.6. Add support for openat2 to the
> audit syscall classifier and for recording openat2 parameters that cannot
> be captured in the syscall parameters of the SYSCALL record.
>
> Supporting userspace code can be found in
> https://github.com/rgbriggs/audit-userspace/tree/ghau-openat2
>
> Supporting test case can be found in
> https://github.com/linux-audit/audit-testsuite/pull/103

Seems sensible, thank you.

2021-03-18 10:55:55

by Christian Brauner

[permalink] [raw]

Subject: Re: [PATCH 1/2] audit: add support for the openat2 syscall

---

On Thu, Mar 18, 2021 at 11:48:45AM +0100, Christian Brauner wrote:
> [+Cc Aleksa, the author of openat2()]
>
> and a comment below. :)
>
> On Wed, Mar 17, 2021 at 09:47:17PM -0400, Richard Guy Briggs wrote:
> > The openat2(2) syscall was added in kernel v5.6 with commit fddb5d430ad9
> > ("open: introduce openat2(2) syscall")
> >
> > Add the openat2(2) syscall to the audit syscall classifier.
> >
> > See the github issue
> > https://github.com/linux-audit/audit-kernel/issues/67
> >
> > Signed-off-by: Richard Guy Briggs <[email protected]>
> > ---
> > arch/alpha/kernel/audit.c | 2 ++
> > arch/ia64/kernel/audit.c | 2 ++
> > arch/parisc/kernel/audit.c | 2 ++
> > arch/parisc/kernel/compat_audit.c | 2 ++
> > arch/powerpc/kernel/audit.c | 2 ++
> > arch/powerpc/kernel/compat_audit.c | 2 ++
> > arch/s390/kernel/audit.c | 2 ++
> > arch/s390/kernel/compat_audit.c | 2 ++
> > arch/sparc/kernel/audit.c | 2 ++
> > arch/sparc/kernel/compat_audit.c | 2 ++
> > arch/x86/ia32/audit.c | 2 ++
> > arch/x86/kernel/audit_64.c | 2 ++
> > kernel/auditsc.c | 3 +++
> > lib/audit.c | 4 ++++
> > lib/compat_audit.c | 4 ++++
> > 15 files changed, 35 insertions(+)
> >
> > diff --git a/arch/alpha/kernel/audit.c b/arch/alpha/kernel/audit.c
> > index 96a9d18ff4c4..06a911b685d1 100644
> > --- a/arch/alpha/kernel/audit.c
> > +++ b/arch/alpha/kernel/audit.c
> > @@ -42,6 +42,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > return 3;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/ia64/kernel/audit.c b/arch/ia64/kernel/audit.c
> > index 5192ca899fe6..5eaa888c8fd3 100644
> > --- a/arch/ia64/kernel/audit.c
> > +++ b/arch/ia64/kernel/audit.c
> > @@ -43,6 +43,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > return 3;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/parisc/kernel/audit.c b/arch/parisc/kernel/audit.c
> > index 9eb47b2225d2..fc721a7727ba 100644
> > --- a/arch/parisc/kernel/audit.c
> > +++ b/arch/parisc/kernel/audit.c
> > @@ -52,6 +52,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > return 3;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/parisc/kernel/compat_audit.c b/arch/parisc/kernel/compat_audit.c
> > index 20c39c9d86a9..fc6d35918c44 100644
> > --- a/arch/parisc/kernel/compat_audit.c
> > +++ b/arch/parisc/kernel/compat_audit.c
> > @@ -35,6 +35,8 @@ int parisc32_classify_syscall(unsigned syscall)
> > return 3;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 1;
> > }
> > diff --git a/arch/powerpc/kernel/audit.c b/arch/powerpc/kernel/audit.c
> > index a2dddd7f3d09..8f32700b0baa 100644
> > --- a/arch/powerpc/kernel/audit.c
> > +++ b/arch/powerpc/kernel/audit.c
> > @@ -54,6 +54,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/powerpc/kernel/compat_audit.c b/arch/powerpc/kernel/compat_audit.c
> > index 55c6ccda0a85..ebe45534b1c9 100644
> > --- a/arch/powerpc/kernel/compat_audit.c
> > +++ b/arch/powerpc/kernel/compat_audit.c
> > @@ -38,6 +38,8 @@ int ppc32_classify_syscall(unsigned syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 1;
> > }
> > diff --git a/arch/s390/kernel/audit.c b/arch/s390/kernel/audit.c
> > index d395c6c9944c..d964cb94cfaf 100644
> > --- a/arch/s390/kernel/audit.c
> > +++ b/arch/s390/kernel/audit.c
> > @@ -54,6 +54,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/s390/kernel/compat_audit.c b/arch/s390/kernel/compat_audit.c
> > index 444fb1f66944..f7b32933ce0e 100644
> > --- a/arch/s390/kernel/compat_audit.c
> > +++ b/arch/s390/kernel/compat_audit.c
> > @@ -39,6 +39,8 @@ int s390_classify_syscall(unsigned syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 1;
> > }
> > diff --git a/arch/sparc/kernel/audit.c b/arch/sparc/kernel/audit.c
> > index a6e91bf34d48..b6dcca9c6520 100644
> > --- a/arch/sparc/kernel/audit.c
> > +++ b/arch/sparc/kernel/audit.c
> > @@ -55,6 +55,8 @@ int audit_classify_syscall(int abi, unsigned int syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/sparc/kernel/compat_audit.c b/arch/sparc/kernel/compat_audit.c
> > index 10eeb4f15b20..d2652a1083ad 100644
> > --- a/arch/sparc/kernel/compat_audit.c
> > +++ b/arch/sparc/kernel/compat_audit.c
> > @@ -39,6 +39,8 @@ int sparc32_classify_syscall(unsigned int syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 1;
> > }
> > diff --git a/arch/x86/ia32/audit.c b/arch/x86/ia32/audit.c
> > index 6efe6cb3768a..57a02ade5503 100644
> > --- a/arch/x86/ia32/audit.c
> > +++ b/arch/x86/ia32/audit.c
> > @@ -39,6 +39,8 @@ int ia32_classify_syscall(unsigned syscall)
> > case __NR_execve:
> > case __NR_execveat:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 1;
> > }
> > diff --git a/arch/x86/kernel/audit_64.c b/arch/x86/kernel/audit_64.c
> > index 83d9cad4e68b..39de1e021258 100644
> > --- a/arch/x86/kernel/audit_64.c
> > +++ b/arch/x86/kernel/audit_64.c
> > @@ -53,6 +53,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > case __NR_execve:
> > case __NR_execveat:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 8bb9ac84d2fb..f5616e70d129 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -76,6 +76,7 @@
> > #include <linux/fsnotify_backend.h>
> > #include <uapi/linux/limits.h>
> > #include <uapi/linux/netfilter/nf_tables.h>
> > +#include <uapi/linux/openat2.h>
> >
> > #include "audit.h"
> >
> > @@ -195,6 +196,8 @@ static int audit_match_perm(struct audit_context *ctx, int mask)
> > return ((mask & AUDIT_PERM_WRITE) && ctx->argv[0] == SYS_BIND);
> > case 5: /* execve */
> > return mask & AUDIT_PERM_EXEC;
> > + case 6: /* openat2 */
> > + return mask & ACC_MODE((u32)((struct open_how *)ctx->argv[2])->flags);
>
> That looks a bit dodgy. Maybe sm like the below would be a bit better?
>
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 47fb48f42c93..531e882a5096 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -159,6 +159,7 @@ static const struct audit_nfcfgop_tab audit_nfcfgs[] = {
>
> static int audit_match_perm(struct audit_context *ctx, int mask)
> {
> + struct open_how *openat2;
> unsigned n;
> if (unlikely(!ctx))
> return 0;
> @@ -195,6 +196,12 @@ static int audit_match_perm(struct audit_context *ctx, int mask)
> return ((mask & AUDIT_PERM_WRITE) && ctx->argv[0] == SYS_BIND);
> case 5: /* execve */
> return mask & AUDIT_PERM_EXEC;
> + case 6: /* openat2 */
> + openat2 = ctx->argv[2];
> + if (upper_32_bits(openat2->flags))
> + pr_warn("Some sensible warning about unknown flags");
> +
> + return mask & ACC_MODE(lower_32_bits(openat2->flags));
> default:
> return 0;
> }
>
> (Ideally we'd probably notice at build-time that we've got flags
> exceeding 32bits. Could probably easily been done by exposing an all
> flags macro somewhere and then we can place a BUILD_BUG_ON() or sm into
> such places.)

And one more comment, why return a hard-coded integer from all of these
architectures instead of introducing an enum in a central place with
proper names idk:

enum audit_match_perm_t {
.
.
.
AUDIT_MATCH_PERM_EXECVE = 5,
AUDIT_MATCH_PERM_OPENAT2 = 6,
.
.
.
}

Then you can drop these hard-coded comments too and it's way less
brittle overall.

Christian

2021-03-18 12:05:16

by Richard Guy Briggs

[permalink] [raw]

Subject: Re: [PATCH 1/2] audit: add support for the openat2 syscall

---

On 2021-03-18 11:52, Christian Brauner wrote:
> On Thu, Mar 18, 2021 at 11:48:45AM +0100, Christian Brauner wrote:
> > On Wed, Mar 17, 2021 at 09:47:17PM -0400, Richard Guy Briggs wrote:
> > > The openat2(2) syscall was added in kernel v5.6 with commit fddb5d430ad9
> > > ("open: introduce openat2(2) syscall")
> > > Add the openat2(2) syscall to the audit syscall classifier.
> > > See the github issue
> > > https://github.com/linux-audit/audit-kernel/issues/67
> > > Signed-off-by: Richard Guy Briggs <[email protected]>

...

> And one more comment, why return a hard-coded integer from all of these
> architectures instead of introducing an enum in a central place with
> proper names idk:

Oh, believe me, I tried hard to do that because I really don't like
hard-coded magic values, but for expediency I continued the same
approach until I could sort out the header file mess. There was an
extra preparatory patch (attached) in this patchset with a different
audit syscall perms patch (also attached). By including "#include
<linux/audit.h>" in each of the compat source files there were warnings
of redefinitions of every __NR_* syscall number. The easiest way to get
rid of it would have been to pull the new AUDITSC_* definitions into a
new <linux/auditsc.h> file and include that from <linux/audit.h> and
each of the arch/*/*/*audit.c (and lib/*audit.c) files.

> enum audit_match_perm_t {
> .
> .
> .
> AUDIT_MATCH_PERM_EXECVE = 5,
> AUDIT_MATCH_PERM_OPENAT2 = 6,
> .
> .
> .
> }
>
> Then you can drop these hard-coded comments too and it's way less
> brittle overall.

Totally agree.

> Christian

- RGB

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

---

Attachments:

(No filename) (1.83 kB)
0001-audit-replace-magic-audit-syscall-class-numbers-with.patch (12.37 kB)
0002-audit-add-support-for-the-openat2-syscall.patch (8.07 kB)
Download all attachments

2021-03-18 12:10:15

by Richard Guy Briggs

[permalink] [raw]

Subject: Re: [PATCH 1/2] audit: add support for the openat2 syscall

---

On 2021-03-18 11:48, Christian Brauner wrote:
> [+Cc Aleksa, the author of openat2()]

Ah! Thanks for pulling in Aleksa. I thought I caught everyone...

> and a comment below. :)

Same...

> On Wed, Mar 17, 2021 at 09:47:17PM -0400, Richard Guy Briggs wrote:
> > The openat2(2) syscall was added in kernel v5.6 with commit fddb5d430ad9
> > ("open: introduce openat2(2) syscall")
> >
> > Add the openat2(2) syscall to the audit syscall classifier.
> >
> > See the github issue
> > https://github.com/linux-audit/audit-kernel/issues/67
> >
> > Signed-off-by: Richard Guy Briggs <[email protected]>
> > ---
> > arch/alpha/kernel/audit.c | 2 ++
> > arch/ia64/kernel/audit.c | 2 ++
> > arch/parisc/kernel/audit.c | 2 ++
> > arch/parisc/kernel/compat_audit.c | 2 ++
> > arch/powerpc/kernel/audit.c | 2 ++
> > arch/powerpc/kernel/compat_audit.c | 2 ++
> > arch/s390/kernel/audit.c | 2 ++
> > arch/s390/kernel/compat_audit.c | 2 ++
> > arch/sparc/kernel/audit.c | 2 ++
> > arch/sparc/kernel/compat_audit.c | 2 ++
> > arch/x86/ia32/audit.c | 2 ++
> > arch/x86/kernel/audit_64.c | 2 ++
> > kernel/auditsc.c | 3 +++
> > lib/audit.c | 4 ++++
> > lib/compat_audit.c | 4 ++++
> > 15 files changed, 35 insertions(+)
> >
> > diff --git a/arch/alpha/kernel/audit.c b/arch/alpha/kernel/audit.c
> > index 96a9d18ff4c4..06a911b685d1 100644
> > --- a/arch/alpha/kernel/audit.c
> > +++ b/arch/alpha/kernel/audit.c
> > @@ -42,6 +42,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > return 3;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/ia64/kernel/audit.c b/arch/ia64/kernel/audit.c
> > index 5192ca899fe6..5eaa888c8fd3 100644
> > --- a/arch/ia64/kernel/audit.c
> > +++ b/arch/ia64/kernel/audit.c
> > @@ -43,6 +43,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > return 3;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/parisc/kernel/audit.c b/arch/parisc/kernel/audit.c
> > index 9eb47b2225d2..fc721a7727ba 100644
> > --- a/arch/parisc/kernel/audit.c
> > +++ b/arch/parisc/kernel/audit.c
> > @@ -52,6 +52,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > return 3;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/parisc/kernel/compat_audit.c b/arch/parisc/kernel/compat_audit.c
> > index 20c39c9d86a9..fc6d35918c44 100644
> > --- a/arch/parisc/kernel/compat_audit.c
> > +++ b/arch/parisc/kernel/compat_audit.c
> > @@ -35,6 +35,8 @@ int parisc32_classify_syscall(unsigned syscall)
> > return 3;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 1;
> > }
> > diff --git a/arch/powerpc/kernel/audit.c b/arch/powerpc/kernel/audit.c
> > index a2dddd7f3d09..8f32700b0baa 100644
> > --- a/arch/powerpc/kernel/audit.c
> > +++ b/arch/powerpc/kernel/audit.c
> > @@ -54,6 +54,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/powerpc/kernel/compat_audit.c b/arch/powerpc/kernel/compat_audit.c
> > index 55c6ccda0a85..ebe45534b1c9 100644
> > --- a/arch/powerpc/kernel/compat_audit.c
> > +++ b/arch/powerpc/kernel/compat_audit.c
> > @@ -38,6 +38,8 @@ int ppc32_classify_syscall(unsigned syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 1;
> > }
> > diff --git a/arch/s390/kernel/audit.c b/arch/s390/kernel/audit.c
> > index d395c6c9944c..d964cb94cfaf 100644
> > --- a/arch/s390/kernel/audit.c
> > +++ b/arch/s390/kernel/audit.c
> > @@ -54,6 +54,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/s390/kernel/compat_audit.c b/arch/s390/kernel/compat_audit.c
> > index 444fb1f66944..f7b32933ce0e 100644
> > --- a/arch/s390/kernel/compat_audit.c
> > +++ b/arch/s390/kernel/compat_audit.c
> > @@ -39,6 +39,8 @@ int s390_classify_syscall(unsigned syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 1;
> > }
> > diff --git a/arch/sparc/kernel/audit.c b/arch/sparc/kernel/audit.c
> > index a6e91bf34d48..b6dcca9c6520 100644
> > --- a/arch/sparc/kernel/audit.c
> > +++ b/arch/sparc/kernel/audit.c
> > @@ -55,6 +55,8 @@ int audit_classify_syscall(int abi, unsigned int syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/arch/sparc/kernel/compat_audit.c b/arch/sparc/kernel/compat_audit.c
> > index 10eeb4f15b20..d2652a1083ad 100644
> > --- a/arch/sparc/kernel/compat_audit.c
> > +++ b/arch/sparc/kernel/compat_audit.c
> > @@ -39,6 +39,8 @@ int sparc32_classify_syscall(unsigned int syscall)
> > return 4;
> > case __NR_execve:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 1;
> > }
> > diff --git a/arch/x86/ia32/audit.c b/arch/x86/ia32/audit.c
> > index 6efe6cb3768a..57a02ade5503 100644
> > --- a/arch/x86/ia32/audit.c
> > +++ b/arch/x86/ia32/audit.c
> > @@ -39,6 +39,8 @@ int ia32_classify_syscall(unsigned syscall)
> > case __NR_execve:
> > case __NR_execveat:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 1;
> > }
> > diff --git a/arch/x86/kernel/audit_64.c b/arch/x86/kernel/audit_64.c
> > index 83d9cad4e68b..39de1e021258 100644
> > --- a/arch/x86/kernel/audit_64.c
> > +++ b/arch/x86/kernel/audit_64.c
> > @@ -53,6 +53,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > case __NR_execve:
> > case __NR_execveat:
> > return 5;
> > + case __NR_openat2:
> > + return 6;
> > default:
> > return 0;
> > }
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 8bb9ac84d2fb..f5616e70d129 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -76,6 +76,7 @@
> > #include <linux/fsnotify_backend.h>
> > #include <uapi/linux/limits.h>
> > #include <uapi/linux/netfilter/nf_tables.h>
> > +#include <uapi/linux/openat2.h>
> >
> > #include "audit.h"
> >
> > @@ -195,6 +196,8 @@ static int audit_match_perm(struct audit_context *ctx, int mask)
> > return ((mask & AUDIT_PERM_WRITE) && ctx->argv[0] == SYS_BIND);
> > case 5: /* execve */
> > return mask & AUDIT_PERM_EXEC;
> > + case 6: /* openat2 */
> > + return mask & ACC_MODE((u32)((struct open_how *)ctx->argv[2])->flags);
>
> That looks a bit dodgy. Maybe sm like the below would be a bit better?

Ah, ok, fair enough, since original flags use a u32 and this was picked
as u64 for alignment. It was just occurring to me last night that I
might have the dubious honour of being the first usage of 0%llo format
specifier in the kernel... ;-)

> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 47fb48f42c93..531e882a5096 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -159,6 +159,7 @@ static const struct audit_nfcfgop_tab audit_nfcfgs[] = {
>
> static int audit_match_perm(struct audit_context *ctx, int mask)
> {
> + struct open_how *openat2;
> unsigned n;
> if (unlikely(!ctx))
> return 0;
> @@ -195,6 +196,12 @@ static int audit_match_perm(struct audit_context *ctx, int mask)
> return ((mask & AUDIT_PERM_WRITE) && ctx->argv[0] == SYS_BIND);
> case 5: /* execve */
> return mask & AUDIT_PERM_EXEC;
> + case 6: /* openat2 */
> + openat2 = ctx->argv[2];
> + if (upper_32_bits(openat2->flags))
> + pr_warn("Some sensible warning about unknown flags");
> +
> + return mask & ACC_MODE(lower_32_bits(openat2->flags));
> default:
> return 0;
> }
>
> (Ideally we'd probably notice at build-time that we've got flags
> exceeding 32bits. Could probably easily been done by exposing an all
> flags macro somewhere and then we can place a BUILD_BUG_ON() or sm into
> such places.)
>
> Christian

- RGB

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

2021-04-23 02:36:36

by Richard Guy Briggs

[permalink] [raw]

Subject: Re: [PATCH 1/2] audit: add support for the openat2 syscall

---

On 2021-03-18 08:08, Richard Guy Briggs wrote:
> On 2021-03-18 11:48, Christian Brauner wrote:
> > [+Cc Aleksa, the author of openat2()]
>
> Ah! Thanks for pulling in Aleksa. I thought I caught everyone...
>
> > and a comment below. :)
>
> Same...
>
> > On Wed, Mar 17, 2021 at 09:47:17PM -0400, Richard Guy Briggs wrote:
> > > The openat2(2) syscall was added in kernel v5.6 with commit fddb5d430ad9
> > > ("open: introduce openat2(2) syscall")
> > >
> > > Add the openat2(2) syscall to the audit syscall classifier.
> > >
> > > See the github issue
> > > https://github.com/linux-audit/audit-kernel/issues/67
> > >
> > > Signed-off-by: Richard Guy Briggs <[email protected]>
> > > ---
> > > arch/alpha/kernel/audit.c | 2 ++
> > > arch/ia64/kernel/audit.c | 2 ++
> > > arch/parisc/kernel/audit.c | 2 ++
> > > arch/parisc/kernel/compat_audit.c | 2 ++
> > > arch/powerpc/kernel/audit.c | 2 ++
> > > arch/powerpc/kernel/compat_audit.c | 2 ++
> > > arch/s390/kernel/audit.c | 2 ++
> > > arch/s390/kernel/compat_audit.c | 2 ++
> > > arch/sparc/kernel/audit.c | 2 ++
> > > arch/sparc/kernel/compat_audit.c | 2 ++
> > > arch/x86/ia32/audit.c | 2 ++
> > > arch/x86/kernel/audit_64.c | 2 ++
> > > kernel/auditsc.c | 3 +++
> > > lib/audit.c | 4 ++++
> > > lib/compat_audit.c | 4 ++++
> > > 15 files changed, 35 insertions(+)
> > >
> > > diff --git a/arch/alpha/kernel/audit.c b/arch/alpha/kernel/audit.c
> > > index 96a9d18ff4c4..06a911b685d1 100644
> > > --- a/arch/alpha/kernel/audit.c
> > > +++ b/arch/alpha/kernel/audit.c
> > > @@ -42,6 +42,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > > return 3;
> > > case __NR_execve:
> > > return 5;
> > > + case __NR_openat2:
> > > + return 6;
> > > default:
> > > return 0;
> > > }
> > > diff --git a/arch/ia64/kernel/audit.c b/arch/ia64/kernel/audit.c
> > > index 5192ca899fe6..5eaa888c8fd3 100644
> > > --- a/arch/ia64/kernel/audit.c
> > > +++ b/arch/ia64/kernel/audit.c
> > > @@ -43,6 +43,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > > return 3;
> > > case __NR_execve:
> > > return 5;
> > > + case __NR_openat2:
> > > + return 6;
> > > default:
> > > return 0;
> > > }
> > > diff --git a/arch/parisc/kernel/audit.c b/arch/parisc/kernel/audit.c
> > > index 9eb47b2225d2..fc721a7727ba 100644
> > > --- a/arch/parisc/kernel/audit.c
> > > +++ b/arch/parisc/kernel/audit.c
> > > @@ -52,6 +52,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > > return 3;
> > > case __NR_execve:
> > > return 5;
> > > + case __NR_openat2:
> > > + return 6;
> > > default:
> > > return 0;
> > > }
> > > diff --git a/arch/parisc/kernel/compat_audit.c b/arch/parisc/kernel/compat_audit.c
> > > index 20c39c9d86a9..fc6d35918c44 100644
> > > --- a/arch/parisc/kernel/compat_audit.c
> > > +++ b/arch/parisc/kernel/compat_audit.c
> > > @@ -35,6 +35,8 @@ int parisc32_classify_syscall(unsigned syscall)
> > > return 3;
> > > case __NR_execve:
> > > return 5;
> > > + case __NR_openat2:
> > > + return 6;
> > > default:
> > > return 1;
> > > }
> > > diff --git a/arch/powerpc/kernel/audit.c b/arch/powerpc/kernel/audit.c
> > > index a2dddd7f3d09..8f32700b0baa 100644
> > > --- a/arch/powerpc/kernel/audit.c
> > > +++ b/arch/powerpc/kernel/audit.c
> > > @@ -54,6 +54,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > > return 4;
> > > case __NR_execve:
> > > return 5;
> > > + case __NR_openat2:
> > > + return 6;
> > > default:
> > > return 0;
> > > }
> > > diff --git a/arch/powerpc/kernel/compat_audit.c b/arch/powerpc/kernel/compat_audit.c
> > > index 55c6ccda0a85..ebe45534b1c9 100644
> > > --- a/arch/powerpc/kernel/compat_audit.c
> > > +++ b/arch/powerpc/kernel/compat_audit.c
> > > @@ -38,6 +38,8 @@ int ppc32_classify_syscall(unsigned syscall)
> > > return 4;
> > > case __NR_execve:
> > > return 5;
> > > + case __NR_openat2:
> > > + return 6;
> > > default:
> > > return 1;
> > > }
> > > diff --git a/arch/s390/kernel/audit.c b/arch/s390/kernel/audit.c
> > > index d395c6c9944c..d964cb94cfaf 100644
> > > --- a/arch/s390/kernel/audit.c
> > > +++ b/arch/s390/kernel/audit.c
> > > @@ -54,6 +54,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > > return 4;
> > > case __NR_execve:
> > > return 5;
> > > + case __NR_openat2:
> > > + return 6;
> > > default:
> > > return 0;
> > > }
> > > diff --git a/arch/s390/kernel/compat_audit.c b/arch/s390/kernel/compat_audit.c
> > > index 444fb1f66944..f7b32933ce0e 100644
> > > --- a/arch/s390/kernel/compat_audit.c
> > > +++ b/arch/s390/kernel/compat_audit.c
> > > @@ -39,6 +39,8 @@ int s390_classify_syscall(unsigned syscall)
> > > return 4;
> > > case __NR_execve:
> > > return 5;
> > > + case __NR_openat2:
> > > + return 6;
> > > default:
> > > return 1;
> > > }
> > > diff --git a/arch/sparc/kernel/audit.c b/arch/sparc/kernel/audit.c
> > > index a6e91bf34d48..b6dcca9c6520 100644
> > > --- a/arch/sparc/kernel/audit.c
> > > +++ b/arch/sparc/kernel/audit.c
> > > @@ -55,6 +55,8 @@ int audit_classify_syscall(int abi, unsigned int syscall)
> > > return 4;
> > > case __NR_execve:
> > > return 5;
> > > + case __NR_openat2:
> > > + return 6;
> > > default:
> > > return 0;
> > > }
> > > diff --git a/arch/sparc/kernel/compat_audit.c b/arch/sparc/kernel/compat_audit.c
> > > index 10eeb4f15b20..d2652a1083ad 100644
> > > --- a/arch/sparc/kernel/compat_audit.c
> > > +++ b/arch/sparc/kernel/compat_audit.c
> > > @@ -39,6 +39,8 @@ int sparc32_classify_syscall(unsigned int syscall)
> > > return 4;
> > > case __NR_execve:
> > > return 5;
> > > + case __NR_openat2:
> > > + return 6;
> > > default:
> > > return 1;
> > > }
> > > diff --git a/arch/x86/ia32/audit.c b/arch/x86/ia32/audit.c
> > > index 6efe6cb3768a..57a02ade5503 100644
> > > --- a/arch/x86/ia32/audit.c
> > > +++ b/arch/x86/ia32/audit.c
> > > @@ -39,6 +39,8 @@ int ia32_classify_syscall(unsigned syscall)
> > > case __NR_execve:
> > > case __NR_execveat:
> > > return 5;
> > > + case __NR_openat2:
> > > + return 6;
> > > default:
> > > return 1;
> > > }
> > > diff --git a/arch/x86/kernel/audit_64.c b/arch/x86/kernel/audit_64.c
> > > index 83d9cad4e68b..39de1e021258 100644
> > > --- a/arch/x86/kernel/audit_64.c
> > > +++ b/arch/x86/kernel/audit_64.c
> > > @@ -53,6 +53,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > > case __NR_execve:
> > > case __NR_execveat:
> > > return 5;
> > > + case __NR_openat2:
> > > + return 6;
> > > default:
> > > return 0;
> > > }
> > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > > index 8bb9ac84d2fb..f5616e70d129 100644
> > > --- a/kernel/auditsc.c
> > > +++ b/kernel/auditsc.c
> > > @@ -76,6 +76,7 @@
> > > #include <linux/fsnotify_backend.h>
> > > #include <uapi/linux/limits.h>
> > > #include <uapi/linux/netfilter/nf_tables.h>
> > > +#include <uapi/linux/openat2.h>
> > >
> > > #include "audit.h"
> > >
> > > @@ -195,6 +196,8 @@ static int audit_match_perm(struct audit_context *ctx, int mask)
> > > return ((mask & AUDIT_PERM_WRITE) && ctx->argv[0] == SYS_BIND);
> > > case 5: /* execve */
> > > return mask & AUDIT_PERM_EXEC;
> > > + case 6: /* openat2 */
> > > + return mask & ACC_MODE((u32)((struct open_how *)ctx->argv[2])->flags);
> >
> > That looks a bit dodgy. Maybe sm like the below would be a bit better?
>
> Ah, ok, fair enough, since original flags use a u32 and this was picked
> as u64 for alignment. It was just occurring to me last night that I
> might have the dubious honour of being the first usage of 0%llo format
> specifier in the kernel... ;-)

> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 47fb48f42c93..531e882a5096 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -159,6 +159,7 @@ static const struct audit_nfcfgop_tab audit_nfcfgs[] = {
> >
> > static int audit_match_perm(struct audit_context *ctx, int mask)
> > {
> > + struct open_how *openat2;
> > unsigned n;
> > if (unlikely(!ctx))
> > return 0;
> > @@ -195,6 +196,12 @@ static int audit_match_perm(struct audit_context *ctx, int mask)
> > return ((mask & AUDIT_PERM_WRITE) && ctx->argv[0] == SYS_BIND);
> > case 5: /* execve */
> > return mask & AUDIT_PERM_EXEC;
> > + case 6: /* openat2 */
> > + openat2 = ctx->argv[2];
> > + if (upper_32_bits(openat2->flags))
> > + pr_warn("Some sensible warning about unknown flags");
> > +
> > + return mask & ACC_MODE(lower_32_bits(openat2->flags));
> > default:
> > return 0;
> > }
> >
> > (Ideally we'd probably notice at build-time that we've got flags
> > exceeding 32bits. Could probably easily been done by exposing an all
> > flags macro somewhere and then we can place a BUILD_BUG_ON() or sm into
> > such places.)

open_how arguments are translated to open_flags which is limited to 32 bits.

This code is shared with the other open functions that are limited to 32 bits
in open_flags. openat2 was created to avoid the limitations of openat, so at
some point it isn't unreasonable that flags exceed 32 bits, but open_flags
would have to be modified at that point to accommodate.

This value is handed in from userspace, and could be handed in without being
defined in the kernel, so those values need to be properly checked regardless
of the flags defined in the kernel.

The openat2 syscall claims to check all flags but no check is done on the top
32 bits.

build_open_flags() assigns how->flags to an int, effectively dropping the top
32 bits, before being checked against ~VALID_OPEN_FLAGS. This happens after
audit mode filtering, but has the same result.

Audit mode filtering using ACC_MODE() already masks out all but the lowest two
bits with O_ACCMODE, so there is no danger of overflowing a u32.

tomoyo_check_open_permission() assigns ACC_MODE() to u8 without a check.

All FMODE_* flags are clamped at u32.

6 bits remain at top and 4 bits just above O_ACCMODE, so there is no immediate
danger of overflow and if any additional mode bits are needed they are
available.
000377777703 used
037777777777 available
10 bits remaining

So, I don't think a check at this point in the code is useful, but do agree
that there should be some changes and checks added in sys_openat2 and
build_open_flags().


Also noticed: It looks like fddb5d430ad9f left in VALID_UPGRADE_FLAGS for
how->upgrade_mask that was removed. This may be used at a later date, but at
this point is dead code.

> > Christian
>
> - RGB

- RGB

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

2021-04-23 07:50:33

by Christian Brauner

[permalink] [raw]

Subject: Re: [PATCH 1/2] audit: add support for the openat2 syscall

---

On Thu, Apr 22, 2021 at 10:34:08PM -0400, Richard Guy Briggs wrote:
> On 2021-03-18 08:08, Richard Guy Briggs wrote:
> > On 2021-03-18 11:48, Christian Brauner wrote:
> > > [+Cc Aleksa, the author of openat2()]
> >
> > Ah! Thanks for pulling in Aleksa. I thought I caught everyone...
> >
> > > and a comment below. :)
> >
> > Same...
> >
> > > On Wed, Mar 17, 2021 at 09:47:17PM -0400, Richard Guy Briggs wrote:
> > > > The openat2(2) syscall was added in kernel v5.6 with commit fddb5d430ad9
> > > > ("open: introduce openat2(2) syscall")
> > > >
> > > > Add the openat2(2) syscall to the audit syscall classifier.
> > > >
> > > > See the github issue
> > > > https://github.com/linux-audit/audit-kernel/issues/67
> > > >
> > > > Signed-off-by: Richard Guy Briggs <[email protected]>
> > > > ---
> > > > arch/alpha/kernel/audit.c | 2 ++
> > > > arch/ia64/kernel/audit.c | 2 ++
> > > > arch/parisc/kernel/audit.c | 2 ++
> > > > arch/parisc/kernel/compat_audit.c | 2 ++
> > > > arch/powerpc/kernel/audit.c | 2 ++
> > > > arch/powerpc/kernel/compat_audit.c | 2 ++
> > > > arch/s390/kernel/audit.c | 2 ++
> > > > arch/s390/kernel/compat_audit.c | 2 ++
> > > > arch/sparc/kernel/audit.c | 2 ++
> > > > arch/sparc/kernel/compat_audit.c | 2 ++
> > > > arch/x86/ia32/audit.c | 2 ++
> > > > arch/x86/kernel/audit_64.c | 2 ++
> > > > kernel/auditsc.c | 3 +++
> > > > lib/audit.c | 4 ++++
> > > > lib/compat_audit.c | 4 ++++
> > > > 15 files changed, 35 insertions(+)
> > > >
> > > > diff --git a/arch/alpha/kernel/audit.c b/arch/alpha/kernel/audit.c
> > > > index 96a9d18ff4c4..06a911b685d1 100644
> > > > --- a/arch/alpha/kernel/audit.c
> > > > +++ b/arch/alpha/kernel/audit.c
> > > > @@ -42,6 +42,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > > > return 3;
> > > > case __NR_execve:
> > > > return 5;
> > > > + case __NR_openat2:
> > > > + return 6;
> > > > default:
> > > > return 0;
> > > > }
> > > > diff --git a/arch/ia64/kernel/audit.c b/arch/ia64/kernel/audit.c
> > > > index 5192ca899fe6..5eaa888c8fd3 100644
> > > > --- a/arch/ia64/kernel/audit.c
> > > > +++ b/arch/ia64/kernel/audit.c
> > > > @@ -43,6 +43,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > > > return 3;
> > > > case __NR_execve:
> > > > return 5;
> > > > + case __NR_openat2:
> > > > + return 6;
> > > > default:
> > > > return 0;
> > > > }
> > > > diff --git a/arch/parisc/kernel/audit.c b/arch/parisc/kernel/audit.c
> > > > index 9eb47b2225d2..fc721a7727ba 100644
> > > > --- a/arch/parisc/kernel/audit.c
> > > > +++ b/arch/parisc/kernel/audit.c
> > > > @@ -52,6 +52,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > > > return 3;
> > > > case __NR_execve:
> > > > return 5;
> > > > + case __NR_openat2:
> > > > + return 6;
> > > > default:
> > > > return 0;
> > > > }
> > > > diff --git a/arch/parisc/kernel/compat_audit.c b/arch/parisc/kernel/compat_audit.c
> > > > index 20c39c9d86a9..fc6d35918c44 100644
> > > > --- a/arch/parisc/kernel/compat_audit.c
> > > > +++ b/arch/parisc/kernel/compat_audit.c
> > > > @@ -35,6 +35,8 @@ int parisc32_classify_syscall(unsigned syscall)
> > > > return 3;
> > > > case __NR_execve:
> > > > return 5;
> > > > + case __NR_openat2:
> > > > + return 6;
> > > > default:
> > > > return 1;
> > > > }
> > > > diff --git a/arch/powerpc/kernel/audit.c b/arch/powerpc/kernel/audit.c
> > > > index a2dddd7f3d09..8f32700b0baa 100644
> > > > --- a/arch/powerpc/kernel/audit.c
> > > > +++ b/arch/powerpc/kernel/audit.c
> > > > @@ -54,6 +54,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > > > return 4;
> > > > case __NR_execve:
> > > > return 5;
> > > > + case __NR_openat2:
> > > > + return 6;
> > > > default:
> > > > return 0;
> > > > }
> > > > diff --git a/arch/powerpc/kernel/compat_audit.c b/arch/powerpc/kernel/compat_audit.c
> > > > index 55c6ccda0a85..ebe45534b1c9 100644
> > > > --- a/arch/powerpc/kernel/compat_audit.c
> > > > +++ b/arch/powerpc/kernel/compat_audit.c
> > > > @@ -38,6 +38,8 @@ int ppc32_classify_syscall(unsigned syscall)
> > > > return 4;
> > > > case __NR_execve:
> > > > return 5;
> > > > + case __NR_openat2:
> > > > + return 6;
> > > > default:
> > > > return 1;
> > > > }
> > > > diff --git a/arch/s390/kernel/audit.c b/arch/s390/kernel/audit.c
> > > > index d395c6c9944c..d964cb94cfaf 100644
> > > > --- a/arch/s390/kernel/audit.c
> > > > +++ b/arch/s390/kernel/audit.c
> > > > @@ -54,6 +54,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > > > return 4;
> > > > case __NR_execve:
> > > > return 5;
> > > > + case __NR_openat2:
> > > > + return 6;
> > > > default:
> > > > return 0;
> > > > }
> > > > diff --git a/arch/s390/kernel/compat_audit.c b/arch/s390/kernel/compat_audit.c
> > > > index 444fb1f66944..f7b32933ce0e 100644
> > > > --- a/arch/s390/kernel/compat_audit.c
> > > > +++ b/arch/s390/kernel/compat_audit.c
> > > > @@ -39,6 +39,8 @@ int s390_classify_syscall(unsigned syscall)
> > > > return 4;
> > > > case __NR_execve:
> > > > return 5;
> > > > + case __NR_openat2:
> > > > + return 6;
> > > > default:
> > > > return 1;
> > > > }
> > > > diff --git a/arch/sparc/kernel/audit.c b/arch/sparc/kernel/audit.c
> > > > index a6e91bf34d48..b6dcca9c6520 100644
> > > > --- a/arch/sparc/kernel/audit.c
> > > > +++ b/arch/sparc/kernel/audit.c
> > > > @@ -55,6 +55,8 @@ int audit_classify_syscall(int abi, unsigned int syscall)
> > > > return 4;
> > > > case __NR_execve:
> > > > return 5;
> > > > + case __NR_openat2:
> > > > + return 6;
> > > > default:
> > > > return 0;
> > > > }
> > > > diff --git a/arch/sparc/kernel/compat_audit.c b/arch/sparc/kernel/compat_audit.c
> > > > index 10eeb4f15b20..d2652a1083ad 100644
> > > > --- a/arch/sparc/kernel/compat_audit.c
> > > > +++ b/arch/sparc/kernel/compat_audit.c
> > > > @@ -39,6 +39,8 @@ int sparc32_classify_syscall(unsigned int syscall)
> > > > return 4;
> > > > case __NR_execve:
> > > > return 5;
> > > > + case __NR_openat2:
> > > > + return 6;
> > > > default:
> > > > return 1;
> > > > }
> > > > diff --git a/arch/x86/ia32/audit.c b/arch/x86/ia32/audit.c
> > > > index 6efe6cb3768a..57a02ade5503 100644
> > > > --- a/arch/x86/ia32/audit.c
> > > > +++ b/arch/x86/ia32/audit.c
> > > > @@ -39,6 +39,8 @@ int ia32_classify_syscall(unsigned syscall)
> > > > case __NR_execve:
> > > > case __NR_execveat:
> > > > return 5;
> > > > + case __NR_openat2:
> > > > + return 6;
> > > > default:
> > > > return 1;
> > > > }
> > > > diff --git a/arch/x86/kernel/audit_64.c b/arch/x86/kernel/audit_64.c
> > > > index 83d9cad4e68b..39de1e021258 100644
> > > > --- a/arch/x86/kernel/audit_64.c
> > > > +++ b/arch/x86/kernel/audit_64.c
> > > > @@ -53,6 +53,8 @@ int audit_classify_syscall(int abi, unsigned syscall)
> > > > case __NR_execve:
> > > > case __NR_execveat:
> > > > return 5;
> > > > + case __NR_openat2:
> > > > + return 6;
> > > > default:
> > > > return 0;
> > > > }
> > > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > > > index 8bb9ac84d2fb..f5616e70d129 100644
> > > > --- a/kernel/auditsc.c
> > > > +++ b/kernel/auditsc.c
> > > > @@ -76,6 +76,7 @@
> > > > #include <linux/fsnotify_backend.h>
> > > > #include <uapi/linux/limits.h>
> > > > #include <uapi/linux/netfilter/nf_tables.h>
> > > > +#include <uapi/linux/openat2.h>
> > > >
> > > > #include "audit.h"
> > > >
> > > > @@ -195,6 +196,8 @@ static int audit_match_perm(struct audit_context *ctx, int mask)
> > > > return ((mask & AUDIT_PERM_WRITE) && ctx->argv[0] == SYS_BIND);
> > > > case 5: /* execve */
> > > > return mask & AUDIT_PERM_EXEC;
> > > > + case 6: /* openat2 */
> > > > + return mask & ACC_MODE((u32)((struct open_how *)ctx->argv[2])->flags);
> > >
> > > That looks a bit dodgy. Maybe sm like the below would be a bit better?
> >
> > Ah, ok, fair enough, since original flags use a u32 and this was picked
> > as u64 for alignment. It was just occurring to me last night that I
> > might have the dubious honour of being the first usage of 0%llo format
> > specifier in the kernel... ;-)
>
> > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > > index 47fb48f42c93..531e882a5096 100644
> > > --- a/kernel/auditsc.c
> > > +++ b/kernel/auditsc.c
> > > @@ -159,6 +159,7 @@ static const struct audit_nfcfgop_tab audit_nfcfgs[] = {
> > >
> > > static int audit_match_perm(struct audit_context *ctx, int mask)
> > > {
> > > + struct open_how *openat2;
> > > unsigned n;
> > > if (unlikely(!ctx))
> > > return 0;
> > > @@ -195,6 +196,12 @@ static int audit_match_perm(struct audit_context *ctx, int mask)
> > > return ((mask & AUDIT_PERM_WRITE) && ctx->argv[0] == SYS_BIND);
> > > case 5: /* execve */
> > > return mask & AUDIT_PERM_EXEC;
> > > + case 6: /* openat2 */
> > > + openat2 = ctx->argv[2];
> > > + if (upper_32_bits(openat2->flags))
> > > + pr_warn("Some sensible warning about unknown flags");
> > > +
> > > + return mask & ACC_MODE(lower_32_bits(openat2->flags));
> > > default:
> > > return 0;
> > > }
> > >
> > > (Ideally we'd probably notice at build-time that we've got flags
> > > exceeding 32bits. Could probably easily been done by exposing an all
> > > flags macro somewhere and then we can place a BUILD_BUG_ON() or sm into
> > > such places.)
>
> open_how arguments are translated to open_flags which is limited to 32 bits.
>
> This code is shared with the other open functions that are limited to 32 bits
> in open_flags. openat2 was created to avoid the limitations of openat, so at
> some point it isn't unreasonable that flags exceed 32 bits, but open_flags
> would have to be modified at that point to accommodate.
>
> This value is handed in from userspace, and could be handed in without being
> defined in the kernel, so those values need to be properly checked regardless
> of the flags defined in the kernel.
>
> The openat2 syscall claims to check all flags but no check is done on the top
> 32 bits.

Hm, I think this is an oversight because of the different semantics for
openat() and openat2(). We should check that no upper 32 bits are set
for openat2(). That's the intended semantics. For old openat()
we can't error on unknown flags because it has traditionally ignored
unknown flags.

>
> build_open_flags() assigns how->flags to an int, effectively dropping the top
> 32 bits, before being checked against ~VALID_OPEN_FLAGS. This happens after
> audit mode filtering, but has the same result.

Right. That's at bug we should return an error to userspace. We do for
any unkown values that fall within the lower 32 bit range so it's silly
to ignore unknown values in the upper 32 bit range.

>
> Audit mode filtering using ACC_MODE() already masks out all but the lowest two
> bits with O_ACCMODE, so there is no danger of overflowing a u32.
>
> tomoyo_check_open_permission() assigns ACC_MODE() to u8 without a check.
>
> All FMODE_* flags are clamped at u32.
>
> 6 bits remain at top and 4 bits just above O_ACCMODE, so there is no immediate
> danger of overflow and if any additional mode bits are needed they are
> available.
> 000377777703 used
> 037777777777 available
> 10 bits remaining
>
> So, I don't think a check at this point in the code is useful, but do agree

Maybe but note that a defensive posture here might be a good thing
instead of tripping over the issue later.

> that there should be some changes and checks added in sys_openat2 and
> build_open_flags().
>
>
> Also noticed: It looks like fddb5d430ad9f left in VALID_UPGRADE_FLAGS for
> how->upgrade_mask that was removed. This may be used at a later date, but at
> this point is dead code.

I'll take a look now.

Christian