[RFC][PATCH] shm_open/unlink: fix errno if namelen >= NAME_MAX

[RFC][PATCH] shm_open/unlink: fix errno if namelen >= NAME_MAX

[陈力]

According to linux's manpage and posix's doc, errno should be
set to ENAMETOOLONG if the path exceeds the maximuz length:


linux man page:


```
ENAMETOOLONG
   The length of name exceeds PATH_MAX.
```


posix doc:


```
[ENAMETOOLONG]
   The length of the name argument exceeds {PATH_MAX} or a pathname component is longer than {NAME_MAX}.
```
glibc doesn't handle ENAMETOOLONG correctly previously. When the path
exceeds the maximum value, errno was set to EINVAL instead, which
doesn't conform the man page and posix standard.


This patch fix this behavior.
---
 sysdeps/posix/shm-directory.h | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)


diff --git a/sysdeps/posix/shm-directory.h b/sysdeps/posix/shm-directory.h
index c7979ebb72..d3773165d5 100644
--- a/sysdeps/posix/shm-directory.h
+++ b/sysdeps/posix/shm-directory.h
@@ -33,9 +33,10 @@ extern const char *__shm_directory (size_t *len);
 
    This uses the local variable NAME as an lvalue, and increments it past
    any leading slashes.  It then defines the local variable NAMELEN, giving
-   strlen (NAME) + 1.  If NAME is invalid, it sets errno to
-   ERRNO_FOR_INVALID and returns RETVAL_FOR_INVALID.  Finally, it defines
-   the local variable SHM_NAME, giving the absolute file name of the shm
+   strlen (NAME) + 1. If length of NAME exceeds NAME_MAX, it sets errno to
+   ENAMETOOLONG and returns RETVAL_FOR_INVALID. Otherwise, if NAME is invalid,
+   it sets errno to ERRNO_FOR_INVALID and returns RETVAL_FOR_INVALID.  Finally, it
+   defines the local variable SHM_NAME, giving the absolute file name of the shm
    file corresponding to NAME.  PREFIX is a string constant used as a
    prefix on NAME.  */
 
@@ -53,7 +54,12 @@ extern const char *__shm_directory (size_t *len);
     ++name;								      \
   size_t namelen = strlen (name) + 1;					      \
   /* Validate the filename.  */						      \
-  if (namelen == 1 || namelen >= NAME_MAX || strchr (name, '/') != NULL)      \
+  if (namelen >= NAME_MAX)                          \
+    {                                               \
+      __set_errno (ENAMETOOLONG);               \
+      return retval_for_invalid;					      \
+    }                                                                   \
+  else if (namelen == 1 || strchr (name, '/') != NULL) \
     {									      \
       __set_errno (errno_for_invalid);					      \
       return retval_for_invalid;					      \
-- 
2.24.1

Re: [RFC][PATCH] shm_open/unlink: fix errno if namelen >= NAME_MAX

[Adhemerval Zanella]

On 23/12/2019 21:56, 陈力 wrote:
> According to linux's manpage and posix's doc, errno should be
> set to ENAMETOOLONG if the path exceeds the maximuz length:

Thanks for the patch and interest on working on glibc, however your patch
seems to be mangled somehow (maybe a misconfigured email client?) and this 
is not the correct maillist for patch submission, you should send to 
libc-alpha instead).

> 
> 
> linux man page:
> 
> 
> ```
> ENAMETOOLONG
>    The length of name exceeds PATH_MAX.
> ```
> 
> 
> posix doc:
> 
> 
> ```
> [ENAMETOOLONG]
>    The length of the name argument exceeds {PATH_MAX} or a pathname component is longer than {NAME_MAX}.
> ```
> glibc doesn't handle ENAMETOOLONG correctly previously. When the path
> exceeds the maximum value, errno was set to EINVAL instead, which
> doesn't conform the man page and posix standard.

The Austin Group Interpretation 1003.1-2001 #077 changed the [ENAMETOOLONG] from a
*shall fail* to a *may fail* error.  So essentially if the underlying file system
does support names longer than NAME_MAX, shm_open may not fail.

I think what would be better is in fact remove the NAME_MAX check on SHM_GET_NAME
and instead let the __libc_open call returns ENAMETOOLONG (and maybe a SHM_GET_NAME
refactor as well).

> 
> 
> This patch fix this behavior.
> ---
>  sysdeps/posix/shm-directory.h | 14 ++++++++++----
>  1 file changed, 10 insertions(+), 4 deletions(-)
> 
> 
> diff --git a/sysdeps/posix/shm-directory.h b/sysdeps/posix/shm-directory.h
> index c7979ebb72..d3773165d5 100644
> --- a/sysdeps/posix/shm-directory.h
> +++ b/sysdeps/posix/shm-directory.h
> @@ -33,9 +33,10 @@ extern const char *__shm_directory (size_t *len);
>  
>     This uses the local variable NAME as an lvalue, and increments it past
>     any leading slashes.  It then defines the local variable NAMELEN, giving
> -   strlen (NAME) + 1.  If NAME is invalid, it sets errno to
> -   ERRNO_FOR_INVALID and returns RETVAL_FOR_INVALID.  Finally, it defines
> -   the local variable SHM_NAME, giving the absolute file name of the shm
> +   strlen (NAME) + 1. If length of NAME exceeds NAME_MAX, it sets errno to
> +   ENAMETOOLONG and returns RETVAL_FOR_INVALID. Otherwise, if NAME is invalid,
> +   it sets errno to ERRNO_FOR_INVALID and returns RETVAL_FOR_INVALID.  Finally, it
> +   defines the local variable SHM_NAME, giving the absolute file name of the shm
>     file corresponding to NAME.  PREFIX is a string constant used as a
>     prefix on NAME.  */
>  
> @@ -53,7 +54,12 @@ extern const char *__shm_directory (size_t *len);
>      ++name;								      \
>    size_t namelen = strlen (name) + 1;					      \
>    /* Validate the filename.  */						      \
> -  if (namelen == 1 || namelen >= NAME_MAX || strchr (name, '/') != NULL)      \
> +  if (namelen >= NAME_MAX)                          \
> +    {                                               \
> +      __set_errno (ENAMETOOLONG);               \
> +      return retval_for_invalid;					      \
> +    }                                                                   \
> +  else if (namelen == 1 || strchr (name, '/') != NULL) \
>      {									      \
>        __set_errno (errno_for_invalid);					      \
>        return retval_for_invalid;					      \
>