linux-kernel - Re: [GIT PULL] direct IO support for loop driver

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, 20 Nov 2013 13:38:19 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Dave Kleikamp <dave.kleikamp@...cle.com>
Cc:	Christoph Hellwig <hch@...radead.org>,
	LKML <linux-kernel@...r.kernel.org>,
	"linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
	"Maxim V. Patlasov" <mpatlasov@...allels.com>, linux-aio@...ck.org,
	Kent Overstreet <kmo@...erainc.com>,
	Jens Axboe <axboe@...nel.dk>
Subject: Re: [GIT PULL] direct IO support for loop driver

On Wed, Nov 20, 2013 at 1:19 PM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> At that point, I just couldn't take it any more.

Just to clarify, I think it might be fixable. But it does need fixing,
because I really feel dirty from reading it. And I may not care all
that deeply about what random drivers or low-level filesystems do, but
I *do* care about generic code in mm/ and fs/, so making those iovec
functions uglier makes me go all "Hulk angry! Hulk smash" on the code.

The whole "separate out checking from user copy" needs to go away.
There's no excuse for it.

The whole "if (atomic) do_atomic_ops() else do_regular_ops()" crap
needs to go away. You can do it either by just duplicating the
function, or by having it use a indirect function for the copy (and
that indirect function acts like copy_from/to_user() and checks the
address range - and you can obviously then also have it be a "copy
from kernel" thing too if you want/need it). And no, you don't then
make it do *both* the conditional *and* the function pointer like you
did in that discusting commit that mixes the two with the struct
iov_iter_ops).

The "__" versions that don't check the user address range needs to die entirely.

The whole crazy "ii_iov_xyz" naming needs to go away. It doesn't even
make sense (one of the "i"s is for "iov".

That "unsigned long data" that contains an iovec *? WTF? How did that
ever start making sense?

IOW, there are many many details that just make me absolutely detest
this series. Enough that there's no way in hell I feel comfortable
pulling it. But they are likely fixable.

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