[PATCH] Fix build w/o NAME_MAX
Günther Deschner
gd at samba.org
Fri Apr 26 11:07:21 UTC 2019
The new patchset has now been RB+'ed by Ralph off-list and pushed to
autobuild.
Thanks,
Guenther
On 26/04/2019 08:03, Anoop C S via samba-technical wrote:
> 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
>>>
>>
>>
>>
>>
--
Günther Deschner GPG-ID: 8EE11688
Red Hat gdeschner at redhat.com
Samba Team gd at samba.org
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20190426/419ea630/signature.sig>
More information about the samba-technical
mailing list