linux-ext4 - Re: Checks in ext4_ext_fiemap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGBYx2YV5-yNA9cRwU0ojmMnOqNsc_gLLGrc+jenDBk4uYbM8g@mail.gmail.com>
Date:	Tue, 26 Jul 2011 20:48:21 +0800
From:	Yongqiang Yang <xiaoqiangnk@...il.com>
To:	Jan Kara <jack@...e.cz>,
	Allison Henderson <achender@...ux.vnet.ibm.com>
Cc:	"Ted Ts'o" <tytso@....edu>, linux-ext4@...r.kernel.org,
	Andreas Dilger <adilger@...ger.ca>
Subject: Re: Checks in ext4_ext_fiemap_cb() broken

On Tue, Jul 26, 2011 at 8:12 PM, Jan Kara <jack@...e.cz> wrote:
>  Hi Yongqiang,
>
> On Tue 26-07-11 09:20:28, Yongqiang Yang wrote:
>> I have been thinking if we can handle fiemap much simpler for a while.
>>  Current code is very ugly due to page cache look up.  I have a
>> thought on simplifying these code.  The reason leading us to looking
>> up page cache is that delayed extents are not in extents tree.  I
>> think we can add an in-memory delayed extents list in inode, and we
>> can delete entries in the list after we allocate blocks for them.
>> There is no limit on length of extents in the list, this way can an
>> entry contain as many blocks as they are contiguous logically.
>>
>> What's your opinion?
>  Yes, that should be doable and shouldn't have too big overhead. It's just
> stupid we'll do all this stuff only for fiemap call which is relatively
> rare.

I guess there are other places where delayed extents should be handled
by looking up page cache.

SEEK_HOLE and SEEK_DATA also need to lookup page cache to handle
delayed extents.

Hi Allison,

If a delayed extents list added in the inode, could punch hole code be simpler?


Yongqiang.
>
>                                                                Honza
>
>> On Mon, Jul 25, 2011 at 11:58 PM, Jan Kara <jack@...e.cz> wrote:
>> >  Hello,
>> >
>> >  I just had a look at the code checking delayed allocated buffers in
>> > ext4_ext_fiemap_cb(). I believe the checks there could use some elimiation
>> > of common patterns but that's just a minor thing. The main problem is that
>> > the code can easily crash the kernel when it races with page reclaim. You
>> > just cannot access most of the page contents (and for buffers it is
>> > especially true) without locking the page. Getting a reference via
>> > find_get_pages_tag() guarantees you the structure cannot go away but mm is
>> > still free to detach the page from the mapping at any moment. So you must
>> > always lock a page and check that it still belongs to the desired mapping
>> > before you check 'page_has_buffers()'.
>> >
>> >                                                                Honza
>> > --
>> > Jan Kara <jack@...e.cz>
>> > SUSE Labs, CR
>> >
>>
>>
>>
>> --
>> Best Wishes
>> Yongqiang Yang
> --
> Jan Kara <jack@...e.cz>
> SUSE Labs, CR
>



-- 
Best Wishes
Yongqiang Yang
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html