On Tue, Feb 28, 2017, at 02:23 PM, Eric Blake wrote:
> Might also be worth mentioning that this patch is required in order to
> solve CVE-2016-9602, per discussion at
> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg06089.html
I only briefly looked at this, but can't `open(..., O_PATH)` be used to solve
this today?
On 04/11/2017 12:52 PM, Colin Walters wrote:
>
>
> On Tue, Feb 28, 2017, at 02:23 PM, Eric Blake wrote:
>
>> Might also be worth mentioning that this patch is required in order to
>> solve CVE-2016-9602, per discussion at
>> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg06089.html
>
> I only briefly looked at this, but can't `open(..., O_PATH)` be used to solve
> this today?
O_PATH was the fallback that qemu used - but that's non-POSIX, which
means we have to have a different solution for POSIX systems than for
Linux systems, while waiting for Linux to catch up to POSIX.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
On 04/11/2017 12:55 PM, Eric Blake wrote:
> On 04/11/2017 12:52 PM, Colin Walters wrote:
>>
>>
>> On Tue, Feb 28, 2017, at 02:23 PM, Eric Blake wrote:
>>
>>> Might also be worth mentioning that this patch is required in order to
>>> solve CVE-2016-9602, per discussion at
>>> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg06089.html
>>
>> I only briefly looked at this, but can't `open(..., O_PATH)` be used to solve
>> this today?
>
> O_PATH was the fallback that qemu used
Hmm - actually, qemu used O_PATH for the directory portion of *at
traversals:
git.qemu-project.org/?p=qemu.git;a=commitdiff;h=918112c
but did not use O_PATH for its chmod() fallback:
git.qemu-project.org/?p=qemu.git;a=commitdiff;h=e3187a4
A good idea on the surface. But reading the man page of openat(), the
section on O_PATH says:
The file
itself is not opened, and other file operations (e.g.,
read(2),
write(2), fchmod(2), fchown(2), fgetxattr(2), mmap(2))
fail with
the error EBADF.
> - but that's non-POSIX, which
> means we have to have a different solution for POSIX systems than for
> Linux systems, while waiting for Linux to catch up to POSIX.
But even if using open(O_PATH)/fchmod() works, it is not immediately
obvious whether it can catch all the same cases that chmodat(O_NOFOLLOW)
would cover, as there are cases where you have permissions to change
mode bits but not open() the file for reading or writing. And even if
it gets rid of a TOCTTOU race, it still is a 2-syscall hit rather than
an atomic single syscall.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
On 04/11/2017 01:07 PM, Eric Blake wrote:
>
> But even if using open(O_PATH)/fchmod() works, it is not immediately
> obvious whether it can catch all the same cases that chmodat(O_NOFOLLOW)
Typo; I obviously meant fchmodat(AT_SYMLINK_NOFOLLOW)
> would cover, as there are cases where you have permissions to change
> mode bits but not open() the file for reading or writing. And even if
> it gets rid of a TOCTTOU race, it still is a 2-syscall hit rather than
> an atomic single syscall.
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
On Tue, Apr 11, 2017, at 02:07 PM, Eric Blake wrote:
>
> A good idea on the surface. But reading the man page of openat(), the
> section on O_PATH says:
> The file
> itself is not opened, and other file operations (e.g.,
> read(2),
> write(2), fchmod(2), fchown(2), fgetxattr(2), mmap(2))
> fail with
> the error EBADF.
Right, though more topically I'd have expected
fchmodat() (not fchmod()) to take AT_EMPTY_PATH,
just like fstatat() does.
But it doesn't appear to be supported...oh, even at
the syscall level, interesting. Ah, I see, glibc does:
int
fchmodat (int fd, const char *file, mode_t mode, int flag)
{
if (flag & ~AT_SYMLINK_NOFOLLOW)
return INLINE_SYSCALL_ERROR_RETURN_VALUE (EINVAL);
...
}
And indeed the syscall doesn't have flags, bringing us back
to the start here. Sorry, that seems obvious in retrospect,
but I was "working forwards" from the O_PATH userspace API
mindset.
On Tue, 11 Apr 2017 15:09:40 -0400
Colin Walters <[email protected]> wrote:
> On Tue, Apr 11, 2017, at 02:07 PM, Eric Blake wrote:
> >
> > A good idea on the surface. But reading the man page of openat(), the
> > section on O_PATH says:
> > The file
> > itself is not opened, and other file operations (e.g.,
> > read(2),
> > write(2), fchmod(2), fchown(2), fgetxattr(2), mmap(2))
> > fail with
> > the error EBADF.
>
> Right, though more topically I'd have expected
> fchmodat() (not fchmod()) to take AT_EMPTY_PATH,
> just like fstatat() does.
>
Like Eric said in another mail, this would still require to open() the file
first... ie, we cannot change mode if initial bits are 0000, whereas it
succeeds with chmod().
> But it doesn't appear to be supported...oh, even at
> the syscall level, interesting. Ah, I see, glibc does:
>
> int
> fchmodat (int fd, const char *file, mode_t mode, int flag)
> {
> if (flag & ~AT_SYMLINK_NOFOLLOW)
> return INLINE_SYSCALL_ERROR_RETURN_VALUE (EINVAL);
> ...
> }
>
> And indeed the syscall doesn't have flags, bringing us back
> to the start here. Sorry, that seems obvious in retrospect,
> but I was "working forwards" from the O_PATH userspace API
> mindset.
>
>
The use case is to fix CVE-2016-9602 in QEMU. We need to be able to change
the mode bits of a file that resides under a specific directory, which is
shared between the host and the guest.
Since untrusted code in a guest can create symlinks, we need to be sure
that the file isn't a symlink, otherwise the mode bit change could affect
an arbitrary file not residing under the shared directory.
This could be handled with chroot() or unshare()+chdir() but this isn't an
option because we want this to work even if QEMU is unprivileged.
According to POSIX, this is exactly how fchmodat(AT_SYMLINK_NOFOLLOW) should
behave on Linux:
[EOPNOTSUPP]
The AT_SYMLINK_NOFOLLOW bit is set in the flag argument, path names a
symbolic link, and the system does not support changing the mode of a
symbolic link.
I hope this is clear enough.
--
Greg