linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Marcin Nowakowski <marcin.nowakowski@imgtec.com>
To: Andy Lutomirski <luto@amacapital.net>
Cc: Linux API <linux-api@vger.kernel.org>,
	Ingo Molnar <mingo@redhat.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	open list <linux-kernel@vger.kernel.org>,
	<linux-mips@linux-mips.org>
Subject: Re: [PATCH 1/2] tracing/syscalls: allow multiple syscall numbers per syscall
Date: Tue, 30 Aug 2016 10:14:34 +0200	[thread overview]
Message-ID: <f40020e1-200c-f113-5174-5fe4d4c000dc@imgtec.com> (raw)
In-Reply-To: <CALCETrW1m9ozck-ugX6AKnL7oNA8rvMTjhFGqtVSvKL9BMXMZA@mail.gmail.com>



On 30.08.2016 01:55, Andy Lutomirski wrote:
> On Aug 29, 2016 11:30 AM, "Marcin Nowakowski"
> <marcin.nowakowski@imgtec.com> wrote:
>>
>> Syscall metadata makes an assumption that only a single syscall number
>> corresponds to a given method. This is true for most archs, but
>> can break tracing otherwise.
>>
>> For MIPS platforms, depending on the choice of supported ABIs, up to 3
>> system call numbers can correspond to the same call - depending on which
>> ABI the userspace app uses.
>
> MIPS isn't special here.  x86 does the same thing.  Why isn't this a
> problem on x86?
>

Hi Andy,

My understanding is that MIPS is quite different to what most other 
architectures do ...
First of all x86 disables tracing of compat syscalls as that didn't work 
properly because of wrong mapping of syscall numbers to syscalls:
http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=f431b634f

Moreover, when trace_syscalls is initialised, the syscall metadata is 
updated to include the right syscall numbers. That uses 
arch_syscall_addr method, which has a default implementation in 
kernel/trace/trace_syscalls.c:

unsigned long __init __weak arch_syscall_addr(int nr)
{
         return (unsigned long)sys_call_table[nr];
}

that works for x86 and only uses 'native' syscalls, ie. for x86_64 will 
not map any of the ia32_sys_call_table entries. So on one hand we have 
the code that disables tracing for x86_64 compat, on the other we only 
ensure that the native calls are mapped.
It is quite different for MIPS where syscall numbers for different ABIs 
have distinct call numbers, so the following code maps the syscalls
(for O32 -> 4xxx, N64 -> 5xxx, N32 -> 6xxx):

unsigned long __init arch_syscall_addr(int nr)
{
         if (nr >= __NR_N32_Linux && nr <= __NR_N32_Linux + 
__NR_N32_Linux_syscalls)
                 return (unsigned long)sysn32_call_table[nr - 
__NR_N32_Linux];
         if (nr >= __NR_64_Linux  && nr <= __NR_64_Linux + 
__NR_64_Linux_syscalls)
                 return (unsigned long)sys_call_table[nr - __NR_64_Linux];
         if (nr >= __NR_O32_Linux && nr <= __NR_O32_Linux + 
__NR_O32_Linux_syscalls)
                 return (unsigned long)sys32_call_table[nr - 
__NR_O32_Linux];
         return (unsigned long) &sys_ni_syscall;
}

As a result when init_ftrace_syscalls() loops through all the possible 
syscall numbers,  it first finds an O32 implementation, then N64 and 
finally N32. As the current code doesn't expect multiple references to a 
given syscall number, it always overrides the metadata with the last 
found - as a result only N32 syscalls are mapped.
This is generally unexpected and wrong behaviour, and to makes things 
worse - since when N32 support is enabled, it overwrites N64 entries, it 
becomes impossible to trace native syscalls.

 > Also, you seem to be partially reinventing AUDIT_ARCH here.  Can you
 > use that and integrate with syscall_get_arch()?

Please correct me if I don't understand what you meant here, but I don't 
see how these can be integrated ...
For MIPS syscall_get_arch() properly determines arch type and calling 
convention, but that information is not enough to determine what call 
was made and how to map it to syscall metadata from another calling 
convention.

Marcin

  reply	other threads:[~2016-08-30  8:14 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-29  9:30 [PATCH 1/2] tracing/syscalls: allow multiple syscall numbers per syscall Marcin Nowakowski
2016-08-29  9:30 ` [PATCH 2/2] MIPS: set NR_syscall_tables appropriately Marcin Nowakowski
2016-09-27 12:04   ` Ralf Baechle
2016-09-28  6:58     ` Marcin Nowakowski
2016-08-29 23:55 ` [PATCH 1/2] tracing/syscalls: allow multiple syscall numbers per syscall Andy Lutomirski
2016-08-30  8:14   ` Marcin Nowakowski [this message]
2016-08-30 18:52     ` Andy Lutomirski
2016-08-30 19:29       ` Steven Rostedt
2016-08-30 19:53         ` Andy Lutomirski
2016-08-30 20:58           ` Steven Rostedt
2016-08-30 21:45             ` Andy Lutomirski
2016-08-30 22:03               ` Steven Rostedt
2016-08-30 22:08                 ` Andy Lutomirski
2016-08-30 22:30                   ` Steven Rostedt
2016-08-30 23:09                     ` Andy Lutomirski
2016-08-30 23:28                       ` Steven Rostedt
2016-08-31  0:01                         ` Andy Lutomirski
2016-08-31 14:08                           ` Marcin Nowakowski
2016-08-31  7:00                         ` Marcin Nowakowski
2016-08-31  8:24           ` Arnd Bergmann
2016-09-01 15:24             ` Steven Rostedt

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=f40020e1-200c-f113-5174-5fe4d4c000dc@imgtec.com \
    --to=marcin.nowakowski@imgtec.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@linux-mips.org \
    --cc=luto@amacapital.net \
    --cc=mingo@redhat.com \
    --cc=rostedt@goodmis.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).