ARM: ptrace: Include linux/audit.h to fix build errors

PATCH In review Axel Lin 6 participants 12 messages Feb 15, 2012

Subsystems (2)

Arm (32-bit) ptrace support

lore

Share & tools

lore

Axel Lin

author

2012-02-15 05:36 UTC #0 lore raw

PATCH 1/1 ARM: ptrace: Include linux/audit.h to fix build errors

Include linux/audit.h to fix below build errors:

CC arch/arm/kernel/ptrace.o
arch/arm/kernel/ptrace.c: In function 'syscall_trace':
arch/arm/kernel/ptrace.c:919: error: implicit declaration of function 'audit_syscall_exit' arch/arm/kernel/ptrace.c:921: error: implicit declaration of function 'audit_syscall_entry' arch/arm/kernel/ptrace.c:921: error: 'AUDIT_ARCH_ARMEB' undeclared (first use in this function) arch/arm/kernel/ptrace.c:921: error: (Each undeclared identifier is reported only once arch/arm/kernel/ptrace.c:921: error: for each function it appears in.)
make[1]: *** [arch/arm/kernel/ptrace.o] Error 1
make: *** [arch/arm/kernel] Error 2

Signed-off-by Axel Lin <[email protected]>

---
This patch is against linux-next 20120215.

 arch/arm/kernel/ptrace.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

arch/arm/kernel/ptrace.c +1 -0

index e33870f..48970ad 100644
@@ -23,6 +23,7 @@
 #include <linux/perf_event.h>
 #include <linux/hw_breakpoint.h>
 #include <linux/regset.h>
+#include <linux/audit.h>
 
 #include <asm/pgtable.h>
 #include <asm/system.h>

Signature · 1 line

1.7.5.4

Russell King - ARM Linux

2012-02-15 08:36 UTC #1 lore raw

Quoted 7 lines · Axel Lin

Include linux/audit.h to fix below build errors:
CC arch/arm/kernel/ptrace.o
arch/arm/kernel/ptrace.c: In function 'syscall_trace':
arch/arm/kernel/ptrace.c:919: error: implicit declaration of function 'audit_syscall_exit' arch/arm/kernel/ptrace.c:921: error: implicit declaration of function 'audit_syscall_entry' arch/arm/kernel/ptrace.c:921: error: 'AUDIT_ARCH_ARMEB' undeclared (first use in this function)

Err, can someone explain why we seem to tell the audit code that we're always big endian?

Will Deacon

2012-02-15 09:52 UTC #2 lore raw

Quoted 11 lines · Russell King - ARM Linux

Quoted 7 lines · Axel Lin
Include linux/audit.h to fix below build errors:
CC arch/arm/kernel/ptrace.o
arch/arm/kernel/ptrace.c: In function 'syscall_trace':
arch/arm/kernel/ptrace.c:919: error: implicit declaration of function 'audit_syscall_exit' arch/arm/kernel/ptrace.c:921: error: implicit declaration of function 'audit_syscall_entry' arch/arm/kernel/ptrace.c:921: error: 'AUDIT_ARCH_ARMEB' undeclared (first use in this function)
Err, can someone explain why we seem to tell the audit code that we're always big endian?

I can't see this patch in the linux-arm-kernel archives, so that's probably why we missed it. The fix should be pretty straightforward though - Axel, care to add the appropriate ifdeffery?

Will

Russell King - ARM Linux

2012-02-15 10:18 UTC #3 lore raw

Quoted 10 lines · Will Deacon

Quoted 9 lines · Russell King - ARM Linux
Quoted 7 lines · Axel Lin
Include linux/audit.h to fix below build errors:
CC arch/arm/kernel/ptrace.o
arch/arm/kernel/ptrace.c: In function 'syscall_trace':
arch/arm/kernel/ptrace.c:919: error: implicit declaration of function 'audit_syscall_exit' arch/arm/kernel/ptrace.c:921: error: implicit declaration of function 'audit_syscall_entry' arch/arm/kernel/ptrace.c:921: error: 'AUDIT_ARCH_ARMEB' undeclared (first use in this function)

Quoted 6 lines

Quoted 2 lines
Err, can someone explain why we seem to tell the audit code that we're always big endian?
I can't see this patch in the linux-arm-kernel archives, so that's probably why we missed it. The fix should be pretty straightforward though - Axel, care to add the appropriate ifdeffery?

In which case the BIG QUESTION is how the hell this ever got into mainline without being seen by any ARM people. I've checked my mailboxes and, like you, can find no trace of this patch. The only trace of it I can find is:

http://www.redhat.com/archives/linux-audit/2011-August/msg00000.html

