From: Paul Moore <email@example.com> To: Michael Ellerman <firstname.lastname@example.org>, email@example.com Cc: Benjamin Herrenschmidt <firstname.lastname@example.org>, email@example.com, Eric Paris <firstname.lastname@example.org>, email@example.com, Paul Mackerras <firstname.lastname@example.org>, Christophe Leroy <email@example.com>, firstname.lastname@example.org Subject: Re: [PATCH v2 RESEND] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC Date: Thu, 26 Aug 2021 20:52:17 -0400 [thread overview] Message-ID: <CAHC9VhRAbsjifuO+fw4_KpK3ErVM0Dk0Ru3LuZKkeTZvWYc5email@example.com> (raw) In-Reply-To: <firstname.lastname@example.org> On Thu, Aug 26, 2021 at 10:37 AM Michael Ellerman <email@example.com> wrote: > Paul Moore <firstname.lastname@example.org> writes: > > On Tue, Aug 24, 2021 at 1:11 PM Christophe Leroy > > <email@example.com> wrote: > >> Le 24/08/2021 à 16:47, Paul Moore a écrit : > >> > On Tue, Aug 24, 2021 at 9:36 AM Christophe Leroy > >> > <firstname.lastname@example.org> wrote: > >> >> > >> >> Commit e65e1fc2d24b ("[PATCH] syscall class hookup for all normal > >> >> targets") added generic support for AUDIT but that didn't include > >> >> support for bi-arch like powerpc. > >> >> > >> >> Commit 4b58841149dc ("audit: Add generic compat syscall support") > >> >> added generic support for bi-arch. > >> >> > >> >> Convert powerpc to that bi-arch generic audit support. > >> >> > >> >> Cc: Paul Moore <email@example.com> > >> >> Cc: Eric Paris <firstname.lastname@example.org> > >> >> Signed-off-by: Christophe Leroy <email@example.com> > >> >> --- > >> >> Resending v2 with Audit people in Cc > >> >> > >> >> v2: > >> >> - Missing 'git add' for arch/powerpc/include/asm/unistd32.h > >> >> - Finalised commit description > >> >> --- > >> >> arch/powerpc/Kconfig | 5 +- > >> >> arch/powerpc/include/asm/unistd32.h | 7 +++ > >> >> arch/powerpc/kernel/Makefile | 3 -- > >> >> arch/powerpc/kernel/audit.c | 84 ----------------------------- > >> >> arch/powerpc/kernel/compat_audit.c | 44 --------------- > >> >> 5 files changed, 8 insertions(+), 135 deletions(-) > >> >> create mode 100644 arch/powerpc/include/asm/unistd32.h > >> >> delete mode 100644 arch/powerpc/kernel/audit.c > >> >> delete mode 100644 arch/powerpc/kernel/compat_audit.c > >> > > >> > Can you explain, in detail please, the testing you have done to verify > >> > this patch? > >> > > >> > >> I built ppc64_defconfig and checked that the generated code is functionnaly equivalent. > >> > >> ppc32_classify_syscall() is exactly the same as audit_classify_compat_syscall() except that the > >> later takes the syscall as second argument (ie in r4) whereas the former takes it as first argument > >> (ie in r3). > >> > >> audit_classify_arch() and powerpc audit_classify_syscall() are slightly different between the > >> powerpc version and the generic version because the powerpc version checks whether it is > >> AUDIT_ARCH_PPC or not (ie value 20), while the generic one checks whether it has bit > >> __AUDIT_ARCH_64BIT set or not (__AUDIT_ARCH_64BIT is the sign bit of a word), but taking into > >> account that the abi is either AUDIT_ARCH_PPC, AUDIT_ARCH_PPC64 or AUDIT_ARCH_PPC64LE, the result is > >> the same. > >> > >> If you are asking I guess you saw something wrong ? > > > > I was asking because I didn't see any mention of testing, and when you > > are enabling something significant like this it is nice to see that it > > has been verified to work :) > > > > While binary dumps and comparisons are nice, it is always good to see > > verification from a test suite. I don't have access to the necessary > > hardware to test this, but could you verify that the audit-testsuite > > passes on your test system with your patches applied? > > > > * https://github.com/linux-audit/audit-testsuite > > I tested on ppc64le. Both before and after the patch I get the result > below. > > So I guess the patch is OK, but maybe we have some existing issue. > > I had a bit of a look at the test code, but my perl is limited. I think > it was running the command below, and it returned "<no matches>", but > not really sure what that means. If it makes you feel any better, my perl is *very* limited; thankfully this isn't my first time looking at that test :) It's a little odd, but after some basic sanity tests at the top, the test sets a watch on a file, /tmp/<rando_string>, and tells the kernel to generate audit records for any syscall that operates on that file. It then creates, and removes, a series of exclude audit filters to check if the exclude filtering is working as expected, e.g. when syscall filtering is excluded there should be no syscall records in the audit log. In the case you describe, it looks like it looks like the audit exclude filter is removed (that's what line 147 does), the /tmp/<rando_string> file is removed (line 152), and then we check to see if any syscall records exist (line 164, and yes, there should be *something* there for the unlink/rm). It may be of little consolation, but this test works just fine on recent kernels running on both x86_64 and aarch64. I don't have access to a powerpc system of any vintage, but I added Richard to the To line above in case he has easier access to a test system (I suspect the RH/IBM linkage should help in this regard). Otherwise I would suggest starting to debug this by simply doing some basic tests using auditctl to create rules and exclude rules to see what is working, and what isn't; that might provide some clues. Sorry I'm not much more help at this point :/ > $ sudo ausearch -i -m SYSCALL -p 216440 -ui 0 -gi 0 -ul 0 -su unconfined _u:unconfined_r:unconfined_t:s0-s0:c0.c1023 -ts recent > <no matches> > > cheers > > > > Running as user root > with context unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 > on system Fedora > > backlog_wait_time_actual_reset/test .. ok > exec_execve/test ..................... ok > exec_name/test ....................... ok > file_create/test ..................... ok > file_delete/test ..................... ok > file_rename/test ..................... ok > filter_exclude/test .................. 1/21 > # Test 20 got: "256" (filter_exclude/test at line 167) > # Expected: "0" > # filter_exclude/test line 167 is: ok( $result, 0 ); > # Test 21 got: "0" (filter_exclude/test at line 179) > # Expected: "1" > # filter_exclude/test line 179 is: ok( $found_msg, 1 ); > filter_exclude/test .................. Failed 2/21 subtests > filter_saddr_fam/test ................ ok > filter_sessionid/test ................ ok > login_tty/test ....................... ok > lost_reset/test ...................... ok > netfilter_pkt/test ................... ok > syscalls_file/test ................... ok > syscall_module/test .................. ok > time_change/test ..................... ok > user_msg/test ........................ ok > fanotify/test ........................ ok > bpf/test ............................. ok > > Test Summary Report > ------------------- > filter_exclude/test (Wstat: 0 Tests: 21 Failed: 2) > Failed tests: 20-21 > Files=18, Tests=202, 45 wallclock secs ( 0.18 usr 0.03 sys + 20.15 cusr 0.92 csys = 21.28 CPU) > Result: FAIL > Failed 1/18 test programs. 2/202 subtests failed. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linuxfirstname.lastname@example.org https://listman.redhat.com/mailman/listinfo/linux-audit
next prev parent reply other threads:[~2021-08-27 0:52 UTC|newest] Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-08-24 13:36 Christophe Leroy 2021-08-24 14:47 ` Paul Moore 2021-08-24 17:11 ` Christophe Leroy 2021-08-24 23:01 ` Paul Moore 2021-08-26 14:37 ` Michael Ellerman 2021-08-27 0:52 ` Paul Moore [this message] 2021-11-02 10:11 ` Michael Ellerman 2021-11-02 13:01 ` Paul Moore 2021-11-02 23:18 ` Michael Ellerman 2021-11-02 23:54 ` Konstantin Ryabitsev 2021-11-04 6:20 ` Michael Ellerman 2021-11-04 14:32 ` Paul Moore 2021-11-03 1:15 ` Paul Moore 2021-11-02 23:17 ` Michael Ellerman
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='CAHC9VhRAbsjifuO+fw4_KpK3ErVM0Dk0Ru3LuZKkeTZvWYc5email@example.com' \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCH v2 RESEND] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).