2021-08-24 13:38:45

by Christophe Leroy

[permalink] [raw]
Subject: [PATCH v2 RESEND] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC

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 protected]>
Cc: Eric Paris <[email protected]>
Signed-off-by: Christophe Leroy <[email protected]>
---
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

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 663766fbf505..5472358609d2 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -163,6 +163,7 @@ config PPC
select ARCH_WANT_IRQS_OFF_ACTIVATE_MM
select ARCH_WANT_LD_ORPHAN_WARN
select ARCH_WEAK_RELEASE_ACQUIRE
+ select AUDIT_ARCH_COMPAT_GENERIC
select BINFMT_ELF
select BUILDTIME_TABLE_SORT
select CLONE_BACKWARDS
@@ -316,10 +317,6 @@ config GENERIC_TBSYNC
bool
default y if PPC32 && SMP

-config AUDIT_ARCH
- bool
- default y
-
config GENERIC_BUG
bool
default y
diff --git a/arch/powerpc/include/asm/unistd32.h b/arch/powerpc/include/asm/unistd32.h
new file mode 100644
index 000000000000..07689897d206
--- /dev/null
+++ b/arch/powerpc/include/asm/unistd32.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef _ASM_POWERPC_UNISTD32_H_
+#define _ASM_POWERPC_UNISTD32_H_
+
+#include <asm/unistd_32.h>
+
+#endif /* _ASM_POWERPC_UNISTD32_H_ */
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 7be36c1e1db6..825121eba3c2 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -125,9 +125,6 @@ obj-$(CONFIG_PCI) += pci_$(BITS).o $(pci64-y) \
pci-common.o pci_of_scan.o
obj-$(CONFIG_PCI_MSI) += msi.o

-obj-$(CONFIG_AUDIT) += audit.o
-obj64-$(CONFIG_AUDIT) += compat_audit.o
-
obj-$(CONFIG_PPC_IO_WORKAROUNDS) += io-workarounds.o

