Re: [PATCH 06/13] arc: define syscall_get_arch()
On 11/9/18 11:03 AM, Andy Lutomirski wrote: >> Does ptrace (or user of this API) need a unique value per arch. Otherwise >> instead >> of adding the boilerplate code to all arches, they could simply define >> AUDIT_ARCH >> and common code could return it. Also the EM_xxx are not there in >> include/uapi/linux/elf.h to begin with since libc elf.h already defines them. >> > A lot of architectures allow multiple audit_arches at runtime due to compat > support and similar features, so it really does want to be a function. The > goal of this patch set is to get it supported everywhere. > I was wondering if we could have a common syscall_get_arch() simply returning an per-arch AUDIT_ARCH. But seems like the function itself needs to be per arch anyways due to the nuances above. -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH 06/13] arc: define syscall_get_arch()
> On Nov 9, 2018, at 8:50 AM, Vineet Gupta wrote:
>
>> On 11/8/18 7:16 PM, Dmitry V. Levin wrote:
>> syscall_get_arch() is required to be implemented on all architectures
>> that use tracehook_report_syscall_entry() in order to extend
>> the generic ptrace API with PTRACE_GET_SYSCALL_INFO request.
>>
>> Signed-off-by: Dmitry V. Levin
>> ---
>> arch/arc/include/asm/syscall.h | 6 ++
>> include/uapi/linux/audit.h | 1 +
>> 2 files changed, 7 insertions(+)
>>
>> diff --git a/arch/arc/include/asm/syscall.h b/arch/arc/include/asm/syscall.h
>> index 29de09804306..5662778a7411 100644
>> --- a/arch/arc/include/asm/syscall.h
>> +++ b/arch/arc/include/asm/syscall.h
>> @@ -9,6 +9,7 @@
>> #ifndef _ASM_ARC_SYSCALL_H
>> #define _ASM_ARC_SYSCALL_H 1
>>
>> +#include
>> #include
>> #include
>> #include
>> @@ -68,4 +69,9 @@ syscall_get_arguments(struct task_struct *task, struct
>> pt_regs *regs,
>>}
>> }
>>
>> +static inline int syscall_get_arch(void)
>> +{
>> +return AUDIT_ARCH_ARC;
>> +}
>> +
>
> Does ptrace (or user of this API) need a unique value per arch. Otherwise
> instead
> of adding the boilerplate code to all arches, they could simply define
> AUDIT_ARCH
> and common code could return it. Also the EM_xxx are not there in
> include/uapi/linux/elf.h to begin with since libc elf.h already defines them.
A lot of architectures allow multiple audit_arches at runtime due to compat
support and similar features, so it really does want to be a function. The
goal of this patch set is to get it supported everywhere.
>> #endif
>> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
>> index 818ae690ab79..a7149ceb5b98 100644
>> --- a/include/uapi/linux/audit.h
>> +++ b/include/uapi/linux/audit.h
>> @@ -375,6 +375,7 @@ enum {
>>
>> #define AUDIT_ARCH_AARCH64(EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
>> #define AUDIT_ARCH_ALPHA(EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
>> +#define AUDIT_ARCH_ARC(EM_ARC)
>> #define AUDIT_ARCH_ARM(EM_ARM|__AUDIT_ARCH_LE)
>> #define AUDIT_ARCH_ARMEB(EM_ARM)
>> #define AUDIT_ARCH_CRIS(EM_CRIS|__AUDIT_ARCH_LE)
>
> So I don't have the context of this patch (or coverletter) but what exactly
> are we
> trying to do with this (adding LE to audit) - what happens when an arch is
> capable of either and is say built for BE ?
The primary intent is that the triple (audit_arch, syscall_nr, arg1, ..., arg6)
should describe what system call is being called and what its arguments are.
I’m personally not sure what, if any, technical value there is in the LE bit.
I do think it makes sense for BE and LE to have different values.
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH 06/13] arc: define syscall_get_arch()
On 11/8/18 7:16 PM, Dmitry V. Levin wrote:
> syscall_get_arch() is required to be implemented on all architectures
> that use tracehook_report_syscall_entry() in order to extend
> the generic ptrace API with PTRACE_GET_SYSCALL_INFO request.
>
> Signed-off-by: Dmitry V. Levin
> ---
> arch/arc/include/asm/syscall.h | 6 ++
> include/uapi/linux/audit.h | 1 +
> 2 files changed, 7 insertions(+)
>
> diff --git a/arch/arc/include/asm/syscall.h b/arch/arc/include/asm/syscall.h
> index 29de09804306..5662778a7411 100644
> --- a/arch/arc/include/asm/syscall.h
> +++ b/arch/arc/include/asm/syscall.h
> @@ -9,6 +9,7 @@
> #ifndef _ASM_ARC_SYSCALL_H
> #define _ASM_ARC_SYSCALL_H 1
>
> +#include
> #include
> #include
> #include
> @@ -68,4 +69,9 @@ syscall_get_arguments(struct task_struct *task, struct
> pt_regs *regs,
> }
> }
>
> +static inline int syscall_get_arch(void)
> +{
> + return AUDIT_ARCH_ARC;
> +}
> +
Does ptrace (or user of this API) need a unique value per arch. Otherwise
instead
of adding the boilerplate code to all arches, they could simply define
AUDIT_ARCH
and common code could return it. Also the EM_xxx are not there in
include/uapi/linux/elf.h to begin with since libc elf.h already defines them.
> #endif
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 818ae690ab79..a7149ceb5b98 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -375,6 +375,7 @@ enum {
>
> #define AUDIT_ARCH_AARCH64 (EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> #define AUDIT_ARCH_ALPHA (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> +#define AUDIT_ARCH_ARC (EM_ARC)
> #define AUDIT_ARCH_ARM (EM_ARM|__AUDIT_ARCH_LE)
> #define AUDIT_ARCH_ARMEB (EM_ARM)
> #define AUDIT_ARCH_CRIS (EM_CRIS|__AUDIT_ARCH_LE)
So I don't have the context of this patch (or coverletter) but what exactly are
we
trying to do with this (adding LE to audit) - what happens when an arch is
capable of either and is say built for BE ?
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH 06/13] arc: define syscall_get_arch()
On Fri, Nov 9, 2018 at 8:11 AM Alexey Brodkin
wrote:
>
> Hi Andy,
>
> On Fri, 2018-11-09 at 07:56 -0800, Andy Lutomirski wrote:
> > > On Nov 9, 2018, at 7:27 AM, Alexey Brodkin
> > > wrote:
> > >
> > > Hi Andy,
> > >
> > > > On Fri, 2018-11-09 at 07:17 -0800, Andy Lutomirski wrote:
> > > > On Fri, Nov 9, 2018 at 6:22 AM Alexey Brodkin
> > > > wrote:
> > > > > Hi Dmitry,
> > > > >
> > > > > > On Fri, 2018-11-09 at 06:16 +0300, Dmitry V. Levin wrote:
> > > > > > syscall_get_arch() is required to be implemented on all
> > > > > > architectures
> > > > > > that use tracehook_report_syscall_entry() in order to extend
> > > > > > the generic ptrace API with PTRACE_GET_SYSCALL_INFO request.
> > > > > >
> > > > > > Signed-off-by: Dmitry V. Levin
> > > > > > ---
> > > > > > arch/arc/include/asm/syscall.h | 6 ++
> > > > > > include/uapi/linux/audit.h | 1 +
> > > > > > 2 files changed, 7 insertions(+)
> > > > >
> > > > > [snip]
> > > > >
> > > > > > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > > > > > index 818ae690ab79..a7149ceb5b98 100644
> > > > > > --- a/include/uapi/linux/audit.h
> > > > > > +++ b/include/uapi/linux/audit.h
> > > > > > @@ -375,6 +375,7 @@ enum {
> > > > > >
> > > > > > #define AUDIT_ARCH_AARCH64
> > > > > > (EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> > > > > > #define AUDIT_ARCH_ALPHA
> > > > > > (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> > > > > > +#define AUDIT_ARCH_ARC (EM_ARC)
> > > > >
> > > > > Similarly here we need to have:
> > > > > >8-
> > > > > +#define AUDIT_ARCH_ARC (EM_ARC|EM_ARCV2)
> > > > > >8-
> > > > >
> > > >
> > > > Huh? How does the bitwise or of two ELF machine codes make any sense?
> > >
> > > Oops... I didn't read examples of AUDIT_ARCH_ALPHA above :(
> > > Indeed that was stupid.
> > >
> > > But what would be a proper fix then?
> > >
> > > Something like that?
> > > >8-
> > > #define AUDIT_ARCH_ARC (EM_ARC)
> > > #define AUDIT_ARCH_ARCV2 (EM_ARCV2)
> > >
> > >
> > > static inline int syscall_get_arch(void)
> > > {
> > > #ifdef __ARC700__
> > > return AUDIT_ARCH_ARC;
> > > #else
> > > return AUDIT_ARCH_ARCV2;
> > > #endif
> > > }
> > > >8-
> > >
> >
> > Maybe, but I know basically nothing about ARC. Is the syscall numbering or
> > calling convention different on ARC vs ARCv2?
>
> Syscall numbering should be the same as we use UAPI for both ARCompact (AKA
> ARCv1)
> and ARCv2. As for calling convention I think it indeed differs.
>
> Note ARCompact and ARCv2 ISAs are binary incompatible!
>
> Even though assembly look pretty much the same (sans instructions
> available only for either ARCompact or ARCv2) encodings are different so
> in that sense these are completely different architectures.
>
> Also I'm wondering what could be other cases for use of syscall_get_arch().
The intent of syscall_get_arch() is that the tuple:
(arch, nr, arg1, ..., arg6)
fully identifies a system call and its arguments. So it sounds like
we do indeed need to arch values.
>
> So I'd say it's better to report different values for ARC ISAs.
> And given we use the same values as in Binutils IMHO it would be good
> to not mix IDs here.
>
> -Alexey
--
Andy Lutomirski
AMA Capital Management, LLC
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH 06/13] arc: define syscall_get_arch()
> On Nov 9, 2018, at 7:27 AM, Alexey Brodkin
> wrote:
>
> Hi Andy,
>
>> On Fri, 2018-11-09 at 07:17 -0800, Andy Lutomirski wrote:
>> On Fri, Nov 9, 2018 at 6:22 AM Alexey Brodkin
>> wrote:
>>> Hi Dmitry,
>>>
On Fri, 2018-11-09 at 06:16 +0300, Dmitry V. Levin wrote:
syscall_get_arch() is required to be implemented on all architectures
that use tracehook_report_syscall_entry() in order to extend
the generic ptrace API with PTRACE_GET_SYSCALL_INFO request.
Signed-off-by: Dmitry V. Levin
---
arch/arc/include/asm/syscall.h | 6 ++
include/uapi/linux/audit.h | 1 +
2 files changed, 7 insertions(+)
>>>
>>> [snip]
>>>
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 818ae690ab79..a7149ceb5b98 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -375,6 +375,7 @@ enum {
#define AUDIT_ARCH_AARCH64
(EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
#define AUDIT_ARCH_ALPHA (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
+#define AUDIT_ARCH_ARC (EM_ARC)
>>>
>>> Similarly here we need to have:
>>> >8-
>>> +#define AUDIT_ARCH_ARC (EM_ARC|EM_ARCV2)
>>> >8-
>>>
>>
>> Huh? How does the bitwise or of two ELF machine codes make any sense?
>
> Oops... I didn't read examples of AUDIT_ARCH_ALPHA above :(
> Indeed that was stupid.
>
> But what would be a proper fix then?
>
> Something like that?
> >8-
> #define AUDIT_ARCH_ARC (EM_ARC)
> #define AUDIT_ARCH_ARCV2 (EM_ARCV2)
>
>
> static inline int syscall_get_arch(void)
> {
> #ifdef __ARC700__
> return AUDIT_ARCH_ARC;
> #else
> return AUDIT_ARCH_ARCV2;
> #endif
> }
> >8-
>
Maybe, but I know basically nothing about ARC. Is the syscall numbering or
calling convention different on ARC vs ARCv2?
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH 06/13] arc: define syscall_get_arch()
Hi Andy,
On Fri, 2018-11-09 at 07:56 -0800, Andy Lutomirski wrote:
> > On Nov 9, 2018, at 7:27 AM, Alexey Brodkin
> > wrote:
> >
> > Hi Andy,
> >
> > > On Fri, 2018-11-09 at 07:17 -0800, Andy Lutomirski wrote:
> > > On Fri, Nov 9, 2018 at 6:22 AM Alexey Brodkin
> > > wrote:
> > > > Hi Dmitry,
> > > >
> > > > > On Fri, 2018-11-09 at 06:16 +0300, Dmitry V. Levin wrote:
> > > > > syscall_get_arch() is required to be implemented on all architectures
> > > > > that use tracehook_report_syscall_entry() in order to extend
> > > > > the generic ptrace API with PTRACE_GET_SYSCALL_INFO request.
> > > > >
> > > > > Signed-off-by: Dmitry V. Levin
> > > > > ---
> > > > > arch/arc/include/asm/syscall.h | 6 ++
> > > > > include/uapi/linux/audit.h | 1 +
> > > > > 2 files changed, 7 insertions(+)
> > > >
> > > > [snip]
> > > >
> > > > > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > > > > index 818ae690ab79..a7149ceb5b98 100644
> > > > > --- a/include/uapi/linux/audit.h
> > > > > +++ b/include/uapi/linux/audit.h
> > > > > @@ -375,6 +375,7 @@ enum {
> > > > >
> > > > > #define AUDIT_ARCH_AARCH64
> > > > > (EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> > > > > #define AUDIT_ARCH_ALPHA
> > > > > (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> > > > > +#define AUDIT_ARCH_ARC (EM_ARC)
> > > >
> > > > Similarly here we need to have:
> > > > >8-
> > > > +#define AUDIT_ARCH_ARC (EM_ARC|EM_ARCV2)
> > > > >8-
> > > >
> > >
> > > Huh? How does the bitwise or of two ELF machine codes make any sense?
> >
> > Oops... I didn't read examples of AUDIT_ARCH_ALPHA above :(
> > Indeed that was stupid.
> >
> > But what would be a proper fix then?
> >
> > Something like that?
> > >8-
> > #define AUDIT_ARCH_ARC (EM_ARC)
> > #define AUDIT_ARCH_ARCV2 (EM_ARCV2)
> >
> >
> > static inline int syscall_get_arch(void)
> > {
> > #ifdef __ARC700__
> > return AUDIT_ARCH_ARC;
> > #else
> > return AUDIT_ARCH_ARCV2;
> > #endif
> > }
> > >8-
> >
>
> Maybe, but I know basically nothing about ARC. Is the syscall numbering or
> calling convention different on ARC vs ARCv2?
Syscall numbering should be the same as we use UAPI for both ARCompact (AKA
ARCv1)
and ARCv2. As for calling convention I think it indeed differs.
Note ARCompact and ARCv2 ISAs are binary incompatible!
Even though assembly look pretty much the same (sans instructions
available only for either ARCompact or ARCv2) encodings are different so
in that sense these are completely different architectures.
Also I'm wondering what could be other cases for use of syscall_get_arch().
So I'd say it's better to report different values for ARC ISAs.
And given we use the same values as in Binutils IMHO it would be good
to not mix IDs here.
-Alexey
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH 06/13] arc: define syscall_get_arch()
Hi Andy,
On Fri, 2018-11-09 at 07:17 -0800, Andy Lutomirski wrote:
> On Fri, Nov 9, 2018 at 6:22 AM Alexey Brodkin
> wrote:
> > Hi Dmitry,
> >
> > On Fri, 2018-11-09 at 06:16 +0300, Dmitry V. Levin wrote:
> > > syscall_get_arch() is required to be implemented on all architectures
> > > that use tracehook_report_syscall_entry() in order to extend
> > > the generic ptrace API with PTRACE_GET_SYSCALL_INFO request.
> > >
> > > Signed-off-by: Dmitry V. Levin
> > > ---
> > > arch/arc/include/asm/syscall.h | 6 ++
> > > include/uapi/linux/audit.h | 1 +
> > > 2 files changed, 7 insertions(+)
> >
> > [snip]
> >
> > > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > > index 818ae690ab79..a7149ceb5b98 100644
> > > --- a/include/uapi/linux/audit.h
> > > +++ b/include/uapi/linux/audit.h
> > > @@ -375,6 +375,7 @@ enum {
> > >
> > > #define AUDIT_ARCH_AARCH64
> > > (EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> > > #define AUDIT_ARCH_ALPHA
> > > (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> > > +#define AUDIT_ARCH_ARC (EM_ARC)
> >
> > Similarly here we need to have:
> > >8-
> > +#define AUDIT_ARCH_ARC (EM_ARC|EM_ARCV2)
> > >8-
> >
>
> Huh? How does the bitwise or of two ELF machine codes make any sense?
Oops... I didn't read examples of AUDIT_ARCH_ALPHA above :(
Indeed that was stupid.
But what would be a proper fix then?
Something like that?
>8-
#define AUDIT_ARCH_ARC (EM_ARC)
#define AUDIT_ARCH_ARCV2 (EM_ARCV2)
static inline int syscall_get_arch(void)
{
#ifdef __ARC700__
return AUDIT_ARCH_ARC;
#else
return AUDIT_ARCH_ARCV2;
#endif
}
>8-
-Alexey
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH 06/13] arc: define syscall_get_arch()
On Fri, Nov 9, 2018 at 6:22 AM Alexey Brodkin
wrote:
>
> Hi Dmitry,
>
> On Fri, 2018-11-09 at 06:16 +0300, Dmitry V. Levin wrote:
> > syscall_get_arch() is required to be implemented on all architectures
> > that use tracehook_report_syscall_entry() in order to extend
> > the generic ptrace API with PTRACE_GET_SYSCALL_INFO request.
> >
> > Signed-off-by: Dmitry V. Levin
> > ---
> > arch/arc/include/asm/syscall.h | 6 ++
> > include/uapi/linux/audit.h | 1 +
> > 2 files changed, 7 insertions(+)
>
> [snip]
>
> > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > index 818ae690ab79..a7149ceb5b98 100644
> > --- a/include/uapi/linux/audit.h
> > +++ b/include/uapi/linux/audit.h
> > @@ -375,6 +375,7 @@ enum {
> >
> > #define AUDIT_ARCH_AARCH64
> > (EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> > #define AUDIT_ARCH_ALPHA (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> > +#define AUDIT_ARCH_ARC (EM_ARC)
>
> Similarly here we need to have:
> >8-
> +#define AUDIT_ARCH_ARC (EM_ARC|EM_ARCV2)
> >8-
>
Huh? How does the bitwise or of two ELF machine codes make any sense?
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH 06/13] arc: define syscall_get_arch()
Hi Dmitry,
On Fri, 2018-11-09 at 06:16 +0300, Dmitry V. Levin wrote:
> syscall_get_arch() is required to be implemented on all architectures
> that use tracehook_report_syscall_entry() in order to extend
> the generic ptrace API with PTRACE_GET_SYSCALL_INFO request.
>
> Signed-off-by: Dmitry V. Levin
> ---
> arch/arc/include/asm/syscall.h | 6 ++
> include/uapi/linux/audit.h | 1 +
> 2 files changed, 7 insertions(+)
[snip]
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 818ae690ab79..a7149ceb5b98 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -375,6 +375,7 @@ enum {
>
> #define AUDIT_ARCH_AARCH64 (EM_AARCH64|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> #define AUDIT_ARCH_ALPHA (EM_ALPHA|__AUDIT_ARCH_64BIT|__AUDIT_ARCH_LE)
> +#define AUDIT_ARCH_ARC (EM_ARC)
Similarly here we need to have:
>8-
+#define AUDIT_ARCH_ARC (EM_ARC|EM_ARCV2)
>8-
-Alexey
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
