splice: __generic_file_splice_read: fix read/truncate race
authorJens Axboe <jens.axboe@oracle.com>
Thu, 7 Jun 2007 07:39:42 +0000 (09:39 +0200)
committerJens Axboe <jens.axboe@oracle.com>
Fri, 8 Jun 2007 06:34:11 +0000 (08:34 +0200)
Original patch and description from Neil Brown <neilb@suse.de>,
merged and adapted to splice branch by me. Neils text follows:

__generic_file_splice_read() currently samples the i_size at the start
and doesn't do so again unless it needs to call ->readpage to load
a page.  After ->readpage it has to re-sample i_size as a truncate
may have caused that page to be filled with zeros, and the read()
call should not see these.

However there are other activities that might cause ->readpage to be
called on a page between the time that __generic_file_splice_read()
samples i_size and when it finds that it has an uptodate page. These
include at least read-ahead and possibly another thread performing a
read

So we must sample i_size *after* it has an uptodate page.  Thus the
current sampling at the start and after a read can be replaced with a
sampling before page addition into spd.

Signed-off-by: Jens Axboe <jens.axboe@oracle.com>
fs/splice.c

index 123fcdb2e4d99207e95c1993dea799ff0331c397..cb211360273ac77e21a7b4a5f44f4a401385d1a2 100644 (file)
@@ -413,37 +413,37 @@ __generic_file_splice_read(struct file *in, loff_t *ppos,
 
                                break;
                        }
+               }
+fill_it:
+               /*
+                * i_size must be checked after PageUptodate.
+                */
+               isize = i_size_read(mapping->host);
+               end_index = (isize - 1) >> PAGE_CACHE_SHIFT;
+               if (unlikely(!isize || index > end_index))
+                       break;
+
+               /*
+                * if this is the last page, see if we need to shrink
+                * the length and stop
+                */
+               if (end_index == index) {
+                       unsigned int plen;
 
                        /*
-                        * i_size must be checked after ->readpage().
+                        * max good bytes in this page
                         */
-                       isize = i_size_read(mapping->host);
-                       end_index = (isize - 1) >> PAGE_CACHE_SHIFT;
-                       if (unlikely(!isize || index > end_index))
+                       plen = ((isize - 1) & ~PAGE_CACHE_MASK) + 1;
+                       if (plen <= loff)
                                break;
 
                        /*
-                        * if this is the last page, see if we need to shrink
-                        * the length and stop
+                        * force quit after adding this page
                         */
-                       if (end_index == index) {
-                               unsigned int plen;
-
-                               /*
-                                * max good bytes in this page
-                                */
-                               plen = ((isize - 1) & ~PAGE_CACHE_MASK) + 1;
-                               if (plen <= loff)
-                                       break;
-
-                               /*
-                                * force quit after adding this page
-                                */
-                               this_len = min(this_len, plen - loff);
-                               len = this_len;
-                       }
+                       this_len = min(this_len, plen - loff);
+                       len = this_len;
                }
-fill_it:
+
                partial[page_nr].offset = loff;
                partial[page_nr].len = this_len;
                len -= this_len;