CC Steven
On Fri, Mar 29, 2019 at 11:04 PM Dmitry V. Levin <ldv@xxxxxxxxxxxx> wrote:
On Wed, Dec 12, 2018 at 11:55:16AM +0300, Dmitry V. Levin wrote:
On Mon, Dec 10, 2018 at 04:30:25PM +0300, Dmitry V. Levin wrote:
On Mon, Dec 10, 2018 at 02:06:28PM +0100, Geert Uytterhoeven wrote:
On Mon, Dec 10, 2018 at 1:41 PM Dmitry V. Levin <ldv@xxxxxxxxxxxx> wrote:
On Mon, Dec 10, 2018 at 09:45:42AM +0100, Geert Uytterhoeven wrote:
On Mon, Dec 10, 2018 at 5:30 AM Dmitry V. Levin <ldv@xxxxxxxxxxxx> wrote:
syscall_get_* functions are required to be implemented on all
architectures in order to extend the generic ptrace API with
PTRACE_GET_SYSCALL_INFO request.
This introduces asm/syscall.h on m68k implementing all 5 syscall_get_*
functions as documented in asm-generic/syscall.h: syscall_get_nr,
syscall_get_arguments, syscall_get_error, syscall_get_return_value,
and syscall_get_arch.
Cc: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
Cc: Oleg Nesterov <oleg@xxxxxxxxxx>
Cc: Andy Lutomirski <luto@xxxxxxxxxx>
Cc: Elvira Khabirova <lineprinter@xxxxxxxxxxxx>
Cc: Eugene Syromyatnikov <esyr@xxxxxxxxxx>
Cc: linux-m68k@xxxxxxxxxxxxxxxxxxxx
Signed-off-by: Dmitry V. Levin <ldv@xxxxxxxxxxxx>
---
Notes:
v5: added syscall_get_nr, syscall_get_arguments, syscall_get_error,
and syscall_get_return_value
v1: added syscall_get_arch
--- /dev/null
+++ b/arch/m68k/include/asm/syscall.h
@@ -0,0 +1,39 @@
+static inline void
+syscall_get_arguments(struct task_struct *task, struct pt_regs *regs,
+ unsigned int i, unsigned int n, unsigned long *args)
+{
+ BUG_ON(i + n > 6);
Does this have to crash the kernel?
This is what most of other architectures do, but we could choose
a softer approach, e.g. use WARN_ON_ONCE instead.
Perhaps you can return an error code instead?
That would be problematic given the signature of this function
and the nature of the potential bug which would most likely be a usage error.
Of course to handle that, the function's signature need to be changed.
Changing it has the advantage that the error handling can be done at the
caller, in common code, instead of duplicating it for all
architectures, possibly
leading to different semantics.
Given that *all* current users of syscall_get_arguments specify i == 0
(and there is an architecture that has BUG_ON(i)),
it should be really a usage error to get into situation where i + n > 6,
I wish a BUILD_BUG_ON could be used here instead.
I don't think it worths pushing the change of API just to convert
a "cannot happen" assertion into an error that would have to be dealt with
on the caller side.
I suggest the following BUG_ON replacement for syscall_get_arguments:
#define SYSCALL_MAX_ARGS 6
static inline void
syscall_get_arguments(struct task_struct *task, struct pt_regs *regs,
unsigned int i, unsigned int n, unsigned long *args)
{
/*
* Ideally there should have been
* BUILD_BUG_ON(i + n > SYSCALL_MAX_ARGS);
* instead of these checks.
*/
if (unlikely(i > SYSCALL_MAX_ARGS)) {
WARN_ONCE(1, "i > SYSCALL_MAX_ARGS");
return;
}
if (unlikely(n > SYSCALL_MAX_ARGS - i)) {
WARN_ONCE(1, "i + n > SYSCALL_MAX_ARGS");
n = SYSCALL_MAX_ARGS - i;
}
BUILD_BUG_ON(sizeof(regs->d1) != sizeof(args[0]));
memcpy(args, ®s->d1 + i, n * sizeof(args[0]));
}
There seems to be a more straightforward approach to this issue.
Assuming there is a general consensus [1] to get rid of "i" and "n"
arguments of syscall_get_arguments(), the implementation could be
simplified to
static inline void
syscall_get_arguments(struct task_struct *task, struct pt_regs *regs,
unsigned long *args)
{
memcpy(args, ®s->d1, 6 * sizeof(args[0]));
}
[1] https://lore.kernel.org/lkml/20190328230512.486297455@xxxxxxxxxxx/
Yeah, no longer a need for all these ugly checks, good.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
