[PATCHES] Two patches for gpfs sharemodes

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCHES] Two patches for gpfs sharemodes

Samba - samba-technical mailing list

patches (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCHES] Two patches for gpfs sharemodes

Samba - samba-technical mailing list
Hi Christof,

On Wed, Aug 23, 2017 at 01:40:54PM -0700, Christof Schmitt via samba-technical wrote:
> From 9474f1b1865636c7cc53a302b3225e3197f154c1 Mon Sep 17 00:00:00 2001
> From: Christof Schmitt <[hidden email]>
> Date: Wed, 23 Aug 2017 10:33:42 -0700
> Subject: [PATCH 1/2] vfs_gpfs: Do not map DELETE sharemode access to WRITE
>
> A SMB client can deny the WRITE sharemode, but still grant the DELETE
> sharemode. Mapping the requested DELETE access to WRITE access breaks
> this case. Fix this by removing the incorrect mapping from DELETE access
> to WRITE access.

I'b be happy to review, alas I fail to understand gpfs_set_share()
API/semantics. Would you or anyone else care to explain how the "allow" and
"deny" parameters are supposed to work? Lacking API documentation for
gpfs_set_share() makes me feel a bit uncomfortable when reviewing this...

From looking at the semantics of the existing code set_gpfs_sharemode(), it
seems as if "allow" is just used to pass the (mapped) access_mask to the
filesystem. Supposedly GPFS then uses the "allow" parameter in a sense of
"requested access" to match against possible deny modes from other opens?

*scratches head*

Thanks!
-slow

Reply | Threaded
Open this post in threaded view
|

Re: [PATCHES] Two patches for gpfs sharemodes

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Wed, Aug 23, 2017 at 01:40:54PM -0700, Christof Schmitt via samba-technical wrote:
> From 9474f1b1865636c7cc53a302b3225e3197f154c1 Mon Sep 17 00:00:00 2001
> From: Christof Schmitt <[hidden email]>
> Date: Wed, 23 Aug 2017 10:33:42 -0700
> Subject: [PATCH 1/2] vfs_gpfs: Do not map DELETE sharemode access to WRITE
>
> A SMB client can deny the WRITE sharemode, but still grant the DELETE
> sharemode. Mapping the requested DELETE access to WRITE access breaks
> this case. Fix this by removing the incorrect mapping from DELETE access
> to WRITE access.

Isn't the problem with GPFS_DENY_DELETE that it is mapped to a
combination of other flags internally? The last time we discussed this
GPFS_DENY_DELETE was not an independent bit in GPFS. This goes along
with Ralph's question about the precise semantics. Can we get someone
from the GPFS team to describe exactly what's going on internally?

Thanks, Volker

