linux-ext4 - Re: [REGRESSION] 998ef75ddb and aio-dio-invalidate-failure w/ data=journal

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]

Date:	Mon, 5 Oct 2015 16:58:15 +0100
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	"Theodore Ts'o" <tytso@....edu>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [REGRESSION] 998ef75ddb and aio-dio-invalidate-failure w/ data=journal

On Mon, Oct 5, 2015 at 4:22 PM, Theodore Ts'o <tytso@....edu> wrote:
>
> What I think is going on is that when we do attempt the copy, we end
> up marking the page dirty before we notice that we need to page fault
> in the page, which ends up triggering the warning that jbd2
> buffer_head that is supposed to be journaled has been marked dirty
> without calling ext4_handle_dirty_metadata() --- which is handled by
> ext4_journalled_write_end(), but which is now happening out of order
> given this commit.

Hmm. I suspect that we'll just need to revert that commit for now.

It does smell like jbd2 might be a bit too fragile here, and you might
be able to trigger the same issue by having some random race condition
where the user unmaps the memory in another thread in between the
iov_iter_fault_in_readable() and the actual
iov_iter_copy_from_user_atomic() call later. So I think that this
commit may not be buggy per se, as much as just exposing a problem in
jbd2, but I don't think that is something we can really fix at this
point in the release schedule.

Dave, comments?

> Is it possible that we can change iov_iter_copy_from_user_atomic(), to
> check for the error case before it marks the page dirty?

iov_iter_copy_from_user_atomic() doesn't mark anything dirty, it just
does the copy afaik. The dirtying is up to the write_begin/write_end
logic.

>  Or can we
> create a light-weight function which checks to see if the page needs
> to be faulted in which is lighter weight than
> iov_iter_fault_in_readable?

I'm not actually sure why Dave finds that function to be expensive
as-is. It should be a very cheap thing to do if it's already mapped,
and if the area isn't mapped it does need to be faulted in later
anyway, so it's not like you're really doing any extra work.

Dave, mind sharing the micro-benchmark or perhaps even just a kernel
profile of it? How is that "iov_iter_fault_in_readable()" so
noticeable? It really shouldn't be a big deal.

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