From: James Hogan <james.hogan@imgtec.com>
To: Lars Persson <lars.persson@axis.com>
Cc: <ralf@linux-mips.org>, <linux-mips@linux-mips.org>,
Lars Persson <larper@axis.com>
Subject: Re: [PATCH] MIPS: disable page faults in syscall_get_arguments
Date: Fri, 17 Mar 2017 14:59:47 +0000 [thread overview]
Message-ID: <20170317145947.GK2878@jhogan-linux.le.imgtec.org> (raw)
In-Reply-To: <1489744570-16929-1-git-send-email-larper@axis.com>
[-- Attachment #1: Type: text/plain, Size: 2515 bytes --]
On Fri, Mar 17, 2017 at 10:56:10AM +0100, Lars Persson wrote:
> We call a sleeping function in MIPS' get_syscall_arguments to read
> from the userspace stack. Disable page faults around these calls to
> prevent sleep in atomic contexts.
>
> The problem was observed as the following splat with ftrace system
> call tracing enabled:
>
> BUG: sleeping function called from invalid context at arch/mips/include/asm/syscall.h:48
> in_atomic(): 1, irqs_disabled(): 0, pid: 1389, name: sh
> Preemption disabled at:
> [<801d31d4>] __fd_install+0x50/0x194
> CPU: 2 PID: 1389 Comm: sh Not tainted 4.9.14-axis4-devel #6
> Stack : 809d7bfa 0000003b 00000000 00000000 00000000 00000000 800a5438 808c3507
> 8e77599c 0000056d 00000002 809c4f44 8fc060e0 8dc07f68 00000005 800a5468
> 00000005 80403b30 00000000 00000000 807f5b64 8dc07e0c 8dc07df4 8014a36c
> 00000000 8004927c 809d7bf8 00000024 8dc07e0c 00000000 00000002 807eaf58
> 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> ...
> Call Trace:
> [<80021a68>] show_stack+0x94/0xb0
> [<803f4e38>] dump_stack+0x98/0xd0
> [<80074838>] ___might_sleep+0x160/0x1e4
> [<801205f8>] ftrace_syscall_enter+0x1c8/0x3ac
> [<8001f77c>] syscall_trace_enter+0x18c/0x1f8
> [<8002b3f8>] syscall_trace_entry+0x44/0x94
Hmm, tricky. If for some reason the stack page was swapped out before
syscall_get_arguments() was called, then by disabling page faults the
args would be left uninitialised, even when not called from IRQ disabled
context...
We already copy the args to the kernel stack in handle_sys (with IRQs
enabled), but thats as arguments to the syscall handler function and I
suppose there's nothing to stop it overwriting those arguments.
Cheers
James
>
> Signed-off-by: Lars Persson <larper@axis.com>
> ---
> arch/mips/include/asm/syscall.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/mips/include/asm/syscall.h b/arch/mips/include/asm/syscall.h
> index d878825..270a6d5 100644
> --- a/arch/mips/include/asm/syscall.h
> +++ b/arch/mips/include/asm/syscall.h
> @@ -104,8 +104,10 @@ static inline void syscall_get_arguments(struct task_struct *task,
> (regs->regs[2] == __NR_syscall))
> i++;
>
> + pagefault_disable();
> while (n--)
> ret |= mips_get_syscall_arg(args++, task, regs, i++);
> + pagefault_enable();
>
> /*
> * No way to communicate an error because this is a void function.
> --
> 2.1.4
>
>
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
WARNING: multiple messages have this Message-ID (diff)
From: James Hogan <james.hogan@imgtec.com>
To: Lars Persson <lars.persson@axis.com>
Cc: ralf@linux-mips.org, linux-mips@linux-mips.org,
Lars Persson <larper@axis.com>
Subject: Re: [PATCH] MIPS: disable page faults in syscall_get_arguments
Date: Fri, 17 Mar 2017 14:59:47 +0000 [thread overview]
Message-ID: <20170317145947.GK2878@jhogan-linux.le.imgtec.org> (raw)
Message-ID: <20170317145947.76VZzNi2SYalTVl1oosk_g2nURvCtR0Mv4FSblHh4h4@z> (raw)
In-Reply-To: <1489744570-16929-1-git-send-email-larper@axis.com>
[-- Attachment #1: Type: text/plain, Size: 2515 bytes --]
On Fri, Mar 17, 2017 at 10:56:10AM +0100, Lars Persson wrote:
> We call a sleeping function in MIPS' get_syscall_arguments to read
> from the userspace stack. Disable page faults around these calls to
> prevent sleep in atomic contexts.
>
> The problem was observed as the following splat with ftrace system
> call tracing enabled:
>
> BUG: sleeping function called from invalid context at arch/mips/include/asm/syscall.h:48
> in_atomic(): 1, irqs_disabled(): 0, pid: 1389, name: sh
> Preemption disabled at:
> [<801d31d4>] __fd_install+0x50/0x194
> CPU: 2 PID: 1389 Comm: sh Not tainted 4.9.14-axis4-devel #6
> Stack : 809d7bfa 0000003b 00000000 00000000 00000000 00000000 800a5438 808c3507
> 8e77599c 0000056d 00000002 809c4f44 8fc060e0 8dc07f68 00000005 800a5468
> 00000005 80403b30 00000000 00000000 807f5b64 8dc07e0c 8dc07df4 8014a36c
> 00000000 8004927c 809d7bf8 00000024 8dc07e0c 00000000 00000002 807eaf58
> 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> ...
> Call Trace:
> [<80021a68>] show_stack+0x94/0xb0
> [<803f4e38>] dump_stack+0x98/0xd0
> [<80074838>] ___might_sleep+0x160/0x1e4
> [<801205f8>] ftrace_syscall_enter+0x1c8/0x3ac
> [<8001f77c>] syscall_trace_enter+0x18c/0x1f8
> [<8002b3f8>] syscall_trace_entry+0x44/0x94
Hmm, tricky. If for some reason the stack page was swapped out before
syscall_get_arguments() was called, then by disabling page faults the
args would be left uninitialised, even when not called from IRQ disabled
context...
We already copy the args to the kernel stack in handle_sys (with IRQs
enabled), but thats as arguments to the syscall handler function and I
suppose there's nothing to stop it overwriting those arguments.
Cheers
James
>
> Signed-off-by: Lars Persson <larper@axis.com>
> ---
> arch/mips/include/asm/syscall.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/mips/include/asm/syscall.h b/arch/mips/include/asm/syscall.h
> index d878825..270a6d5 100644
> --- a/arch/mips/include/asm/syscall.h
> +++ b/arch/mips/include/asm/syscall.h
> @@ -104,8 +104,10 @@ static inline void syscall_get_arguments(struct task_struct *task,
> (regs->regs[2] == __NR_syscall))
> i++;
>
> + pagefault_disable();
> while (n--)
> ret |= mips_get_syscall_arg(args++, task, regs, i++);
> + pagefault_enable();
>
> /*
> * No way to communicate an error because this is a void function.
> --
> 2.1.4
>
>
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
next prev parent reply other threads:[~2017-03-17 15:00 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-17 9:56 [PATCH] MIPS: disable page faults in syscall_get_arguments Lars Persson
2017-03-17 14:59 ` James Hogan [this message]
2017-03-17 14:59 ` James Hogan
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=20170317145947.GK2878@jhogan-linux.le.imgtec.org \
--to=james.hogan@imgtec.com \
--cc=larper@axis.com \
--cc=lars.persson@axis.com \
--cc=linux-mips@linux-mips.org \
--cc=ralf@linux-mips.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