[PATCH] Fix build w/o NAME_MAX

Jeremy Allison jra at samba.org
Thu Apr 25 23:26:31 UTC 2019


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 :-).

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
> 







More information about the samba-technical mailing list