--
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:[hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [PATCHES] Two patches for gpfs sharemodes

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Thu, Aug 24, 2017 at 12:31:10PM +0200, Ralph Böhme wrote:

> Hi Christof,
>
> On Wed, Aug 23, 2017 at 01:40:54PM -0700, Christof Schmitt via samba-technical wrote:
> > From 9474f1b1865636c7cc53a302b3225e3197f154c1 Mon Sep 17 00:00:00 2001
> > From: Christof Schmitt <[hidden email]>
> > Date: Wed, 23 Aug 2017 10:33:42 -0700
> > Subject: [PATCH 1/2] vfs_gpfs: Do not map DELETE sharemode access to WRITE
> >
> > A SMB client can deny the WRITE sharemode, but still grant the DELETE
> > sharemode. Mapping the requested DELETE access to WRITE access breaks
> > this case. Fix this by removing the incorrect mapping from DELETE access
> > to WRITE access.
>
> I'b be happy to review, alas I fail to understand gpfs_set_share()
> API/semantics. Would you or anyone else care to explain how the "allow" and
> "deny" parameters are supposed to work? Lacking API documentation for
> gpfs_set_share() makes me feel a bit uncomfortable when reviewing this...
>
> From looking at the semantics of the existing code set_gpfs_sharemode(), it
> seems as if "allow" is just used to pass the (mapped) access_mask to the
> filesystem. Supposedly GPFS then uses the "allow" parameter in a sense of
> "requested access" to match against possible deny modes from other opens?
>
> *scratches head*

I have done some extensive testing, so i am confident that this is
accurate:

Samba can always open() a file and get a file descriptor. For other
applications, the file system will return an error on open() when there
is a conflicting sharemode for the file in the filesystem.

Through the "allow" parameter, Samba is registering the currently
required access to the file. This is somewhat redundant to the flags in
the open() call, but this is the path in the filesystem to prevent
conflicting sharemodes from other applications. As the gpfs_set_share
API is tied to a file descriptor, this does not include requesting
DELETE access, as this is done through unlink() without a file
descriptor.

The "deny" parameter is the inverse of the actual sharemode requested.
If the file is opened to allow only READ access for others, then
DENY_READ is not requested, but DENY_WRITE is.

The main point of the first patch is that when a SMB client is only
requesting READ and DELETE access, Samba should not request WRITE
access. A second access to the same file should be able to deny the
WRITE sharemode. The problem can be seen by running the
smb2.sharemode-access and smb2.access-sharemode tests against Samba
running on a gpfs file system. All combinations tested there are valid
against a Windows file server and a Samba server without the gpfs vfs
module. Loading the gpfs vfs module with gpfs:sharemodes=true results
in a test error that is fixed by the first patch here.

Christof

Reply | Threaded
Open this post in threaded view
|

Re: [PATCHES] Two patches for gpfs sharemodes

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
On Thu, Aug 24, 2017 at 01:46:36PM +0200, Volker Lendecke wrote:

> On Wed, Aug 23, 2017 at 01:40:54PM -0700, Christof Schmitt via samba-technical wrote:
> > From 9474f1b1865636c7cc53a302b3225e3197f154c1 Mon Sep 17 00:00:00 2001
> > From: Christof Schmitt <[hidden email]>
> > Date: Wed, 23 Aug 2017 10:33:42 -0700
> > Subject: [PATCH 1/2] vfs_gpfs: Do not map DELETE sharemode access to WRITE
> >
> > A SMB client can deny the WRITE sharemode, but still grant the DELETE
> > sharemode. Mapping the requested DELETE access to WRITE access breaks
> > this case. Fix this by removing the incorrect mapping from DELETE access
> > to WRITE access.
>
> Isn't the problem with GPFS_DENY_DELETE that it is mapped to a
> combination of other flags internally? The last time we discussed this
> GPFS_DENY_DELETE was not an independent bit in GPFS. This goes along
> with Ralph's question about the precise semantics. Can we get someone
> from the GPFS team to describe exactly what's going on internally?

The limitation of DENY_DELETE is that this bit can only be set together
with either DENY_READ, DENY_WRITE or both. So this does not cover all
possible cases (a SMB client allowing READ and WRITE access to others,
but preventing DELETE cannot be enforced through the current API).

On the other hand, we should leverage the current API as much as
possible and allow a SMB client to prevent others from deleting the file
when that can be enforced.

Christof

Reply | Threaded
Open this post in threaded view
|

Re: [PATCHES] Two patches for gpfs sharemodes

Samba - samba-technical mailing list
On Thu, Aug 24, 2017 at 10:22:41AM -0700, Christof Schmitt wrote:

> On Thu, Aug 24, 2017 at 01:46:36PM +0200, Volker Lendecke wrote:
> > On Wed, Aug 23, 2017 at 01:40:54PM -0700, Christof Schmitt via samba-technical wrote:
> > > From 9474f1b1865636c7cc53a302b3225e3197f154c1 Mon Sep 17 00:00:00 2001
> > > From: Christof Schmitt <[hidden email]>
> > > Date: Wed, 23 Aug 2017 10:33:42 -0700
> > > Subject: [PATCH 1/2] vfs_gpfs: Do not map DELETE sharemode access to WRITE
> > >
> > > A SMB client can deny the WRITE sharemode, but still grant the DELETE
> > > sharemode. Mapping the requested DELETE access to WRITE access breaks
> > > this case. Fix this by removing the incorrect mapping from DELETE access
> > > to WRITE access.
> >
> > Isn't the problem with GPFS_DENY_DELETE that it is mapped to a
> > combination of other flags internally? The last time we discussed this
> > GPFS_DENY_DELETE was not an independent bit in GPFS. This goes along
> > with Ralph's question about the precise semantics. Can we get someone
> > from the GPFS team to describe exactly what's going on internally?
>
> The limitation of DENY_DELETE is that this bit can only be set together
> with either DENY_READ, DENY_WRITE or both. So this does not cover all
> possible cases (a SMB client allowing READ and WRITE access to others,
> but preventing DELETE cannot be enforced through the current API).
>
> On the other hand, we should leverage the current API as much as
> possible and allow a SMB client to prevent others from deleting the file
> when that can be enforced.

To follow-up on the offline discussion we had: I did some additional
testing: The file system sharemodes are enforced across cluster nodes as
expected, including the DENY_DELETE one. The only limitation is the one
mentioned in the comment, that DENY_DELETE is only possible together
with one of the other DENY flags.

Christof

Reply | Threaded
Open this post in threaded view
|

Re: [PATCHES] Two patches for gpfs sharemodes

Samba - samba-technical mailing list
In reply to this post by Samba - samba-technical mailing list
Hi,

after private email conversation: lgtm&pushed.

Cheerio!
-slow