obj-y += trace/
diff --git a/arch/powerpc/kernel/audit.c b/arch/powerpc/kernel/audit.c
deleted file mode 100644
index a2dddd7f3d09..000000000000
--- a/arch/powerpc/kernel/audit.c
+++ /dev/null
@@ -1,84 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-#include <linux/init.h>
-#include <linux/types.h>
-#include <linux/audit.h>
-#include <asm/unistd.h>
-
-static unsigned dir_class[] = {
-#include <asm-generic/audit_dir_write.h>
-~0U
-};
-
-static unsigned read_class[] = {
-#include <asm-generic/audit_read.h>
-~0U
-};
-
-static unsigned write_class[] = {
-#include <asm-generic/audit_write.h>
-~0U
-};
-
-static unsigned chattr_class[] = {
-#include <asm-generic/audit_change_attr.h>
-~0U
-};
-
-static unsigned signal_class[] = {
-#include <asm-generic/audit_signal.h>
-~0U
-};
-
-int audit_classify_arch(int arch)
-{
-#ifdef CONFIG_PPC64
- if (arch == AUDIT_ARCH_PPC)
- return 1;
-#endif
- return 0;
-}
-
-int audit_classify_syscall(int abi, unsigned syscall)
-{
-#ifdef CONFIG_PPC64
- extern int ppc32_classify_syscall(unsigned);
- if (abi == AUDIT_ARCH_PPC)
- return ppc32_classify_syscall(syscall);
-#endif
- switch(syscall) {
- case __NR_open:
- return 2;
- case __NR_openat:
- return 3;
- case __NR_socketcall:
- return 4;
- case __NR_execve:
- return 5;
- default:
- return 0;
- }
-}
-
-static int __init audit_classes_init(void)
-{
-#ifdef CONFIG_PPC64
- extern __u32 ppc32_dir_class[];
- extern __u32 ppc32_write_class[];
- extern __u32 ppc32_read_class[];
- extern __u32 ppc32_chattr_class[];
- extern __u32 ppc32_signal_class[];
- audit_register_class(AUDIT_CLASS_WRITE_32, ppc32_write_class);
- audit_register_class(AUDIT_CLASS_READ_32, ppc32_read_class);
- audit_register_class(AUDIT_CLASS_DIR_WRITE_32, ppc32_dir_class);
- audit_register_class(AUDIT_CLASS_CHATTR_32, ppc32_chattr_class);
- audit_register_class(AUDIT_CLASS_SIGNAL_32, ppc32_signal_class);
-#endif
- audit_register_class(AUDIT_CLASS_WRITE, write_class);
- audit_register_class(AUDIT_CLASS_READ, read_class);
- audit_register_class(AUDIT_CLASS_DIR_WRITE, dir_class);
- audit_register_class(AUDIT_CLASS_CHATTR, chattr_class);
- audit_register_class(AUDIT_CLASS_SIGNAL, signal_class);
- return 0;
-}
-
-__initcall(audit_classes_init);
diff --git a/arch/powerpc/kernel/compat_audit.c b/arch/powerpc/kernel/compat_audit.c
deleted file mode 100644
index 55c6ccda0a85..000000000000
--- a/arch/powerpc/kernel/compat_audit.c
+++ /dev/null
@@ -1,44 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-#undef __powerpc64__
-#include <asm/unistd.h>
-
-unsigned ppc32_dir_class[] = {
-#include <asm-generic/audit_dir_write.h>
-~0U
-};
-
-unsigned ppc32_chattr_class[] = {
-#include <asm-generic/audit_change_attr.h>
-~0U
-};
-
-unsigned ppc32_write_class[] = {
-#include <asm-generic/audit_write.h>
-~0U
-};
-
-unsigned ppc32_read_class[] = {
-#include <asm-generic/audit_read.h>
-~0U
-};
-
-unsigned ppc32_signal_class[] = {
-#include <asm-generic/audit_signal.h>
-~0U
-};
-
-int ppc32_classify_syscall(unsigned syscall)
-{
- switch(syscall) {
- case __NR_open:
- return 2;
- case __NR_openat:
- return 3;
- case __NR_socketcall:
- return 4;
- case __NR_execve:
- return 5;
- default:
- return 1;
- }
-}
--
2.25.0


2021-08-24 14:49:43

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH v2 RESEND] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC

On Tue, Aug 24, 2021 at 9:36 AM Christophe Leroy
<[email protected]> 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 protected]>
> Cc: Eric Paris <[email protected]>
> Signed-off-by: Christophe Leroy <[email protected]>
> ---
> 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?

--
paul moore
http://www.paul-moore.com

2021-08-24 17:17:14

by Christophe Leroy

[permalink] [raw]
Subject: Re: [PATCH v2 RESEND] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC



Le 24/08/2021 à 16:47, Paul Moore a écrit :
> On Tue, Aug 24, 2021 at 9:36 AM Christophe Leroy
> <[email protected]> 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 protected]>
>> Cc: Eric Paris <[email protected]>
>> Signed-off-by: Christophe Leroy <[email protected]>
>> ---
>> 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 ?


arch/powerpc/kernel/compat_audit.o: file format elf64-powerpc


Disassembly of section .text:

0000000000000000 <.ppc32_classify_syscall>:
0: 28 03 00 66 cmplwi r3,102
4: 7c 69 1b 78 mr r9,r3
8: 41 82 00 48 beq 50 <.ppc32_classify_syscall+0x50>
c: 41 81 00 24 bgt 30 <.ppc32_classify_syscall+0x30>
10: 28 03 00 05 cmplwi r3,5
14: 38 60 00 02 li r3,2
18: 4d 82 00 20 beqlr
1c: 28 09 00 0b cmplwi r9,11
20: 40 82 00 38 bne 58 <.ppc32_classify_syscall+0x58>
24: 38 60 00 05 li r3,5
28: 4e 80 00 20 blr
2c: 60 00 00 00 nop
30: 28 03 01 1e cmplwi r3,286
34: 38 60 00 01 li r3,1
38: 4c 82 00 20 bnelr
3c: 38 60 00 03 li r3,3
40: 4e 80 00 20 blr
44: 60 00 00 00 nop
48: 60 00 00 00 nop
4c: 60 00 00 00 nop
50: 38 60 00 04 li r3,4
54: 4e 80 00 20 blr
58: 38 60 00 01 li r3,1
5c: 4e 80 00 20 blr


