Re: I finally prepared a testcase for read(inotify_fd) getting EINTR on PTRACE_ATTACH

[Oleg Nesterov <oleg@redhat.com>]

Add CC's,

On 02/10, Denys Vlasenko wrote:
>
> I recalled that I saw a spurious EINTR on strace attach.
> I found time/inspiration to create an isolated testcase for it.
> Already committed it to ptrace-tests.
>
> The code: skip all the cruft, read reproduce() function body.
> It's quite straightforward.
>
> $ gcc -Wall -Os eintr-on-attach.c -oeintr-on-attach
> $ ./eintr-on-attach 1;echo 1
> bug: read was interrupted by attach, errno: Interrupted system call

At first glance this looks obvious? I never used inotify and I never
looked into fs/notify/inotify/, but it seems that inotify_read() simply
returns -EINTR if signal_pending() and doesn't implement restarts.

Probably this trivial change


	--- x/fs/notify/inotify/inotify_user.c
	+++ x/fs/notify/inotify/inotify_user.c
	@@ -264,7 +264,7 @@ static ssize_t inotify_read(struct file 
			ret = -EAGAIN;
			if (file->f_flags & O_NONBLOCK)
				break;
	-		ret = -EINTR;
	+		ret = -ERESTARTSYS;
			if (signal_pending(current))
				break;
	 

makes sense.

> 1
>
> -- 
> vda
>
>
> /* Attach to a process which is blocked in read syscall on inotify fd.
>    Syscall should be restarted. It is not.
>
>    This software is provided 'as-is', without any express or implied
>    warranty.  In no event will the authors be held liable for any damages
>    arising from the use of this software.
>
>    Permission is granted to anyone to use this software for any purpose,
>    including commercial applications, and to alter it and redistribute it
>    freely.  */
>
> #define _GNU_SOURCE 1
> #include <assert.h>
> #include <limits.h>
> #include <stddef.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> #include <errno.h>
> #include <time.h>
> #include <stdio.h>
> #include <sched.h>
> #include <signal.h>
> #include <dirent.h>
> #include <fcntl.h>
> #include <sys/types.h>
> #include <sys/wait.h>
> #include <sys/syscall.h>
> /* #include <pthread.h> */
> /* Dance around ptrace.h + user.h incompatibility */
> #ifdef __ia64__
> # define ia64_fpreg ia64_fpreg_DISABLE
> # define pt_all_user_regs pt_all_user_regs_DISABLE
> #endif
> #include <sys/ptrace.h>
> #include <linux/ptrace.h>
> #ifdef __ia64__
> # undef ia64_fpreg
> # undef pt_all_user_regs
> #endif
> #include <sys/user.h>
> #if defined __i386__ || defined __x86_64__
> # include <sys/debugreg.h>
> #endif
>
> static int verbose;
>
> #define VERBOSE(...) do { \
>   if (verbose) \
>     { \
>       printf (__VA_ARGS__); \
>       fflush (stdout); \
>     } \
> } while (0)
>
> static pid_t child;
> /*static pid_t grandchild;*/
>
> static void
> sigkill (pid_t * pp)
> {
>   pid_t pid = *pp;
>   *pp = 0;
>   if (pid > 0)
>     kill (pid, SIGKILL);
> }
>
> static void
> cleanup (void)
> {
>   /*sigkill (&grandchild); */
>   sigkill (&child);
>   while (waitpid (-1, NULL, __WALL) > 0)
>     continue;
> }
>
> static void
> handler_fail (int signo)
> {
>   sigset_t set;
>   signal (SIGABRT, SIG_DFL);
>   signal (SIGALRM, SIG_DFL);
>   /* SIGALRM may be blocked in sighandler, need to unblock */
>   sigfillset (&set);
>   sigprocmask (SIG_UNBLOCK, &set, NULL);
>   /* Due to kernel bugs, waitpid may block. Need to have a timeout */
>   alarm (1);
>   cleanup ();
>   assert (0);
> }
>
> /****************** Standard scaffolding ends here ****************/
>
> #include <sys/inotify.h>
>
> /*
>  * Read from inotify fd is spuriously interrupted by PTRACE_ATTACH.
>  *
>  * Kernel 3.2.1-3.fc16 (and probably many kernels before it) is affected.
>  * The bug is deterministic.
>  */
>
> /* If the test is not deterministic:
>  * Amount of seconds needed to almost 100% catch it */
> //#define DEFAULT_TESTTIME 5
> /* or (if reproducible in a few loops only) */
> //#define DEFAULT_LOOPS 10
>
> /* If nothing strange happens, just returns.
>  * Notable events (which are not bugs) print some sort of marker
>  * if verbose is on, but still continue and return normally.
>  * Known bugs also print a message if verbose, but they exit (1).
>  * New bugs are likely to trip asserts or cause hang/kernel crash :)
>  */
> static void
> reproduce(void)
> {
>   int status;
>
>   alarm(1);
>
>   child = fork();
>   assert (child != -1);
>   if (child == 0)
>     {
>       char buf[4096];
>       int inotify_fd, wd;
>       int len;
>       const char *filename = getenv("TEST_FILENAME");
>       if (!filename)
>         filename = "/dev/null";
>
>       inotify_fd = inotify_init();
>       assert(inotify_fd >= 0);
>       wd = inotify_add_watch(inotify_fd, filename, IN_DELETE_SELF);
>       assert(wd >= 0);
>
>       signal(SIGALRM, SIG_DFL);
>       raise(SIGSTOP);
>
>       errno = 0;
>       len = read(inotify_fd, buf, sizeof(buf));
>       if (len < 0)
>         VERBOSE("bug: read was interrupted by attach, errno: %s\n", strerror(errno));
>       exit(0);
>     }
>
>   /* Wait for child to stop before read */
>   assert(waitpid(-1, &status, WUNTRACED) == child);
>   assert(WIFSTOPPED(status));
>   assert(WSTOPSIG(status) == SIGSTOP);
>   kill(child, SIGCONT);
>   usleep(500*1000);
>   /* now child has to be blocked in read syscall */
>
>   /* Attach and continue */
>   assert(ptrace(PTRACE_ATTACH, child, 0, 0) == 0);
>   assert(waitpid(-1, &status, 0) == child);
>   assert(ptrace(PTRACE_CONT, child, 0, 0) == 0);
>
>   /* Kernel should restart read syscall. SIGALRM should kill the child in ~0.5 second */
>   assert(waitpid(-1, &status, 0) == child);
>   if (WIFEXITED(status)) /* we saw the bug */
>     exit(1);
>   assert(WIFSIGNALED(status));
>   assert(WTERMSIG(status) == SIGALRM);
>   cleanup();
> }
>
> int
> main (int argc, char **argv)
> {
> #if defined DEFAULT_TESTTIME || defined DEFAULT_LOOPS
>   int i;
>   char *env_testtime = getenv ("TESTTIME");	/* misnomer */
>   int testtime = (env_testtime ? atoi (env_testtime) : 1);
> #endif
>
>   setbuf (stdout, NULL);
>   atexit (cleanup);
>   signal (SIGINT, handler_fail);
>   signal (SIGABRT, handler_fail);
>   signal (SIGALRM, handler_fail);
>   verbose = (argc - 1);
>
> #if defined DEFAULT_TESTTIME
>   testtime *= DEFAULT_TESTTIME;
>   for (i = 0; i < testtime; i++)
>     {
>       time_t t = time (NULL);
>       while (t == time (NULL))
>         {
>           VERBOSE (".");
> 	  reproduce ();
>         }
>     }
>   VERBOSE ("\n");
> #elif defined DEFAULT_LOOPS
>   testtime *= DEFAULT_LOOPS;
>   for (i = 0; i < testtime; i++)
>     {
>       VERBOSE (".");
>       reproduce ();
>     }
>   VERBOSE ("\n");
> #else
>   reproduce ();
> #endif
>
>   return 0;
> }