From: Amir Goldstein <amir73il@gmail.com>
To: Dave Chinner <david@fromorbit.com>
Cc: "Darrick J . Wong" <djwong@kernel.org>,
Brian Foster <bfoster@redhat.com>, Christoph Hellwig <hch@lst.de>,
Luis Chamberlain <mcgrof@kernel.org>,
Adam Manzanares <a.manzanares@samsung.com>,
Tyler Hicks <code@tyhicks.com>,
linux-xfs <linux-xfs@vger.kernel.org>,
fstests <fstests@vger.kernel.org>,
Christian Brauner <christian.brauner@ubuntu.com>,
Yang Xu <xuyang2018.jy@fujitsu.com>
Subject: Re: [PATCH 5.10 CANDIDATE 1/8] xfs: fix up non-directory creation in SGID directories
Date: Thu, 2 Jun 2022 07:13:56 +0300 [thread overview]
Message-ID: <CAOQ4uxjcumjxeWypahgYd9wLExLuipd9MTCc_8vfq6SFY7L4dA@mail.gmail.com> (raw)
In-Reply-To: <20220602005238.GK227878@dread.disaster.area>
On Thu, Jun 2, 2022 at 3:52 AM Dave Chinner <david@fromorbit.com> wrote:
>
> On Wed, Jun 01, 2022 at 01:45:40PM +0300, Amir Goldstein wrote:
> > From: Christoph Hellwig <hch@lst.de>
> >
> > commit 01ea173e103edd5ec41acec65b9261b87e123fc2 upstream.
> >
> > XFS always inherits the SGID bit if it is set on the parent inode, while
> > the generic inode_init_owner does not do this in a few cases where it can
> > create a possible security problem, see commit 0fa3ecd87848
> > ("Fix up non-directory creation in SGID directories") for details.
>
> inode_init_owner() introduces a bunch more SGID problems because
> it strips the SGID bit from the mode passed to it, but all the code
> outside it still sees the SGID bit set. IIRC, that means we do the
> wrong thing when ACLs are present. IIRC, there's an LTP test for
> this CVE now, and it also has a variant which uses ACLs and that
> fails too....
Good point.
I think Christian's vfstests probably tests more cases than what LTP
does at this point.
Christian, Yang,
It would be nice if you could annotate the relevant fstests with
_fixed_by_kernel_commit, which will make it easier to find
all relevant commits to backport when tests are failing on LTS
kernel.
>
> I'm kinda wary about mentioning a security fix and then not
> backporting the entire set of fixes the CVE requires in the same
> patchset. I have no idea what the status of the VFS level fixes
> that are needed to fix this properly - I thought they were done and
> reviewed, but they don't appear to be in 5.19 yet.
>
No, it looks like tihs is still in review.
Christoph, Cristian, Yang,
What do you think is best to do w.r.t this patch?
Wait for all the current known issues to be fixed in upstream and then
backport all known fixes?
Backport whatever fixes are available in upstream now at the same
backport series?
Take this now and the rest later?
To be on the safe side, until there is consensus about the best way
to fix LTS, I will omit this fix from my weekly post to stable.
Thanks,
Amir.
next prev parent reply other threads:[~2022-06-02 4:14 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-01 10:45 [PATCH 5.10 CANDIDATE 0/8] xfs stable candidate patches for 5.10.y (part 2) Amir Goldstein
2022-06-01 10:45 ` [PATCH 5.10 CANDIDATE 1/8] xfs: fix up non-directory creation in SGID directories Amir Goldstein
2022-06-02 0:52 ` Dave Chinner
2022-06-02 4:13 ` Amir Goldstein [this message]
2022-06-02 10:31 ` Christian Brauner
2022-06-08 8:25 ` Amir Goldstein
2022-06-08 8:26 ` Christoph Hellwig
2022-06-08 9:15 ` Amir Goldstein
2022-06-02 10:17 ` Christian Brauner
2022-06-01 10:45 ` [PATCH 5.10 CANDIDATE 2/8] xfs: set inode size after creating symlink Amir Goldstein
2022-06-01 10:45 ` [PATCH 5.10 CANDIDATE 3/8] xfs: sync lazy sb accounting on quiesce of read-only mounts Amir Goldstein
2022-06-01 10:45 ` [PATCH 5.10 CANDIDATE 4/8] xfs: fix chown leaking delalloc quota blocks when fssetxattr fails Amir Goldstein
2022-06-01 10:45 ` [PATCH 5.10 CANDIDATE 5/8] xfs: fix incorrect root dquot corruption error when switching group/project quota types Amir Goldstein
2022-06-01 10:45 ` [PATCH 5.10 CANDIDATE 6/8] xfs: restore shutdown check in mapped write fault path Amir Goldstein
2022-06-01 10:45 ` [PATCH 5.10 CANDIDATE 7/8] xfs: consider shutdown in bmapbt cursor delete assert Amir Goldstein
2022-06-02 0:38 ` Dave Chinner
2022-06-02 4:24 ` Amir Goldstein
2022-06-02 5:15 ` Dave Chinner
2022-06-02 5:55 ` Amir Goldstein
2022-06-03 9:39 ` Amir Goldstein
2022-06-01 10:45 ` [PATCH 5.10 CANDIDATE 8/8] xfs: force log and push AIL to clear pinned inodes when aborting mount Amir Goldstein
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=CAOQ4uxjcumjxeWypahgYd9wLExLuipd9MTCc_8vfq6SFY7L4dA@mail.gmail.com \
--to=amir73il@gmail.com \
--cc=a.manzanares@samsung.com \
--cc=bfoster@redhat.com \
--cc=christian.brauner@ubuntu.com \
--cc=code@tyhicks.com \
--cc=david@fromorbit.com \
--cc=djwong@kernel.org \
--cc=fstests@vger.kernel.org \
--cc=hch@lst.de \
--cc=linux-xfs@vger.kernel.org \
--cc=mcgrof@kernel.org \
--cc=xuyang2018.jy@fujitsu.com \
/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).