lib/compat_audit.o: file format elf64-powerpc


Disassembly of section .text:

0000000000000000 <.audit_classify_compat_syscall>:
0: 28 04 00 66 cmplwi r4,102
4: 41 82 00 4c beq 50 <.audit_classify_compat_syscall+0x50>
8: 41 81 00 28 bgt 30 <.audit_classify_compat_syscall+0x30>
c: 28 04 00 05 cmplwi r4,5
10: 38 60 00 02 li r3,2
14: 4d 82 00 20 beqlr
18: 28 04 00 0b cmplwi r4,11
1c: 40 82 00 3c bne 58 <.audit_classify_compat_syscall+0x58>
20: 38 60 00 05 li r3,5
24: 4e 80 00 20 blr
28: 60 00 00 00 nop
2c: 60 00 00 00 nop
30: 28 04 01 1e cmplwi r4,286
34: 38 60 00 01 li r3,1
38: 4c 82 00 20 bnelr
3c: 38 60 00 03 li r3,3
40: 4e 80 00 20 blr
44: 60 00 00 00 nop
48: 60 00 00 00 nop
4c: 60 00 00 00 nop
50: 38 60 00 04 li r3,4
54: 4e 80 00 20 blr
58: 38 60 00 01 li r3,1
5c: 4e 80 00 20 blr


arch/powerpc/kernel/audit.o: file format elf64-powerpc


Disassembly of section .text:

0000000000000000 <.audit_classify_arch>:
0: 68 63 00 14 xori r3,r3,20
4: 7c 63 00 34 cntlzw r3,r3
8: 54 63 d9 7e rlwinm r3,r3,27,5,31
c: 4e 80 00 20 blr

0000000000000010 <.audit_classify_syscall>:
10: 2c 03 00 14 cmpwi r3,20
14: 41 82 00 5c beq 70 <.audit_classify_syscall+0x60>
18: 28 04 00 66 cmplwi r4,102
1c: 41 82 00 44 beq 60 <.audit_classify_syscall+0x50>
20: 41 81 00 20 bgt 40 <.audit_classify_syscall+0x30>
24: 28 04 00 05 cmplwi r4,5
28: 38 60 00 02 li r3,2
2c: 4d 82 00 20 beqlr
30: 28 04 00 0b cmplwi r4,11
34: 40 82 00 64 bne 98 <.audit_classify_syscall+0x88>
38: 38 60 00 05 li r3,5
3c: 4e 80 00 20 blr
40: 28 04 01 1e cmplwi r4,286
44: 38 60 00 00 li r3,0
48: 4c 82 00 20 bnelr
4c: 38 60 00 03 li r3,3
50: 4e 80 00 20 blr
54: 60 00 00 00 nop
58: 60 00 00 00 nop
5c: 60 00 00 00 nop
60: 38 60 00 04 li r3,4
64: 4e 80 00 20 blr
68: 60 00 00 00 nop
6c: 60 00 00 00 nop
70: 7c 08 02 a6 mflr r0
74: 7c 83 23 78 mr r3,r4
78: f8 01 00 10 std r0,16(r1)
7c: f8 21 ff 91 stdu r1,-112(r1)
80: 48 00 00 01 bl 80 <.audit_classify_syscall+0x70>
80: R_PPC64_REL24 .ppc32_classify_syscall
84: 60 00 00 00 nop
88: 38 21 00 70 addi r1,r1,112
8c: e8 01 00 10 ld r0,16(r1)
90: 7c 08 03 a6 mtlr r0
94: 4e 80 00 20 blr
98: 38 60 00 00 li r3,0
9c: 4e 80 00 20 blr

Disassembly of section .init.text:

