On 7.04.2017 23:11, Liu Bo wrote:
> Commit 2dabb3248453 ("Btrfs: Direct I/O read: Work on sectorsized blocks")
> introduced this bug during iterating bio pages in dio read's endio hook,
> and it could end up with segment fault of the dio reading task.
>
> So the reason is 'if (nr_sectors--)', and it makes the code assume that
> there is one more block in the same page, so page offset is increased and
> the bio which is created to repair the bad block then has an incorrect
> bvec.bv_offset, and a later access of the page content would throw a
> segment fault.
>
> This also adds ASSERT to check page offset against page size.
>
> Signed-off-by: Liu Bo <[email protected]>
> ---
> fs/btrfs/inode.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index c875e68..5e71f1e 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -7972,8 +7972,10 @@ static int __btrfs_correct_data_nocsum(struct inode
> *inode,
>
> start += sectorsize;
>
> - if (nr_sectors--) {
> + nr_sectors--;
Why not if(--nr_sectors)? I know it's more of a style issue but it will
reduce the size of the diff ?
> + if (nr_sectors) {
> pgoff += sectorsize;
> + ASSERT(pgoff < PAGE_SIZE);
> goto next_block_or_try_again;
> }
> }
> @@ -8074,8 +8076,10 @@ static int __btrfs_subio_endio_read(struct inode
> *inode,
>
> ASSERT(nr_sectors);
>
> - if (--nr_sectors) {
> + nr_sectors--;
> + if (nr_sectors) {
> pgoff += sectorsize;
> + ASSERT(pgoff < PAGE_SIZE);
> goto next_block;
> }
> }
>
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
