Hi all,
I found a bug that cause the kernel to lockup.
The problem is in mm/shmem.c do_shmem_file_read() (the tmpfs)
at line 959 there is a call to file_read_actor(desc, page, offset, nr);
The problem is that inside the function file_read_actor() desc->error is
set to -EFAULT (this happens when the buffer supplied by the user for the
read is wrong)
but there is no check right after the return from file_read_actor() to
test this situation.
The result is that the desc->count field always stay the same and the
while loop in do_shmem_file_read never end and the kernel locksup.
The fix is very simple just add the following line after the call to
file_read_actor():
-------------------
if(desc->error)
break
-------------------
If you need any other info please mail me
Regards
--lior
________________________________________________________________
Lior Amar Distributed Computing Lab MOSIX
E-mail : [email protected]
________________________________________________________________
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;