0000000000000000 <.audit_classes_init>:
0: 7c 08 02 a6 mflr r0
4: fb e1 ff f8 std r31,-8(r1)
8: 3d 22 00 00 addis r9,r2,0
a: R_PPC64_TOC16_HA .toc
c: 38 60 00 07 li r3,7
10: e8 89 00 00 ld r4,0(r9)
12: R_PPC64_TOC16_LO_DS .toc
14: 3f e2 00 00 addis r31,r2,0
16: R_PPC64_TOC16_HA .data
18: 3b ff 00 00 addi r31,r31,0
1a: R_PPC64_TOC16_LO .data
1c: f8 01 00 10 std r0,16(r1)
20: f8 21 ff 81 stdu r1,-128(r1)
24: 48 00 00 01 bl 24 <.audit_classes_init+0x24>
24: R_PPC64_REL24 .audit_register_class
28: 60 00 00 00 nop
2c: 3d 22 00 00 addis r9,r2,0
2e: R_PPC64_TOC16_HA .toc+0x8
30: 38 60 00 05 li r3,5
34: e8 89 00 00 ld r4,0(r9)
36: R_PPC64_TOC16_LO_DS .toc+0x8
38: 48 00 00 01 bl 38 <.audit_classes_init+0x38>
38: R_PPC64_REL24 .audit_register_class
3c: 60 00 00 00 nop
40: 3d 22 00 00 addis r9,r2,0
42: R_PPC64_TOC16_HA .toc+0x10
44: 38 60 00 01 li r3,1
48: e8 89 00 00 ld r4,0(r9)
4a: R_PPC64_TOC16_LO_DS .toc+0x10
4c: 48 00 00 01 bl 4c <.audit_classes_init+0x4c>
4c: R_PPC64_REL24 .audit_register_class
50: 60 00 00 00 nop
54: 3d 22 00 00 addis r9,r2,0
56: R_PPC64_TOC16_HA .toc+0x18
58: 38 60 00 03 li r3,3
5c: e8 89 00 00 ld r4,0(r9)
5e: R_PPC64_TOC16_LO_DS .toc+0x18
60: 48 00 00 01 bl 60 <.audit_classes_init+0x60>
60: R_PPC64_REL24 .audit_register_class
64: 60 00 00 00 nop
68: 3d 22 00 00 addis r9,r2,0
6a: R_PPC64_TOC16_HA .toc+0x20
6c: 38 60 00 09 li r3,9
70: e8 89 00 00 ld r4,0(r9)
72: R_PPC64_TOC16_LO_DS .toc+0x20
74: 48 00 00 01 bl 74 <.audit_classes_init+0x74>
74: R_PPC64_REL24 .audit_register_class
78: 60 00 00 00 nop
7c: 7f e4 fb 78 mr r4,r31
80: 38 60 00 06 li r3,6
84: 48 00 00 01 bl 84 <.audit_classes_init+0x84>
84: R_PPC64_REL24 .audit_register_class
88: 60 00 00 00 nop
8c: 38 9f 00 5c addi r4,r31,92
90: 38 60 00 04 li r3,4
94: 48 00 00 01 bl 94 <.audit_classes_init+0x94>
94: R_PPC64_REL24 .audit_register_class
98: 60 00 00 00 nop
9c: 38 9f 00 84 addi r4,r31,132
a0: 38 60 00 00 li r3,0
a4: 48 00 00 01 bl a4 <.audit_classes_init+0xa4>
a4: R_PPC64_REL24 .audit_register_class
a8: 60 00 00 00 nop
ac: 38 9f 00 c4 addi r4,r31,196
b0: 38 60 00 02 li r3,2
b4: 48 00 00 01 bl b4 <.audit_classes_init+0xb4>
b4: R_PPC64_REL24 .audit_register_class
b8: 60 00 00 00 nop
bc: 38 9f 01 04 addi r4,r31,260
c0: 38 60 00 08 li r3,8
c4: 48 00 00 01 bl c4 <.audit_classes_init+0xc4>
c4: R_PPC64_REL24 .audit_register_class
c8: 60 00 00 00 nop
cc: 38 60 00 00 li r3,0
d0: 38 21 00 80 addi r1,r1,128
d4: e8 01 00 10 ld r0,16(r1)
d8: eb e1 ff f8 ld r31,-8(r1)
dc: 7c 08 03 a6 mtlr r0
e0: 4e 80 00 20 blr


