Re: [patch] fix infinite loop in generic_file_splice_read()

[Andrew Morton]

On Wed, 09 Apr 2008 17:57:56 +0200
Miklos Szeredi <miklos@xxxxxxxxxx> wrote:

> generic_file_splice_read() goes into an infinite loop if it races with
> truncation.  I've found this with fsx-linux on NFS over fuse.
> 
> Perhaps the whole while() loop is bogus, but I can't tell from a
> cursory glance at __generic_file_splice_read() if it will return zero
> only on EOF, or it can do that for other reasons as well.  In the
> latter case the loop is obviously needed.
> 
> This simplistic patch fixes the issue for me.
> 

We found suspicious-looking code in generic_file_splice_read() back in
February.  See http://lkml.org/lkml/2008/2/29/443.  I suspect that patch
(if it works) will address the truncate lockup as well - it zaps the loop
entirely.

Unfortunately Allard never got back to us (probably because he's running
2.6.24 which has a quite different generic_file_splice_read()) and the
patch didn't get anywhere.

Here it is again.

It needs a changelog.

Nobody has tested this at all, to my knowledge.

It's going to take some serious and sudden effort to get these bugs fixed
for 2.6.25.


 fs/splice.c |   31 ++++++-------------------------
 1 file changed, 6 insertions(+), 25 deletions(-)

diff -puN fs/splice.c~generic_file_splice_read-fix-lockups fs/splice.c
--- a/fs/splice.c~generic_file_splice_read-fix-lockups
+++ a/fs/splice.c
@@ -370,8 +370,10 @@ __generic_file_splice_read(struct file *
 			 * for an in-flight io page
 			 */
 			if (flags & SPLICE_F_NONBLOCK) {
-				if (TestSetPageLocked(page))
+				if (TestSetPageLocked(page)) {
+					error = -EAGAIN;
 					break;
+				}
 			} else
 				lock_page(page);
 
@@ -479,9 +481,8 @@ ssize_t generic_file_splice_read(struct 
 				 struct pipe_inode_info *pipe, size_t len,
 				 unsigned int flags)
 {
-	ssize_t spliced;
-	int ret;
 	loff_t isize, left;
+	int ret;
 
 	isize = i_size_read(in->f_mapping->host);
 	if (unlikely(*ppos >= isize))
@@ -491,29 +492,9 @@ ssize_t generic_file_splice_read(struct 
 	if (unlikely(left < len))
 		len = left;
 
-	ret = 0;
-	spliced = 0;
-	while (len && !spliced) {
-		ret = __generic_file_splice_read(in, ppos, pipe, len, flags);
-
-		if (ret < 0)
-			break;
-		else if (!ret) {
-			if (spliced)
-				break;
-			if (flags & SPLICE_F_NONBLOCK) {
-				ret = -EAGAIN;
-				break;
-			}
-		}
-
+	ret = __generic_file_splice_read(in, ppos, pipe, len, flags);
+	if (ret > 0)
 		*ppos += ret;
-		len -= ret;
-		spliced += ret;
-	}
-
-	if (spliced)
-		return spliced;
 
 	return ret;
 }
_

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/