and a repost as part of a patch set in November.

This is a patch which should have been reviewed by ARM folk to catch bugs like the above.

So, this gives us two options: either we revert the damned thing and start over with it (and hopefully audit folk will start co-operating with ARM folk) or we try to fix up their mess (and potentially they learn they can dump crap on us.)

Eric Paris

2012-02-15 15:10 UTC #4 lore raw

Quoted 4 lines · Russell King - ARM Linux

So, this gives us two options: either we revert the damned thing and start over with it (and hopefully audit folk will start co-operating with ARM folk) or we try to fix up their mess (and potentially they learn they can dump crap on us.)

It must be a conspiracy to screw with the ARM people! Your jobs were obviously too easy to begin with. Rahrrrrr! Down with ARM!

No seriously. I am the audit folks. You got us all right here in this one e-mail. You're right, I should have paid more attention to who was in the cc list. If you want to rewrite it, please feel free! If you want to push fixes yourself, please feel free! If you want me to push fixes, please just let me know what is broken and I'll do my best.

Sorry. I just got the patch and was contacted by a number of people off list asking when it was going to go in because they wanted it for their project. I obviously should have known who the ARM maintainers that would have looked at a patch for the audit system was and should have made sure they commented. Sorry! I'll be sure to hit the ARM list if another patch goes by....

Russell King - ARM Linux

2012-02-15 16:10 UTC #5 lore raw

Quoted 16 lines · Eric Paris

Quoted 4 lines · Russell King - ARM Linux
So, this gives us two options: either we revert the damned thing and start over with it (and hopefully audit folk will start co-operating with ARM folk) or we try to fix up their mess (and potentially they learn they can dump crap on us.)
It must be a conspiracy to screw with the ARM people! Your jobs were obviously too easy to begin with. Rahrrrrr! Down with ARM!
No seriously. I am the audit folks. You got us all right here in this one e-mail. You're right, I should have paid more attention to who was in the cc list. If you want to rewrite it, please feel free! If you want to push fixes yourself, please feel free! If you want me to push fixes, please just let me know what is broken and I'll do my best.

Quoted 6 lines

Sorry. I just got the patch and was contacted by a number of people off list asking when it was going to go in because they wanted it for their project. I obviously should have known who the ARM maintainers that would have looked at a patch for the audit system was and should have made sure they commented. Sorry! I'll be sure to hit the ARM list if another patch goes by....

And... what about this endian-ness issue?

Frustrated.

