[2/6] sysvipc: Return EINVAL for invalid semctl commands

Message ID 20200928144556.239160-2-adhemerval.zanella@linaro.org
State Superseded
Headers
Series [1/6] sysvipc: Fix SEM_STAT_ANY kernel argument pass [BZ #26637] |

Commit Message

Adhemerval Zanella Netto Sept. 28, 2020, 2:45 p.m. UTC
  It avoids regressions on possible future commands that might require
additional libc support.  The downside is new commands added by newer
kernels will need further glibc support.

Checked on x86_64-linux-gnu and i686-linux-gnu (Linux v4.15 and v5.4).
---
 sysdeps/unix/sysv/linux/semctl.c |  10 +++
 sysvipc/test-sysvipc.h           | 135 +++++++++++++++++++++++++++++++
 sysvipc/test-sysvsem.c           |   5 ++
 3 files changed, 150 insertions(+)
 create mode 100644 sysvipc/test-sysvipc.h
  

Comments

Florian Weimer Sept. 28, 2020, 4:33 p.m. UTC | #1
* Adhemerval Zanella via Libc-alpha:

> diff --git a/sysdeps/unix/sysv/linux/semctl.c b/sysdeps/unix/sysv/linux/semctl.c
> index 1cdabde8f2..dcff6e691d 100644
> --- a/sysdeps/unix/sysv/linux/semctl.c
> +++ b/sysdeps/unix/sysv/linux/semctl.c
> @@ -158,6 +158,15 @@ __semctl64 (int semid, int semnum, int cmd, ...)
>        arg64 = va_arg (ap, union semun64);
>        va_end (ap);
>        break;
> +    case IPC_RMID:      /* arg ignored. */

Missing space after “.”.

> +/* Return the first invalid command SysV IPC command for shared memory.  */
> +static inline int
> +first_shm_invalid_cmd (void)
> +{
> +  const int shm_cmds[] = {
> +    SHM_STAT,
> +    SHM_INFO,
> +#ifdef SHM_STAT_ANY
> +    SHM_STAT_ANY,
> +#endif
> +  };
> +
> +  int invalid = first_common_invalid_cmd ();
> +  for (int i = 0; i < array_length (shm_cmds); i++)
> +    {
> +      if (invalid == shm_cmds[i])
> +	{
> +	  invalid++;
> +	  i = 0;
> +	}
> +    }
> +
> +  return invalid;
> +}

I think this change should go into the shmctl patch?  Maybe add the
tests separately at the end? 

Rest looks okay to me.

Thanks,
Florian
  
Adhemerval Zanella Netto Sept. 28, 2020, 9:07 p.m. UTC | #2
On 28/09/2020 13:33, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:
> 
>> diff --git a/sysdeps/unix/sysv/linux/semctl.c b/sysdeps/unix/sysv/linux/semctl.c
>> index 1cdabde8f2..dcff6e691d 100644
>> --- a/sysdeps/unix/sysv/linux/semctl.c
>> +++ b/sysdeps/unix/sysv/linux/semctl.c
>> @@ -158,6 +158,15 @@ __semctl64 (int semid, int semnum, int cmd, ...)
>>        arg64 = va_arg (ap, union semun64);
>>        va_end (ap);
>>        break;
>> +    case IPC_RMID:      /* arg ignored. */
> 
> Missing space after “.”.

Ack.

> 
>> +/* Return the first invalid command SysV IPC command for shared memory.  */
>> +static inline int
>> +first_shm_invalid_cmd (void)
>> +{
>> +  const int shm_cmds[] = {
>> +    SHM_STAT,
>> +    SHM_INFO,
>> +#ifdef SHM_STAT_ANY
>> +    SHM_STAT_ANY,
>> +#endif
>> +  };
>> +
>> +  int invalid = first_common_invalid_cmd ();
>> +  for (int i = 0; i < array_length (shm_cmds); i++)
>> +    {
>> +      if (invalid == shm_cmds[i])
>> +	{
>> +	  invalid++;
>> +	  i = 0;
>> +	}
>> +    }
>> +
>> +  return invalid;
>> +}
> 
> I think this change should go into the shmctl patch?  Maybe add the
> tests separately at the end? 

Ok, I move each first_*_invalid_cmd to its own patch.

> 
> Rest looks okay to me.
> 
> Thanks,
> Florian
>
  

Patch

diff --git a/sysdeps/unix/sysv/linux/semctl.c b/sysdeps/unix/sysv/linux/semctl.c
index 1cdabde8f2..dcff6e691d 100644
--- a/sysdeps/unix/sysv/linux/semctl.c
+++ b/sysdeps/unix/sysv/linux/semctl.c
@@ -158,6 +158,15 @@  __semctl64 (int semid, int semnum, int cmd, ...)
       arg64 = va_arg (ap, union semun64);
       va_end (ap);
       break;
+    case IPC_RMID:      /* arg ignored. */
+    case GETNCNT:
+    case GETPID:
+    case GETVAL:
+    case GETZCNT:
+      break;
+    default:
+      __set_errno (EINVAL);
+      return -1;
     }
 
 #if __IPC_TIME64
