Re: [PATCH 5/9] fs: remove various compat readv/writev helpers
On Wed, Sep 23, 2020 at 12:39 PM Al Viro wrote: > > On Wed, Sep 23, 2020 at 03:59:01PM +0100, Al Viro wrote: > > > > That's a very good question. But it does not just compile but actually > > > works. Probably because all the syscall wrappers mean that we don't > > > actually generate the normal names. I just tried this: > > > > > > --- a/include/linux/syscalls.h > > > +++ b/include/linux/syscalls.h > > > @@ -468,7 +468,7 @@ asmlinkage long sys_lseek(unsigned int fd, off_t > > > offset, > > > asmlinkage long sys_read(unsigned int fd, char __user *buf, size_t > > > count); > > > asmlinkage long sys_write(unsigned int fd, const char __user *buf, > > > size_t count); > > > -asmlinkage long sys_readv(unsigned long fd, > > > +asmlinkage long sys_readv(void *fd, > > > > > > for fun, and the compiler doesn't care either.. > > > > Try to build it for sparc or ppc... > > FWIW, declarations in syscalls.h used to serve 4 purposes: > 1) syscall table initializers needed symbols declared > 2) direct calls needed the same > 3) catching mismatches between the declarations and definitions > 4) centralized list of all syscalls > > (2) has been (thankfully) reduced for some time; in any case, ksys_... is > used for the remaining ones. > > (1) and (3) are served by syscalls.h in architectures other than x86, arm64 > and s390. On those 3 (1) is done otherwise (near the syscall table > initializer) > and (3) is not done at all. > > I wonder if we should do something like > > SYSCALL_DECLARE3(readv, unsigned long, fd, const struct iovec __user *, vec, > unsigned long, vlen); > in syscalls.h instead, and not under that ifdef. > > Let it expand to declaration of sys_...() in generic case and, on x86, into > __do_sys_...() and __ia32_sys_...()/__x64_sys_...(), with types matching > what SYSCALL_DEFINE ends up using. > > Similar macro would cover compat_sys_...() declarations. That would > restore mismatch checking for x86 and friends. AFAICS, the cost wouldn't > be terribly high - cpp would have more to chew through in syscalls.h, > but it shouldn't be all that costly. Famous last words, of course... > > Does anybody see fundamental problems with that? I think this would be a good idea. I have been working on a patchset to clean up the conditional syscall handling (sys_ni.c), and conflicts with the prototypes in syscalls.h have been getting in the way. Having the prototypes use SYSCALL_DECLAREx(...) would solve that issue. -- Brian Gerst
Re: [PATCH 2/6] exec: simplify the compat syscall handling
On Mon, Jun 15, 2020 at 2:47 PM Arnd Bergmann wrote: > > On Mon, Jun 15, 2020 at 4:48 PM Brian Gerst wrote: > > On Mon, Jun 15, 2020 at 10:13 AM Christoph Hellwig wrote: > > > On Mon, Jun 15, 2020 at 03:31:35PM +0200, Arnd Bergmann wrote: > > > > > > > I'd rather keep it in common code as that allows all the low-level > > > exec stuff to be marked static, and avoid us growing new pointless > > > compat variants through copy and paste. > > > smart compiler to d > > > > > > > I don't really understand > > > > the comment, why can't this just use this? > > > > > > That errors out with: > > > > > > ld: arch/x86/entry/syscall_x32.o:(.rodata+0x1040): undefined reference to > > > `__x32_sys_execve' > > > ld: arch/x86/entry/syscall_x32.o:(.rodata+0x1108): undefined reference to > > > `__x32_sys_execveat' > > > make: *** [Makefile:1139: vmlinux] Error 1 > > > > I think I have a fix for this, by modifying the syscall wrappers to > > add an alias for the __x32 variant to the native __x64_sys_foo(). > > I'll get back to you with a patch. > > Do we actually need the __x32 prefix any more, or could we just > change all x32 specific calls to use __x64_compat_sys_foo()? I suppose that would work too. The prefix really describes the register mapping. -- Brian Gerst
Re: [PATCH 2/6] exec: simplify the compat syscall handling
On Mon, Jun 15, 2020 at 11:10 AM Christoph Hellwig wrote: > > On Mon, Jun 15, 2020 at 04:46:15PM +0200, Arnd Bergmann wrote: > > How about this one: > > > > diff --git a/arch/x86/entry/syscall_x32.c b/arch/x86/entry/syscall_x32.c > > index 3d8d70d3896c..0ce15807cf54 100644 > > --- a/arch/x86/entry/syscall_x32.c > > +++ b/arch/x86/entry/syscall_x32.c > > @@ -16,6 +16,9 @@ > > #undef __SYSCALL_X32 > > #undef __SYSCALL_COMMON > > > > +#define __x32_sys_execve __x64_sys_execve > > +#define __x32_sys_execveat __x64_sys_execveat > > + > > > arch/x86/entry/syscall_x32.c:19:26: error: ‘__x64_sys_execve’ undeclared here > (not in a function); did you mean ‘__x32_sys_execve’? >19 | #define __x32_sys_execve __x64_sys_execve > | ^~~~ > arch/x86/entry/syscall_x32.c:22:39: note: in expansion of macro > ‘__x32_sys_execve’ >22 | #define __SYSCALL_X32(nr, sym) [nr] = __x32_##sym, > | ^~ > ./arch/x86/include/generated/asm/syscalls_64.h:344:1: note: in expansion of > macro ‘__SYSCALL_X32’ > 344 | __SYSCALL_X32(520, sys_execve) > | ^ > arch/x86/entry/syscall_x32.c:20:28: error: ‘__x64_sys_execveat’ undeclared > here (not in a function); did you mean ‘__x32_sys_execveat’? >20 | #define __x32_sys_execveat __x64_sys_execveat > |^~ > arch/x86/entry/syscall_x32.c:22:39: note: in expansion of macro > ‘__x32_sys_execveat’ >22 | #define __SYSCALL_X32(nr, sym) [nr] = __x32_##sym, > | ^~ > ./arch/x86/include/generated/asm/syscalls_64.h:369:1: note: in expansion of > macro ‘__SYSCALL_X32’ > 369 | __SYSCALL_X32(545, sys_execveat) > | ^ > make[2]: *** [scripts/Makefile.build:281: arch/x86/entry/syscall_x32.o] Error > 1 > make[1]: *** [scripts/Makefile.build:497: arch/x86/entry] Error 2 > make[1]: *** Waiting for unfinished jobs > kernel/exit.o: warning: objtool: __x64_sys_exit_group()+0x14: unreachable > instruction > make: *** [Makefile:1764: arch/x86] Error 2 > make: *** Waiting for unfinished jobs If you move those aliases above all the __SYSCALL_* defines it will work, since that will get the forward declaration too. This would be the simplest workaround. -- Brian Gerst
Re: [PATCH 2/6] exec: simplify the compat syscall handling
On Mon, Jun 15, 2020 at 10:13 AM Christoph Hellwig wrote:
>
> On Mon, Jun 15, 2020 at 03:31:35PM +0200, Arnd Bergmann wrote:
> > > #ifdef CONFIG_COMPAT
> > > - if (unlikely(argv.is_compat)) {
> > > + if (in_compat_syscall()) {
> > > + const compat_uptr_t __user *compat_argv =
> > > + compat_ptr((unsigned long)argv);
> > > compat_uptr_t compat;
> > >
> > > - if (get_user(compat, argv.ptr.compat + nr))
> > > + if (get_user(compat, compat_argv + nr))
> > > return ERR_PTR(-EFAULT);
> > >
> > > return compat_ptr(compat);
> > > }
> > > #endif
> >
> > I would expect that the "#ifdef CONFIG_COMPAT" can be removed
> > now, since compat_ptr() and in_compat_syscall() are now defined
> > unconditionally. I have not tried that though.
>
> True, I'll give it a spin.
>
> > > +/*
> > > + * x32 syscalls are listed in the same table as x86_64 ones, so we need
> > > to
> > > + * define compat syscalls that are exactly the same as the native
> > > version for
> > > + * the syscall table machinery to work. Sigh..
> > > + */
> > > +#ifdef CONFIG_X86_X32
> > > COMPAT_SYSCALL_DEFINE3(execve, const char __user *, filename,
> > > - const compat_uptr_t __user *, argv,
> > > - const compat_uptr_t __user *, envp)
> > > + const char __user *const __user *, argv,
> > > + const char __user *const __user *, envp)
> > > {
> > > - return do_compat_execve(AT_FDCWD, getname(filename), argv, envp,
> > > 0);
> > > + return do_execveat(AT_FDCWD, getname(filename), argv, envp, 0,
> > > NULL);
> > > }
> >
> > Maybe move it to arch/x86/kernel/process_64.c or
> > arch/x86/entry/syscall_x32.c
> > to keep it out of the common code if this is needed.
>
> I'd rather keep it in common code as that allows all the low-level
> exec stuff to be marked static, and avoid us growing new pointless
> compat variants through copy and paste.
> smart compiler to d
>
> > I don't really understand
> > the comment, why can't this just use this?
>
> That errors out with:
>
> ld: arch/x86/entry/syscall_x32.o:(.rodata+0x1040): undefined reference to
> `__x32_sys_execve'
> ld: arch/x86/entry/syscall_x32.o:(.rodata+0x1108): undefined reference to
> `__x32_sys_execveat'
> make: *** [Makefile:1139: vmlinux] Error 1
I think I have a fix for this, by modifying the syscall wrappers to
add an alias for the __x32 variant to the native __x64_sys_foo().
I'll get back to you with a patch.
--
Brian Gerst
Re: [PATCH v5 18/32] signal: Consolidate {TS, TLF}_RESTORE_SIGMASK code
On Mon, Jul 11, 2016 at 4:53 PM, Andy Lutomirski <l...@kernel.org> wrote:
> In general, there's no need for the "restore sigmask" flag to live in
> ti->flags. alpha, ia64, microblaze, powerpc, sh, sparc (64-bit only),
> tile, and x86 use essentially identical alternative implementations,
> placing the flag in ti->status.
>
> Replace those optimized implementations with an equally good common
> implementation that stores it in a bitfield in struct task_struct
> and drop the custom implementations.
>
> Additional architectures can opt in by removing their
> TIF_RESTORE_SIGMASK defines.
There is a small typo in the subject, should be "signal: Consolidate
{TS,TIF}_RESTORE_SIGMASK code"
--
Brian Gerst
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH v2 03/18] x86/asm/head: standardize the bottom of the stack for idle tasks
On Thu, Apr 28, 2016 at 4:44 PM, Josh Poimboeuf <jpoim...@redhat.com> wrote: > Thanks to all the recent x86 entry code refactoring, most tasks' kernel > stacks start at the same offset right above their saved pt_regs, > regardless of which syscall was used to enter the kernel. That creates > a nice convention which makes it straightforward to identify the > "bottom" of the stack, which can be useful for stack walking code which > needs to verify the stack is sane. > > However there are still a few types of tasks which don't yet follow that > convention: > > 1) CPU idle tasks, aka the "swapper" tasks > > 2) freshly forked TIF_FORK tasks which don't have a stack at all > > Make the idle tasks conform to the new stack bottom convention by > starting their stack at a sizeof(pt_regs) offset from the end of the > stack page. > > Signed-off-by: Josh Poimboeuf <jpoim...@redhat.com> > --- > arch/x86/kernel/head_64.S | 7 --- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S > index 6dbd2c0..0b12311 100644 > --- a/arch/x86/kernel/head_64.S > +++ b/arch/x86/kernel/head_64.S > @@ -296,8 +296,9 @@ ENTRY(start_cpu) > * REX.W + FF /5 JMP m16:64 Jump far, absolute indirect, > * address given in m16:64. > */ > - movqinitial_code(%rip),%rax > - pushq $0 # fake return address to stop unwinder > + call1f # put return address on stack for unwinder > +1: xorq%rbp, %rbp # clear frame pointer > + movqinitial_code(%rip), %rax > pushq $__KERNEL_CS# set correct cs > pushq %rax# target address in negative space > lretq This chunk looks like it should be a separate patch. -- Brian Gerst ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
