Message ID | 20230307083229.629411-1-abushwangs@gmail.com |
---|---|
State | New |
Headers | show |
Series | rt: fix shm_open not set ENAMETOOLONG when name exceeds {_POSIX_PATH_MAX} | expand |
On Tue, 2023-03-07 at 16:32 +0800, abushwang via Libc-alpha wrote: > according to man-pages-posix-2017, shm_open() function may fail if the length > of the name argument exceeds {_POSIX_PATH_MAX} and set ENAMETOOLONG It's "may" fail, not "shall" fail. POSIX 2017 says "may" means: may Describes a feature or behavior that is optional for an implementation that conforms to POSIX.1-2017. An application should not rely on the existence of the feature or behavior. An application that relies on such a feature or behavior cannot be assured to be portable across conforming implementations. We should not break existing programs just for a "may" clause. /* snip */ > int > __shm_get_name (struct shmdir_name *result, const char *name, bool > sem_prefix) > @@ -50,13 +51,15 @@ __shm_get_name (struct shmdir_name *result, const > char *name, bool sem_prefix) > while (name[0] == '/') > ++name; > namelen = strlen (name); > + if (namelen > NAME_MAX) > + return ENAMETOOLONG; > > if (sem_prefix) > alloc_buffer_copy_bytes (&buffer, "sem.", strlen ("sem.")); > alloc_buffer_copy_bytes (&buffer, name, namelen + 1); > if (namelen == 0 || memchr (name, '/', namelen) != NULL > || alloc_buffer_has_failed (&buffer)) If you really want ENAMETOOLONG I guess you can check if namelen > NAME_MAX here (as a very long namelen would have caused an allocation failure). By the way if namelen <= NAME_MAX but alloc_buffer_has_failed (due to a high memory usage) should we say ENOSPC instead of EINVAL? POSIX 2017 says: [ENOSPC] There is insufficient space for the creation of the new shared memory object. > - return -1; > + return EINVAL; > return 0; > }
I have re-send this patch according to your suggest. Thinks abushwang Xi Ruoyao <xry111@xry111.site> 于2023年3月7日周二 16:54写道: > On Tue, 2023-03-07 at 16:32 +0800, abushwang via Libc-alpha wrote: > > according to man-pages-posix-2017, shm_open() function may fail if the > length > > of the name argument exceeds {_POSIX_PATH_MAX} and set ENAMETOOLONG > > It's "may" fail, not "shall" fail. POSIX 2017 says "may" means: > > may > > Describes a feature or behavior that is optional for an implementation > that conforms to POSIX.1-2017. An application should not rely on the > existence of the feature or behavior. An application that relies on such > a feature or behavior cannot be assured to be portable across conforming > implementations. > > We should not break existing programs just for a "may" clause. > > /* snip */ > > > int > > __shm_get_name (struct shmdir_name *result, const char *name, bool > > sem_prefix) > > @@ -50,13 +51,15 @@ __shm_get_name (struct shmdir_name *result, const > > char *name, bool sem_prefix) > > while (name[0] == '/') > > ++name; > > namelen = strlen (name); > > + if (namelen > NAME_MAX) > > + return ENAMETOOLONG; > > > > if (sem_prefix) > > alloc_buffer_copy_bytes (&buffer, "sem.", strlen ("sem.")); > > alloc_buffer_copy_bytes (&buffer, name, namelen + 1); > > if (namelen == 0 || memchr (name, '/', namelen) != NULL > > || alloc_buffer_has_failed (&buffer)) > > If you really want ENAMETOOLONG I guess you can check if namelen > > NAME_MAX here (as a very long namelen would have caused an allocation > failure). > > By the way if namelen <= NAME_MAX but alloc_buffer_has_failed (due to a > high memory usage) should we say ENOSPC instead of EINVAL? POSIX 2017 > says: > > [ENOSPC] > There is insufficient space for the creation of the new shared memory > object. > > > - return -1; > > + return EINVAL; > > return 0; > > } > > -- > Xi Ruoyao <xry111@xry111.site> > School of Aerospace Science and Technology, Xidian University >
ENOSPC mean insufficient shared memory object, alloc_buffer_has_failed mean insufficient memory for name, maybe they are different. abush wang <abushwangs@gmail.com> 于2023年3月7日周二 17:46写道: > I have re-send this patch according to your suggest. > > Thinks > abushwang > > Xi Ruoyao <xry111@xry111.site> 于2023年3月7日周二 16:54写道: > >> On Tue, 2023-03-07 at 16:32 +0800, abushwang via Libc-alpha wrote: >> > according to man-pages-posix-2017, shm_open() function may fail if the >> length >> > of the name argument exceeds {_POSIX_PATH_MAX} and set ENAMETOOLONG >> >> It's "may" fail, not "shall" fail. POSIX 2017 says "may" means: >> >> may >> >> Describes a feature or behavior that is optional for an implementation >> that conforms to POSIX.1-2017. An application should not rely on the >> existence of the feature or behavior. An application that relies on such >> a feature or behavior cannot be assured to be portable across conforming >> implementations. >> >> We should not break existing programs just for a "may" clause. >> >> /* snip */ >> >> > int >> > __shm_get_name (struct shmdir_name *result, const char *name, bool >> > sem_prefix) >> > @@ -50,13 +51,15 @@ __shm_get_name (struct shmdir_name *result, const >> > char *name, bool sem_prefix) >> > while (name[0] == '/') >> > ++name; >> > namelen = strlen (name); >> > + if (namelen > NAME_MAX) >> > + return ENAMETOOLONG; >> > >> > if (sem_prefix) >> > alloc_buffer_copy_bytes (&buffer, "sem.", strlen ("sem.")); >> > alloc_buffer_copy_bytes (&buffer, name, namelen + 1); >> > if (namelen == 0 || memchr (name, '/', namelen) != NULL >> > || alloc_buffer_has_failed (&buffer)) >> >> If you really want ENAMETOOLONG I guess you can check if namelen > >> NAME_MAX here (as a very long namelen would have caused an allocation >> failure). >> >> By the way if namelen <= NAME_MAX but alloc_buffer_has_failed (due to a >> high memory usage) should we say ENOSPC instead of EINVAL? POSIX 2017 >> says: >> >> [ENOSPC] >> There is insufficient space for the creation of the new shared memory >> object. >> >> > - return -1; >> > + return EINVAL; >> > return 0; >> > } >> >> -- >> Xi Ruoyao <xry111@xry111.site> >> School of Aerospace Science and Technology, Xidian University >> >
diff --git a/posix/shm-directory.c b/posix/shm-directory.c index 86d9fd8e4f..93aaeb60a0 100644 --- a/posix/shm-directory.c +++ b/posix/shm-directory.c @@ -25,6 +25,7 @@ #include <string.h> #include <sys/mman.h> #include <fcntl.h> +#include <errno.h> int __shm_get_name (struct shmdir_name *result, const char *name, bool sem_prefix) @@ -50,13 +51,15 @@ __shm_get_name (struct shmdir_name *result, const char *name, bool sem_prefix) while (name[0] == '/') ++name; namelen = strlen (name); + if (namelen > NAME_MAX) + return ENAMETOOLONG; if (sem_prefix) alloc_buffer_copy_bytes (&buffer, "sem.", strlen ("sem.")); alloc_buffer_copy_bytes (&buffer, name, namelen + 1); if (namelen == 0 || memchr (name, '/', namelen) != NULL || alloc_buffer_has_failed (&buffer)) - return -1; + return EINVAL; return 0; } libc_hidden_def (__shm_get_name) diff --git a/rt/shm_open.c b/rt/shm_open.c index 6c1f4d604f..fc1dc96bb4 100644 --- a/rt/shm_open.c +++ b/rt/shm_open.c @@ -30,9 +30,10 @@ int __shm_open (const char *name, int oflag, mode_t mode) { struct shmdir_name dirname; - if (__shm_get_name (&dirname, name, false) != 0) + int ret =__shm_get_name (&dirname, name, false); + if (ret != 0) { - __set_errno (EINVAL); + __set_errno (ret); return -1; }
according to man-pages-posix-2017, shm_open() function may fail if the length of the name argument exceeds {_POSIX_PATH_MAX} and set ENAMETOOLONG Signed-off-by: abushwang <abushwangs@gmail.com> --- posix/shm-directory.c | 5 ++++- rt/shm_open.c | 5 +++-- 2 files changed, 7 insertions(+), 3 deletions(-)