lib/audit.o: file format elf64-powerpc


Disassembly of section .text:

0000000000000000 <.audit_classify_arch>:
0: 7c 63 18 f8 not r3,r3
4: 54 63 0f fe rlwinm r3,r3,1,31,31
8: 4e 80 00 20 blr
c: 60 00 00 00 nop

0000000000000010 <.audit_classify_syscall>:
10: 2c 03 00 00 cmpwi r3,0
14: 40 80 00 4c bge 60 <.audit_classify_syscall+0x50>
18: 28 04 00 66 cmplwi r4,102
1c: 41 82 00 74 beq 90 <.audit_classify_syscall+0x80>
20: 41 81 00 20 bgt 40 <.audit_classify_syscall+0x30>
24: 28 04 00 05 cmplwi r4,5
28: 38 60 00 02 li r3,2
2c: 4d 82 00 20 beqlr
30: 28 04 00 0b cmplwi r4,11
34: 41 82 00 20 beq 54 <.audit_classify_syscall+0x44>
38: 38 60 00 00 li r3,0
3c: 4e 80 00 20 blr
40: 28 04 01 1e cmplwi r4,286
44: 38 60 00 03 li r3,3
48: 4d 82 00 20 beqlr
4c: 28 04 01 6a cmplwi r4,362
50: 40 82 ff e8 bne 38 <.audit_classify_syscall+0x28>
54: 38 60 00 05 li r3,5
58: 4e 80 00 20 blr
5c: 60 00 00 00 nop
60: 7c 08 02 a6 mflr r0
64: f8 01 00 10 std r0,16(r1)
68: f8 21 ff 91 stdu r1,-112(r1)
6c: 48 00 00 01 bl 6c <.audit_classify_syscall+0x5c>
6c: R_PPC64_REL24 .audit_classify_compat_syscall
70: 60 00 00 00 nop
74: 38 21 00 70 addi r1,r1,112
78: e8 01 00 10 ld r0,16(r1)
7c: 7c 08 03 a6 mtlr r0
80: 4e 80 00 20 blr
84: 60 00 00 00 nop
88: 60 00 00 00 nop
8c: 60 00 00 00 nop
90: 38 60 00 04 li r3,4
94: 4e 80 00 20 blr

Disassembly of section .init.text:

