linux-ext4 - Re: [PATCH] jbd_commit_transaction() races with journal_try_to_drop

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: <20080514181444.GI24363@duck.suse.cz>
Date:	Wed, 14 May 2008 20:14:45 +0200
From:	Jan Kara <jack@...e.cz>
To:	Mingming Cao <cmm@...ibm.com>
Cc:	Badari Pulavarty <pbadari@...ibm.com>, akpm@...ux-foundation.org,
	linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] jbd_commit_transaction() races with
	journal_try_to_drop_buffers() causing DIO failures

On Wed 14-05-08 10:41:12, Mingming Cao wrote:
> > > Bummer, we can't free buffers in ext3_launder_page() before calling
> > > try_to_free_page, as later
> > > invalidate_complete_page2()->try_to_free_page() expecting the page
> > > buffers are still here, and will return EIO if it launder_page() has
> > > already freed those buffers.:(
> >   Are you sure? Because if bufferes are released in ext3_launder_page(),
> > PagePrivate() has been set to 0 and we should directly fall through to
> > releasing the page without ever calling try_to_release_page()... So I'd
> > want to find out why PagePrivate is still set in
> > invalidate_complete_page2().
> > 
> 
> You are right. PagePrivate() is being set to 0 in drop_buffers(). 
> 
> The problem is do_launder_page() returns successfully if the page is not
> dirty (our case), so ext3_launder_page() is not even get called. This
> also explains why the log_wait_commit() approach doesn't work for me:(
  I didn't realize PageDirty() is going to be already cleared by previous
writes... :(

> Have to think other ways...could we pass some flag to
> journal_try_to_free_buffers(), and ask journal_try_to_free_buffers()
> wait for jbd commit to finish flushing the data, if the request is from
> directo IO?
  Well, we could do that but we'd have to change try_to_release_page() call
to accept an extra argument which would consequently mean changing all the
filesystems... But yes, it probably makes sence because it is really
different whether we should just release the page because of memory
pressure or because direct IO needs to write to that area of the file.
So adding the parameter to releasepage() callback is probably a reasonable
thing to do.

								Honza
-- 
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