Re: Problem

[Hugh Dickins <hugh@veritas.com>]

On Tue, 31 Dec 2002, Amar Lior wrote:
> 
> I found a bug that cause the kernel to lockup.

In 2.4, yes.  Coincidentally, Mikael Starvik reported this just a
couple of weeks ago, though it has been lurking there for a long time.
In 2.5 it was fixed (in ignorance of the problem) a little while ago,
and Marcelo already has fix below in his BK tree towards 2.4.20-pre3.

Anyway, thanks a lot for making sure we know about it.  (The code was
_nearly_ right, the loop should have terminated when nr returned from
file_read_actor becomes 0: but there were _two_ declarations of nr,
and the nr tested to terminate the loop remained 1 throughout).

Hugh

diff -Nru a/mm/shmem.c b/mm/shmem.c
--- a/mm/shmem.c	Thu Dec 26 22:32:38 2002
+++ b/mm/shmem.c	Thu Dec 26 22:32:38 2002
@@ -919,14 +919,13 @@
 	struct inode *inode = filp->f_dentry->d_inode;
 	struct address_space *mapping = inode->i_mapping;
 	unsigned long index, offset;
-	int nr = 1;
 
 	index = *ppos >> PAGE_CACHE_SHIFT;
 	offset = *ppos & ~PAGE_CACHE_MASK;
 
-	while (nr && desc->count) {
+	for (;;) {
 		struct page *page;
-		unsigned long end_index, nr;
+		unsigned long end_index, nr, ret;
 
 		end_index = inode->i_size >> PAGE_CACHE_SHIFT;
 		if (index > end_index)
@@ -956,12 +955,14 @@
 		 * "pos" here (the actor routine has to update the user buffer
 		 * pointers and the remaining count).
 		 */
-		nr = file_read_actor(desc, page, offset, nr);
-		offset += nr;
+		ret = file_read_actor(desc, page, offset, nr);
+		offset += ret;
 		index += offset >> PAGE_CACHE_SHIFT;
 		offset &= ~PAGE_CACHE_MASK;
 	
 		page_cache_release(page);
+		if (ret != nr || !desc->count)
+			break;
 	}
 
 	*ppos = ((loff_t) index << PAGE_CACHE_SHIFT) + offset;