[5/5] rt: Set shm_open as a non cancellation point (BZ #18243)

Message ID 1471876053-780-5-git-send-email-adhemerval.zanella@linaro.org
State Dropped
Headers

Commit Message

Adhemerval Zanella Netto Aug. 22, 2016, 2:27 p.m. UTC
  This patch changes shm_open to not act as a cancellation point.
Cancellation is disable at start and reenable in function exit.
It fixes BZ #18243.

Tested on x86_64 and i686.

	[BZ #18243]
	* rt/Makefile (test): Add tst-shm-cancel.
	* rt/tst-shm-cancel.c: New file.
	* sysdeps/posix/shm_open.c: Disable asynchronous cancellation.
---
 rt/Makefile              |   2 +-
 rt/tst-shm-cancel.c      | 135 +++++++++++++++++++++++++++++++++++++++++++++++
 sysdeps/posix/shm_open.c |   7 +++
 3 files changed, 143 insertions(+), 1 deletion(-)
 create mode 100644 rt/tst-shm-cancel.c
  

Comments

Torvald Riegel Sept. 5, 2016, 6:08 p.m. UTC | #1
On Mon, 2016-08-22 at 11:27 -0300, Adhemerval Zanella wrote:
> This patch changes shm_open to not act as a cancellation point.
> Cancellation is disable at start and reenable in function exit.
> It fixes BZ #18243.

The patch looks good to me.  If we agree that the sem_wait part of this
BZ is NOTABUG, then this patch indeed fixes the BZ.
  

Patch

diff --git a/rt/Makefile b/rt/Makefile
index cfa6837..7593b11 100644
--- a/rt/Makefile
+++ b/rt/Makefile
@@ -53,7 +53,7 @@  tests := tst-shm tst-clock tst-clock_nanosleep tst-timer tst-timer2 \
 	 tst-timer3 tst-timer4 tst-timer5 \
 	 tst-cpuclock1 tst-cpuclock2 \
 	 tst-cputimer1 tst-cputimer2 tst-cputimer3 \
-	 tst-clock2
+	 tst-clock2 tst-shm-cancel
 
 extra-libs := librt
 extra-libs-others := $(extra-libs)
diff --git a/rt/tst-shm-cancel.c b/rt/tst-shm-cancel.c
new file mode 100644
index 0000000..de70c1a
--- /dev/null
+++ b/rt/tst-shm-cancel.c
@@ -0,0 +1,135 @@ 
+/* Test for shm_open cancellation handling: BZ #18243.
+   Copyright (C) 2016 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
+   <http://www.gnu.org/licenses/>.  */
+
+#include <pthread.h>
+#include <sys/mman.h>
+#include <semaphore.h>
+#include <stdio.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <errno.h>
+#include <stdlib.h>
+
+static sem_t sem;	/* Use to sync with thread start.  */
+volatile int thread_ret;
+static const char shm_name[] = "/glibc-shm_open-cancel";
+
+static void
+remove_shm (int status, void *arg)
+{
+  shm_unlink (shm_name);
+}
+
+static void *
+tf (void *arg)
+{
+  thread_ret = 0;
+
+  pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, 0);
+
+  if (sem_wait (&sem) != 0)
+    { 
+      printf ("error: sem_wait failed: %m");
+      thread_ret = 1;
+      return NULL;
+    }
+
+  if (pthread_setcancelstate (PTHREAD_CANCEL_ENABLE, 0) != 0)
+    {
+      printf ("error: pthread_setcancelstate failed: %m");
+      thread_ret = 1;
+      return NULL;
+    }
+
+  /* Neither sem_unlink or sem_open should act on thread cancellation.  */
+  shm_unlink (shm_name);
+  on_exit (remove_shm, NULL);
+
+  int fd = shm_open (shm_name, O_CREAT, 0600);
+  if (fd == -1)
+    {
+      if (errno == ENOSYS || errno == EACCES)
+	thread_ret = 77;
+      else
+	thread_ret = 1;
+      return NULL;
+    }
+
+  if (pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, 0) != 0)
+    {
+      printf ("error: pthread_setcancelstate failed: %m");
+      thread_ret = 1;
+      return NULL;
+    }
+
+  if (close (fd) != 0)
+    {
+      printf ("error: pthread_setcancelstate failed: %m");
+      thread_ret = 1;
+    }
+
+  return NULL;
+}
+
+static int
+do_test (void)
+{
+  pthread_t td;
+
+  if (sem_init (&sem, 0, 0))
+    {
+      printf ("error: sem_init failed: %m\n");
+      return 1;
+    }
+
+  if (pthread_create (&td, NULL, tf, NULL) != 0)
+    {
+      printf ("error: pthread_create failed: %m\n");
+      return 1;
+    }
+
+  if (pthread_cancel (td) != 0)
+    {
+      printf ("error: pthread_cancel failed: %m\n");
+      return 1;
+    }
+
+  if (sem_post (&sem) != 0)
+    {
+      printf ("error: sem_post failed: %m\n");
+      return 1;
+    }
+
+  void *r;
+  if (pthread_join (td, &r) != 0)
+    {
+      printf ("error: pthread_join failed: %m\n");
+      return 1;
+    }
+
+  if (r == PTHREAD_CANCELED)
+    {
+      puts ("error: pthread_join returned PTHREAD_CANCELED");
+      return 1;
+    }
+
+  return thread_ret;
+}
+
+#define TEST_FUNCTION do_test ()
+#include <test-skeleton.c>
diff --git a/sysdeps/posix/shm_open.c b/sysdeps/posix/shm_open.c
index f296162..0182e7b 100644
--- a/sysdeps/posix/shm_open.c
+++ b/sysdeps/posix/shm_open.c
@@ -40,6 +40,11 @@  shm_open (const char *name, int oflag, mode_t mode)
 # ifdef O_CLOEXEC
   oflag |= O_CLOEXEC;
 # endif
+
+  /* Disable asynchronous cancellation.  */
+  int state;
+  pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, &state);
+
   int fd = open (shm_name, oflag, mode);
   if (fd == -1 && __glibc_unlikely (errno == EISDIR))
     /* It might be better to fold this error with EINVAL since
@@ -70,6 +75,8 @@  shm_open (const char *name, int oflag, mode_t mode)
     }
 # endif
 
+  pthread_setcancelstate (state, NULL);
+
   return fd;
 }