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

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

[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)
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 <axel.lin@gmail.com>
---
This patch is against linux-next 20120215.

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

diff --git a/arch/arm/kernel/ptrace.c b/arch/arm/kernel/ptrace.c
index e33870f..48970ad 100644
--- a/arch/arm/kernel/ptrace.c
+++ b/arch/arm/kernel/ptrace.c
@@ -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>
-- 
1.7.5.4

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

[Russell King - ARM Linux]

On Wed, Feb 15, 2012 at 01:36:28PM +0800, Axel Lin wrote:
> 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?

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

[Will Deacon]

On Wed, Feb 15, 2012 at 08:36:09AM +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 15, 2012 at 01:36:28PM +0800, Axel Lin wrote:
> > 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

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

[Russell King - ARM Linux]

On Wed, Feb 15, 2012 at 09:52:16AM +0000, Will Deacon wrote:
> On Wed, Feb 15, 2012 at 08:36:09AM +0000, Russell King - ARM Linux wrote:
> > On Wed, Feb 15, 2012 at 01:36:28PM +0800, Axel Lin wrote:
> > > 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?

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

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

[Eric Paris]

On Wed, 2012-02-15 at 10:18 +0000, Russell King - ARM Linux wrote:

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

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

[Russell King - ARM Linux]

On Wed, Feb 15, 2012 at 10:10:58AM -0500, Eric Paris wrote:
> On Wed, 2012-02-15 at 10:18 +0000, Russell King - ARM Linux wrote:
> 
> > 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....

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.

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

[Nathaniel Husted]

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

On Wed, Feb 15, 2012 at 11:10 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Feb 15, 2012 at 10:10:58AM -0500, Eric Paris wrote:
>> On Wed, 2012-02-15 at 10:18 +0000, Russell King - ARM Linux wrote:
>>
>> > 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....
>
> 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.

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

[Eric Paris]

On Wed, 2012-02-15 at 08:36 +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 15, 2012 at 01:36:28PM +0800, Axel Lin wrote:
> > 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....

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

[Will Deacon]

On Wed, Feb 15, 2012 at 05:07:02PM +0000, Eric Paris wrote:
> 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

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

[Russell King - ARM Linux]

On Wed, Feb 15, 2012 at 12:07:02PM -0500, Eric Paris wrote:
> 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,

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

[Eric Paris]

On Wed, 2012-02-15 at 17:34 +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 15, 2012 at 12:07:02PM -0500, Eric Paris wrote:
> > 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

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

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

[Nicolas Pitre]

On Wed, 15 Feb 2012, Eric Paris wrote:

> On Wed, 2012-02-15 at 17:34 +0000, Russell King - ARM Linux wrote:
> > On Wed, Feb 15, 2012 at 12:07:02PM -0500, Eric Paris wrote:
> > > 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
> 
> 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