[PATCH] Fix build w/o NAME_MAX

Anoop C S anoopcs at cryptolab.net
Fri Apr 26 06:03:31 UTC 2019


On Thu, 2019-04-25 at 16:26 -0700, Jeremy Allison via samba-technical
wrote:
> On Thu, Apr 25, 2019 at 06:27:34PM +0200, Günther Deschner via samba-
> technical wrote:
> > Hi,
> > 
> > please review and push.
> > 
> > Thanks,
> > Guenther
> 
> Close, but there's one paranoia thing I'd like to add.
> 
> In patchsets #1 and #2 You do:
> 
> +       long name_max;
> ..
> +       name_max = pathconf(path, _PC_NAME_MAX);
> ..
> +       val_buf = talloc_zero_array(mem_ctx, char, name_max + 1);
> 
> After getting name_max from pathconf I'd like to
> see an integer wrap test added. i.e.
> 
> 	if (name_max + 1 < 1) {
> 		errno = EINVAL;
> 		return -1;
> 	}
> 
> Just for carefulness's sake :-).

Please find the attached patch set including suggested check for
name_max.

Pipeline triggered:
https://gitlab.com/samba-team/devel/samba/pipelines/58525568

> Yeah I know it can't happen, but I don't
> want to see unchecked arithmetic on anything
> that we didn't initialize ourselves :-).
> 
> Thanks,
> 
> 	Jeremy.
> 
> > From 859cf8b7da7b5e0bede12fb4d4b8a74bd95ec031 Mon Sep 17 00:00:00
> > 2001
> > From: Anoop C S <anoopcs at redhat.com>
> > Date: Thu, 25 Apr 2019 16:41:53 +0530
> > Subject: [PATCH 1/3] s3/vfs_glusterfs: Dynamically determine
> > NAME_MAX
> > 
> > BUG: https://bugzilla.samba.org/show_bug.cgi?id=13872
> > 
> > Signed-off-by: Anoop C S <anoopcs at redhat.com>
> > Reviewed-by: Guenther Deschner <gd at samba.org>
> > ---
> >  source3/modules/vfs_glusterfs.c | 33 +++++++++++++++++++++++++--
> > ------
> >  1 file changed, 25 insertions(+), 8 deletions(-)
> > 
> > diff --git a/source3/modules/vfs_glusterfs.c
> > b/source3/modules/vfs_glusterfs.c
> > index 82a7c2ce9b4..16ba0b25a82 100644
> > --- a/source3/modules/vfs_glusterfs.c
> > +++ b/source3/modules/vfs_glusterfs.c
> > @@ -1454,20 +1454,32 @@ static int vfs_gluster_chflags(struct
> > vfs_handle_struct *handle,
> >  
> >  static int vfs_gluster_get_real_filename(struct vfs_handle_struct
> > *handle,
> >  					 const char *path, const char
> > *name,
> > -					 TALLOC_CTX *mem_ctx, char
> > **found_name)
> > +					 TALLOC_CTX *mem_ctx, char
> > **_found_name)
> >  {
> >  	int ret;
> > -	char key_buf[NAME_MAX + 64];
> > -	char val_buf[NAME_MAX + 1];
> > +	char *key_buf = NULL, *val_buf = NULL;
> > +	long name_max;
> > +	char *found_name = NULL;
> >  
> > -	if (strlen(name) >= NAME_MAX) {
> > +	name_max = pathconf(path, _PC_NAME_MAX);
> > +
> > +	if (strlen(name) >= name_max) {
> >  		errno = ENAMETOOLONG;
> >  		return -1;
> >  	}
> >  
> > -	snprintf(key_buf, NAME_MAX + 64,
> > -		 "glusterfs.get_real_filename:%s", name);
> > +	key_buf = talloc_asprintf(mem_ctx,
> > "glusterfs.get_real_filename:%s",
> > +				  name);
> > +	if (key_buf == NULL) {
> > +		errno = ENOMEM;
> > +		return -1;
> > +	}
> >  
> > +	val_buf = talloc_zero_array(mem_ctx, char, name_max + 1);
> > +	if (val_buf == NULL) {
> > +		errno = ENOMEM;
> > +		return -1;
> > +	}
> >  	ret = glfs_getxattr(handle->data, path, key_buf, val_buf,
> > NAME_MAX + 1);
> >  	if (ret == -1) {
> >  		if (errno == ENOATTR) {
> > @@ -1476,11 +1488,16 @@ static int
> > vfs_gluster_get_real_filename(struct vfs_handle_struct *handle,
> >  		return -1;
> >  	}
> >  
> > -	*found_name = talloc_strdup(mem_ctx, val_buf);
> > -	if (found_name[0] == NULL) {
> > +	found_name = talloc_strdup(mem_ctx, val_buf);
> > +	if (found_name == NULL) {
> >  		errno = ENOMEM;
> >  		return -1;
> >  	}
> > +	*_found_name = found_name;
> > +
> > +	TALLOC_FREE(key_buf);
> > +	TALLOC_FREE(val_buf);
> > +
> >  	return 0;
> >  }
> >  
> > -- 
> > 2.20.1
> > 
> > 
> > From e35e96878f058021ff6f9f5872a9a8c1d0f84aa9 Mon Sep 17 00:00:00
> > 2001
> > From: Anoop C S <anoopcs at redhat.com>
> > Date: Thu, 25 Apr 2019 16:42:01 +0530
> > Subject: [PATCH 2/3] s3/vfs_glusterfs_fuse: Dynamically determine
> > NAME_MAX
> > 
> > This allows the vfs_glusterfs_fuse build to complete on AIX.
> > 
> > BUG: https://bugzilla.samba.org/show_bug.cgi?id=13872
> > 
> > Signed-off-by: Anoop C S <anoopcs at redhat.com>
> > Reviewed-by: Guenther Deschner <gd at samba.org>
> > ---
> >  source3/modules/vfs_glusterfs_fuse.c | 28 ++++++++++++++++++++++
> > ------
> >  1 file changed, 22 insertions(+), 6 deletions(-)
> > 
> > diff --git a/source3/modules/vfs_glusterfs_fuse.c
> > b/source3/modules/vfs_glusterfs_fuse.c
> > index 8855cd18d01..bbad95650bd 100644
> > --- a/source3/modules/vfs_glusterfs_fuse.c
> > +++ b/source3/modules/vfs_glusterfs_fuse.c
> > @@ -28,19 +28,31 @@ static int
> > vfs_gluster_fuse_get_real_filename(struct vfs_handle_struct
> > *handle,
> >  					      char **_found_name)
> >  {
> >  	int ret;
> > -	char key_buf[NAME_MAX + 64];
> > -	char val_buf[NAME_MAX + 1];
> > +	char *key_buf = NULL, *val_buf = NULL;
> > +	long name_max;
> >  	char *found_name = NULL;
> >  
> > -	if (strlen(name) >= NAME_MAX) {
> > +	name_max = pathconf(path, _PC_NAME_MAX);
> > +
> > +	if (strlen(name) >= name_max) {
> >  		errno = ENAMETOOLONG;
> >  		return -1;
> >  	}
> >  
> > -	snprintf(key_buf, NAME_MAX + 64,
> > -		 "glusterfs.get_real_filename:%s", name);
> > +	key_buf = talloc_asprintf(mem_ctx,
> > "glusterfs.get_real_filename:%s",
> > +				  name);
> > +	if (key_buf == NULL) {
> > +		errno = ENOMEM;
> > +		return -1;
> > +	}
> > +
> > +	val_buf = talloc_zero_array(mem_ctx, char, name_max + 1);
> > +	if (val_buf == NULL) {
> > +		errno = ENOMEM;
> > +		return -1;
> > +	}
> >  
> > -	ret = getxattr(path, key_buf, val_buf, NAME_MAX + 1);
> > +	ret = getxattr(path, key_buf, val_buf, name_max + 1);
> >  	if (ret == -1) {
> >  		if (errno == ENOATTR) {
> >  			errno = EOPNOTSUPP;
> > @@ -54,6 +66,10 @@ static int
> > vfs_gluster_fuse_get_real_filename(struct vfs_handle_struct
> > *handle,
> >  		return -1;
> >  	}
> >  	*_found_name = found_name;
> > +
> > +	TALLOC_FREE(key_buf);
> > +	TALLOC_FREE(val_buf);
> > +
> >  	return 0;
> >  }
> >  
> > -- 
> > 2.20.1
> > 
> > 
> > From 03b59c1aedc36a326d889f528bc796eb0b6dc974 Mon Sep 17 00:00:00
> > 2001
> > From: =?UTF-8?q?G=C3=BCnther=20Deschner?= <gd at samba.org>
> > Date: Thu, 25 Apr 2019 14:49:48 +0200
> > Subject: [PATCH 3/3] Revert "lib/replace: define NAME_MAX for
> > platforms that
> >  don't have it"
> > MIME-Version: 1.0
> > Content-Type: text/plain; charset=UTF-8
> > Content-Transfer-Encoding: 8bit
> > 
> > BUG: https://bugzilla.samba.org/show_bug.cgi?id=13872
> > 
> > This reverts commit e3c894fb6b87df8aa56e29ef3b16ae1ef456a875.
> > 
> > Signed-off-by: G??nther Deschner <gd at samba.org>
> > ---
> >  lib/replace/replace.h | 4 ----
> >  1 file changed, 4 deletions(-)
> > 
> > diff --git a/lib/replace/replace.h b/lib/replace/replace.h
> > index 4d9b81f6825..212ed265d4a 100644
> > --- a/lib/replace/replace.h
> > +++ b/lib/replace/replace.h
> > @@ -858,10 +858,6 @@ typedef unsigned long long ptrdiff_t ;
> >  #define PATH_MAX 1024
> >  #endif
> >  
> > -#ifndef NAME_MAX
> > -#define NAME_MAX 255
> > -#endif
> > -
> >  #ifndef MAX_DNS_NAME_LENGTH
> >  #define MAX_DNS_NAME_LENGTH 256 /* Actually 255 but +1 for
> > terminating null. */
> >  #endif
> > -- 
> > 2.20.1
> > 
> 
> 
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: glusterfs.patch
Type: text/x-patch
Size: 5331 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20190426/f4a5f95a/glusterfs.bin>


More information about the samba-technical mailing list