0000000000000000 <.audit_classes_init>:
0: 7c 08 02 a6 mflr r0
4: fb e1 ff f8 std r31,-8(r1)
8: 3d 22 00 00 addis r9,r2,0
a: R_PPC64_TOC16_HA .toc
c: 38 60 00 07 li r3,7
10: e8 89 00 00 ld r4,0(r9)
12: R_PPC64_TOC16_LO_DS .toc
14: 3f e2 00 00 addis r31,r2,0
16: R_PPC64_TOC16_HA .data
18: 3b ff 00 00 addi r31,r31,0
1a: R_PPC64_TOC16_LO .data
1c: f8 01 00 10 std r0,16(r1)
20: f8 21 ff 81 stdu r1,-128(r1)
24: 48 00 00 01 bl 24 <.audit_classes_init+0x24>
24: R_PPC64_REL24 .audit_register_class
28: 60 00 00 00 nop
2c: 3d 22 00 00 addis r9,r2,0
2e: R_PPC64_TOC16_HA .toc+0x8
30: 38 60 00 05 li r3,5
34: e8 89 00 00 ld r4,0(r9)
36: R_PPC64_TOC16_LO_DS .toc+0x8
38: 48 00 00 01 bl 38 <.audit_classes_init+0x38>
38: R_PPC64_REL24 .audit_register_class
3c: 60 00 00 00 nop
40: 3d 22 00 00 addis r9,r2,0
42: R_PPC64_TOC16_HA .toc+0x10
44: 38 60 00 01 li r3,1
48: e8 89 00 00 ld r4,0(r9)
4a: R_PPC64_TOC16_LO_DS .toc+0x10
4c: 48 00 00 01 bl 4c <.audit_classes_init+0x4c>
4c: R_PPC64_REL24 .audit_register_class
50: 60 00 00 00 nop
54: 3d 22 00 00 addis r9,r2,0
56: R_PPC64_TOC16_HA .toc+0x18
58: 38 60 00 03 li r3,3
5c: e8 89 00 00 ld r4,0(r9)
5e: R_PPC64_TOC16_LO_DS .toc+0x18
60: 48 00 00 01 bl 60 <.audit_classes_init+0x60>
60: R_PPC64_REL24 .audit_register_class
64: 60 00 00 00 nop
68: 3d 22 00 00 addis r9,r2,0
6a: R_PPC64_TOC16_HA .toc+0x20
6c: 38 60 00 09 li r3,9
70: e8 89 00 00 ld r4,0(r9)
72: R_PPC64_TOC16_LO_DS .toc+0x20
74: 48 00 00 01 bl 74 <.audit_classes_init+0x74>
74: R_PPC64_REL24 .audit_register_class
78: 60 00 00 00 nop
7c: 7f e4 fb 78 mr r4,r31
80: 38 60 00 06 li r3,6
84: 48 00 00 01 bl 84 <.audit_classes_init+0x84>
84: R_PPC64_REL24 .audit_register_class
88: 60 00 00 00 nop
8c: 38 9f 00 5c addi r4,r31,92
90: 38 60 00 04 li r3,4
94: 48 00 00 01 bl 94 <.audit_classes_init+0x94>
94: R_PPC64_REL24 .audit_register_class
98: 60 00 00 00 nop
9c: 38 9f 00 84 addi r4,r31,132
a0: 38 60 00 00 li r3,0
a4: 48 00 00 01 bl a4 <.audit_classes_init+0xa4>
a4: R_PPC64_REL24 .audit_register_class
a8: 60 00 00 00 nop
ac: 38 9f 00 c4 addi r4,r31,196
b0: 38 60 00 02 li r3,2
b4: 48 00 00 01 bl b4 <.audit_classes_init+0xb4>
b4: R_PPC64_REL24 .audit_register_class
b8: 60 00 00 00 nop
bc: 38 9f 01 04 addi r4,r31,260
c0: 38 60 00 08 li r3,8
c4: 48 00 00 01 bl c4 <.audit_classes_init+0xc4>
c4: R_PPC64_REL24 .audit_register_class
c8: 60 00 00 00 nop
cc: 38 60 00 00 li r3,0
d0: 38 21 00 80 addi r1,r1,128
d4: e8 01 00 10 ld r0,16(r1)
d8: eb e1 ff f8 ld r31,-8(r1)
dc: 7c 08 03 a6 mtlr r0
e0: 4e 80 00 20 blr

2021-08-24 23:04:23

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH v2 RESEND] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC

On Tue, Aug 24, 2021 at 1:11 PM Christophe Leroy
<[email protected]> wrote:
> Le 24/08/2021 à 16:47, Paul Moore a écrit :
> > On Tue, Aug 24, 2021 at 9:36 AM Christophe Leroy
> > <[email protected]> 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 protected]>
> >> Cc: Eric Paris <[email protected]>
> >> Signed-off-by: Christophe Leroy <[email protected]>
> >> ---
> >> 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

--
paul moore
http://www.paul-moore.com

2021-08-26 14:38:50

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v2 RESEND] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC

Paul Moore <[email protected]> writes:
> On Tue, Aug 24, 2021 at 1:11 PM Christophe Leroy
> <[email protected]> wrote:
>> Le 24/08/2021 à 16:47, Paul Moore a écrit :
>> > On Tue, Aug 24, 2021 at 9:36 AM Christophe Leroy
>> > <[email protected]> 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 protected]>
>> >> Cc: Eric Paris <[email protected]>
>> >> Signed-off-by: Christophe Leroy <[email protected]>
>> >> ---
>> >> 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.

