* Re: [PATCH v9 00/24] ILP32 for ARM64
From: Arnd Bergmann @ 2018-10-14 19:49 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Yury Norov, Catalin Marinas, Linux ARM, Linux Kernel Mailing List,
open list:DOCUMENTATION, linux-arch, Linux API, Adam Borowski,
Alexander Graf, Alexey Klimov, Andreas Schwab, Andrew Pinski,
Bamvor Zhangjian, Chris Metcalf, Christoph Muellner, Dave Martin,
David Miller, Florian Weimer
In-Reply-To: <CALCETrVuabkb5ZQOSgQQTEq8CrS602NBpAkE2=4n982r1f5reQ@mail.gmail.com>
On Sat, Oct 13, 2018 at 9:36 PM Andy Lutomirski <luto@amacapital.net> wrote:
>
> On Wed, May 16, 2018 at 1:19 AM Yury Norov <ynorov@caviumnetworks.com> wrote:
> >
> > This series enables AARCH64 with ILP32 mode.
> >
> > As supporting work, it introduces ARCH_32BIT_OFF_T configuration
> > option that is enabled for existing 32-bit architectures but disabled
> > for new arches (so 64-bit off_t userspace type is used by new userspace).
> > Also it deprecates getrlimit and setrlimit syscalls prior to prlimit64.
>
> Second, ILP32 user code is highly unlikely
> to end up with the same struct layout as ILP64 code. The latter seems
> like it should be solved entirely in userspace by adding a way to
> annotate a structure as being a kernel ABI structure and getting the
> toolchain to lay it out as if it were ILP64 even though the target is
> ILP32.
The syscall ABI could be almost completely abstracted in glibc, the
main issue is ioctl and a couple of related interfaces that pass data
structures (read() on /dev/input/*, mmap on /dev/snd/*
or raw sockets, fcntl).
The question whether a data type is laid out like a 64-bit architecture
would cannot be a property of the type in most of those cases,
because the same types are used elsewhere. Many ioctls just
take a pointer to a 'long' or similar, and then you have structures
like 'timespec' that are used both in syscall/ioctl ABI and in normal
user space code, but are required to be laid out differently there.
(timespec is a bad example because y2038 of course, but it
illustrates the point).
> 2. I think you should make a conscious decision as to whether the
> ILP32-ness of a syscall is a property of the task or of the syscall.
> On x86, x32-ness is a property of the syscall, but historically it
> also got rather entangled with the state of the task, and the result
> was a mess. It looks like you're making it be a property of the task,
> which is fine, but you're making it impossible for very clever ILP32
> libraries to include little ILP64 stubs that do fancy things with full
> 64-bit syscalls.
>
> 3. Make very certain that you aren't exploitable by malicious
> processes that set the high bits in ILP32 syscall args. x86 compat
> has issues like that in the past.
This point was actually the most important one for keeping the
aarch64 ilp32 interface as restricted as it is: it doesn't allow
anything that the normal aarch32/armv7 emulation doesn't
already provide.
Arnd
^ permalink raw reply
* rseq system call incompletely wired up in microblaze
From: Mathieu Desnoyers @ 2018-10-14 16:46 UTC (permalink / raw)
To: Michal Simek
Cc: Thomas Gleixner, Paul E. McKenney, Peter Zijlstra, Boqun Feng,
linux-kernel, linux-api
Hi Michal,
I notice the following commit went into 4.18:
commit 54b0a2011dfcd2e3fe2c28062694fbbe3eb377a3
Author: Michal Simek <michal.simek@xilinx.com>
Date: Thu Jun 14 08:24:39 2018 +0200
microblaze: Add new syscalls io_pgetevents and rseq
Wire up new syscalls io_pgetevents and rseq.
Signed-off-by: Michal Simek <michal.simek@xilinx.com>
It adds the rseq system call to the microblaze syscall list, but
does not add the required hooks in the architecture code, nor does
it select HAVE_RSEQ. So effectively, even though sys_rseq is reserved
on microblaze, it currently always returns ENOSYS.
Is your intent to simply reserve the system call number, or to
also get rseq to work on microblaze ?
A good example of the architecture changes required to
wire up rseq can be found here:
commit 9d6d99e3ac8ccfd0945edb3c83cd912838775056
Author: Heiko Carstens <heiko.carstens@de.ibm.com>
Date: Sat Jun 30 10:54:15 2018 +0200
s390: wire up rseq system call
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
Also, please make sure the tools/testing/selftests/rseq/ tests
pass before pushing a commit enabling it for your architecture.
Thanks,
Mathieu
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
^ permalink raw reply
* Re: [PATCH 31/34] vfs: syscall: Add fspick() to select a superblock for reconfiguration [ver #12]
From: Andy Lutomirski @ 2018-10-13 23:04 UTC (permalink / raw)
To: alan.christopher.jenkins
Cc: Al Viro, David Howells, Linux API, Linus Torvalds,
Eric W. Biederman, Linux FS Devel, LKML, Miklos Szeredi
In-Reply-To: <68a2107f-bf70-055b-86cf-1ba2ba9422bf@gmail.com>
On Sat, Oct 13, 2018 at 2:45 AM Alan Jenkins
<alan.christopher.jenkins@gmail.com> wrote:
>
> On 13/10/2018 07:11, Al Viro wrote:
> > On Fri, Oct 12, 2018 at 03:49:50PM +0100, Alan Jenkins wrote:
> >>> +SYSCALL_DEFINE3(fspick, int, dfd, const char __user *, path, unsigned int, flags)
> >>> +{
> >>> + struct fs_context *fc;
> >>> + struct path target;
> >>> + unsigned int lookup_flags;
> >>> + int ret;
> >>> +
> >>> + if (!ns_capable(current->nsproxy->mnt_ns->user_ns, CAP_SYS_ADMIN))
> >>> + return -EPERM;
> >>
> >> This seems to accept basically any mount. Specifically: are you sure it's
> >> OK to return a handle to a SB_NO_USER superblock?
> > Umm... As long as we don't try to do pathname resolution from its ->s_root,
> > shouldn't be a problem and I don't see anything that would do that. I might've
> > missed something, but...
>
> Sorry, I guess SB_NOUSER was the wrong word. I was trying find if
> anything stopped things like
>
> int memfd = memfd_create("foo", 0);
> int fsfd = fspick(memfd, "", FSPICK_EMPTY_PATH);
>
> fsconfig(fsfd, FSCONFIG_SET_FLAG, "ro", NULL, 0);
> fsconfig(fsfd, FSCONFIG_SET_STRING, "size", "100M", 0);
> fsconfig(fsfd, FSCONFIG_CMD_RECONFIGURE, NULL, NULL, 0);
>
> So far I'm getting -EBUSY if I try to apply the "ro", -EINVAL if I try
> to apply the "size=100M". But if I don't apply either, then
> FSCONFIG_CMD_RECONFIGURE succeeds.
>
> It seems worrying that it might let me set options on shm_mnt. Or at
> least letting me get as far as the -EBUSY check for the "ro" superblock
> flag.
>
> I'm not sure why I'm getting the -EINVAL setting the "size" option. But
> it would be much more reassuring if I was getting -EPERM :-).
>
I would argue that the filesystem associated with a memfd, and even
the fact that there *is* a filesystem, is none of user code's
business. So that fspick() call should return -EINVAL or similar.
^ permalink raw reply
* Re: [PATCH v9 00/24] ILP32 for ARM64
From: Andy Lutomirski @ 2018-10-13 19:36 UTC (permalink / raw)
To: ynorov
Cc: Catalin Marinas, Arnd Bergmann, linux-arm-kernel, LKML, linux-doc,
linux-arch, Linux API, Adam Borowski, Alexander Graf,
klimov.linux, schwab, Andrew Pinski, bamv2005, Chris Metcalf,
christoph.muellner, Dave Martin, David S. Miller, Florian Weimer,
Geert Uytterhoeven, Heiko Carstens, James Hogan
In-Reply-To: <20180516081910.10067-1-ynorov@caviumnetworks.com>
On Wed, May 16, 2018 at 1:19 AM Yury Norov <ynorov@caviumnetworks.com> wrote:
>
> This series enables AARCH64 with ILP32 mode.
>
> As supporting work, it introduces ARCH_32BIT_OFF_T configuration
> option that is enabled for existing 32-bit architectures but disabled
> for new arches (so 64-bit off_t userspace type is used by new userspace).
> Also it deprecates getrlimit and setrlimit syscalls prior to prlimit64.
A few thoughts:
1. I've never been able to shake the feeling that x32 should not need
kernel support at all. As far as I can tell, the kernel support is an
enormous amount of code and complexity to deal with exactly two
issues. First, ILP32 only has 32-bit pointers, so there needs to be a
way to tell the kernel to only use the lower 4 GB of user address
space. That part is easy. Second, ILP32 user code is highly unlikely
to end up with the same struct layout as ILP64 code. The latter seems
like it should be solved entirely in userspace by adding a way to
annotate a structure as being a kernel ABI structure and getting the
toolchain to lay it out as if it were ILP64 even though the target is
ILP32.
2. I think you should make a conscious decision as to whether the
ILP32-ness of a syscall is a property of the task or of the syscall.
On x86, x32-ness is a property of the syscall, but historically it
also got rather entangled with the state of the task, and the result
was a mess. It looks like you're making it be a property of the task,
which is fine, but you're making it impossible for very clever ILP32
libraries to include little ILP64 stubs that do fancy things with full
64-bit syscalls.
3. Make very certain that you aren't exploitable by malicious
processes that set the high bits in ILP32 syscall args. x86 compat
has issues like that in the past.
^ permalink raw reply
* Re: [PATCH v9 00/24] ILP32 for ARM64
From: Andy Lutomirski @ 2018-10-13 16:54 UTC (permalink / raw)
To: Catalin Marinas
Cc: Eugene Syromiatnikov, linux-doc, szabolcs.nagy, palmer,
Heiko Carstens, ynorov, Pavel Machek, philipp.tomsich,
Joseph S. Myers, linux-arch, sellcey, Prasun.Kapoor, schwab,
Alexander Graf, bamv2005, Geert Uytterhoeven, Dave Martin,
Adam Borowski, manuel.montezelo, James Hogan, Chris Metcalf,
Arnd Bergmann, Andrew Pinski, linyongting
In-Reply-To: <20181013093411.o3id6yzkspsxr5jt@mbp>
> On Oct 13, 2018, at 2:34 AM, Catalin Marinas <catalin.marinas@arm.com> wrote:
>
>> On Sat, Oct 13, 2018 at 04:14:16AM +0200, Eugene Syromiatnikov wrote:
>>> On Wed, Oct 10, 2018 at 04:36:56PM +0100, Catalin Marinas wrote:
>>>> On Wed, Oct 10, 2018 at 04:10:21PM +0200, Eugene Syromiatnikov wrote:
>>>> I have some questions regarding AArch64 ILP32 implementation for which I
>>>> failed to find an answer myself:
>>>> * How ptrace() tracer is supposed to distinguish between ILP32 and LP64
>>>> tracees? For MIPS N32 and x32 this is possible based on syscall
>>>> number, but for AArch64 ILP32 I do not see such a sign. There's also
>>>> ARM_ip is employed for signalling entering/exiting, I wonder whether
>>>> it's possible to employ it also for signalling tracee's personality.
>>>
>>> With the current implementation, I don't think you can distinguish. From
>>> the kernel perspective, the register set is the same. What is the
>>> use-case for this?
>>
>> Err, a ptrace()-based tracer trying to trace a process, for example?
>
> I first thought it wouldn't matter for ptrace-based tracers since the
> syscall numbers are (mostly) the same. But the arguments layout in
> register is indeed different, so I see your point now about having to
> distinguish.
>
>>> We could add a new regset to expose the ILP32 state (NT_ARM_..., I can't
>>> think of a name now but probably not PER* as this implies PER_LINUX_...
>>> which is independent from TIF_32BIT_*).
>>
>> So that would require an additional ptrace() call on each syscall stop,
>> is that correct?
>
> The ILP32 state does not change at run-time, so it could only do a
> ptrace() call once and save the information. No need to re-read it on
> each syscall stop.
>
Please solve this in an arch independent way. This situation is
basically unusably broken on x86 right now. Please solve it for real,
by, for example, adding a new ptrace operation that returns something
like this:
enum ptrace_syscall_state {
NO_SYSCALL,
SYSCALL_ENTRY,
SYSCALL_EXIT,
/* other values may be defined in the future. */
};
struct ptrace_syscall_info {
enum ptrace_syscall_state state;
unsigned long arch;
union {
struct {
unsigned long nr;
unsigned long args[6];
} entry;
struct {
unsigned long ret;
} exit;
};
where arch is an AUDIT_ARCH_XYZ constant.
On x86, it's currently essentially impossible for tools like strace to
correctly decode syscalls.
> We could set a high bit in the syscall number reported to the ptrace
> caller (though not changing the syscall ABI) but I haven't thought of
> other consequences. For example, can the ptrace caller change the
> syscall number?
Yes it can.
>
>>>> * What's the reasoning behind capping syscall arguments to 32 bit? x32
>>>> and MIPS N32 do not have such a restriction (and do not need special
>>>> wrappers for syscalls that pass 64-bit values as a result, except
>>>> when they do, as it is the case for preadv2 on x32); moreover, that
>>>> would lead to insurmountable difficulties for AArch64 ILP32 tracers
>>>> that try to trace LP64 tracees, as it would be impossible to pass
>>>> 64-bit addresses to process_vm_{read,write} or ptrace PEEK/POKE.
>>>
>>> We've attempted in earlier versions to allow a mix of 32 and 64-bit
>>> register values from ILP32 but it got pretty complicated. The entry code
>>> would need to know which registers need zeroing of the top 32-bit
>>
>> If kernel specifies 64-bit wide registers for syscalls, then it's the
>> caller's (libc's) responsibility to properly sign-extend arguments when
>> needed, and glibc, for example, already has proper type definitions that
>> aimed to handle this.
>
> We tried, see my other reply.
>
> --
> Catalin
^ permalink raw reply
* Re: [PATCH v9 00/24] ILP32 for ARM64
From: Yury Norov @ 2018-10-13 13:43 UTC (permalink / raw)
To: Catalin Marinas
Cc: Eugene Syromiatnikov, linux-doc@vger.kernel.org, Szabolcs Nagy,
Palmer Dabbelt, Heiko Carstens, Pavel Machek, Philipp Tomsich,
Joseph Myers, linux-arch@vger.kernel.org, Ellcey, Steve,
Kapoor, Prasun, Andreas Schwab, Alexander Graf, Bamvor Zhangjian,
Geert Uytterhoeven, Dave Martin, Adam Borowski, Manuel Montezelo
In-Reply-To: <20181013093411.o3id6yzkspsxr5jt@mbp>
On Sat, Oct 13, 2018 at 10:34:11AM +0100, Catalin Marinas wrote:
>
> Lines: 73
>
> External Email
>
> On Sat, Oct 13, 2018 at 04:14:16AM +0200, Eugene Syromiatnikov wrote:
> > On Wed, Oct 10, 2018 at 04:36:56PM +0100, Catalin Marinas wrote:
> > > On Wed, Oct 10, 2018 at 04:10:21PM +0200, Eugene Syromiatnikov wrote:
> > > > I have some questions regarding AArch64 ILP32 implementation for which I
> > > > failed to find an answer myself:
> > > > * How ptrace() tracer is supposed to distinguish between ILP32 and LP64
> > > > tracees? For MIPS N32 and x32 this is possible based on syscall
> > > > number, but for AArch64 ILP32 I do not see such a sign. There's also
> > > > ARM_ip is employed for signalling entering/exiting, I wonder whether
> > > > it's possible to employ it also for signalling tracee's personality.
> > >
> > > With the current implementation, I don't think you can distinguish. From
> > > the kernel perspective, the register set is the same. What is the
> > > use-case for this?
> >
> > Err, a ptrace()-based tracer trying to trace a process, for example?
>
> I first thought it wouldn't matter for ptrace-based tracers since the
> syscall numbers are (mostly) the same. But the arguments layout in
> register is indeed different, so I see your point now about having to
> distinguish.
>
> > > We could add a new regset to expose the ILP32 state (NT_ARM_..., I can't
> > > think of a name now but probably not PER* as this implies PER_LINUX_...
> > > which is independent from TIF_32BIT_*).
> >
> > So that would require an additional ptrace() call on each syscall stop,
> > is that correct?
>
> The ILP32 state does not change at run-time, so it could only do a
> ptrace() call once and save the information. No need to re-read it on
> each syscall stop.
>
> We could set a high bit in the syscall number reported to the ptrace
> caller (though not changing the syscall ABI) but I haven't thought of
> other consequences. For example, can the ptrace caller change the
> syscall number?
I believe, /proc/PID/auxv is enough to distinguish between arm64, ilp32
and aarch32 ABis. If no, I think it's better to do it there.
I don't have ILP32 machine available at the moment, but I'll check it soon.
> > > > * What's the reasoning behind capping syscall arguments to 32 bit? x32
> > > > and MIPS N32 do not have such a restriction (and do not need special
> > > > wrappers for syscalls that pass 64-bit values as a result, except
> > > > when they do, as it is the case for preadv2 on x32); moreover, that
> > > > would lead to insurmountable difficulties for AArch64 ILP32 tracers
> > > > that try to trace LP64 tracees, as it would be impossible to pass
> > > > 64-bit addresses to process_vm_{read,write} or ptrace PEEK/POKE.
> > >
> > > We've attempted in earlier versions to allow a mix of 32 and 64-bit
> > > register values from ILP32 but it got pretty complicated. The entry code
> > > would need to know which registers need zeroing of the top 32-bit
> >
> > If kernel specifies 64-bit wide registers for syscalls, then it's the
> > caller's (libc's) responsibility to properly sign-extend arguments when
> > needed, and glibc, for example, already has proper type definitions that
> > aimed to handle this.
>
> We tried, see my other reply.
A couple of links to recall the story:
https://www.spinics.net/lists/linux-s390/msg11593.html
http://linux-kernel.2935.n7.nabble.com/RFC6-PATCH-v6-00-21-ILP32-for-ARM64-td1345105.html
Cover-letter of the series has links to previous discussions.
I would also notice that even if we pass 64-bit parameters in a single
register, we cannot avoid using the compat layer. It looks more natural
not to split the 64-bit register, but from performance point of view
there is almost no difference, either we split registers or not (2.6%
for empty syscall, as I measured). And the cost of overcomplication was
considered too much. So we chose to stick to more standard compat layer
and gain in maintainability.
Yury
^ permalink raw reply
* Re: [PATCH 31/34] vfs: syscall: Add fspick() to select a superblock for reconfiguration [ver #12]
From: Alan Jenkins @ 2018-10-13 9:45 UTC (permalink / raw)
To: Al Viro
Cc: David Howells, linux-api, torvalds, ebiederm, linux-fsdevel,
linux-kernel, mszeredi
In-Reply-To: <20181013061141.GR32577@ZenIV.linux.org.uk>
On 13/10/2018 07:11, Al Viro wrote:
> On Fri, Oct 12, 2018 at 03:49:50PM +0100, Alan Jenkins wrote:
>>> +SYSCALL_DEFINE3(fspick, int, dfd, const char __user *, path, unsigned int, flags)
>>> +{
>>> + struct fs_context *fc;
>>> + struct path target;
>>> + unsigned int lookup_flags;
>>> + int ret;
>>> +
>>> + if (!ns_capable(current->nsproxy->mnt_ns->user_ns, CAP_SYS_ADMIN))
>>> + return -EPERM;
>>
>> This seems to accept basically any mount. Specifically: are you sure it's
>> OK to return a handle to a SB_NO_USER superblock?
> Umm... As long as we don't try to do pathname resolution from its ->s_root,
> shouldn't be a problem and I don't see anything that would do that. I might've
> missed something, but...
Sorry, I guess SB_NOUSER was the wrong word. I was trying find if
anything stopped things like
int memfd = memfd_create("foo", 0);
int fsfd = fspick(memfd, "", FSPICK_EMPTY_PATH);
fsconfig(fsfd, FSCONFIG_SET_FLAG, "ro", NULL, 0);
fsconfig(fsfd, FSCONFIG_SET_STRING, "size", "100M", 0);
fsconfig(fsfd, FSCONFIG_CMD_RECONFIGURE, NULL, NULL, 0);
So far I'm getting -EBUSY if I try to apply the "ro", -EINVAL if I try
to apply the "size=100M". But if I don't apply either, then
FSCONFIG_CMD_RECONFIGURE succeeds.
It seems worrying that it might let me set options on shm_mnt. Or at
least letting me get as far as the -EBUSY check for the "ro" superblock
flag.
I'm not sure why I'm getting the -EINVAL setting the "size" option. But
it would be much more reassuring if I was getting -EPERM :-).
Alan
^ permalink raw reply
* Re: [PATCH v9 00/24] ILP32 for ARM64
From: Catalin Marinas @ 2018-10-13 9:34 UTC (permalink / raw)
To: Eugene Syromiatnikov
Cc: linux-doc, Szabolcs Nagy, Palmer Dabbelt, Heiko Carstens,
Yury Norov, Pavel Machek, Philipp Tomsich, Joseph Myers,
linux-arch, Steve Ellcey, Prasun Kapoor, Andreas Schwab,
Alexander Graf, Bamvor Zhangjian, Geert Uytterhoeven, Dave Martin,
Adam Borowski, Manuel Montezelo, James Hogan, Chris Metcalf,
Arnd Bergmann, Andrew Pinski, Lin Yongting
In-Reply-To: <20181013021416.GE21972@asgard.redhat.com>
On Sat, Oct 13, 2018 at 04:14:16AM +0200, Eugene Syromiatnikov wrote:
> On Wed, Oct 10, 2018 at 04:36:56PM +0100, Catalin Marinas wrote:
> > On Wed, Oct 10, 2018 at 04:10:21PM +0200, Eugene Syromiatnikov wrote:
> > > I have some questions regarding AArch64 ILP32 implementation for which I
> > > failed to find an answer myself:
> > > * How ptrace() tracer is supposed to distinguish between ILP32 and LP64
> > > tracees? For MIPS N32 and x32 this is possible based on syscall
> > > number, but for AArch64 ILP32 I do not see such a sign. There's also
> > > ARM_ip is employed for signalling entering/exiting, I wonder whether
> > > it's possible to employ it also for signalling tracee's personality.
> >
> > With the current implementation, I don't think you can distinguish. From
> > the kernel perspective, the register set is the same. What is the
> > use-case for this?
>
> Err, a ptrace()-based tracer trying to trace a process, for example?
I first thought it wouldn't matter for ptrace-based tracers since the
syscall numbers are (mostly) the same. But the arguments layout in
register is indeed different, so I see your point now about having to
distinguish.
> > We could add a new regset to expose the ILP32 state (NT_ARM_..., I can't
> > think of a name now but probably not PER* as this implies PER_LINUX_...
> > which is independent from TIF_32BIT_*).
>
> So that would require an additional ptrace() call on each syscall stop,
> is that correct?
The ILP32 state does not change at run-time, so it could only do a
ptrace() call once and save the information. No need to re-read it on
each syscall stop.
We could set a high bit in the syscall number reported to the ptrace
caller (though not changing the syscall ABI) but I haven't thought of
other consequences. For example, can the ptrace caller change the
syscall number?
> > > * What's the reasoning behind capping syscall arguments to 32 bit? x32
> > > and MIPS N32 do not have such a restriction (and do not need special
> > > wrappers for syscalls that pass 64-bit values as a result, except
> > > when they do, as it is the case for preadv2 on x32); moreover, that
> > > would lead to insurmountable difficulties for AArch64 ILP32 tracers
> > > that try to trace LP64 tracees, as it would be impossible to pass
> > > 64-bit addresses to process_vm_{read,write} or ptrace PEEK/POKE.
> >
> > We've attempted in earlier versions to allow a mix of 32 and 64-bit
> > register values from ILP32 but it got pretty complicated. The entry code
> > would need to know which registers need zeroing of the top 32-bit
>
> If kernel specifies 64-bit wide registers for syscalls, then it's the
> caller's (libc's) responsibility to properly sign-extend arguments when
> needed, and glibc, for example, already has proper type definitions that
> aimed to handle this.
We tried, see my other reply.
--
Catalin
^ permalink raw reply
* Re: [PATCH v3 3/3] namei: aggressively check for nd->root escape on ".." resolution
From: Aleksa Sarai @ 2018-10-13 9:27 UTC (permalink / raw)
To: Al Viro
Cc: Aleksa Sarai, Jann Horn, Eric W. Biederman, jlayton, Bruce Fields,
Arnd Bergmann, Andy Lutomirski, David Howells, christian,
Tycho Andersen, David Drysdale, dev, containers, linux-fsdevel,
kernel list, linux-arch, Linux API
In-Reply-To: <20181013090432.GV32577@ZenIV.linux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 1237 bytes --]
On 2018-10-13, Al Viro <viro@ZenIV.linux.org.uk> wrote:
> On Sat, Oct 13, 2018 at 07:53:26PM +1100, Aleksa Sarai wrote:
>
> > I didn't know about path_is_under() -- I just checked and it appears to
> > not take &rename_lock? From my understanding, in order to protect
> > against the rename attack you need to take &rename_lock (or check
> > against &rename_lock at least and retry if it changed).
> >
> > I could definitely use path_is_under() if you prefer, though I think
> > that in this case we'd need to take &rename_lock (right?). Also is there
> > a speed issue with taking the write-side of a seqlock when we are just
> > reading -- is this more efficient than doing a retry like in __d_path?
>
> ???
>
> 1) it uses is_subdir(), which does deal with rename_lock
Oh -- complete brain-fart on my part. Sorry about that.
> 2) what it does is taking mount_lock.lock. I.e. the same
> thing as the second retry in __d_path(). _If_ it shows
> up in profiles, we can switch it to read_seqbegin_or_lock(),
> but I'd like to see the profiling data first.
Sure, I'll switch it to use path_is_under().
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v9 00/24] ILP32 for ARM64
From: Catalin Marinas @ 2018-10-13 9:20 UTC (permalink / raw)
To: Eugene Syromiatnikov
Cc: Szabolcs Nagy, linux-doc, Palmer Dabbelt, Heiko Carstens,
Yury Norov, Pavel Machek, Philipp Tomsich, Joseph Myers,
linux-arch, Steve Ellcey, Prasun Kapoor, Andreas Schwab,
Alexander Graf, Geert Uytterhoeven, Bamvor Zhangjian, Dave Martin,
Adam Borowski, Manuel Montezelo, James Hogan
In-Reply-To: <20181013020731.GD21972@asgard.redhat.com>
On Sat, Oct 13, 2018 at 04:07:31AM +0200, Eugene Syromiatnikov wrote:
> On Wed, Oct 10, 2018 at 03:39:07PM +0100, Szabolcs Nagy wrote:
> > On 10/10/18 15:10, Eugene Syromiatnikov wrote:
> > > * What's the reasoning behind capping syscall arguments to 32 bit? x32
> > > and MIPS N32 do not have such a restriction (and do not need special
> > > wrappers for syscalls that pass 64-bit values as a result, except
> > > when they do, as it is the case for preadv2 on x32); moreover, that
> > > would lead to insurmountable difficulties for AArch64 ILP32 tracers
> > > that try to trace LP64 tracees, as it would be impossible to pass
> > > 64-bit addresses to process_vm_{read,write} or ptrace PEEK/POKE.
> >
> > but that's necessarily the case for all ilp32 abis:
> > the userspace syscall function receives 32bit
> > arguments so even if the kernel abi takes 64bit
> > args you cannot use that from c code. (the libc
> > does not even know which args should be sign or
> > zero extended.)
>
> glibc's syscall() prototype has kernel_ulong_t as its arguments (more
> specifically, to __syscall_ulong_t, which is 64-bit wide on x32; it
> should also have kernel_long_t as its return type instead of long,
> but that's another story), so it works perfectly fine in case of x32.
This would have been my preferred approach but the libc community were
not entirely happy with it as it breaks POSIX compatibility:
https://sourceware.org/bugzilla/show_bug.cgi?id=16437
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/323348.html
(there are other threads around x32 and people arguing whether POSIX is
wrong)
Some sometime around version 4 or 5 of this series, we made the move
(back) to compat-like ABI.
--
Catalin
^ permalink raw reply
* Re: [PATCH v3 3/3] namei: aggressively check for nd->root escape on ".." resolution
From: Al Viro @ 2018-10-13 9:04 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Aleksa Sarai, Jann Horn, Eric W. Biederman, jlayton, Bruce Fields,
Arnd Bergmann, Andy Lutomirski, David Howells, christian,
Tycho Andersen, David Drysdale, dev, containers, linux-fsdevel,
kernel list, linux-arch, Linux API
In-Reply-To: <20181013085326.gx6rvgqbbyuntfvv@ryuk>
On Sat, Oct 13, 2018 at 07:53:26PM +1100, Aleksa Sarai wrote:
> I didn't know about path_is_under() -- I just checked and it appears to
> not take &rename_lock? From my understanding, in order to protect
> against the rename attack you need to take &rename_lock (or check
> against &rename_lock at least and retry if it changed).
>
> I could definitely use path_is_under() if you prefer, though I think
> that in this case we'd need to take &rename_lock (right?). Also is there
> a speed issue with taking the write-side of a seqlock when we are just
> reading -- is this more efficient than doing a retry like in __d_path?
???
1) it uses is_subdir(), which does deal with rename_lock
2) what it does is taking mount_lock.lock. I.e. the same
thing as the second retry in __d_path(). _If_ it shows
up in profiles, we can switch it to read_seqbegin_or_lock(),
but I'd like to see the profiling data first.
^ permalink raw reply
* Re: [PATCH v3 3/3] namei: aggressively check for nd->root escape on ".." resolution
From: Aleksa Sarai @ 2018-10-13 8:53 UTC (permalink / raw)
To: Al Viro
Cc: Aleksa Sarai, Jann Horn, Eric W. Biederman, jlayton, Bruce Fields,
Arnd Bergmann, Andy Lutomirski, David Howells, christian,
Tycho Andersen, David Drysdale, dev, containers, linux-fsdevel,
kernel list, linux-arch, Linux API
In-Reply-To: <20181013082210.GU32577@ZenIV.linux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 2623 bytes --]
On 2018-10-13, Al Viro <viro@ZenIV.linux.org.uk> wrote:
> > > > +static inline int nd_alloc_dpathbuf(struct nameidata *nd)
> > > > +{
> > > > + if (unlikely(!nd->dpathbuf)) {
> > > > + if (nd->flags & LOOKUP_RCU) {
> > > > + nd->dpathbuf = kmalloc(PATH_MAX, GFP_ATOMIC);
> > > > + if (unlikely(!nd->dpathbuf))
> > > > + return -ECHILD;
> > > > + } else {
> > > > + nd->dpathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
> > > > + if (unlikely(!nd->dpathbuf))
> > > > + return -ENOMEM;
> > > > + }
> > > > + }
> > > > + return 0;
> > > > +}
> > >
> > > Note that a fixed-size path buffer means that if the path is very
> > > long, e.g. because you followed long symlinks on the way down, this
> > > can cause lookup failures.
> >
> > This is already an issue with __d_path (even if the buffer was larger)
> > because it will not output a path longer than PATH_MAX. I imagine this
> > is a pretty strong argument for why we should refactor __d_path so that
> > we can *just* use the escape checking to avoid -ENAMETOOLONG.
>
> Let me get it straight - the whole point of that buffer is to check
> if __d_path() returns NULL? So you allocate it so that you would have
> place to copy the path components into... only to have them completely
> ignored?
Yes (and it was definitely the wrong thing to do).
Since writing that mail, I changed it to not have to allocate a buffer
-- though this is done in the fairly ugly way of changing prepend_path()
to be able to take @buffer==NULL which then skips all of the
string-related code, and then having a dumb wrapper which calls
prepend_path(root, path, NULL, NULL).
I was planning on sending out the updated patches after LPC.
> How is that different from path_is_under()?
I didn't know about path_is_under() -- I just checked and it appears to
not take &rename_lock? From my understanding, in order to protect
against the rename attack you need to take &rename_lock (or check
against &rename_lock at least and retry if it changed).
I could definitely use path_is_under() if you prefer, though I think
that in this case we'd need to take &rename_lock (right?). Also is there
a speed issue with taking the write-side of a seqlock when we are just
reading -- is this more efficient than doing a retry like in __d_path?
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 3/3] namei: aggressively check for nd->root escape on ".." resolution
From: Al Viro @ 2018-10-13 8:22 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Jann Horn, cyphar, Eric W. Biederman, jlayton, Bruce Fields,
Arnd Bergmann, Andy Lutomirski, David Howells, christian,
Tycho Andersen, David Drysdale, dev, containers, linux-fsdevel,
kernel list, linux-arch, Linux API
In-Reply-To: <20181009153728.2altaqxclntvyc7b@mikami>
On Wed, Oct 10, 2018 at 02:37:28AM +1100, Aleksa Sarai wrote:
> > > +static inline int nd_alloc_dpathbuf(struct nameidata *nd)
> > > +{
> > > + if (unlikely(!nd->dpathbuf)) {
> > > + if (nd->flags & LOOKUP_RCU) {
> > > + nd->dpathbuf = kmalloc(PATH_MAX, GFP_ATOMIC);
> > > + if (unlikely(!nd->dpathbuf))
> > > + return -ECHILD;
> > > + } else {
> > > + nd->dpathbuf = kmalloc(PATH_MAX, GFP_KERNEL);
> > > + if (unlikely(!nd->dpathbuf))
> > > + return -ENOMEM;
> > > + }
> > > + }
> > > + return 0;
> > > +}
> >
> > Note that a fixed-size path buffer means that if the path is very
> > long, e.g. because you followed long symlinks on the way down, this
> > can cause lookup failures.
>
> This is already an issue with __d_path (even if the buffer was larger)
> because it will not output a path longer than PATH_MAX. I imagine this
> is a pretty strong argument for why we should refactor __d_path so that
> we can *just* use the escape checking to avoid -ENAMETOOLONG.
Let me get it straight - the whole point of that buffer is to check
if __d_path() returns NULL? So you allocate it so that you would have
place to copy the path components into... only to have them completely
ignored?
How is that different from path_is_under()?
^ permalink raw reply
* Re: [PATCH v3 1/3] namei: implement O_BENEATH-style AT_* flags
From: Aleksa Sarai @ 2018-10-13 8:20 UTC (permalink / raw)
To: Al Viro
Cc: Eric Biederman, Christian Brauner, Jeff Layton, J. Bruce Fields,
Arnd Bergmann, Andy Lutomirski, David Howells, Jann Horn,
Tycho Andersen, David Drysdale, dev, containers, linux-fsdevel,
linux-kernel, linux-arch, linux-api
In-Reply-To: <20181013080511.GT32577@ZenIV.linux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 2126 bytes --]
On 2018-10-13, Al Viro <viro@ZenIV.linux.org.uk> wrote:
> > Pardon me, but... huh? The reason for your two calls of dirfd_path_init() is,
> > AFAICS, the combination of absolute pathname with both LOOKUP_XDEV and
> > LOOKUP_BENEATH at the same time. That combination is treated as if the pathname
> > had been relative. Note that LOOKUP_BENEATH alone is ignored for absolute ones
> > (and with a good reason - it's a no-op on path_init() level in that case).
> >
> > What the hell? It complicates your code and doesn't seem to provide any benefits
> > whatsoever -- you could bloody well have passed the relative pathname to start with.
> >
> > IDGI... Without that kludge it becomes simply "do as we currently do for absolute
> > pathnames, call dirfd_path_init() for relative ones". And I would argue that
> > taking LOOKUP_BENEATH handling out of dirfd_path_init() into path_init() (relative)
> > case would be a good idea.
> >
> > As it is, the logics is very hard to follow.
>
> ... and it fails on LOOKUP_BENEATH anyway. Egads... So that's for your
> LOOKUP_CHROOT ;-/ IMO that's awful, especially with the way you've spread those
> LOOKUP_CHROOT cases between these two.
Yeah, the ->root setting in dirfd_path_init() is ugly. :/
> Why not simply have O_THISROOT pick root by dirfd and call file_open_root()?
Wouldn't this require replicating the dirfd_path_init()-like code inside
all of the path_*at() callers which use path_init()? Or is there another
common place we could put it?
> And if something wants it for stat(), etc. just have them use it combined with
> O_PATH and pass the result to ...at()...
This works for stat and quite a few other things (which is why I only
added openat(2) support for the moment), but I think we'd eventually
need something like this for renameat2(2) as well as a few other choice
*at(2) syscalls. Though I also think that more AT_EMPTY_PATH support
would removed the need for _most_ *at(2) implementations to use this.
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/3] namei: implement O_BENEATH-style AT_* flags
From: Aleksa Sarai @ 2018-10-13 8:09 UTC (permalink / raw)
To: Al Viro
Cc: Eric Biederman, Christian Brauner, Jeff Layton, J. Bruce Fields,
Arnd Bergmann, Andy Lutomirski, David Howells, Jann Horn,
Tycho Andersen, David Drysdale, dev, containers, linux-fsdevel,
linux-kernel, linux-arch, linux-api
In-Reply-To: <20181013073319.GS32577@ZenIV.linux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 5408 bytes --]
On 2018-10-13, Al Viro <viro@ZenIV.linux.org.uk> wrote:
> First of all, dirfd_path_init() part should be in a separate commit. And I'm
> really not happy with the logics in there. dirfd_path_init() itself is
> kinda-sorta reasonable.
Sure, I can do that.
> It is equivalent to setting the starting point for
> relative pathnames + setting ->root for LOOKUP_BENEATH, right?
Right.
> But the part in path_init() is too bloody convoluted for its own good. Let me
> try to translate:
>
> > + if (unlikely(flags & LOOKUP_XDEV)) {
> > + error = dirfd_path_init(nd);
> > + if (unlikely(error))
> > + return ERR_PTR(error);
> > + }
>
> * if LOOKUP_XDEV is set, set the starting point as if it was a relative
> pathname. If LOOKUP_BENEATH was set as well, set ->root to the same
> point.
Right. This is for two reasons (though if you disagree with these
semantics we can change this as well):
1. It's not clear to me whether openat(somefd->"/", "/tmp", O_XDEV)
should return an -EXDEV or completely ignore the starting point. Same
argument with AT_FDCWD. I opted to make it so that the starting point
has to be on the same mountpoint, but I totally understand if you
feel this is insane -- and I'd be happy to change it. The real
problem comes from (2).
2. AT_THIS_ROOT chroot-scope absolute paths, and so in the second patch
LOOKUP_CHROOT also triggers this codepath. The main argument for this
semantic is somewhat elaborated in the cover letter -- but
the short version is because AT_THIS_ROOT has to chroot-scope
absolute symlinks it would be somewhat strange if it didn't scope
absolute paths you give it -- otherwise it could either be a footgun
or would require always returning -EXDEV here.
Though, as above, if you feel that the current semantics (absolute
paths override whatever dirfd you give), then -EXDEV is the
alternative I would pitch.
> * if it's an absolute pathname,
> > if (*s == '/') {
> ... and we hadn't come here with LOOKUP_XDEV + LOOKUP_BENEATH, set ->root.
> > + if (likely(!nd->root.mnt))
> > + set_root(nd);
> * if it's an absolute pathname, set the starting point to ->root. Note that
> if we came here with LOOKUP_XDEV, we'll discard the starting point we'd
> calculated.
We wouldn't discard it -- nd_jump_root() will check whether a mount
crossing was implied here (otherwise an absolute symlink could cause you
to cross a mountpoint).
But as above, if you'd prefer that absolute paths disable all dirfd
handling (as is the case now), I can remove this semantic.
> > + error = nd_jump_root(nd);
> > + if (unlikely(error))
> > + s = ERR_PTR(error);
> > return s;
> > }
> > + if (likely(!nd->path.mnt)) {
> * if we didn't have LOOKUP_XDEV, set the starting point as if it was a relative
> pathname (which it is) and, if LOOKUP_BENEATH is also there, set ->root there
> as well.
> > + error = dirfd_path_init(nd);
> > + if (unlikely(error))
> > + return ERR_PTR(error);
> > + }
> > + return s;
> > }
>
> Pardon me, but... huh? The reason for your two calls of dirfd_path_init() is,
> AFAICS, the combination of absolute pathname with both LOOKUP_XDEV and
> LOOKUP_BENEATH at the same time. That combination is treated as if the pathname
> had been relative. Note that LOOKUP_BENEATH alone is ignored for absolute ones
> (and with a good reason - it's a no-op on path_init() level in that case).
>
> What the hell? It complicates your code and doesn't seem to provide any benefits
> whatsoever
The reasoning for this is because of how AT_THIS_ROOT uses both of these
codepaths (it causes dirfd_path_init() to be called before the absolute
check, and also causes ->root to be set).
I wrote the features in parallel and then split out the code for
AT_THIS_ROOT so it could be discussed separately (and so removing it if
it was rejected would be simpler). But unfortunately this does result in
the dirfd_path_init() code looking completely superfluous without seeing
the second patch.
> -- you could bloody well have passed the relative pathname to start with.
(I think you mean always doing dirfd_path_init() first here?)
Right, but I didn't want to discard nd->path unnecessarily -- if we do
all of the code to grab AT_FDCWD and then it is completely unused (not
even in the AT_XDEV sense of "unused") it seems like a waste.
Did I misunderstand your suggestion? Were you referring to userspace
just being able to "[pass] the relative pathname to start with"?
> IDGI... Without that kludge it becomes simply "do as we currently do for absolute
> pathnames, call dirfd_path_init() for relative ones". And I would argue that
> taking LOOKUP_BENEATH handling out of dirfd_path_init() into path_init() (relative)
> case would be a good idea.
Right, I could definitely do that -- though for AT_THIS_ROOT we'd
duplicate the ->root setting in both places.
> As it is, the logics is very hard to follow.
Sorry about that. Would you prefer if the two patches (AT_BENEATH family
and AT_THIS_ROOT) were sent as a single patch -- with the
dirfd_path_init() code split out? Or that the second patch do all of the
structural changes to refactor dirfd_path_init() usage?
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/3] namei: implement O_BENEATH-style AT_* flags
From: Al Viro @ 2018-10-13 8:05 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Eric Biederman, Christian Brauner, Jeff Layton, J. Bruce Fields,
Arnd Bergmann, Andy Lutomirski, David Howells, Jann Horn,
Tycho Andersen, David Drysdale, dev, containers, linux-fsdevel,
linux-kernel, linux-arch, linux-api
In-Reply-To: <20181013073319.GS32577@ZenIV.linux.org.uk>
On Sat, Oct 13, 2018 at 08:33:19AM +0100, Al Viro wrote:
> Pardon me, but... huh? The reason for your two calls of dirfd_path_init() is,
> AFAICS, the combination of absolute pathname with both LOOKUP_XDEV and
> LOOKUP_BENEATH at the same time. That combination is treated as if the pathname
> had been relative. Note that LOOKUP_BENEATH alone is ignored for absolute ones
> (and with a good reason - it's a no-op on path_init() level in that case).
>
> What the hell? It complicates your code and doesn't seem to provide any benefits
> whatsoever -- you could bloody well have passed the relative pathname to start with.
>
> IDGI... Without that kludge it becomes simply "do as we currently do for absolute
> pathnames, call dirfd_path_init() for relative ones". And I would argue that
> taking LOOKUP_BENEATH handling out of dirfd_path_init() into path_init() (relative)
> case would be a good idea.
>
> As it is, the logics is very hard to follow.
... and it fails on LOOKUP_BENEATH anyway. Egads... So that's for your
LOOKUP_CHROOT ;-/ IMO that's awful, especially with the way you've spread those
LOOKUP_CHROOT cases between these two.
Why not simply have O_THISROOT pick root by dirfd and call file_open_root()?
And if something wants it for stat(), etc. just have them use it combined with
O_PATH and pass the result to ...at()...
^ permalink raw reply
* Re: [PATCH v3 1/3] namei: implement O_BENEATH-style AT_* flags
From: Al Viro @ 2018-10-13 7:33 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Eric Biederman, Christian Brauner, Jeff Layton, J. Bruce Fields,
Arnd Bergmann, Andy Lutomirski, David Howells, Jann Horn,
Tycho Andersen, David Drysdale, dev, containers, linux-fsdevel,
linux-kernel, linux-arch, linux-api
In-Reply-To: <20181009070230.12884-2-cyphar@cyphar.com>
On Tue, Oct 09, 2018 at 06:02:28PM +1100, Aleksa Sarai wrote:
First of all, dirfd_path_init() part should be in a separate commit. And I'm
really not happy with the logics in there. dirfd_path_init() itself is
kinda-sorta reasonable. It is equivalent to setting the starting point for
relative pathnames + setting ->root for LOOKUP_BENEATH, right?
But the part in path_init() is too bloody convoluted for its own good. Let me
try to translate:
> + if (unlikely(flags & LOOKUP_XDEV)) {
> + error = dirfd_path_init(nd);
> + if (unlikely(error))
> + return ERR_PTR(error);
> + }
* if LOOKUP_XDEV is set, set the starting point as if it was a relative
pathname. If LOOKUP_BENEATH was set as well, set ->root to the same
point.
* if it's an absolute pathname,
> if (*s == '/') {
... and we hadn't come here with LOOKUP_XDEV + LOOKUP_BENEATH, set ->root.
> + if (likely(!nd->root.mnt))
> + set_root(nd);
* if it's an absolute pathname, set the starting point to ->root. Note that
if we came here with LOOKUP_XDEV, we'll discard the starting point we'd
calculated.
> + error = nd_jump_root(nd);
> + if (unlikely(error))
> + s = ERR_PTR(error);
> return s;
> }
> + if (likely(!nd->path.mnt)) {
* if we didn't have LOOKUP_XDEV, set the starting point as if it was a relative
pathname (which it is) and, if LOOKUP_BENEATH is also there, set ->root there
as well.
> + error = dirfd_path_init(nd);
> + if (unlikely(error))
> + return ERR_PTR(error);
> + }
> + return s;
> }
Pardon me, but... huh? The reason for your two calls of dirfd_path_init() is,
AFAICS, the combination of absolute pathname with both LOOKUP_XDEV and
LOOKUP_BENEATH at the same time. That combination is treated as if the pathname
had been relative. Note that LOOKUP_BENEATH alone is ignored for absolute ones
(and with a good reason - it's a no-op on path_init() level in that case).
What the hell? It complicates your code and doesn't seem to provide any benefits
whatsoever -- you could bloody well have passed the relative pathname to start with.
IDGI... Without that kludge it becomes simply "do as we currently do for absolute
pathnames, call dirfd_path_init() for relative ones". And I would argue that
taking LOOKUP_BENEATH handling out of dirfd_path_init() into path_init() (relative)
case would be a good idea.
As it is, the logics is very hard to follow.
^ permalink raw reply
* Re: [PATCH 31/34] vfs: syscall: Add fspick() to select a superblock for reconfiguration [ver #12]
From: Al Viro @ 2018-10-13 6:11 UTC (permalink / raw)
To: Alan Jenkins
Cc: David Howells, linux-api, torvalds, ebiederm, linux-fsdevel,
linux-kernel, mszeredi
In-Reply-To: <9b8bf436-65de-13b9-0002-0479d11c18ca@gmail.com>
On Fri, Oct 12, 2018 at 03:49:50PM +0100, Alan Jenkins wrote:
> > +SYSCALL_DEFINE3(fspick, int, dfd, const char __user *, path, unsigned int, flags)
> > +{
> > + struct fs_context *fc;
> > + struct path target;
> > + unsigned int lookup_flags;
> > + int ret;
> > +
> > + if (!ns_capable(current->nsproxy->mnt_ns->user_ns, CAP_SYS_ADMIN))
> > + return -EPERM;
>
>
> This seems to accept basically any mount. Specifically: are you sure it's
> OK to return a handle to a SB_NO_USER superblock?
Umm... As long as we don't try to do pathname resolution from its ->s_root,
shouldn't be a problem and I don't see anything that would do that. I might've
missed something, but...
^ permalink raw reply
* Re: [PATCH v9 00/24] ILP32 for ARM64
From: Eugene Syromiatnikov @ 2018-10-13 2:14 UTC (permalink / raw)
To: Catalin Marinas
Cc: Yury Norov, Arnd Bergmann, linux-arm-kernel, linux-kernel,
linux-doc, linux-arch, linux-api, Adam Borowski, Alexander Graf,
Alexey Klimov, Andreas Schwab, Andrew Pinski, Bamvor Zhangjian,
Chris Metcalf, Christoph Muellner, Dave Martin, David S . Miller,
Florian Weimer, Geert Uytterhoeven, Heiko Carstens,
James Hogan <james.h>
In-Reply-To: <20181010153655.GA212880@arrakis.emea.arm.com>
On Wed, Oct 10, 2018 at 04:36:56PM +0100, Catalin Marinas wrote:
> On Wed, Oct 10, 2018 at 04:10:21PM +0200, Eugene Syromiatnikov wrote:
> > I have some questions regarding AArch64 ILP32 implementation for which I
> > failed to find an answer myself:
> > * How ptrace() tracer is supposed to distinguish between ILP32 and LP64
> > tracees? For MIPS N32 and x32 this is possible based on syscall
> > number, but for AArch64 ILP32 I do not see such a sign. There's also
> > ARM_ip is employed for signalling entering/exiting, I wonder whether
> > it's possible to employ it also for signalling tracee's personality.
>
> With the current implementation, I don't think you can distinguish. From
> the kernel perspective, the register set is the same. What is the
> use-case for this?
Err, a ptrace()-based tracer trying to trace a process, for example?
> We could add a new regset to expose the ILP32 state (NT_ARM_..., I can't
> think of a name now but probably not PER* as this implies PER_LINUX_...
> which is independent from TIF_32BIT_*).
So that would require an additional ptrace() call on each syscall stop,
is that correct?
> > * What's the reasoning behind capping syscall arguments to 32 bit? x32
> > and MIPS N32 do not have such a restriction (and do not need special
> > wrappers for syscalls that pass 64-bit values as a result, except
> > when they do, as it is the case for preadv2 on x32); moreover, that
> > would lead to insurmountable difficulties for AArch64 ILP32 tracers
> > that try to trace LP64 tracees, as it would be impossible to pass
> > 64-bit addresses to process_vm_{read,write} or ptrace PEEK/POKE.
>
> We've attempted in earlier versions to allow a mix of 32 and 64-bit
> register values from ILP32 but it got pretty complicated. The entry code
> would need to know which registers need zeroing of the top 32-bit
If kernel specifies 64-bit wide registers for syscalls, then it's the
caller's (libc's) responsibility to properly sign-extend arguments when
needed, and glibc, for example, already has proper type definitions that
aimed to handle this.
> and the generic unistd.h wrapper hacks were not very nice.
They are already implemented in glibc during x32 introduction period.
> Some past discussions:
>
> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1211716.html
>
> --
> Catalin
^ permalink raw reply
* Re: [PATCH v9 00/24] ILP32 for ARM64
From: Eugene Syromiatnikov @ 2018-10-13 2:07 UTC (permalink / raw)
To: Szabolcs Nagy
Cc: Yury Norov, nd, Catalin Marinas, Arnd Bergmann, linux-arm-kernel,
linux-kernel, linux-doc, linux-arch, linux-api, Adam Borowski,
Alexander Graf, Alexey Klimov, Andreas Schwab, Andrew Pinski,
Bamvor Zhangjian, Chris Metcalf, Christoph Muellner, Dave Martin,
David S . Miller, Florian Weimer, Geert Uytterhoeven, Heiko
In-Reply-To: <7aac1a08-8948-1a04-cbd3-fbc6a53f9ff0@arm.com>
On Wed, Oct 10, 2018 at 03:39:07PM +0100, Szabolcs Nagy wrote:
> On 10/10/18 15:10, Eugene Syromiatnikov wrote:
> > * What's the reasoning behind capping syscall arguments to 32 bit? x32
> > and MIPS N32 do not have such a restriction (and do not need special
> > wrappers for syscalls that pass 64-bit values as a result, except
> > when they do, as it is the case for preadv2 on x32); moreover, that
> > would lead to insurmountable difficulties for AArch64 ILP32 tracers
> > that try to trace LP64 tracees, as it would be impossible to pass
> > 64-bit addresses to process_vm_{read,write} or ptrace PEEK/POKE.
>
> but that's necessarily the case for all ilp32 abis:
> the userspace syscall function receives 32bit
> arguments so even if the kernel abi takes 64bit
> args you cannot use that from c code. (the libc
> does not even know which args should be sign or
> zero extended.)
glibc's syscall() prototype has kernel_ulong_t as its arguments (more
specifically, to __syscall_ulong_t, which is 64-bit wide on x32; it
should also have kernel_long_t as its return type instead of long,
but that's another story), so it works perfectly fine in case of x32.
> process_vm_readv/writev is limited by the ilp32
> iovec struct, not by the syscall arguments.
Right, on x32/N32 this issue is worked around by the usage of the respective
x86_64/N64 call, and it looks like another thing that is impossible
with AArch64 ilp32.
> ptrace is specified to take void* addr argument,
> and void* is 32bit on all ilp32 targets.
> so again on the c language level there is no
> way around the 32bit limitation.
Which is an issue.
^ permalink raw reply
* Re: [PATCH v5 07/27] mm/mmap: Create a guard area between VMAs
From: Yu-cheng Yu @ 2018-10-12 21:49 UTC (permalink / raw)
To: Andy Lutomirski, Jann Horn
Cc: X86 ML, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, LKML,
linux-doc, Linux-MM, linux-arch, Linux API, Arnd Bergmann,
Balbir Singh, Cyrill Gorcunov, Dave Hansen, Eugene Syromiatnikov,
Florian Weimer, H. J. Lu, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov,
Pavel Machek <pave>
In-Reply-To: <CALCETrUJ1t_K=FQExa_K0yg+aXkPot6wn6RHBPDc3BsAxtmMBw@mail.gmail.com>
On Thu, 2018-10-11 at 13:55 -0700, Andy Lutomirski wrote:
> On Thu, Oct 11, 2018 at 1:39 PM Jann Horn <jannh@google.com> wrote:
> >
> > On Thu, Oct 11, 2018 at 5:20 PM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> > > Create a guard area between VMAs to detect memory corruption.
> >
> > [...]
> > > +config VM_AREA_GUARD
> > > + bool "VM area guard"
> > > + default n
> > > + help
> > > + Create a guard area between VM areas so that access beyond
> > > + limit can be detected.
> > > +
> > > endmenu
> >
> > Sorry to bring this up so late, but Daniel Micay pointed out to me
> > that, given that VMA guards will raise the number of VMAs by
> > inhibiting vma_merge(), people are more likely to run into
> > /proc/sys/vm/max_map_count (which limits the number of VMAs to ~65k by
> > default, and can't easily be raised without risking an overflow of
> > page->_mapcount on systems with over ~800GiB of RAM, see
> > https://lore.kernel.org/lkml/20180208021112.GB14918@bombadil.infradead.org/
> > and replies) with this change.
> >
> > Playing with glibc's memory allocator, it looks like glibc will use
> > mmap() for 128KB allocations; so at 65530*128KB=8GB of memory usage in
> > 128KB chunks, an application could run out of VMAs.
>
> Ugh.
>
> Do we have a free VM flag so we could do VM_GUARD to force a guard
> page? (And to make sure that, when a new VMA is allocated, it won't
> be directly adjacent to a VM_GUARD VMA.)
Maybe something like the following? These vm_start_gap()/vm_end_gap() are used
in many architectures. Do we want to put them in a different series? Comments?
Yu-cheng
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 0416a7204be3..92b580542411 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -224,11 +224,13 @@ extern unsigned int kobjsize(const void *objp);
#define VM_HIGH_ARCH_BIT_2 34 /* bit only usable on 64-bit
architectures */
#define VM_HIGH_ARCH_BIT_3 35 /* bit only usable on 64-bit
architectures */
#define VM_HIGH_ARCH_BIT_4 36 /* bit only usable on 64-bit
architectures */
+#define VM_HIGH_ARCH_BIT_5 37 /* bit only usable on 64-bit
architectures */
#define VM_HIGH_ARCH_0 BIT(VM_HIGH_ARCH_BIT_0)
#define VM_HIGH_ARCH_1 BIT(VM_HIGH_ARCH_BIT_1)
#define VM_HIGH_ARCH_2 BIT(VM_HIGH_ARCH_BIT_2)
#define VM_HIGH_ARCH_3 BIT(VM_HIGH_ARCH_BIT_3)
#define VM_HIGH_ARCH_4 BIT(VM_HIGH_ARCH_BIT_4)
+#define VM_HIGH_ARCH_5 BIT(VM_HIGH_ARCH_BIT_5)
#endif /* CONFIG_ARCH_USES_HIGH_VMA_FLAGS */
#ifdef CONFIG_ARCH_HAS_PKEYS
@@ -266,6 +268,12 @@ extern unsigned int kobjsize(const void *objp);
# define VM_MPX VM_NONE
#endif
+#ifdef CONFIG_ARCH_USES_HIGH_VMA_FLAGS
+#define VM_GUARD VM_HIGH_ARCH_5
+#else
+#define VM_GUARD VM_NONE
+#endif
+
#ifndef VM_GROWSUP
# define VM_GROWSUP VM_NONE
#endif
@@ -2417,24 +2425,34 @@ static inline struct vm_area_struct *
find_vma_intersection(struct mm_struct * m
-static inline unsigned long vm_start_gap(struct vm_area_struct *vma)
+static inline unsigned long vm_start_gap(struct vm_area_struct *vma, vm_flags_t
flags)
{
unsigned long vm_start = vma->vm_start;
+ unsigned long gap = 0;
+
+ if (vma->vm_flags & VM_GROWSDOWN)
+ gap = stack_guard_gap;
+ else if ((vma->vm_flags & VM_GUARD) || (flags & VM_GUARD))
+ gap = PAGE_SIZE;
+
+ vm_start -= gap;
+ if (vm_start > vma->vm_start)
+ vm_start = 0;
- if (vma->vm_flags & VM_GROWSDOWN) {
- vm_start -= stack_guard_gap;
- if (vm_start > vma->vm_start)
- vm_start = 0;
- }
return vm_start;
}
-static inline unsigned long vm_end_gap(struct vm_area_struct *vma)
+static inline unsigned long vm_end_gap(struct vm_area_struct *vma, vm_flags_t
flags)
{
unsigned long vm_end = vma->vm_end;
+ unsigned long gap = 0;
+
+ if (vma->vm_flags & VM_GROWSUP)
+ gap = stack_guard_gap;
+ else if ((vma->vm_flags & VM_GUARD) || (flags & VM_GUARD))
+ gap = PAGE_SIZE;
+
+ vm_end += gap;
+ if (vm_end < vma->vm_end)
+ vm_end = -PAGE_SIZE;
- if (vma->vm_flags & VM_GROWSUP) {
- vm_end += stack_guard_gap;
- if (vm_end < vma->vm_end)
- vm_end = -PAGE_SIZE;
- }
return vm_end;
}
^ permalink raw reply related
* Re: [PATCH v7 3/6] seccomp: add a way to get a listener fd from ptrace
From: Christian Brauner @ 2018-10-12 20:11 UTC (permalink / raw)
To: Tycho Andersen
Cc: Andy Lutomirski, Paul Moore, Jann Horn, Kees Cook, Linux API,
Linux Containers, Akihiro Suda, Oleg Nesterov, LKML,
Eric W. Biederman, Linux FS Devel, Christian Brauner, LSM List,
SELinux-NSA, Stephen Smalley, Eric Paris
In-Reply-To: <20181012200220.GC5530@cisco>
On Fri, Oct 12, 2018 at 01:02:20PM -0700, Tycho Andersen wrote:
> On Thu, Oct 11, 2018 at 06:02:06PM -0700, Andy Lutomirski wrote:
> > On Thu, Oct 11, 2018 at 4:10 PM Paul Moore <paul@paul-moore.com> wrote:
> > >
> > > On October 11, 2018 9:40:06 AM Jann Horn <jannh@google.com> wrote:
> > > > On Thu, Oct 11, 2018 at 9:24 AM Paul Moore <paul@paul-moore.com> wrote:
> > > >> On October 10, 2018 11:34:11 AM Jann Horn <jannh@google.com> wrote:
> > > >>> On Wed, Oct 10, 2018 at 5:32 PM Paul Moore <paul@paul-moore.com> wrote:
> > > >>>> On Tue, Oct 9, 2018 at 9:36 AM Jann Horn <jannh@google.com> wrote:
> > > >>>>> +cc selinux people explicitly, since they probably have opinions on this
> > > >>>>
> > > >>>> I just spent about twenty minutes working my way through this thread,
> > > >>>> and digging through the containers archive trying to get a good
> > > >>>> understanding of what you guys are trying to do, and I'm not quite
> > > >>>> sure I understand it all. However, from what I have seen, this
> > > >>>> approach looks very ptrace-y to me (I imagine to others as well based
> > > >>>> on the comments) and because of this I think ensuring the usual ptrace
> > > >>>> access controls are evaluated, including the ptrace LSM hooks, is the
> > > >>>> right thing to do.
> > > >>>
> > > >>> Basically the problem is that this new ptrace() API does something
> > > >>> that doesn't just influence the target task, but also every other task
> > > >>> that has the same seccomp filter. So the classic ptrace check doesn't
> > > >>> work here.
> > > >>
> > > >> Due to some rather unfortunate events today I'm suddenly without easy access to the kernel code, but would it be possible to run the LSM ptrace access control checks against all of the affected tasks? If it is possible, how painful would it be?
> > > >
> > > > There are currently no backlinks from seccomp filters to the tasks
> > > > that use them; the only thing you have is a refcount. If the refcount
> > > > is 1, and the target task uses the filter directly (it is the last
> > > > installed one), you'd be able to infer that the ptrace target is the
> > > > only task with a reference to the filter, and you could just do the
> > > > direct check; but if the refcount is >1, you might end up having to
> > > > take some spinlock and then iterate over all tasks' filters with that
> > > > spinlock held, or something like that.
> > >
> > > That's what I was afraid of.
> > >
> > > Unfortunately, I stand by my previous statements that we still probably want a LSM access check similar to what we currently do for ptrace.
> > >
> >
> > I would argue that once "LSM" enters this conversation, it just means
> > we're on the wrong track. Let's try to make this work without ptrace
> > if possible :) The whole seccomp() mechanism is very carefully
> > designed so that it's perfectly safe to install seccomp filters
> > without involving LSM or even involving credentials at all.
>
> In a last ditch effort to save the ptrace() interface: can we just
> only allow it when refcount_read(filter->usage) == 1?
I mean, the filter->usage == 1 means lets us get rid of
capable(CAP_SYS_ADMIN) making the ptrace() way of getting an fd useable
in nesting scenarios and from within user namespaces. That makes it a
whole lot more useful and aligns it with the seccomp() way of getting
the fd. So I wouldn't argue against it.
I guess it comes down to (for me) whether you consider this a necessary
part of this patchset aka meaning without it it wouldn't be useable.
Christian
^ permalink raw reply
* Re: [PATCH v7 3/6] seccomp: add a way to get a listener fd from ptrace
From: Jann Horn @ 2018-10-12 20:06 UTC (permalink / raw)
To: Tycho Andersen
Cc: Andy Lutomirski, Paul Moore, christian, Kees Cook, Linux API,
containers, suda.akihiro, Oleg Nesterov, kernel list,
Eric W. Biederman, linux-fsdevel, Christian Brauner,
linux-security-module, selinux, Stephen Smalley, Eric Paris
In-Reply-To: <20181012200220.GC5530@cisco>
On Fri, Oct 12, 2018 at 10:02 PM Tycho Andersen <tycho@tycho.ws> wrote:
> On Thu, Oct 11, 2018 at 06:02:06PM -0700, Andy Lutomirski wrote:
> > On Thu, Oct 11, 2018 at 4:10 PM Paul Moore <paul@paul-moore.com> wrote:
> > >
> > > On October 11, 2018 9:40:06 AM Jann Horn <jannh@google.com> wrote:
> > > > On Thu, Oct 11, 2018 at 9:24 AM Paul Moore <paul@paul-moore.com> wrote:
> > > >> On October 10, 2018 11:34:11 AM Jann Horn <jannh@google.com> wrote:
> > > >>> On Wed, Oct 10, 2018 at 5:32 PM Paul Moore <paul@paul-moore.com> wrote:
> > > >>>> On Tue, Oct 9, 2018 at 9:36 AM Jann Horn <jannh@google.com> wrote:
> > > >>>>> +cc selinux people explicitly, since they probably have opinions on this
> > > >>>>
> > > >>>> I just spent about twenty minutes working my way through this thread,
> > > >>>> and digging through the containers archive trying to get a good
> > > >>>> understanding of what you guys are trying to do, and I'm not quite
> > > >>>> sure I understand it all. However, from what I have seen, this
> > > >>>> approach looks very ptrace-y to me (I imagine to others as well based
> > > >>>> on the comments) and because of this I think ensuring the usual ptrace
> > > >>>> access controls are evaluated, including the ptrace LSM hooks, is the
> > > >>>> right thing to do.
> > > >>>
> > > >>> Basically the problem is that this new ptrace() API does something
> > > >>> that doesn't just influence the target task, but also every other task
> > > >>> that has the same seccomp filter. So the classic ptrace check doesn't
> > > >>> work here.
> > > >>
> > > >> Due to some rather unfortunate events today I'm suddenly without easy access to the kernel code, but would it be possible to run the LSM ptrace access control checks against all of the affected tasks? If it is possible, how painful would it be?
> > > >
> > > > There are currently no backlinks from seccomp filters to the tasks
> > > > that use them; the only thing you have is a refcount. If the refcount
> > > > is 1, and the target task uses the filter directly (it is the last
> > > > installed one), you'd be able to infer that the ptrace target is the
> > > > only task with a reference to the filter, and you could just do the
> > > > direct check; but if the refcount is >1, you might end up having to
> > > > take some spinlock and then iterate over all tasks' filters with that
> > > > spinlock held, or something like that.
> > >
> > > That's what I was afraid of.
> > >
> > > Unfortunately, I stand by my previous statements that we still probably want a LSM access check similar to what we currently do for ptrace.
> > >
> >
> > I would argue that once "LSM" enters this conversation, it just means
> > we're on the wrong track. Let's try to make this work without ptrace
> > if possible :) The whole seccomp() mechanism is very carefully
> > designed so that it's perfectly safe to install seccomp filters
> > without involving LSM or even involving credentials at all.
>
> In a last ditch effort to save the ptrace() interface: can we just
> only allow it when refcount_read(filter->usage) == 1?
>From a security perspective, I think that would be fine, assuming that
we know that the target task is stopped. (But note that if the target
process e.g. uses the filter on multiple threads, the refcount will be
higher.)
^ permalink raw reply
* Re: [PATCH v7 3/6] seccomp: add a way to get a listener fd from ptrace
From: Tycho Andersen @ 2018-10-12 20:02 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Paul Moore, Jann Horn, Christian Brauner, Kees Cook, Linux API,
Linux Containers, Akihiro Suda, Oleg Nesterov, LKML,
Eric W. Biederman, Linux FS Devel, Christian Brauner, LSM List,
SELinux-NSA, Stephen Smalley, Eric Paris
In-Reply-To: <CALCETrV1fWKJe96ow3XGy_kmZoXoehb99e4AZehsO84YGxKjgQ@mail.gmail.com>
On Thu, Oct 11, 2018 at 06:02:06PM -0700, Andy Lutomirski wrote:
> On Thu, Oct 11, 2018 at 4:10 PM Paul Moore <paul@paul-moore.com> wrote:
> >
> > On October 11, 2018 9:40:06 AM Jann Horn <jannh@google.com> wrote:
> > > On Thu, Oct 11, 2018 at 9:24 AM Paul Moore <paul@paul-moore.com> wrote:
> > >> On October 10, 2018 11:34:11 AM Jann Horn <jannh@google.com> wrote:
> > >>> On Wed, Oct 10, 2018 at 5:32 PM Paul Moore <paul@paul-moore.com> wrote:
> > >>>> On Tue, Oct 9, 2018 at 9:36 AM Jann Horn <jannh@google.com> wrote:
> > >>>>> +cc selinux people explicitly, since they probably have opinions on this
> > >>>>
> > >>>> I just spent about twenty minutes working my way through this thread,
> > >>>> and digging through the containers archive trying to get a good
> > >>>> understanding of what you guys are trying to do, and I'm not quite
> > >>>> sure I understand it all. However, from what I have seen, this
> > >>>> approach looks very ptrace-y to me (I imagine to others as well based
> > >>>> on the comments) and because of this I think ensuring the usual ptrace
> > >>>> access controls are evaluated, including the ptrace LSM hooks, is the
> > >>>> right thing to do.
> > >>>
> > >>> Basically the problem is that this new ptrace() API does something
> > >>> that doesn't just influence the target task, but also every other task
> > >>> that has the same seccomp filter. So the classic ptrace check doesn't
> > >>> work here.
> > >>
> > >> Due to some rather unfortunate events today I'm suddenly without easy access to the kernel code, but would it be possible to run the LSM ptrace access control checks against all of the affected tasks? If it is possible, how painful would it be?
> > >
> > > There are currently no backlinks from seccomp filters to the tasks
> > > that use them; the only thing you have is a refcount. If the refcount
> > > is 1, and the target task uses the filter directly (it is the last
> > > installed one), you'd be able to infer that the ptrace target is the
> > > only task with a reference to the filter, and you could just do the
> > > direct check; but if the refcount is >1, you might end up having to
> > > take some spinlock and then iterate over all tasks' filters with that
> > > spinlock held, or something like that.
> >
> > That's what I was afraid of.
> >
> > Unfortunately, I stand by my previous statements that we still probably want a LSM access check similar to what we currently do for ptrace.
> >
>
> I would argue that once "LSM" enters this conversation, it just means
> we're on the wrong track. Let's try to make this work without ptrace
> if possible :) The whole seccomp() mechanism is very carefully
> designed so that it's perfectly safe to install seccomp filters
> without involving LSM or even involving credentials at all.
In a last ditch effort to save the ptrace() interface: can we just
only allow it when refcount_read(filter->usage) == 1?
Tycho
^ permalink raw reply
* Re: [PATCH 31/34] vfs: syscall: Add fspick() to select a superblock for reconfiguration [ver #12]
From: Alan Jenkins @ 2018-10-12 14:49 UTC (permalink / raw)
To: David Howells, viro
Cc: linux-api, torvalds, ebiederm, linux-fsdevel, linux-kernel,
mszeredi
In-Reply-To: <153754766004.17872.9829232103614083565.stgit@warthog.procyon.org.uk>
On 21/09/2018 17:34, David Howells wrote:
> Provide an fspick() system call that can be used to pick an existing
> mountpoint into an fs_context which can thereafter be used to reconfigure a
> superblock (equivalent of the superblock side of -o remount).
>
> This looks like:
>
> int fd = fspick(AT_FDCWD, "/mnt",
> FSPICK_CLOEXEC | FSPICK_NO_AUTOMOUNT);
> fsconfig(fd, FSCONFIG_SET_FLAG, "intr", NULL, 0);
> fsconfig(fd, FSCONFIG_SET_FLAG, "noac", NULL, 0);
> fsconfig(fd, FSCONFIG_CMD_RECONFIGURE, NULL, NULL, 0);
>
> At the point of fspick being called, the file descriptor referring to the
> filesystem context is in exactly the same state as the one that was created
> by fsopen() after fsmount() has been successfully called.
>
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: linux-api@vger.kernel.org
> ---
>
> arch/x86/entry/syscalls/syscall_32.tbl | 1 +
> arch/x86/entry/syscalls/syscall_64.tbl | 1 +
> fs/fsopen.c | 54 ++++++++++++++++++++++++++++++++
> include/linux/syscalls.h | 1 +
> include/uapi/linux/fs.h | 5 +++
> 5 files changed, 62 insertions(+)
>
> diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
> index c78b68256f8a..d1eb6c815790 100644
> --- a/arch/x86/entry/syscalls/syscall_32.tbl
> +++ b/arch/x86/entry/syscalls/syscall_32.tbl
> @@ -403,3 +403,4 @@
> 389 i386 fsopen sys_fsopen __ia32_sys_fsopen
> 390 i386 fsconfig sys_fsconfig __ia32_sys_fsconfig
> 391 i386 fsmount sys_fsmount __ia32_sys_fsmount
> +392 i386 fspick sys_fspick __ia32_sys_fspick
> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
> index d44ead5d4368..d3ab703c02bb 100644
> --- a/arch/x86/entry/syscalls/syscall_64.tbl
> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> @@ -348,6 +348,7 @@
> 337 common fsopen __x64_sys_fsopen
> 338 common fsconfig __x64_sys_fsconfig
> 339 common fsmount __x64_sys_fsmount
> +340 common fspick __x64_sys_fspick
>
> #
> # x32-specific system call numbers start at 512 to avoid cache impact
> diff --git a/fs/fsopen.c b/fs/fsopen.c
> index 5955a6b65596..9ead9220e2cb 100644
> --- a/fs/fsopen.c
> +++ b/fs/fsopen.c
> @@ -155,6 +155,60 @@ SYSCALL_DEFINE2(fsopen, const char __user *, _fs_name, unsigned int, flags)
> return ret;
> }
>
> +/*
> + * Pick a superblock into a context for reconfiguration.
> + */
> +SYSCALL_DEFINE3(fspick, int, dfd, const char __user *, path, unsigned int, flags)
> +{
> + struct fs_context *fc;
> + struct path target;
> + unsigned int lookup_flags;
> + int ret;
> +
> + if (!ns_capable(current->nsproxy->mnt_ns->user_ns, CAP_SYS_ADMIN))
> + return -EPERM;
This seems to accept basically any mount. Specifically: are you sure
it's OK to return a handle to a SB_NO_USER superblock?
# strace -f -v -e trace=154 \
./fspick 3</proc/self/ns/mnt 3 \
stat -f /dev/fd/3
syscall_0x154(0x3, 0x4009a1, 0x8, ...) = 0x4
File: "/dev/fd/3"
ID: 0 Namelen: 255 Type: anon-inode FS
Block size: 4096 Fundamental block size: 4096
Blocks: Total: 0 Free: 0 Available: 0
Inodes: Total: 0 Free: 0
+++ exited with 0 +++
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox