linux-kernel - Re: [PATCH 00/17] [RFC] AFS: Implement OpenAFS pioctls(version)s

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: <alpine.LFD.2.01.0906171112530.16802@localhost.localdomain>
Date:	Wed, 17 Jun 2009 11:24:00 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	David Howells <dhowells@...hat.com>
cc:	Andreas Dilger <adilger@....com>, linux-kernel@...r.kernel.org,
	linux-fsdevel@...r.kernel.org, akpm@...ux-foundation.org,
	linux-afs@...ts.infradead.org, Alan Cox <alan@...rguk.ukuu.org.uk>
Subject: Re: [PATCH 00/17] [RFC] AFS: Implement OpenAFS pioctls(version)s

On Wed, 17 Jun 2009, David Howells wrote:

> Linus Torvalds <torvalds@...ux-foundation.org> wrote:
> 
> > What _I_ mean is that THIS IS IMPOSSIBLE TO DO FROM USER SPACE!
> > 
> > Try it. Not doable. User space simply doesn't know enough, and has 
> > fundamental races with mount/umount.
> 
> Ummm...  I'm not sure I completely agree.  If you've managed to open, say,
> "/afs", where's the race with mount/umount?

Well, if you mean that you're going to have a new system call that then 
passes in both the 'fd' from that /afs open, _and_ the pathname you want 
to work on, then sure.

But if you do that new system call, then what's the point again? You're 
back to pinfo() anyway.

> > I just don't think "pioctl()" is a good one.
> 
> Out of interest, why not?  Is it just because it's another multipexor?  Or is
> it because it's been abused to have pathless commands?

No. It's because it's another _typeless_ multiplexor.

Look at ioctl. It's a F*CKING DISASTER. Look at all the compat crap, and 
at the ioctl numbers that mean different things for different file types, 
and all the random sizing crap. You fixed the random sizing crap (at least 
it has well-defined "input" and "output" areas), and that's an 
improvement, but it's still just random numbers with no semantics.

Now, you can take two approaches:

 - learn from your mistake, and not do another f*cking disaster that just 
   takes a pathname instead of a fd. Do something else, that actually has 
   semantics and has a well-defined input and output buffer.

 - do the same stupid thing over again, and never learn.

And quite frankly, I know which of those choices I'd call "intelligent", 
and which of them I'd call "you're a f*cking moron for doing it".

And guess which one "pioctl()" is. Just take a wild stab at it.

> > You'd be better off with some modification of open and then use ioctl.
> 
> So you'd say use:
> 
> 	fd = open("/the/target/file", O_SUPPRESS | (nofollow?O_NOFOLLOW:0));
> 	ioctl(fd, cmd, &args);
> 	close(fd);

Yes, I think that would be better. It's not perfect, because I think ioctl 
is still a f*cking broken mess (and with the sizing issue, it's arguably 
_worse_ then your pioctl), but at least we're not adding _another_ broken 
mess.

So I don't think the above is great either.

What I'd really prefer is something that actually has semantics. Not just 
"here's input, here's output, do something random to it".

> Do you also disagree with OpenAFS's idea of creating a proc file to open so
> that you can do ioctls on that to emulate pioctl()?  That would serve also.

Oh yes, I think that's a piece of crap too.

			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/