linux-kernel - Re: [PATCH] Introduce a method to catch mmap_region (was: Recent kernel "mount" slow)

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:	Wed, 28 Nov 2012 14:50:56 -0500 (EST)
From:	Mikulas Patocka <mpatocka@...hat.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
cc:	Jens Axboe <axboe@...nel.dk>,
	Jeff Chua <jeff.chua.linux@...il.com>,
	Lai Jiangshan <laijs@...fujitsu.com>, Jan Kara <jack@...e.cz>,
	lkml <linux-kernel@...r.kernel.org>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH] Introduce a method to catch mmap_region (was: Recent
 kernel "mount" slow)

On Wed, 28 Nov 2012, Linus Torvalds wrote:

> No, this is crap.
> 
> We don't introduce random hooks like this just because the block layer
> has shit-for-brains and cannot be bothered to do things right.
> 
> The fact is, the whole locking in the block layer open routine is
> total and utter crap. It doesn't lock the right thing, even with your
> change *anyway* (or with the change Jens had). Absolutely nothing in
> "mmap_region()" cares at all about the block-size anywhere - it's
> generic, after all - so locking around it is f*cking pointless. There
> is no way in hell that the caller of ->mmap can *ever* care about the
> block size, since it never even looks at it.
> 
> Don't do random crap like this.
> 
> Why does the code think that mmap matters so much anyway? As you say,
> the mmap itself does *nothing*. It has no impact for the block size.
> 
>                  Linus

mmap_region() doesn't care about the block size. But a lot of 
page-in/page-out code does.

The problem is that once the block device is mapped, page faults or page 
writeback can happen anytime - so the simplest solution is to not allow 
the block device being mapped while we change block size.

The function set_blocksize takes bd_block_size_semaphore for write (that 
blocks read/write/mmap), then it calls sync_blockdev (now we are sure that 
there is no more writeback), then it changes the block size, then it calls 
kill_bdev (now we are sure that there are no more any pages with buffers 
with the old blocksize).

If you want to allow to change block size while a block device is mapped, 
you'd have to add explicit locks around all mm callbacks (so that the 
block size can't change while the callback is in progress) - and even 
then, there are some unsolvable cases - i.e. what are you going to do if 
the user mlocks a mapped block device and you change block size of that 
device? - you can't drop the pages (that would violate mlock semantics) 
and you can leave them there (because they have buffers with wrong size).

If you don't like what I sent, propose a different solution.

Mikulas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/