@@ -277,6 +286,7 @@  __semctl (int semid, int semnum, int cmd, ...)
       arg = va_arg (ap, union semun);
       va_end (ap);
       break;
+    /* __semctl64 handles non-supported commands.  */
     }
 
   struct __semid64_ds semid64;
diff --git a/sysvipc/test-sysvipc.h b/sysvipc/test-sysvipc.h
new file mode 100644
index 0000000000..86aa8df83d
--- /dev/null
+++ b/sysvipc/test-sysvipc.h
@@ -0,0 +1,135 @@ 
+/* Basic definition for Sysv IPC test functions.
+   Copyright (C) 2020 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _TEST_SYSV_H
+#define _TEST_SYSV_H
+
+#include <sys/ipc.h>
+#include <sys/sem.h>
+#include <sys/msg.h>
+#include <sys/shm.h>
+#include <include/array_length.h>
+
+/* Return the first invalid command SysV IPC command from common shared
+   between message queue, shared memory, and semaphore.  */
+static inline int
+first_common_invalid_cmd (void)
+{
+  const int common_cmds[] = {
+    IPC_RMID,
+    IPC_SET,
+    IPC_STAT,
+    IPC_INFO,
+  };
+
+  int invalid = 0;
+  for (int i = 0; i < array_length (common_cmds); i++)
+    {
+      if (invalid == common_cmds[i])
+	{
+	  invalid++;
+	  i = 0;
+        }
+    }
+
+  return invalid;
+}
+
+/* Return the first invalid command SysV IPC command for message queue.  */
+static inline int
+first_msg_invalid_cmd (void)
+{
+  const int msg_cmds[] = {
+    MSG_STAT,
+    MSG_INFO,
+#ifdef MSG_STAT_ANY
+    MSG_STAT_ANY,
+#endif
+  };
+
+  int invalid = first_common_invalid_cmd ();
+  for (int i = 0; i < array_length (msg_cmds); i++)
+    {
+      if (invalid == msg_cmds[i])
+	{
+	  invalid++;
+	  i = 0;
+	}
+    }
+
+  return invalid;
+}
+
+/* Return the first invalid command SysV IPC command for semaphore.  */
+static inline int
+first_sem_invalid_cmd (void)
+{
+  const int sem_cmds[] = {
+    GETPID,
+    GETVAL,
+    GETALL,
+    GETNCNT,
+    GETZCNT,
+    SETVAL,
+    SETALL,
+    SEM_STAT,
+    SEM_INFO,
+#ifdef SEM_STAT_ANY
+    SEM_STAT_ANY,
+#endif
+  };
+
+  int invalid = first_common_invalid_cmd ();
+  for (int i = 0; i < array_length (sem_cmds); i++)
+    {
+      if (invalid == sem_cmds[i])
+	{
+	  invalid++;
+	  i = 0;
+	}
+    }
+
+  return invalid;
+}
+
+/* Return the first invalid command SysV IPC command for shared memory.  */
+static inline int
+first_shm_invalid_cmd (void)
+{
+  const int shm_cmds[] = {
+    SHM_STAT,
+    SHM_INFO,
+#ifdef SHM_STAT_ANY
+    SHM_STAT_ANY,
+#endif
+  };
+
+  int invalid = first_common_invalid_cmd ();
+  for (int i = 0; i < array_length (shm_cmds); i++)
+    {
+      if (invalid == shm_cmds[i])
+	{
+	  invalid++;
+	  i = 0;
+	}
+    }
+
+  return invalid;
+}
+
+#endif /* _TEST_SYSV_H  */
diff --git a/sysvipc/test-sysvsem.c b/sysvipc/test-sysvsem.c
index b7284e0b48..61837d8f3d 100644
--- a/sysvipc/test-sysvsem.c
+++ b/sysvipc/test-sysvsem.c
@@ -25,6 +25,8 @@ 
 #include <sys/ipc.h>
 #include <sys/sem.h>
 
+#include <test-sysvipc.h>
+
 #include <support/support.h>
 #include <support/check.h>
 #include <support/temp_file.h>
@@ -80,6 +82,9 @@  do_test (void)
       FAIL_EXIT1 ("semget failed (errno=%d)", errno);
     }
 
+  TEST_COMPARE (semctl (semid, 0, first_sem_invalid_cmd (), NULL), -1);
+  TEST_COMPARE (errno, EINVAL);
+
   /* Get semaphore kernel information and do some sanity checks.  */
   struct semid_ds seminfo;
   if (semctl (semid, 0, IPC_STAT, (union semun) { .buf = &seminfo }) == -1)