All of lore.kernel.org
 help / color / mirror / Atom feed
From: Omar Sandoval <osandov@osandov.com>
To: Josef Bacik <josef@toxicpanda.com>
Cc: linux-fsdevel@vger.kernel.org, linux-btrfs@vger.kernel.org,
	Al Viro <viro@zeniv.linux.org.uk>,
	Christoph Hellwig <hch@infradead.org>,
	Dave Chinner <david@fromorbit.com>, Jann Horn <jannh@google.com>,
	Amir Goldstein <amir73il@gmail.com>,
	Aleksa Sarai <cyphar@cyphar.com>,
	linux-api@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH v5 2/9] fs: add O_ALLOW_ENCODED open flag
Date: Mon, 24 Aug 2020 14:11:15 -0700	[thread overview]
Message-ID: <20200824211115.GB197795@exodia.localdomain> (raw)
In-Reply-To: <3c44cdf2-e0fd-cea1-4028-d6315ba3b7fd@toxicpanda.com>

On Mon, Aug 24, 2020 at 02:28:39PM -0400, Josef Bacik wrote:
> On 8/21/20 3:38 AM, Omar Sandoval wrote:
> > From: Omar Sandoval <osandov@fb.com>
> > 
> > The upcoming RWF_ENCODED operation introduces some security concerns:
> > 
> > 1. Compressed writes will pass arbitrary data to decompression
> >     algorithms in the kernel.
> > 2. Compressed reads can leak truncated/hole punched data.
> > 
> > Therefore, we need to require privilege for RWF_ENCODED. It's not
> > possible to do the permissions checks at the time of the read or write
> > because, e.g., io_uring submits IO from a worker thread. So, add an open
> > flag which requires CAP_SYS_ADMIN. It can also be set and cleared with
> > fcntl(). The flag is not cleared in any way on fork or exec; it should
> > probably be used with O_CLOEXEC in most cases.
> > 
> > Note that the usual issue that unknown open flags are ignored doesn't
> > really matter for O_ALLOW_ENCODED; if the kernel doesn't support
> > O_ALLOW_ENCODED, then it doesn't support RWF_ENCODED, either.
> 
> It seemed like you agreed to require O_CLOEXEC to be set when using
> O_ALLOW_ENCODED in your last go around, what happened to that?  I know I'd
> feel better if we had that requirement, and if we aren't I'd like to know
> why we can't.  Thanks,
> 
> Josef

Yup I was still on the fence about it since it's a bit of an awkward
requirement, but I'm convinced now that we might as well be safe and
require it.

  reply	other threads:[~2020-08-24 21:11 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-21  7:38 [PATCH v5 0/9] fs: interface for directly reading/writing compressed data Omar Sandoval
2020-08-21  7:38 ` [PATCH man-pages v5] Document encoded I/O Omar Sandoval
2020-08-21  9:24   ` Amir Goldstein
2020-08-24 18:15     ` Omar Sandoval
2020-08-21  7:38 ` [PATCH v5 1/9] iov_iter: add copy_struct_from_iter() Omar Sandoval
2020-08-24 18:52   ` Josef Bacik
2020-08-24 21:09     ` Omar Sandoval
2020-08-21  7:38 ` [PATCH v5 2/9] fs: add O_ALLOW_ENCODED open flag Omar Sandoval
2020-08-24 18:28   ` Josef Bacik
2020-08-24 21:11     ` Omar Sandoval [this message]
2020-08-21  7:38 ` [PATCH v5 3/9] fs: add RWF_ENCODED for reading/writing compressed data Omar Sandoval
2020-08-21  8:47   ` Amir Goldstein
2020-08-24 23:49     ` Omar Sandoval
2020-08-25  8:25       ` Amir Goldstein
2020-08-25 17:20         ` Omar Sandoval
2020-08-24 19:07   ` Josef Bacik
2020-08-21  7:38 ` [PATCH v5 4/9] btrfs: don't advance offset for compressed bios in btrfs_csum_one_bio() Omar Sandoval
2020-08-24 19:17   ` Josef Bacik
2020-08-21  7:38 ` [PATCH v5 5/9] btrfs: add ram_bytes and offset to btrfs_ordered_extent Omar Sandoval
2020-08-24 19:23   ` Josef Bacik
2020-08-21  7:38 ` [PATCH v5 6/9] btrfs: support different disk extent size for delalloc Omar Sandoval
2020-08-24 19:26   ` Josef Bacik
2020-08-21  7:38 ` [PATCH v5 7/9] btrfs: optionally extend i_size in cow_file_range_inline() Omar Sandoval
2020-08-24 19:33   ` Josef Bacik
2020-08-21  7:38 ` [PATCH v5 8/9] btrfs: implement RWF_ENCODED reads Omar Sandoval
2020-08-24 19:54   ` Josef Bacik
2020-08-24 21:23     ` Omar Sandoval
2020-08-21  7:38 ` [PATCH v5 9/9] btrfs: implement RWF_ENCODED writes Omar Sandoval
2020-08-24 20:30   ` Josef Bacik
2020-08-24 21:30     ` Omar Sandoval

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=20200824211115.GB197795@exodia.localdomain \
    --to=osandov@osandov.com \
    --cc=amir73il@gmail.com \
    --cc=cyphar@cyphar.com \
    --cc=david@fromorbit.com \
    --cc=hch@infradead.org \
    --cc=jannh@google.com \
    --cc=josef@toxicpanda.com \
    --cc=kernel-team@fb.com \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /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.