All of lore.kernel.org
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linuxfoundation.org>
To: Josef Bacik <josef@toxicpanda.com>
Cc: ansgar.loesser@kom.tu-darmstadt.de,
	"Darrick J. Wong" <djwong@kernel.org>,
	"Christoph Hellwig" <hch@lst.de>,
	"Amir Goldstein" <amir73il@gmail.com>,
	"Mark Fasheh" <mark@fasheh.com>,
	"Matthew Wilcox" <willy@infradead.org>,
	"Miklos Szeredi" <mszeredi@redhat.com>,
	"Al Viro" <viro@zeniv.linux.org.uk>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	"Security Officers" <security@kernel.org>,
	"Max Schlecht" <max.schlecht@informatik.hu-berlin.de>,
	"Björn Scheuermann" <scheuermann@kom.tu-darmstadt.de>
Subject: Re: Information Leak: FIDEDUPERANGE ioctl allows reading writeonly files
Date: Tue, 12 Jul 2022 12:07:46 -0700	[thread overview]
Message-ID: <CAHk-=wgEgAjX5gRntm0NutaNtjkzN+OaJVMaJAqved4dxPtAqw@mail.gmail.com> (raw)
In-Reply-To: <CAEzrpqdweuZ2ufMKDJwSzP5W021F7mgS+7toSo6VDgvDzd0ZqA@mail.gmail.com>

On Tue, Jul 12, 2022 at 12:02 PM Josef Bacik <josef@toxicpanda.com> wrote:
>
> > Any permission checks done at IO time are basically always buggy:
> > things may have changed since the 'open()', and those changes
> > explicitly should *not* matter for the IO. That's really fundamentally
> > how UNIX file permissions work.
>
> I don't think we should go this far, after all the normal
> write()/read() syscalls do the permission checking each time as well,

No, they really don't.

The permission check is ONLY DONE AT OPEN TIME.

Really. Go look.

Anything else is a bug. If you open a file, and then change the
permissions of the file (or the ownership, or whatever) afterwards,
the open file descriptor is still supposed to be readable or writable.

Doing IO time permission checks is not only wrong, it's ACTIVELY
BUGGY, and is a fairly common source of security problems (ie passing
a file descriptor off to a suid binary, and then using the suid
permissions to make changes that the original opener didn't have the
rights to do).

So if you do permission checks at read/write time, you are a buggy
mess.  It really is that simple.

This is why read and write check FMODE_READ and FMODE_WRITE. That's
the *open* time check.

The fact that dedupe does that inode_permission() check at IO time
really looks completely bogus and buggy.

                 Linus

  reply	other threads:[~2022-07-12 19:32 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-12 12:11 Information Leak: FIDEDUPERANGE ioctl allows reading writeonly files Ansgar Lößer
2022-07-12 17:33 ` Linus Torvalds
2022-07-12 18:43   ` Matthew Wilcox
2022-07-12 18:47     ` Linus Torvalds
2022-07-12 18:51       ` Linus Torvalds
2022-07-12 19:02   ` Josef Bacik
2022-07-12 19:07     ` Linus Torvalds [this message]
2022-07-12 19:23       ` Linus Torvalds
2022-07-12 20:03       ` Josef Bacik
2022-07-12 20:48         ` Linus Torvalds
2022-07-13  0:48           ` Darrick J. Wong
2022-07-13  2:58             ` Linus Torvalds
2022-07-13  4:14               ` Linus Torvalds
2022-07-13  6:46               ` Dave Chinner
2022-07-13  7:49                 ` [PATCH] fs/remap: constrain dedupe of EOF blocks Dave Chinner
2022-07-13  8:19                   ` Linus Torvalds
2022-07-13 17:18                   ` Ansgar Lößer
2022-07-13 17:26                     ` Linus Torvalds
2022-07-13 18:51                       ` [PATCH] vf/remap: return the amount of bytes actually deduplicated Ansgar Lößer
2022-07-13 19:09                         ` Linus Torvalds
2022-07-14  0:22                         ` Dave Chinner
2022-07-14  1:03                           ` Linus Torvalds
2022-07-16 21:15                           ` Mark Fasheh
2022-07-14 22:32                         ` Dave Chinner
2022-07-14 22:42                           ` Linus Torvalds
2022-07-14 23:15                             ` Dave Chinner
2022-07-13  8:16                 ` Information Leak: FIDEDUPERANGE ioctl allows reading writeonly files Linus Torvalds
2022-07-13 23:48                   ` Dave Chinner
2022-07-13 17:17                 ` Ansgar Lößer
2022-07-13 17:16             ` Ansgar Lößer
2022-07-13 22:43               ` Dave Chinner
2022-07-13 17:14   ` Ansgar Lößer
2022-07-13 18:03     ` Linus Torvalds

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAHk-=wgEgAjX5gRntm0NutaNtjkzN+OaJVMaJAqved4dxPtAqw@mail.gmail.com' \
    --to=torvalds@linuxfoundation.org \
    --cc=amir73il@gmail.com \
    --cc=ansgar.loesser@kom.tu-darmstadt.de \
    --cc=djwong@kernel.org \
    --cc=hch@lst.de \
    --cc=josef@toxicpanda.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=mark@fasheh.com \
    --cc=max.schlecht@informatik.hu-berlin.de \
    --cc=mszeredi@redhat.com \
    --cc=scheuermann@kom.tu-darmstadt.de \
    --cc=security@kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.