Attachment 312564 Details for Bug 455729 – patch -- fix wrong error code on interrupted close() syscalls

[patch] patch -- fix wrong error code on interrupted close() syscalls

0001--fix-wrong-error-code-on-interrupted-close-syscalls.patch (text/plain), 2.60 KB, created by Jeff Layton on 2008-07-24 14:33:03 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeff Layton

Created: 2008-07-24 14:33:03 UTC

Size: 2.60 KB

patch

obsolete

>From ee731f4f7880b09ca147008ab46ad4e5f72cb8bf Mon Sep 17 00:00:00 2001
>From: Ernie Petrides <petrides@redhat.com>
>Date: Fri, 29 Sep 2006 02:00:13 -0700
>Subject: [PATCH] [PATCH] fix wrong error code on interrupted close syscalls
>
>The problem is that close() syscalls can call a file system's flush
>handler, which in turn might sleep interruptibly and ultimately pass back
>an -ERESTARTSYS return value.  This happens for files backed by an
>interruptible NFS mount under nfs_file_flush() when a large file has just
>been written and nfs_wait_bit_interruptible() detects that there is a
>signal pending.
>
>I have a test case where the "strace" command is used to attach to a
>process sleeping in such a close().  Since the SIGSTOP is forced onto the
>victim process (removing it from the thread's "blocked" mask in
>force_sig_info()), the RPC wait is interrupted and the close() is
>terminated early.
>
>But the file table entry has already been cleared before the flush handler
>was called.  Thus, when the syscall is restarted, the file descriptor
>appears closed and an EBADF error is returned (which is wrong).  What's
>worse, there is the hypothetical case where another thread of a
>multi-threaded application might have reused the file descriptor, in which
>case that file would be mistakenly closed.
>
>The bottom line is that close() syscalls are not restartable, and thus
>-ERESTARTSYS return values should be mapped to -EINTR.  This is consistent
>with the close(2) manual page.  The fix is below.
>
>Signed-off-by: Ernie Petrides <petrides@redhat.com>
>Cc: Roland McGrath <roland@redhat.com>
>Signed-off-by: Andrew Morton <akpm@osdl.org>
>Signed-off-by: Linus Torvalds <torvalds@osdl.org>
>---
> fs/open.c |   12 +++++++++++-
> 1 files changed, 11 insertions(+), 1 deletions(-)
>
>diff --git a/fs/open.c b/fs/open.c
>index 1574d8f..304c1c7 100644
>--- a/fs/open.c
>+++ b/fs/open.c
>@@ -1173,6 +1173,7 @@ asmlinkage long sys_close(unsigned int fd)
> 	struct file * filp;
> 	struct files_struct *files = current->files;
> 	struct fdtable *fdt;
>+	int retval;
> 
> 	spin_lock(&files->file_lock);
> 	fdt = files_fdtable(files);
>@@ -1185,7 +1186,16 @@ asmlinkage long sys_close(unsigned int fd)
> 	FD_CLR(fd, fdt->close_on_exec);
> 	__put_unused_fd(files, fd);
> 	spin_unlock(&files->file_lock);
>-	return filp_close(filp, files);
>+	retval = filp_close(filp, files);
>+
>+	/* can't restart close syscall because file table entry was cleared */
>+	if (unlikely(retval == -ERESTARTSYS ||
>+		     retval == -ERESTARTNOINTR ||
>+		     retval == -ERESTARTNOHAND ||
>+		     retval == -ERESTART_RESTARTBLOCK))
>+		retval = -EINTR;
>+
>+	return retval;
> 
> out_unlock:
> 	spin_unlock(&files->file_lock);
>-- 
>1.5.5.1
>

Actions: View | Diff

Attachments on bug 455729: 312564