Hello,
On Fri 02-05-14 22:52:43, Anton Altaparmakov wrote:
> Perhaps I am missing something (in which case I would appreciate a hit
> with a clue bat!) but it looks to me like there is a bug in
> fs/buffer.c::block_page_mkwrite().
>
> Consider the case where "size = i_size_read()" is X and page_offset() is
> also X, i.e. the page being made dirty starts at the end of the file.
>
> In this case the first check passes because page_offset == size rather
> than being > size - This looks wrong to me - surely the page is entirely
> outside the file size "size" even when its first byte is at "size" offset
> as "size" is the first byte outside the file:
>
> if ((page->mapping != inode->i_mapping) ||
> (page_offset(page) > size)) {
Yup, that looks like a bug, although a pretty harmless one since such a
call can only happen when truncating mmaped file before we manage to
invalidate the page beyond EOF.
Honza
>
> After that is passed the next code happens:
>
> /* page is wholly or partially inside EOF */
> if (((page->index + 1) << PAGE_CACHE_SHIFT) > size)
> end = size & ~PAGE_CACHE_MASK;
> else
> end = PAGE_CACHE_SIZE;
>
> We now do have a "true" condition in the if clause and we also know that "size" is page aligned thus "end = size & ~PAGE_CACHE_MASK" evaluates as "end = 0;".
>
> Thus we then get __block_write_begin() being called with "end" equal 0:
>
> ret = __block_write_begin(page, 0, end, get_block);
> if (!ret)
> ret = block_commit_write(page, 0, end);
>
> So we get __block_write_begin an then block_commit_write with both pos and len being zero.
>
> Then the page is marked dirty and we return it.
>
> Unless I am correct that this is a bug, what is the reasoning for doing this instead of simply bailing out on the page straight away at the top of the function by changing the test to page_offset(page) >= size?
>
> Best regards,
>
> Anton
> --
> Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
> Unix Support, Computing Service, University of Cambridge
> J.J. Thomson Avenue, Cambridge, CB3 0RB, UK
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
