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
| ||
|
Message-Id: <1203398652.3612.7.camel@localhost.localdomain> Date: Mon, 18 Feb 2008 21:24:12 -0800 From: Mingming Cao <cmm@...ibm.com> To: "Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com> Cc: tytso@....edu, linux-ext4@...r.kernel.org Subject: Re: [PATCH] ext4: When reading from fallocated blocks make sure we return zero. On Tue, 2008-02-19 at 08:49 +0530, Aneesh Kumar K.V wrote: > On Mon, Feb 18, 2008 at 04:14:34PM -0800, Mingming Cao wrote: > > On Sat, 2008-02-16 at 08:53 +0530, Aneesh Kumar K.V wrote: > > > > How about the following patch? > > > > Regards, > > Mingming > > > > ext4: ext4_get_blocks_wrap fix for writing to preallocated > > From: Mingming Cao <cmm@...ibm.com> > > > > This patch fixed a issue with wrting to a preallocated blocks. > > A write hit a BUG_ON() in fs/buffer.c saying the buffer is not mapped. > > > > On the write path, ext4_get_block_wrap() is called with create=1, but it > > will pass create=0 down to the underlying ext4ext_get_blocks() > > to do a look up first. In the preallocation case, ext4_ext_get_blocks() > > with create = 0, will return number of blocks pre-allocated and buffer > > head unmapped. ext4_get_blocks_wrap() thinks it succeeds too early, without > > checking if it needs again call ext4_ext_get_blocks with create = 1 > > which would do proper handling for writing to preallocated blocks: > > split the extent to initialized and uninitialized one and > > returns the mapped buffer head. > > > > Treating preallocated blocks as holes equally(i.e. ignoring the number of blocks > > pre-allocated and returns 0) when get_blocks() is called with create = 0 is not enough. > > ext4_ext_get_blocks() needs to differentiate these two cases for delayed allocation > > purpose, as for holes it need to do reservation and prepare for later > > delayed allocation, but for pre-allocated blocks it needs skip that work. > > > > It would makes things more clear if we have clear definition of what > > get_blocks() return value means. > > > > Similar to ext4_get_blocks_handle(), the following > > * return > 0, # of blocks already allocated > > * if these are pre-allocated blocks and create = 0 > > * buffer head is unmapped > > * otherwise blocks are mapped. > > * > > * return = 0, if plain look up failed (blocks have not been allocated) > > * buffer head is unmapped > > * > > * return < 0, error case. > > > > The for the write path, at ext4_ext_get_blocks_wrap(), it could check the > > buffer_mapped() status for preallocated extent before quit too early. > > > > Signed-off-by: Mingming Cao <cmm@...ibm.com> > > Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@...ux.vnet.ibm.co> > > > > I guess we also need to make sure the buffer head have the mapped bit > set. Something like the patch below. > Good point. I modified the patch with clear_buffer_mapped() added at the begining of the wrapper function. Mingming > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index bc7081f..69ccda9 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -2294,6 +2294,7 @@ int ext4_ext_get_blocks(handle_t *handle, struct inode *inode, > struct ext4_allocation_request ar; > > __clear_bit(BH_New, &bh_result->b_state); > + __clear_bit(BH_Mapped, &bh_result->b_state); > ext_debug("blocks %u/%lu requested for inode %u\n", > iblock, max_blocks, inode->i_ino); > - 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
Powered by blists - more mailing lists