$ 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.

2021-08-27 00:56:20

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH v2 RESEND] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC

On Thu, Aug 26, 2021 at 10:37 AM Michael Ellerman <[email protected]> wrote:
> Paul Moore <[email protected]> writes:
> > On Tue, Aug 24, 2021 at 1:11 PM Christophe Leroy
> > <[email protected]> wrote:
> >> Le 24/08/2021 à 16:47, Paul Moore a écrit :
> >> > On Tue, Aug 24, 2021 at 9:36 AM Christophe Leroy
> >> > <[email protected]> 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 protected]>
> >> >> Cc: Eric Paris <[email protected]>
> >> >> Signed-off-by: Christophe Leroy <[email protected]>
> >> >> ---
> >> >> 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
http://www.paul-moore.com

2021-11-02 11:41:58

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v2 RESEND] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC

On Tue, 24 Aug 2021 13:36:13 +0000 (UTC), Christophe Leroy 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.
>
> [...]

Applied to powerpc/next.

[1/1] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC
https://git.kernel.org/powerpc/c/566af8cda399c088763d07464463dc871c943b54

cheers

2021-11-02 13:06:35

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH v2 RESEND] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC

On Tue, Nov 2, 2021 at 7:38 AM Michael Ellerman
<[email protected]> wrote:
>
> On Tue, 24 Aug 2021 13:36:13 +0000 (UTC), Christophe Leroy 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.
> >
> > [...]
>
> Applied to powerpc/next.
>
> [1/1] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC
> https://git.kernel.org/powerpc/c/566af8cda399c088763d07464463dc871c943b54

Did the test failure discussed earlier in this thread ever get
resolved? If not, this really shouldn't be in linux-next IMO.

--
paul moore
http://www.paul-moore.com

2021-11-02 23:22:17

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v2 RESEND] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC

Paul Moore <[email protected]> writes:
> On Tue, Nov 2, 2021 at 7:38 AM Michael Ellerman
> <[email protected]> wrote:
>>
>> On Tue, 24 Aug 2021 13:36:13 +0000 (UTC), Christophe Leroy 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.
>> >
>> > [...]
>>
>> Applied to powerpc/next.
>>
>> [1/1] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC
>> https://git.kernel.org/powerpc/c/566af8cda399c088763d07464463dc871c943b54
>
> Did the test failure discussed earlier in this thread ever get
> resolved? If not, this really shouldn't be in linux-next IMO.

It's not in next, that notification is from the b4 thanks script, which
didn't notice that the commit has since been reverted.

cheers

2021-11-02 23:22:19

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v2 RESEND] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC

Michael Ellerman <[email protected]> writes:
> On Tue, 24 Aug 2021 13:36:13 +0000 (UTC), Christophe Leroy 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.
>>
>> [...]
>
> Applied to powerpc/next.
>
> [1/1] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC
> https://git.kernel.org/powerpc/c/566af8cda399c088763d07464463dc871c943b54

And then reverted in:

https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git/commit/?h=next&id=b7472e1764bfc0fe3d6578cb281e81c812ca5886

cheers

2021-11-02 23:56:03

by Konstantin Ryabitsev

[permalink] [raw]
Subject: Re: [PATCH v2 RESEND] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC

On Wed, Nov 03, 2021 at 10:18:57AM +1100, Michael Ellerman wrote:
> It's not in next, that notification is from the b4 thanks script, which
> didn't notice that the commit has since been reverted.

Yeah... I'm not sure how to catch that, but I'm open to suggestions.

-K

2021-11-03 01:20:33

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH v2 RESEND] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC

