On Thu, 1 Jun 2006 20:49:38 +0200
Olaf Hering <[email protected]> wrote:
>
> This script will cause cramfs decompression errors, on SMP at least:
>
> #!/bin/bash
> while :;do blockdev --flushbufs /dev/loop0;done </dev/null &>/dev/null&
> while :;do ps faxs </dev/null &>/dev/null&done </dev/null &>/dev/null&
> while :;do dmesg </dev/null &>/dev/null&done </dev/null &>/dev/null&
> while :;do find /mounts/instsys -type f -print0|xargs -0 cat &>/dev/null;done
>
> (The used executables come from the symlinked /mounts/instsys directory)
>
> ...
> Error -3 while decompressing!
> c0000000009592a2(2649)->c0000000edf87000(4096)
> Error -3 while decompressing!
> c000000000959298(2520)->c0000000edbc7000(4096)
> Error -3 while decompressing!
> c000000000959c70(2489)->c0000000f1482000(4096)
> Error -3 while decompressing!
> c00000000095a629(2355)->c0000000edaff000(4096)
> Error -3 while decompressing!
> ...
>
> Its a long standing bug, introduced in 2.6.2.
>
> cramfs_read() clears parts of the src buffer because the page is not uptodate.
> invalidate_bdev() called from block_ioctl(BLKFLSBUF) will set ClearPageUptodate()
> after cramfs_read() got the page from read_cache_page()
> If PageUptodate() fails, read the page again before using it.
> There is still a small window were the page may not be uptodate before copying
> its contents away.
>
> evms_access does the BLKFLSBUF ioctl (lots of them) on the loop device. This will
> corrupt the SuSE installation image on SMP kernels, leading to random segfaults.
>
OK, invalidate_inode_pages().
> +
> for (i = 0; i < BLKS_PER_BUF; i++) {
> - struct page *page = pages[i];
> - if (page) {
> - memcpy(data, kmap(page), PAGE_CACHE_SIZE);
> - kunmap(page);
> - page_cache_release(page);
> - } else
> - memset(data, 0, PAGE_CACHE_SIZE);
> + if (blocknr + i < devsize) {
> + page = NULL;
> + for (readagain = 0; readagain < 5; readagain++) {
> + page = read_cache_page(mapping, blocknr + i,
> + (filler_t *)mapping->a_ops->readpage,
> + NULL);
> + /* synchronous error? */
> + if (IS_ERR(page)) {
> + page = NULL;
> + break;
> + }
> + wait_on_page_locked(page);
> + if (PageUptodate(page))
> + break;
> + /* asynchronous error */
> + /* maybe BLKFLSBUF flushed the page */
> + page_cache_release(page);
> + page = NULL;
> + }
> + if (page) {
> + memcpy(data, kmap(page), PAGE_CACHE_SIZE);
> + kunmap(page);
> + page_cache_release(page);
> + } else
> + memset(data, 0, PAGE_CACHE_SIZE);
> + if (readagain)
> + printk(KERN_DEBUG "cramfs_read got %s Uptodate page after %d attempt(s)\n",
> + page ? "an" : "no", readagain);
> + }
This code absolutely needs a comment telling the poor reader what it's in
there for.
It's still racy. Do the memcpy while the page is locked:
retry:
read_cache_page()
lock_page()
if (!PageUptodate()) {
if (readagain-- != 0)
goto retry;
give_up();
}
memcpy();
unlock_page();
Also, let's use kmap_atomic() while we're in there.
Of course, this will all just fail less often than it presently does. We'd
be better off taking a lock if poss to keep the ioctl away. I'd have
thought that it'd be appropriate to take i_mutex while running
invalidate_inode_pages().
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
