On 12/03/2022 16:17, Paul Moore wrote:
On Fri, Mar 11, 2022 at 8:35 PM Tetsuo Handa
<penguin-kernel@xxxxxxxxxxxxxxxxxxx> wrote:
On 2022/03/12 7:15, Paul Moore wrote:
The silence on this has been deafening :/ No thoughts on fixing, or
not fixing OPEN_FMODE(), Al?
On 2022/03/01 19:15, Mickaël Salaün wrote:
On 01/03/2022 10:22, Christian Brauner wrote:
That specific part seems a bit risky at first glance. Given that the
patch referenced is from 2009 this means we've been allowing O_WRONLY |
O_RDWR to succeed for almost 13 years now.
Yeah, it's an old bug, but we should keep in mind that a file descriptor
created with such flags cannot be used to read nor write. However,
unfortunately, it can be used for things like ioctl, fstat, chdir… I
don't know if there is any user of this trick.
I got a reply from Al at https://lkml.kernel.org/r/20090212032821.GD28946@xxxxxxxxxxxxxxxxxx
that sys_open(path, 3) is for ioctls only. And I'm using this trick when opening something
for ioctls only.
Thanks Tetsuo, that's helpful. After reading your email I went
digging around to see if this was documented anywhere, and buried in
the open(2) manpage, towards the bottom under the "File access mode"
header, is this paragraph:
"Linux reserves the special, nonstandard access mode 3 (binary 11)
in flags to mean: check for read and write permission on the file
and return a file descriptor that can't be used for reading or
writing. This nonstandard access mode is used by some Linux
drivers to return a file descriptor that is to be used only for
device-specific ioctl(2) operations."
Interesting, I missed the reference to this special value in the man page.
I learned something new today :) With this in mind it looks like
doing a SELinux file:ioctl check is the correct thing to do.
Indeed, SELinux uses it in an early ioctl check, but it still seems
inconsistent (without being a bug) with the handling of the other value
of this flag. This FD can also be used for chdir or other inode-related
actions, which may not involve ioctl.
However, it seems there is a more visible inconsistency with the way
Tomoyo checks for read, write (because of the ACC_MODE use) *and* ioctl
rights for an ioctl action. At least, the semantic is not the same and
is not reflected in the documentation.
Because AppArmor and Landlock don't support ioctl, this looks fine for them.