On Tue, Nov 2, 2021 at 7:19 PM Michael Ellerman <[email protected]> wrote:
> Paul Moore <[email protected]> writes:
> > On Tue, Nov 2, 2021 at 7:38 AM Michael Ellerman
> > <[email protected]> wrote:
> >>
> >> On Tue, 24 Aug 2021 13:36:13 +0000 (UTC), Christophe Leroy 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.
> >> >
> >> > [...]
> >>
> >> Applied to powerpc/next.
> >>
> >> [1/1] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC
> >> https://git.kernel.org/powerpc/c/566af8cda399c088763d07464463dc871c943b54
> >
> > Did the test failure discussed earlier in this thread ever get
> > resolved? If not, this really shouldn't be in linux-next IMO.
>
> It's not in next, that notification is from the b4 thanks script, which
> didn't notice that the commit has since been reverted.

Okay, thanks for the clarification. I know we had already talked
about this so I was a little caught off guard when I saw this mail :)

--
paul moore
http://www.paul-moore.com

2021-11-04 06:22:29

by Michael Ellerman

[permalink] [raw]
Subject: Re: [PATCH v2 RESEND] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC

Konstantin Ryabitsev <[email protected]> writes:
> On Wed, Nov 03, 2021 at 10:18:57AM +1100, Michael Ellerman wrote:
>> It's not in next, that notification is from the b4 thanks script, which
>> didn't notice that the commit has since been reverted.
>
> Yeah... I'm not sure how to catch that, but I'm open to suggestions.

I think that's probably the first time I've had a commit and a revert of
the commit in the same batch of thanks mails.

And the notification is not wrong, the commit was applied with that SHA,
it is in the tree.

So I'm not sure it's very common to have a commit & a revert in the tree
at the same time.


On the other hand being able to generate a mail for an arbitrary revert
would be helpful, ie. independent of any thanks state.

eg, picking a random commit from the past:

e95ad5f21693 ("powerpc/head_check: Fix shellcheck errors")


If I revert that in my tree today, it'd be cool if I could run something
that would detect the revert, backtrack to the reverted commit, extract
the message-id from the Link: tag, and generate a reply to the original
submission noting that it's now been reverted.

In fact we could write a bot to do that across all commits ever ...

cheers

2021-11-04 14:36:41

by Paul Moore

[permalink] [raw]
Subject: Re: [PATCH v2 RESEND] powerpc/audit: Convert powerpc to AUDIT_ARCH_COMPAT_GENERIC

On Thu, Nov 4, 2021 at 2:20 AM Michael Ellerman <[email protected]> wrote:
> Konstantin Ryabitsev <[email protected]> writes:
> > On Wed, Nov 03, 2021 at 10:18:57AM +1100, Michael Ellerman wrote:
> >> It's not in next, that notification is from the b4 thanks script, which
> >> didn't notice that the commit has since been reverted.
> >
> > Yeah... I'm not sure how to catch that, but I'm open to suggestions.
>
> I think that's probably the first time I've had a commit and a revert of
> the commit in the same batch of thanks mails.
>
> And the notification is not wrong, the commit was applied with that SHA,
> it is in the tree.
>
> So I'm not sure it's very common to have a commit & a revert in the tree
> at the same time.

I know it is not common for the SELinux and audit trees. I guess
every tree/maintainer is different, but I'm probably more conservative
than most when it comes to merging patches so it's pretty rare that we
need to revert things in those trees.

> On the other hand being able to generate a mail for an arbitrary revert
> would be helpful, ie. independent of any thanks state.
>
> eg, picking a random commit from the past:
>
> e95ad5f21693 ("powerpc/head_check: Fix shellcheck errors")
>
> If I revert that in my tree today, it'd be cool if I could run something
> that would detect the revert, backtrack to the reverted commit, extract
> the message-id from the Link: tag, and generate a reply to the original
> submission noting that it's now been reverted.

Even if it isn't practical to do the find-the-original-commit logic,
simply getting an email that says "FYI, this revert has been merged
into this tree/branch" would be helpful. Although I guess that would
require either the revert having the right metadata, e.g. "Cc:", or
that prior mentioned logic to find the original commit so the proper
To/CC lines could be generated.

--
paul moore
http://www.paul-moore.com