From: Jann Horn <jannh@google.com>
To: Rich Felker <dalias@libc.org>
Cc: linux-fsdevel <linux-fsdevel@vger.kernel.org>,
kernel list <linux-kernel@vger.kernel.org>,
Linux API <linux-api@vger.kernel.org>,
Alexander Viro <viro@zeniv.linux.org.uk>
Subject: Re: [RESEND PATCH] vfs: add RWF_NOAPPEND flag for pwritev2
Date: Sun, 30 Aug 2020 20:31:36 +0200 [thread overview]
Message-ID: <CAG48ez00caDqMomv+PF4dntJkWx7rNYf3E+8gufswis6UFSszw@mail.gmail.com> (raw)
In-Reply-To: <20200830163657.GD3265@brightrain.aerifal.cx>
On Sun, Aug 30, 2020 at 6:36 PM Rich Felker <dalias@libc.org> wrote:
> On Sun, Aug 30, 2020 at 05:05:45PM +0200, Jann Horn wrote:
> > On Sat, Aug 29, 2020 at 4:00 AM Rich Felker <dalias@libc.org> wrote:
> > > The pwrite function, originally defined by POSIX (thus the "p"), is
> > > defined to ignore O_APPEND and write at the offset passed as its
> > > argument. However, historically Linux honored O_APPEND if set and
> > > ignored the offset. This cannot be changed due to stability policy,
> > > but is documented in the man page as a bug.
> > >
> > > Now that there's a pwritev2 syscall providing a superset of the pwrite
> > > functionality that has a flags argument, the conforming behavior can
> > > be offered to userspace via a new flag.
[...]
> > Linux enforces the S_APPEND flag (set by "chattr +a") only at open()
> > time, not at write() time:
[...]
> > It seems to me like your patch will permit bypassing S_APPEND by
> > opening an append-only file with O_WRONLY|O_APPEND, then calling
> > pwritev2() with RWF_NOAPPEND? I think you'll have to add an extra
> > check for IS_APPEND() somewhere.
> >
> >
> > One could also argue that if an O_APPEND file descriptor is handed
> > across privilege boundaries, a programmer might reasonably expect that
> > the recipient will not be able to use the file descriptor for
> > non-append writes; if that is not actually true, that should probably
> > be noted in the open.2 manpage, at the end of the description of
> > O_APPEND.
>
> fcntl F_SETFL can remove O_APPEND, so it is not a security boundary.
> I'm not sure how this interacts with S_APPEND; presumably fcntl
> rechecks it.
Ah, good point, you're right. In fs/fcntl.c:
35 static int setfl(int fd, struct file * filp, unsigned long arg)
36 {
37 struct inode * inode = file_inode(filp);
38 int error = 0;
39
40 /*
41 * O_APPEND cannot be cleared if the file is marked as append-only
42 * and the file is open for write.
43 */
44 if (((arg ^ filp->f_flags) & O_APPEND) && IS_APPEND(inode))
45 return -EPERM;
> So just checking IS_APPEND in the code paths used by
> pwritev2 (and erroring out rather than silently writing output at the
> wrong place) should suffice to preserve all existing security
> invariants.
Makes sense.
next prev parent reply other threads:[~2020-08-30 18:32 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-29 2:00 [RESEND PATCH] vfs: add RWF_NOAPPEND flag for pwritev2 Rich Felker
2020-08-30 15:05 ` Jann Horn
2020-08-30 16:36 ` Rich Felker
2020-08-30 18:31 ` Jann Horn [this message]
2020-08-30 18:43 ` Rich Felker
2020-08-30 19:02 ` Jann Horn
2020-08-30 20:00 ` Rich Felker
2020-08-31 1:15 ` Jann Horn
2020-08-31 1:46 ` Rich Felker
2020-08-31 9:15 ` Jann Horn
2020-08-31 12:57 ` Rich Felker
2020-08-31 13:12 ` Jann Horn
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=CAG48ez00caDqMomv+PF4dntJkWx7rNYf3E+8gufswis6UFSszw@mail.gmail.com \
--to=jannh@google.com \
--cc=dalias@libc.org \
--cc=linux-api@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).