Re: 1352 NUL bytes at the end of a page? (was Re: Assertion `s && s->tree' failed: The saga continues.)

Vladimir Saveliev
2004-05-17T08:39:45Z

Hello

On Mon, 2004-05-17 at 11:46, Andrew Morton wrote:
> Andrew Morton <akpm@osdl.org> wrote:
> >
> > If an application does mmap(MAP_SHARED) of, say, a 2048 byte file and then
> > extends it:
> >
> > p = mmap(..., fd, ...);
> > ftructate(fd, 4096);
> > p[3000] = 1;
> >
> > A racing block_write_full_page() could fail to notice the extended i_size
> > and would decide to zap those 2048 bytes anyway.
>
> This should plug it.
>
> diff -puN mm/memory.c~ftruncate-vs-block_write_full_page mm/memory.c
> --- 25/mm/memory.c~ftruncate-vs-block_write_full_page 2004-05-17 00:33:07.060231368 -0700
> +++ 25-akpm/mm/memory.c 2004-05-17 00:41:00.924193096 -0700
> @@ -1208,6 +1208,8 @@ int vmtruncate(struct inode * inode, lof
> {
> struct address_space *mapping = inode->i_mapping;
> unsigned long limit;
> + loff_t i_size;
> + struct page *page;
>
> if (inode->i_size < offset)
> goto do_expand;
> @@ -1222,8 +1224,22 @@ do_expand:
> goto out_sig;
> if (offset > inode->i_sb->s_maxbytes)
> goto out;
> - i_size_write(inode, offset);
>
> + /*
> + * If there is a pagecache page at the current i_size we need to lock
> + * it while modifying i_size to synchronise against
> + * block_write_full_page()'s sampling of i_size. Otherwise
> + * block_write_full_page may decide to memset part of this page after
> + * the application extended the file size.
> + */

Don't down-ings i_sem in do_truncate and in generic_file_write take care
of this kind of race?

> + i_size = inode->i_size; /* don't need i_size_read() due to i_sem */
> + page = NULL;
> + if (i_size & (PAGE_CACHE_SIZE - 1))
> + page = find_lock_page(inode->i_mapping,
> + i_size >> PAGE_CACHE_SHIFT);
> + i_size_write(inode, offset);
> + if (page)
> + unlock_page(page);
> out_truncate:
> if (inode->i_op && inode->i_op->truncate)
> inode->i_op->truncate(inode);
>
> _
>
> The same could happen with a pwrite() in place of ftruncate:
>
> fd = open("2048-byte-file");
> p = mmap(..., MAP_SHARED, fd, ...);
> pwrite(fd, buf, 1, 4096);
> p[3000] = 1;
>
> But I doubt that bk does extending writes() against a file which is
> concurrently being modified via MAP_SHARED.
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>