linux-audit / audit-kernel

GitHub mirror of the Linux Kernel's audit repository
https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git
Other
140 stars 37 forks source link

BUG: ILP32 (ARM's take on x32) is not audited properly #131

Open weiyuchen opened 3 years ago

weiyuchen commented 3 years ago

I use ILP32 program on 5.10 kernel Recently, and I find that I can't recored log in some case, here is a example: I set one rule on the system:

auditctl -w /etc/shadow -p wa -k test

my test program's core func is:

open("/etc/shadow", O_WRONLY | O_APPEND);

when I use 64 bit program to access /etc/shadow, I can get audit log rightly but when I use ILP32 program, I can't get any audit log

I analyse this problem for days, and I find it's due to this commit:

0fe4141ba63a5dfd425c6d2dd9d8cbafd3497946
.......
 #define AUDIT_ARCH_AARCH64     (EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
+#define AUDIT_ARCH_AARCH64ILP32        (EM_AARCH64|__AUDIT_ARCH_LE)
 #define AUDIT_ARCH_ALPHA       (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
 #define AUDIT_ARCH_ARCOMPACT   (EM_ARCOMPACT|__AUDIT_ARCH_LE)
 #define AUDIT_ARCH_ARCOMPACTBE (EM_ARCOMPACT) 

Beacuse of ILP32 program use 64 bit syscall in most case (according to the arm ilp32 Documents:) when audit enter audit_classify_syscall function, it turns to a branch different from the kernel without AUDIT_ARCH_AARCH64ILP32 defination, however the syscall num on both kernel is same, so audit can't recognize the 64 bit syscall num when it runs to 32ILP branch

weiyuchen commented 3 years ago

the ILP32 whitepaper from https://developer.arm.com/documentation/dai0490/latest/

ILP32 uses the LP64 system calls wherever possible. However, it cannot share system calls 
where structures contain pointers (currently 31 such system calls), in which case ILP32 uses the 
compat (32-bit) version
weiyuchen commented 3 years ago

https://github.com/linux-audit/audit-kernel/issues/131 @YuryNorov @pcmoore

pcmoore commented 3 years ago

From the ILP32 whitepaper:

What is ILP32?

The ARMv8 architecture supports 32-bit and 64-bit instruction sets (AArch32 and AArch64 respectively), both use 32-bit instruction encodings but with different register lengths and data type sizes.

ILP32 is intended for use where, for whatever reason, it is beneficial to use int, long and pointer represented as 32-bit values. This could be for performance or legacy 32-bit compatibility reasons.

Linux on AArch64 uses LP64 as its standard data model, where Long and Pointer are 64-bit, Ints are 32-bit.

AArch64-ILP32 uses the AArch64 instruction set coupled with a data model where Int, Long and Pointer are 32-bit.

... which means that ILP32 is basically the ARM version of x32. Which is pretty much just the worst thing ever; x32 needed to die a painful death and my initial thought is that ILP32 deserves the same fate.

@weiyuchen we will add this to the queue to investigate but it may take a while; if you are interested in working on it you are always welcome to submit patches.

weiyuchen commented 3 years ago

OK, thx, I'd like to try to fix this problem

pcmoore commented 3 years ago

That would be great, thanks!

YuryNorov commented 3 years ago

Not sure you received a notification from github. Please feel free to comment: https://lkml.org/lkml/2021/7/2/475

pcmoore commented 3 years ago

@YuryNorov I think it would be best to decide where you want to discuss this, either here on GH or on LKML, please do not do both. Further, if you want me to comment on LKML you should add me to the To/CC line, like many folks I do not read every LKML posting ... actually, check that, it looks like I'm not even subscribed to LKML anymore.

YuryNorov commented 3 years ago

I found this email in your profile: paul@paul-moore.com I'll add it in CC in case of new postings if you don't mind.

pcmoore commented 3 years ago

Yes, that is fine with me.

My apologies, it has been a busy week.

weiyuchen commented 3 years ago

@YuryNorov I've investigaed all syscalls used by ILP32 application, I find that all the syscalls are same as aarch64, so I'd like to know if I rollback the commit 0fe4141ba63a5dfd425c6d2dd9d8cbafd3497946 locally, if there will be some unknowable problem that impact other parts of kernel? (because recently I find similar problem of seccomp, which filter the syscall failed.)

YuryNorov commented 3 years ago

Weiyuchen, can you point me to a tree with the commit you mentioned. I cannot find the commit, and cannot say more without looking at the code.

My tree is here: https://github.com/norov/linux/tree/ilp32-5.2

I

weiyuchen commented 3 years ago

oh, I'm sorry, the commit name is: arm64: introduce AUDIT_ARCH_AARCH64ILP32 for ilp32 in your tree, the commit id is: 2312ac1edaabb023bcdf90d107ea1198ff9e4bd3

YuryNorov commented 3 years ago

Again, can you please share the link to your repository?

chenjh005 commented 3 years ago

Again, can you please share the link to your repository?

Hi Yury, I guess what Yuchen just mentioned is in this link: https://github.com/norov/linux/commit/2312ac1edaabb023bcdf90d107ea1198ff9e4bd3

weiyuchen commented 3 years ago

Again, can you please share the link to your repository?

https://gitee.com/openeuler/kernel/tree/OLK-5.10/ this is the repository we use, thx

YuryNorov commented 3 years ago

@YuryNorov I've investigaed all syscalls used by ILP32 application, I find that all the syscalls are same as aarch64, so I'd like to know if I rollback the commit 0fe4141ba63a5dfd425c6d2dd9d8cbafd3497946 locally, if there will be some unknowable problem that impact other parts of kernel? (because recently I find similar problem of seccomp, which filter the syscall failed.)

If you roll it back, the seccomp and ptrace will not be able to distinguish between aarch32 and ilp32.

Generally, I walked thru the audit_classify logic, and it looks correct, and there's no recent changes in basic logic. One exception is the patch 0223fad3c98a9 ("audit: enforce op for string fields"), so maybe you'd look at there.

Can you share LTP results? Without that, or better an access to a victim system, I can't say much more.

Thanks, Yury

weiyuchen commented 3 years ago

@YuryNorov I've investigaed all syscalls used by ILP32 application, I find that all the syscalls are same as aarch64, so I'd like to know if I rollback the commit 0fe4141ba63a5dfd425c6d2dd9d8cbafd3497946 locally, if there will be some unknowable problem that impact other parts of kernel? (because recently I find similar problem of seccomp, which filter the syscall failed.)

If you roll it back, the seccomp and ptrace will not be able to distinguish between aarch32 and ilp32.

Generally, I walked thru the audit_classify logic, and it looks correct, and there's no recent changes in basic logic. One exception is the patch 0223fad ("audit: enforce op for string fields"), so maybe you'd look at there.

Can you share LTP results? Without that, or better an access to a victim system, I can't say much more.

Thanks, Yury

Ok, I will show you the LTP results in detail. 1、set the audit rules

-w /etc/shadow -p wa -k identity

2、the test file open.c is

int test(const int argc, const char **argv)
{
    int fd = open("/etc/shadow", O_WRONLY | O_APPEND);
    if (fd < 0)
        return 1;

    close(fd);
    return 0;
}

3、if the program is build in ilp32 format, when execute the open_ilp32, no audit log will be generated 4、if the program is build in aarch64 format, when execute the open_64, audit log will be generated normally 5、I set some points in the kernel to trace the problem, below is the results

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 68296f55d8c6..f1b77f5fde46 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -165,9 +165,11 @@ static int audit_match_perm(struct audit_context *ctx, int mask)
        if (unlikely(!ctx))
                return 0;
        n = ctx->major;
+       pr_info("[wyc] mask: %d, syscall: %u, arch: %x\n", mask, n, ctx->arch);

        switch (audit_classify_syscall(ctx->arch, n)) {
        case 0: /* native */
+               pr_info("[wyc] audit_classify_syscall 0\n");
                if ((mask & AUDIT_PERM_WRITE) &&
                     audit_match_class(AUDIT_CLASS_WRITE, n))
                        return 1;
@@ -179,6 +181,7 @@ static int audit_match_perm(struct audit_context *ctx, int mask)
                        return 1;
                return 0;
        case 1: /* 32bit on biarch */
+               pr_info("[wyc] audit_classify_syscall 1\n");
                if ((mask & AUDIT_PERM_WRITE) &&
                     audit_match_class(AUDIT_CLASS_WRITE_32, n))
                        return 1;
@@ -190,10 +193,13 @@ static int audit_match_perm(struct audit_context *ctx, int mask)
                        return 1;
                return 0;
        case 2: /* open */
+               pr_info("[wyc] audit_classify_syscall 2\n");
                return mask & ACC_MODE(ctx->argv[1]);
        case 3: /* openat */
+               pr_info("[wyc] audit_classify_syscall 3\n");
                return mask & ACC_MODE(ctx->argv[2]);
        case 4: /* socketcall */
+               pr_info("[wyc] audit_classify_syscall 4\n");
                return ((mask & AUDIT_PERM_WRITE) && ctx->argv[0] == SYS_BIND);
        case 5: /* execve */
                return mask & AUDIT_PERM_EXEC;

for ilp32 and aarch64 program, I both get the same syscall number 56 (which is openat in 64bit syscall table), but the arch is different. Generally, openat syscall should enter case3, but if AUDIT_ARCH_AARCH64ILP32 is added, openat syscall will enter case 1 and use 32bit syscall table, which will result in none audit log produced.

# dmesg
auditsc: [wyc] mask: 10, syscall: 56, arch: c00000b7
auditsc: [wyc] audit_classify_syscall 3
auditsc: [wyc] mask: 10, syscall: 56, arch: 400000b7
auditsc: [wyc] audit_classify_syscall 1
weiyuchen commented 3 years ago

Further more, I want to know that in arch/arm64/kernel/sys_ilp32.c file, the ilp32_sys_call_table is defined as below:

const syscall_fn_t ilp32_sys_call_table[__NR_syscalls] = {
        [0 ... __NR_syscalls - 1] = __arm64_sys_ni_syscall,
#include <asm/unistd.h>
};

if it means that ilp32 programs use aarch64 syscalls, if so, whether adding AUDIT_ARCH_AARCH64ILP32 arch will lead most ilp32 filter in audit to a wrong path?

Thanks Yuchen