I've tried discussing that issue with Nathaniel and haven't got anywhere with it (other than, seemingly, it's above his pay grade to know the answer.)

Right, I've committed a revert of it to my tree. When there's more interest in the feature, it can be revisited with proper review and proper testing of it.

Nathaniel Husted

2012-02-15 16:14 UTC #6 lore raw

The code in place regarding the endian-ness was put into place long before my patch and I've no clue the original reasoning behind it. Thus I'd defer the answer to Eric or other powers that be.

Cheers,
Nathaniel

Quoted 35 lines · Russell King - ARM Linux

Quoted 16 lines · Eric Paris
Quoted 4 lines · Russell King - ARM Linux
So, this gives us two options: either we revert the damned thing and start over with it (and hopefully audit folk will start co-operating with ARM folk) or we try to fix up their mess (and potentially they learn they can dump crap on us.)
It must be a conspiracy to screw with the ARM people! ?Your jobs were obviously too easy to begin with. ?Rahrrrrr! ?Down with ARM!
No seriously. ?I am the audit folks. ?You got us all right here in this one e-mail. ?You're right, I should have paid more attention to who was in the cc list. ?If you want to rewrite it, please feel free! ?If you want to push fixes yourself, please feel free! ?If you want me to push fixes, please just let me know what is broken and I'll do my best.
Quoted 6 lines
Sorry. ?I just got the patch and was contacted by a number of people off list asking when it was going to go in because they wanted it for their project. ?I obviously should have known who the ARM maintainers that would have looked at a patch for the audit system was and should have made sure they commented. ?Sorry! ?I'll be sure to hit the ARM list if another patch goes by....
And... what about this endian-ness issue?
Frustrated.
I've tried discussing that issue with Nathaniel and haven't got anywhere with it (other than, seemingly, it's above his pay grade to know the answer.)
Right, I've committed a revert of it to my tree. ?When there's more interest in the feature, it can be revisited with proper review and proper testing of it.

Eric Paris

2012-02-15 17:07 UTC #7 lore raw

In reply to Russell King - ARM Linux

Quoted 11 lines · Russell King - ARM Linux

Quoted 7 lines · Axel Lin
Include linux/audit.h to fix below build errors:
CC arch/arm/kernel/ptrace.o
arch/arm/kernel/ptrace.c: In function 'syscall_trace':
arch/arm/kernel/ptrace.c:919: error: implicit declaration of function 'audit_syscall_exit' arch/arm/kernel/ptrace.c:921: error: implicit declaration of function 'audit_syscall_entry' arch/arm/kernel/ptrace.c:921: error: 'AUDIT_ARCH_ARMEB' undeclared (first use in this function)
Err, can someone explain why we seem to tell the audit code that we're always big endian?

So we have 2 bugs obviously. One is my fault. One is Nathaniel's. Both are easy to fix.

I'm pretty sure I introduced the build failure when I merged Nathaniel's patch into my tree. I completely rewrote audit_syscall_entry() and exit and probably the include wasn't needed in his version. So my fault.

The endian issue is something an ARM-er would have noticed I assume, and again, it's my fault for not forwarding to the ARM list. Let me explain what happens. The kernel audit system is going to dump the raw bits to userspace. Userspace is going to write them to a file exactly how it got them. One of the things we write is the arch (which includes endianness) This means we can send those bits to another machine and it should be able to correctly interpret their meaning. However if you are just looking at the raw bits on your own box, endianness isn't an issue and records will look ok.

Fixing the arch flag to be correct means we could do that translation properly. I haven't seen the patches to support translation of arm raw bits to something higher level in audit userspace, but I assume it's coming as soon as someone cares.

So if someone tells me how the code knows it's endianness I'll gladly write the ifdef to switch from AUDIT_ARCH_ARMEB to AUDIT_ARCH_ARM when appropriate....

Will Deacon

2012-02-15 17:14 UTC #8 lore raw

Quoted 8 lines · Eric Paris

Fixing the arch flag to be correct means we could do that translation properly. I haven't seen the patches to support translation of arm raw bits to something higher level in audit userspace, but I assume it's coming as soon as someone cares.
So if someone tells me how the code knows it's endianness I'll gladly write the ifdef to switch from AUDIT_ARCH_ARMEB to AUDIT_ARCH_ARM when appropriate....

You should just be able to use #ifdef __ARMEB__ for the big-endian case (we don't support a little-endian kernel with a big-endian userspace or anything funky like that). If Russell has reverted the original patch your best bet is probably to spin a v2 of the ARM bits and post it to LAK.

Will

Russell King - ARM Linux

2012-02-15 17:34 UTC #9 lore raw

In reply to Eric Paris

Quoted 3 lines · Eric Paris

So if someone tells me how the code knows it's endianness I'll gladly write the ifdef to switch from AUDIT_ARCH_ARMEB to AUDIT_ARCH_ARM when appropriate....

An easy way would be:

#ifndef __ARMEB__
#define AUDIT_ARCH_NR AUDIT_ARCH_ARMEB
#else
#define AUDIT_ARCH_NR AUDIT_ARCH_ARM
#endif

and then:

	audit_syscall_entry(AUDIT_ARCH_NR, scno, regs->ARM_r0,

Eric Paris

2012-02-15 19:57 UTC #10 lore raw

Quoted 5 lines · Russell King - ARM Linux

Quoted 3 lines · Eric Paris
So if someone tells me how the code knows it's endianness I'll gladly write the ifdef to switch from AUDIT_ARCH_ARMEB to AUDIT_ARCH_ARM when appropriate....

Quoted 7 lines

An easy way would be:
#ifndef __ARMEB__
#define AUDIT_ARCH_NR AUDIT_ARCH_ARMEB
#else
#define AUDIT_ARCH_NR AUDIT_ARCH_ARM
#endif

Is that actually exactly backwards? ifndef? (I don't have a clue when __ARMEB__ is set)

Nicolas Pitre

2012-02-15 20:19 UTC #11 lore raw

Quoted 8 lines · Eric Paris

Quoted 7 lines · Russell King - ARM Linux
Quoted 3 lines · Eric Paris
So if someone tells me how the code knows it's endianness I'll gladly write the ifdef to switch from AUDIT_ARCH_ARMEB to AUDIT_ARCH_ARM when appropriate....
An easy way would be:

Quoted 8 lines

Quoted 5 lines
#ifndef __ARMEB__
#define AUDIT_ARCH_NR AUDIT_ARCH_ARMEB
#else
#define AUDIT_ARCH_NR AUDIT_ARCH_ARM
#endif
Is that actually exactly backwards? ifndef? (I don't have a clue when __ARMEB__ is set)

__ARMEB__ is set on big endian. So the above condition should be ifdef not ifndef.

Nicolas