linux-ext4 - Re: [PATCH] ext4: Check io list state and avoid an unnecessary mutex_lock in ext4_end_io

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: <20111031150206.GC16825@thunk.org>
Date:	Mon, 31 Oct 2011 11:02:06 -0400
From:	Ted Ts'o <tytso@....edu>
To:	Tao Ma <tm@....ma>
Cc:	linux-ext4@...r.kernel.org
Subject: Re: [PATCH] ext4: Check io list state and avoid an unnecessary
 mutex_lock in ext4_end_io_work.

On Sun, Oct 30, 2011 at 03:50:25PM +0800, Tao Ma wrote:
> sorry, but I thought I had considered this case.
> There are 2 callers. One is ext4_end_io_work(which has the bug I pointed
> out), the other is ext4_flush_complete_IO which has already done the
> check before calling ext4_end_io_nolock. And that's the reason why I
> move the check from ext4_end_io_nolock to ext4_end_io_work. So for the
> ext4_flush_complete_IO case, your new patch will spin_lock twice for the
> checking. Do I miss something here?

Ah, you're right; my mistake.  When I looked closely, though, I found
that ext4_flush_completed_IO() had a call to list_empty() without
taking the spinlock, which would also be problematic.  When I looked
more closely, I found more ways to optimize things, which also close
up a few potential (I think theoretical) race conditions.

Let me know what you think....

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