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: <20130802142324.GA20484@quack.suse.cz> Date: Fri, 2 Aug 2013 16:23:24 +0200 From: Jan Kara <jack@...e.cz> To: Ted Tso <tytso@....edu> Cc: linux-ext4@...r.kernel.org, Dave Jones <davej@...hat.com>, Zheng Liu <gnehzuil.liu@...il.com>, Jan Kara <jack@...e.cz> Subject: Re: [PATCH] ext4: Make ext4_writepages() resilient to i_size changes On Thu 01-08-13 00:42:12, Jan Kara wrote: > Inode size can arbitrarily change while writeback is in progress. This > can have various strange effects when we use one value of i_size for one > decision during writeback and another value of i_size for a different > decision during writeback. In particular a check for lblk < blocks in > mpage_map_and_submit_buffers() causes problems when i_size is reduced > while writeback is running because we can end up not using all blocks > we've allocated. Thus these blocks are leaked and also delalloc > accounting gets wrong manifesting as a warning like: > > ext4_da_release_space:1333: ext4_da_release_space: ino 12, to_free 1 > with only 0 reserved data blocks > > The problem can happen only when blocksize < pagesize because the check > for size is performed only after the first iteration of the mapping > loop. > > Fix the problem by removing the size check from the mapping loop. We > have an extent allocated so we have to use it all before checking for > i_size. We may call add_page_bufs_to_extent() unnecessarily but that > function won't do anything if passed block number is beyond file size. > > Also to avoid future surprises like this sample inode size when > starting writeback in ext4_writepages() and then use this sampled size > throughout the writeback call stack. Ted, please disregard this patch. It is buggy. I'll send a better fix soon. Honza > > Reported-by: Dave Jones <davej@...hat.com> > Reported-by: Zheng Liu <gnehzuil.liu@...il.com> > Signed-off-by: Jan Kara <jack@...e.cz> > --- > fs/ext4/inode.c | 21 ++++++++++++--------- > 1 file changed, 12 insertions(+), 9 deletions(-) > > diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c > index ba33c67..8bd0240 100644 > --- a/fs/ext4/inode.c > +++ b/fs/ext4/inode.c > @@ -1413,6 +1413,8 @@ static void ext4_da_page_release_reservation(struct page *page, > struct mpage_da_data { > struct inode *inode; > struct writeback_control *wbc; > + loff_t i_size; /* Inode size can change while we do writeback. > + * Use one fixed value to make things simpler */ > > pgoff_t first_page; /* The first page to write */ > pgoff_t next_page; /* Current page to examine */ > @@ -1943,7 +1945,7 @@ static bool add_page_bufs_to_extent(struct mpage_da_data *mpd, > ext4_lblk_t lblk) > { > struct inode *inode = mpd->inode; > - ext4_lblk_t blocks = (i_size_read(inode) + (1 << inode->i_blkbits) - 1) > + ext4_lblk_t blocks = (mpd->i_size + (1 << inode->i_blkbits) - 1) > >> inode->i_blkbits; > > do { > @@ -1968,12 +1970,11 @@ static bool add_page_bufs_to_extent(struct mpage_da_data *mpd, > static int mpage_submit_page(struct mpage_da_data *mpd, struct page *page) > { > int len; > - loff_t size = i_size_read(mpd->inode); > int err; > > BUG_ON(page->index != mpd->first_page); > - if (page->index == size >> PAGE_CACHE_SHIFT) > - len = size & ~PAGE_CACHE_MASK; > + if (page->index == mpd->i_size >> PAGE_CACHE_SHIFT) > + len = mpd->i_size & ~PAGE_CACHE_MASK; > else > len = PAGE_CACHE_SIZE; > clear_page_dirty_for_io(page); > @@ -2006,8 +2007,6 @@ static int mpage_map_and_submit_buffers(struct mpage_da_data *mpd) > struct inode *inode = mpd->inode; > struct buffer_head *head, *bh; > int bpp_bits = PAGE_CACHE_SHIFT - inode->i_blkbits; > - ext4_lblk_t blocks = (i_size_read(inode) + (1 << inode->i_blkbits) - 1) > - >> inode->i_blkbits; > pgoff_t start, end; > ext4_lblk_t lblk; > sector_t pblock; > @@ -2052,8 +2051,7 @@ static int mpage_map_and_submit_buffers(struct mpage_da_data *mpd) > bh->b_blocknr = pblock++; > } > clear_buffer_unwritten(bh); > - } while (++lblk < blocks && > - (bh = bh->b_this_page) != head); > + } while (lblk++, (bh = bh->b_this_page) != head); > > /* > * FIXME: This is going to break if dioread_nolock > @@ -2200,7 +2198,11 @@ static int mpage_map_and_submit_extent(handle_t *handle, > return err; > } while (map->m_len); > > - /* Update on-disk size after IO is submitted */ > + /* > + * Update on-disk size after IO is submitted. Here we cannot use > + * mpd->i_size as we must avoid increasing i_disksize when racing > + * truncate already set it to a small value. > + */ > disksize = ((loff_t)mpd->first_page) << PAGE_CACHE_SHIFT; > if (disksize > i_size_read(inode)) > disksize = i_size_read(inode); > @@ -2453,6 +2455,7 @@ static int ext4_writepages(struct address_space *mapping, > > mpd.inode = inode; > mpd.wbc = wbc; > + mpd.i_size = i_size_read(inode); > ext4_io_submit_init(&mpd.io_submit, wbc); > retry: > if (wbc->sync_mode == WB_SYNC_ALL || wbc->tagged_writepages) > -- > 1.8.1.4 > -- Jan Kara <jack@...e.cz> SUSE Labs, CR -- 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