[PATCH] vfs_gpfs: Retry getacl with DAC capability if necessary

Christof Schmitt cs at samba.org
Wed Jul 20 22:55:40 UTC 2016


Here is another minor update based on input from Volker. This avoids the
bool parameter for the gpfs_getacl_with_capability function.

Christof

On Fri, Apr 29, 2016 at 10:35:24PM -0700, Christof Schmitt wrote:
> On Fri, Apr 29, 2016 at 04:39:38PM -0700, Jeremy Allison wrote:
> > On Fri, Apr 29, 2016 at 11:02:04AM -0700, Christof Schmitt wrote:
> > > From 45a42c1a3fb2aad6efe3e8fd806adb7ba834fab0 Mon Sep 17 00:00:00 2001
> > > From: Christof Schmitt <cs at samba.org>
> > > Date: Tue, 26 Apr 2016 13:09:09 -0700
> > > Subject: [PATCH] vfs_gpfs: Retry getacl with DAC capability if necessary
> > > 
> > > Samba always tries to read the ACL of a file and checks it internally.
> > > If the READ_ACL permission is missing in GPFS, then then reading the ACL
> > > for Samba internal evaluation will be denied and opening the file or
> > > directory fails. Change this by retrying reading the ACL with the DAC
> > > capability if access was denied.
> > 
> > Comments inline below. Code with capabilities always scares me,
> > but I think this is mostly right :-).
> > 
> > > Signed-off-by: Christof Schmitt <cs at samba.org>
> > > ---
> > >  source3/modules/vfs_gpfs.c | 30 ++++++++++++++++++++++++++++--
> > >  1 file changed, 28 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/source3/modules/vfs_gpfs.c b/source3/modules/vfs_gpfs.c
> > > index 42a3c72..ae2c598 100644
> > > --- a/source3/modules/vfs_gpfs.c
> > > +++ b/source3/modules/vfs_gpfs.c
> > > @@ -358,6 +358,25 @@ static void gpfs_dumpacl(int level, struct gpfs_acl *gacl)
> > >  	}
> > >  }
> > >  
> > > +static int vfs_gpfs_getacl_cap(bool use_capability, const char *fname,
> > > +			       int flags, void *aclbuf)
> > > +{
> > > +	int ret;
> > > +
> > > +	if (use_capability) {
> > > +		set_effective_capability(DAC_OVERRIDE_CAPABILITY);
> > > +	}
> > > +
> > > +	errno = 0;
> > > +	ret = gpfswrap_getacl(discard_const_p(char, fname), flags, aclbuf);
> > 
> > I think you need to check for ret == -1 here and if so
> > save off errno.
> > 
> > > +	if (use_capability) {
> > > +		drop_effective_capability(DAC_OVERRIDE_CAPABILITY);
> > > +	}
> > > +
> > 
> > Then restore it here in order for vfs_gpfs_getacl_cap()
> > to correctly return a ret/errno pair.
> 
> Thank you for catching this. It looks like a successful call to
> drop_effective_capability preserves the errno status, since it worked in
> my testing, but the safer approach is to locally keep errno. See the
> attached patch where this change is included.
> 
> Christof

> From bfb3425ea310919d869bed69ac2bfd95ed02a23b Mon Sep 17 00:00:00 2001
> From: Christof Schmitt <cs at samba.org>
> Date: Tue, 26 Apr 2016 13:09:09 -0700
> Subject: [PATCH] vfs_gpfs: Retry getacl with DAC capability if necessary
> 
> Samba always tries to read the ACL of a file and checks it internally.
> If the READ_ACL permission is missing in GPFS, then then reading the ACL
> for Samba internal evaluation will be denied and opening the file or
> directory fails. Change this by retrying reading the ACL with the DAC
> capability if access was denied.
> 
> Signed-off-by: Christof Schmitt <cs at samba.org>
> ---
>  source3/modules/vfs_gpfs.c | 32 ++++++++++++++++++++++++++++++--
>  1 file changed, 30 insertions(+), 2 deletions(-)
> 
> diff --git a/source3/modules/vfs_gpfs.c b/source3/modules/vfs_gpfs.c
> index 42a3c72..3c03dd6 100644
> --- a/source3/modules/vfs_gpfs.c
> +++ b/source3/modules/vfs_gpfs.c
> @@ -358,6 +358,27 @@ static void gpfs_dumpacl(int level, struct gpfs_acl *gacl)
>  	}
>  }
>  
> +static int vfs_gpfs_getacl_cap(bool use_capability, const char *fname,
> +			       int flags, void *aclbuf)
> +{
> +	int ret, saved_errno;
> +
> +	if (use_capability) {
> +		set_effective_capability(DAC_OVERRIDE_CAPABILITY);
> +	}
> +
> +	errno = 0;
> +	ret = gpfswrap_getacl(discard_const_p(char, fname), flags, aclbuf);
> +	saved_errno = errno;
> +
> +	if (use_capability) {
> +		drop_effective_capability(DAC_OVERRIDE_CAPABILITY);
> +	}
> +
> +	errno = saved_errno;
> +	return ret;
> +}
> +
>  /*
>   * get the ACL from GPFS, allocated on the specified mem_ctx
>   * internally retries when initial buffer was too small
> @@ -378,6 +399,7 @@ static void *vfs_gpfs_getacl(TALLOC_CTX *mem_ctx,
>  	int ret, flags;
>  	unsigned int *len;
>  	size_t struct_size;
> +	bool use_capability = false;
>  
>  again:
>  
> @@ -406,8 +428,14 @@ again:
>  	/* set the length of the buffer as input value */
>  	*len = size;
>  
> -	errno = 0;
> -	ret = gpfswrap_getacl(discard_const_p(char, fname), flags, aclbuf);
> +	ret = vfs_gpfs_getacl_cap(use_capability, fname, flags, aclbuf);
> +
> +	if ((ret != 0) && (errno == EACCES) && !use_capability) {
> +		DBG_DEBUG("Retry with DAC capability for %s\n", fname);
> +		use_capability = true;
> +		ret = vfs_gpfs_getacl_cap(use_capability, fname, flags, aclbuf);
> +	}
> +
>  	if ((ret != 0) && (errno == ENOSPC)) {
>  		/*
>  		 * get the size needed to accommodate the complete buffer
> -- 
> 1.8.3.1
> 

-------------- next part --------------
From 60bbceff4f7fb925d35bdabd6e17a73614df1343 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Wed, 25 May 2016 15:56:49 -0700
Subject: [PATCH] vfs_gpfs: Retry getacl with DAC capability if necessary

Samba always tries to read the ACL of a file and checks it internally.
If the READ_ACL permission is missing in GPFS, then then reading the ACL
for Samba internal evaluation will be denied and opening the file or
directory fails. Change this by retrying reading the ACL with the DAC
capability if access was denied.

Signed-off-by: Christof Schmitt <cs at samba.org>
---
 source3/modules/vfs_gpfs.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/source3/modules/vfs_gpfs.c b/source3/modules/vfs_gpfs.c
index 42a3c72..f096dd5 100644
--- a/source3/modules/vfs_gpfs.c
+++ b/source3/modules/vfs_gpfs.c
@@ -358,6 +358,21 @@ static void gpfs_dumpacl(int level, struct gpfs_acl *gacl)
 	}
 }
 
+static int gpfs_getacl_with_capability(const char *fname, int flags, void *buf)
+{
+	int ret, saved_errno;
+
+	set_effective_capability(DAC_OVERRIDE_CAPABILITY);
+
+	ret = gpfswrap_getacl(discard_const_p(char, fname), flags, buf);
+	saved_errno = errno;
+
+	drop_effective_capability(DAC_OVERRIDE_CAPABILITY);
+
+	errno = saved_errno;
+	return ret;
+}
+
 /*
  * get the ACL from GPFS, allocated on the specified mem_ctx
  * internally retries when initial buffer was too small
@@ -378,6 +393,7 @@ static void *vfs_gpfs_getacl(TALLOC_CTX *mem_ctx,
 	int ret, flags;
 	unsigned int *len;
 	size_t struct_size;
+	bool use_capability = false;
 
 again:
 
@@ -406,8 +422,18 @@ again:
 	/* set the length of the buffer as input value */
 	*len = size;
 
-	errno = 0;
-	ret = gpfswrap_getacl(discard_const_p(char, fname), flags, aclbuf);
+	if (use_capability) {
+		ret = gpfs_getacl_with_capability(fname, flags, aclbuf);
+	} else {
+		ret = gpfswrap_getacl(discard_const_p(char, fname),
+				      flags, aclbuf);
+		if ((ret != 0) && (errno == EACCES)) {
+			DBG_DEBUG("Retry with DAC capability for %s\n", fname);
+			use_capability = true;
+			ret = gpfs_getacl_with_capability(fname, flags, aclbuf);
+		}
+	}
+
 	if ((ret != 0) && (errno == ENOSPC)) {
 		/*
 		 * get the size needed to accommodate the complete buffer
-- 
1.8.3.1



More information about the samba-technical mailing list