Re: [PATCH v2 16/15] syscall_get_arch: add "struct task_struct *" argument
"Dmitry V. Levin" writes:
> diff --git a/arch/powerpc/include/asm/syscall.h
> b/arch/powerpc/include/asm/syscall.h
> index ab9f3f0a8637..d88b34179118 100644
> --- a/arch/powerpc/include/asm/syscall.h
> +++ b/arch/powerpc/include/asm/syscall.h
> @@ -100,9 +100,15 @@ static inline void syscall_set_arguments(struct
> task_struct *task,
> regs->orig_gpr3 = args[0];
> }
>
> -static inline int syscall_get_arch(void)
> +static inline int syscall_get_arch(struct task_struct *task)
> {
> - int arch = is_32bit_task() ? AUDIT_ARCH_PPC : AUDIT_ARCH_PPC64;
> + int arch;
> +
> + if (IS_ENABLED(CONFIG_PPC64) && !test_tsk_thread_flag(task, TIF_32BIT))
> + arch = AUDIT_ARCH_PPC64;
> + else
> + arch = AUDIT_ARCH_PPC;
> +
> #ifdef __LITTLE_ENDIAN__
> arch |= __AUDIT_ARCH_LE;
> #endif
LGTM.
Acked-by: Michael Ellerman (powerpc)
cheers
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2 16/15] syscall_get_arch: add "struct task_struct *" argument
Hi Paul,
On Wed, Nov 21, 2018 at 06:40:06PM +, Paul Burton wrote:
> Hi Dmitry,
>
> On Wed, Nov 21, 2018 at 03:44:22AM +0300, Dmitry V. Levin wrote:
> > This argument is required to extend the generic ptrace API
> > with PTRACE_GET_SYSCALL_INFO request: syscall_get_arch() is going to be
> > called from ptrace_request() along with other syscall_get_* functions
> > with a tracee as their argument.
> >
> > This change partially reverts commit 5e937a9ae913 ("syscall_get_arch:
> > remove useless function arguments").
> >
> >%
> >
> > diff --git a/arch/mips/include/asm/syscall.h
> > b/arch/mips/include/asm/syscall.h
> > index 0170602a1e4e..52b633f20abd 100644
> > --- a/arch/mips/include/asm/syscall.h
> > +++ b/arch/mips/include/asm/syscall.h
> > @@ -73,7 +73,7 @@ static inline unsigned long mips_get_syscall_arg(unsigned
> > long *arg,
> > #ifdef CONFIG_64BIT
> > case 4: case 5: case 6: case 7:
> > #ifdef CONFIG_MIPS32_O32
> > - if (test_thread_flag(TIF_32BIT_REGS))
> > + if (test_ti_thread_flag(task_thread_info(task), TIF_32BIT_REGS))
> > return get_user(*arg, (int *)usp + n);
> > else
> > #endif
>
> This ought to be test_tsk_thread_flag(task, TIF_32BIT_REGS) instead of
> open-coding test_tsk_thread_flag.
This will be corrected, thanks for letting me know.
> More fundamentally though, this change doesn't seem to be (directly)
> related to the change you describe in the commit message - it's not
> syscall_get_arch being modified here. I suspect this should be a
> separate commit, or if not please explain in the commit message why this
> change is included.
Good point, this is a fix that should not have been included into this commit.
The bug was found while preparing the syscall_get_arch change, and this
hunk just slipped in. I'll send it as a separate commit.
> Compounding the lack of clarity is the fact that I only received this
> patch, not the whole series, so I can't view the change in the context
> of the rest of the series.
>
> > @@ -140,14 +140,14 @@ extern const unsigned long sys_call_table[];
> > extern const unsigned long sys32_call_table[];
> > extern const unsigned long sysn32_call_table[];
> >
> > -static inline int syscall_get_arch(void)
> > +static inline int syscall_get_arch(struct task_struct *task)
> > {
> > int arch = AUDIT_ARCH_MIPS;
> > #ifdef CONFIG_64BIT
> > - if (!test_thread_flag(TIF_32BIT_REGS)) {
> > + if (!test_ti_thread_flag(task_thread_info(task), TIF_32BIT_REGS)) {
> > arch |= __AUDIT_ARCH_64BIT;
> > /* N32 sets only TIF_32BIT_ADDR */
> > - if (test_thread_flag(TIF_32BIT_ADDR))
> > + if (test_ti_thread_flag(task_thread_info(task), TIF_32BIT_ADDR))
> > arch |= __AUDIT_ARCH_CONVENTION_MIPS64_N32;
> > }
> > #endif
>
> This does seem like the described change, but there are 2 more instances
> of open-coding test_tsk_thread_flag which ought to be cleaned up.
This will be cleaned up, thanks for letting me know.
--
ldv
signature.asc
Description: PGP signature
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2 16/15] syscall_get_arch: add "struct task_struct *" argument
Hi Dmitry,
On Wed, Nov 21, 2018 at 03:44:22AM +0300, Dmitry V. Levin wrote:
> This argument is required to extend the generic ptrace API
> with PTRACE_GET_SYSCALL_INFO request: syscall_get_arch() is going to be
> called from ptrace_request() along with other syscall_get_* functions
> with a tracee as their argument.
>
> This change partially reverts commit 5e937a9ae913 ("syscall_get_arch:
> remove useless function arguments").
>
>%
>
> diff --git a/arch/mips/include/asm/syscall.h b/arch/mips/include/asm/syscall.h
> index 0170602a1e4e..52b633f20abd 100644
> --- a/arch/mips/include/asm/syscall.h
> +++ b/arch/mips/include/asm/syscall.h
> @@ -73,7 +73,7 @@ static inline unsigned long mips_get_syscall_arg(unsigned
> long *arg,
> #ifdef CONFIG_64BIT
> case 4: case 5: case 6: case 7:
> #ifdef CONFIG_MIPS32_O32
> - if (test_thread_flag(TIF_32BIT_REGS))
> + if (test_ti_thread_flag(task_thread_info(task), TIF_32BIT_REGS))
> return get_user(*arg, (int *)usp + n);
> else
> #endif
This ought to be test_tsk_thread_flag(task, TIF_32BIT_REGS) instead of
open-coding test_tsk_thread_flag.
More fundamentally though, this change doesn't seem to be (directly)
related to the change you describe in the commit message - it's not
syscall_get_arch being modified here. I suspect this should be a
separate commit, or if not please explain in the commit message why this
change is included.
Compounding the lack of clarity is the fact that I only received this
patch, not the whole series, so I can't view the change in the context
of the rest of the series.
> @@ -140,14 +140,14 @@ extern const unsigned long sys_call_table[];
> extern const unsigned long sys32_call_table[];
> extern const unsigned long sysn32_call_table[];
>
> -static inline int syscall_get_arch(void)
> +static inline int syscall_get_arch(struct task_struct *task)
> {
> int arch = AUDIT_ARCH_MIPS;
> #ifdef CONFIG_64BIT
> - if (!test_thread_flag(TIF_32BIT_REGS)) {
> + if (!test_ti_thread_flag(task_thread_info(task), TIF_32BIT_REGS)) {
> arch |= __AUDIT_ARCH_64BIT;
> /* N32 sets only TIF_32BIT_ADDR */
> - if (test_thread_flag(TIF_32BIT_ADDR))
> + if (test_ti_thread_flag(task_thread_info(task), TIF_32BIT_ADDR))
> arch |= __AUDIT_ARCH_CONVENTION_MIPS64_N32;
> }
> #endif
This does seem like the described change, but there are 2 more instances
of open-coding test_tsk_thread_flag which ought to be cleaned up.
Thanks,
Paul
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2 16/15] syscall_get_arch: add "struct task_struct *" argument
On Tue, 20 Nov 2018 16:44:22 PST (-0800), l...@altlinux.org wrote:
This argument is required to extend the generic ptrace API
with PTRACE_GET_SYSCALL_INFO request: syscall_get_arch() is going to be
called from ptrace_request() along with other syscall_get_* functions
with a tracee as their argument.
This change partially reverts commit 5e937a9ae913 ("syscall_get_arch:
remove useless function arguments").
Cc: linux-al...@vger.kernel.org
Cc: linux-a...@vger.kernel.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-au...@redhat.com
Cc: linux-c6x-...@linux-c6x.org
Cc: linux-hexa...@vger.kernel.org
Cc: linux-i...@vger.kernel.org
Cc: linux-m...@lists.linux-m68k.org
Cc: linux-m...@linux-mips.org
Cc: linux-par...@vger.kernel.org
Cc: linux-ri...@lists.infradead.org
Cc: linux-s...@vger.kernel.org
Cc: linux...@vger.kernel.org
Cc: linux-snps-arc@lists.infradead.org
Cc: linux...@lists.infradead.org
Cc: linux-xte...@linux-xtensa.org
Cc: linuxppc-...@lists.ozlabs.org
Cc: nios2-...@lists.rocketboards.org
Cc: openr...@lists.librecores.org
Cc: sparcli...@vger.kernel.org
Cc: uclinux-h8-de...@lists.sourceforge.jp
Cc: x...@kernel.org
Signed-off-by: Dmitry V. Levin
Reviewed-by: Palmer Dabbelt
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH v2 16/15] syscall_get_arch: add "struct task_struct *" argument
On Tue, Nov 20, 2018 at 4:44 PM Dmitry V. Levin wrote:
>
> This argument is required to extend the generic ptrace API
> with PTRACE_GET_SYSCALL_INFO request: syscall_get_arch() is going to be
> called from ptrace_request() along with other syscall_get_* functions
> with a tracee as their argument.
>
> This change partially reverts commit 5e937a9ae913 ("syscall_get_arch:
> remove useless function arguments").
>
Reviewed-by: Andy Lutomirski # for x86
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
