From: Vivek Goyal <vgoyal@redhat.com>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: linux-fsdevel@vger.kernel.org, virtio-fs-list <virtio-fs@redhat.com>
Subject: Re: [PATCH v2 4/6] fuse: Kill suid/sgid using ATTR_MODE if it is not truncate
Date: Tue, 22 Sep 2020 16:08:40 -0400 [thread overview]
Message-ID: <20200922200840.GF57620@redhat.com> (raw)
In-Reply-To: <CAJfpegsncAteUfTAHAttwyVQmhGoK7FCeO_z+xcB_4QkYZEzsQ@mail.gmail.com>
On Tue, Sep 22, 2020 at 03:56:47PM +0200, Miklos Szeredi wrote:
> On Wed, Sep 16, 2020 at 6:18 PM Vivek Goyal <vgoyal@redhat.com> wrote:
>
> > But if this is non-truncate setattr then server will not kill suid/sgid.
> > So continue to send ATTR_MODE to kill suid/sgid for non-truncate setattr,
> > even if ->handle_killpriv_v2 is enabled.
>
> Sending ATTR_MODE doesn't make sense, since that is racy. The
> refresh-recalculate makes the race window narrower, but it doesn't
> eliminate it.
Hi Miklos,
Agreed that it does not eliminate that race.
>
> I think I suggested sending write synchronously if suid/sgid/caps are
> set. Do you see a problem with this?
Sorry, I might have missed it. So you are saying that for the case of
->writeback_cache, force a synchronous WRITE if suid/sgid is set. But
this will only work if client sees the suid/sgid bits. If client B
set the suid/sgid which client A does not see then all the WRITEs
will be cached in client A and not clear suid/sgid bits.
Also another problem is that if client sees suid/sgid and we make
WRITE synchronous, client's suid/sgid attrs are still cached till
next refresh (both for ->writeback_cache and non writeback_cache
case). So server is clearing suid/sgid bits but client still
keeps them cached. I hope none of the code paths end up using
this stale value and refresh attrs before using suid/sgid.
Shall we refresh attrs after WRITE if suid/sgid is set and client
expects it to clear after WRITE finishes to solve this problem. Or
this is something which is actually not a real problem and I am
overdesigning.
Thanks
Vivek
>
> Does this affect anything other than cached writes?
>
> Thanks,
> Miklos
>
WARNING: multiple messages have this Message-ID (diff)
From: Vivek Goyal <vgoyal@redhat.com>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: linux-fsdevel@vger.kernel.org, virtio-fs-list <virtio-fs@redhat.com>
Subject: Re: [Virtio-fs] [PATCH v2 4/6] fuse: Kill suid/sgid using ATTR_MODE if it is not truncate
Date: Tue, 22 Sep 2020 16:08:40 -0400 [thread overview]
Message-ID: <20200922200840.GF57620@redhat.com> (raw)
In-Reply-To: <CAJfpegsncAteUfTAHAttwyVQmhGoK7FCeO_z+xcB_4QkYZEzsQ@mail.gmail.com>
On Tue, Sep 22, 2020 at 03:56:47PM +0200, Miklos Szeredi wrote:
> On Wed, Sep 16, 2020 at 6:18 PM Vivek Goyal <vgoyal@redhat.com> wrote:
>
> > But if this is non-truncate setattr then server will not kill suid/sgid.
> > So continue to send ATTR_MODE to kill suid/sgid for non-truncate setattr,
> > even if ->handle_killpriv_v2 is enabled.
>
> Sending ATTR_MODE doesn't make sense, since that is racy. The
> refresh-recalculate makes the race window narrower, but it doesn't
> eliminate it.
Hi Miklos,
Agreed that it does not eliminate that race.
>
> I think I suggested sending write synchronously if suid/sgid/caps are
> set. Do you see a problem with this?
Sorry, I might have missed it. So you are saying that for the case of
->writeback_cache, force a synchronous WRITE if suid/sgid is set. But
this will only work if client sees the suid/sgid bits. If client B
set the suid/sgid which client A does not see then all the WRITEs
will be cached in client A and not clear suid/sgid bits.
Also another problem is that if client sees suid/sgid and we make
WRITE synchronous, client's suid/sgid attrs are still cached till
next refresh (both for ->writeback_cache and non writeback_cache
case). So server is clearing suid/sgid bits but client still
keeps them cached. I hope none of the code paths end up using
this stale value and refresh attrs before using suid/sgid.
Shall we refresh attrs after WRITE if suid/sgid is set and client
expects it to clear after WRITE finishes to solve this problem. Or
this is something which is actually not a real problem and I am
overdesigning.
Thanks
Vivek
>
> Does this affect anything other than cached writes?
>
> Thanks,
> Miklos
>
next prev parent reply other threads:[~2020-09-22 20:08 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-16 16:17 [PATCH v2 0/6] fuse: Implement FUSE_HANDLE_KILLPRIV_V2 and enable SB_NOSEC Vivek Goyal
2020-09-16 16:17 ` [Virtio-fs] " Vivek Goyal
2020-09-16 16:17 ` [PATCH v2 1/6] fuse: Introduce the notion of FUSE_HANDLE_KILLPRIV_V2 Vivek Goyal
2020-09-16 16:17 ` [Virtio-fs] " Vivek Goyal
2020-09-16 16:17 ` [PATCH v2 2/6] fuse: Set FUSE_WRITE_KILL_PRIV in cached write path Vivek Goyal
2020-09-16 16:17 ` [Virtio-fs] " Vivek Goyal
2020-09-16 16:17 ` [PATCH v2 3/6] fuse: setattr should set FATTR_KILL_PRIV upon size change Vivek Goyal
2020-09-16 16:17 ` [Virtio-fs] " Vivek Goyal
2020-09-16 16:17 ` [PATCH v2 4/6] fuse: Kill suid/sgid using ATTR_MODE if it is not truncate Vivek Goyal
2020-09-16 16:17 ` [Virtio-fs] " Vivek Goyal
2020-09-22 13:56 ` Miklos Szeredi
2020-09-22 13:56 ` [Virtio-fs] " Miklos Szeredi
2020-09-22 20:08 ` Vivek Goyal [this message]
2020-09-22 20:08 ` Vivek Goyal
2020-09-22 21:25 ` Miklos Szeredi
2020-09-22 21:25 ` [Virtio-fs] " Miklos Szeredi
2020-09-22 21:31 ` Vivek Goyal
2020-09-22 21:31 ` [Virtio-fs] " Vivek Goyal
2020-09-16 16:17 ` [PATCH v2 5/6] fuse: Add a flag FUSE_OPEN_KILL_PRIV for open() request Vivek Goyal
2020-09-16 16:17 ` [Virtio-fs] " Vivek Goyal
2020-09-16 16:17 ` [PATCH v2 6/6] virtiofs: Support SB_NOSEC flag to improve direct write performance Vivek Goyal
2020-09-16 16:17 ` [Virtio-fs] " Vivek Goyal
2020-09-16 16:38 ` [PATCH v2 0/6] fuse: Implement FUSE_HANDLE_KILLPRIV_V2 and enable SB_NOSEC Vivek Goyal
2020-09-16 16:38 ` [Virtio-fs] " Vivek Goyal
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=20200922200840.GF57620@redhat.com \
--to=vgoyal@redhat.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=miklos@szeredi.hu \
--cc=virtio-fs@redhat.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 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.