From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751971AbXAWBbR (ORCPT ); Mon, 22 Jan 2007 20:31:17 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1751986AbXAWBbR (ORCPT ); Mon, 22 Jan 2007 20:31:17 -0500 Received: from py-out-1112.google.com ([64.233.166.183]:41257 "EHLO py-out-1112.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751971AbXAWBbQ (ORCPT ); Mon, 22 Jan 2007 20:31:16 -0500 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=beta; h=received:message-id:date:from:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references; b=JUfDLln929V2wXlgqAt5dS68dkCmxy80picg10TRKxe1zmrA09rcVas3BOIOOL6QoCgC+jIhcjaoz5hIc5/DJ72OHSwv7DcixcLQW2844ynX8OuuOW6wQNXMZhBMJE4psDO5S10yUP0ILsm3qsPNYGLod254NDxnXBejZbWrwzM= Message-ID: <34781ae60701221731k508d8e97yed69929de5a38977@mail.gmail.com> Date: Tue, 23 Jan 2007 09:31:15 +0800 From: "yang yin" To: "Neil Brown" Subject: Re: [patch] md: bitmap read_page error Cc: linux-raid@vger.kernel.org, linux-kernel@vger.kernel.org In-Reply-To: <17845.16620.741319.612842@notabene.brown> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <34781ae60701220003v76bd43c8h79c59b2b474b3c57@mail.gmail.com> <17845.16620.741319.612842@notabene.brown> Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org I think your patch is not enough to slove the read_page error completely. I think in the bitmap_init_from_disk we also need to check the 'count' never exceeds the size of file before calling the read_page function. How do your think about it. Thanks your reply. 2007/1/23, Neil Brown : > On Monday January 22, yinyang801120@gmail.com wrote: > > If the bitmap size is less than one page including super_block and > > bitmap and the inode's i_blkbits is also small, when doing the > > read_page function call to read the sb_page, it may return a error. > > For example, if the device is 12800 chunks, its bitmap file size is > > about 1.6KB include the bitmap super block. But the inode i_blkbits > > value of the bitmap file is 10, the read_page will submit 4 bh to > > load the sb_page. Because the size of bitmap is only 1.6KB, in the > > while loop, the error will ocurr when do bmap operation for the block > > 2, which will return 0. Then the bitmap can't be initated because of > > ther read sb page fail. > > > > Another error is in the bitmap_init_from_disk function. Before doing > > read_page,. calculating the count value misses the size of super > > block. When the bitmap just needs one page, It will read two pages > > adding the super block. But at the second read, the count value will > > be set to 0, and not all the bitmap will be read from the disk and > > some bitmap will missed at the second page. > > > > I give a patch as following: > > Thanks a lot for this. > Rather than checking the file size in read_page, I would like to make > sure the 'count' that is passed in never exceeds the size of the > file. This should have the same effect. > > So this is that patch I plan to submit. > > Thanks again, > NeilBrown > > > ------------------ > Avoid reading past the end of a bitmap file. > > In most cases we check the size of the bitmap file before > reading data from it. However when reading the superblock, > we always read the first PAGE_SIZE bytes, which might not > always be appropriate. So limit that read to the size of the > file if appropriate. > > Also, we get the count of available bytes wrong in one place, > so that too can read past the end of the file. > > Cc: "yang yin" > Signed-off-by: Neil Brown > > ### Diffstat output > ./drivers/md/bitmap.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff .prev/drivers/md/bitmap.c ./drivers/md/bitmap.c > --- .prev/drivers/md/bitmap.c 2007-01-23 09:44:11.000000000 +1100 > +++ ./drivers/md/bitmap.c 2007-01-23 09:44:59.000000000 +1100 > @@ -479,9 +479,12 @@ static int bitmap_read_sb(struct bitmap > int err = -EINVAL; > > /* page 0 is the superblock, read it... */ > - if (bitmap->file) > - bitmap->sb_page = read_page(bitmap->file, 0, bitmap, PAGE_SIZE); > - else { > + if (bitmap->file) { > + loff_t isize = i_size_read(bitmap->file->f_mapping->host); > + int bytes = isize > PAGE_SIZE ? PAGE_SIZE : isize; > + > + bitmap->sb_page = read_page(bitmap->file, 0, bitmap, bytes); > + } else { > bitmap->sb_page = read_sb_page(bitmap->mddev, bitmap->offset, 0); > } > if (IS_ERR(bitmap->sb_page)) { > @@ -877,7 +880,8 @@ static int bitmap_init_from_disk(struct > int count; > /* unmap the old page, we're done with it */ > if (index == num_pages-1) > - count = bytes - index * PAGE_SIZE; > + count = bytes + sizeof(bitmap_super_t) > + - index * PAGE_SIZE; > else > count = PAGE_SIZE; > if (index == 0) { >