Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-11-05 Thread Linus Torvalds
On Wed, Nov 5, 2014 at 7:32 AM, Andy Lutomirski wrote: > > I have no idea whether it's a security risk, but it's a serious "wtf?!?" risk. Yeah. And considering that the only apparently user of this read-only O_TMPFILE interface doesn't actually need it anyway because it carries around the whole

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-11-05 Thread Andy Lutomirski
On Wed, Nov 5, 2014 at 7:21 AM, Eric Rannaud wrote: > On Wed, Nov 5, 2014 at 12:27 AM, Christoph Hellwig wrote: >> On Mon, Nov 03, 2014 at 11:04:27AM -0800, Linus Torvalds wrote: >>> Oh, so you don't actually need any file contents at all? >>> >>> If that is actually a real usage, then maybe we

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-11-05 Thread Eric Rannaud
On Wed, Nov 5, 2014 at 12:27 AM, Christoph Hellwig wrote: > On Mon, Nov 03, 2014 at 11:04:27AM -0800, Linus Torvalds wrote: >> Oh, so you don't actually need any file contents at all? >> >> If that is actually a real usage, then maybe we should just say that >> "O_TMPFILE|O_RDONLY" is fine, and

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-11-05 Thread Christoph Hellwig
On Mon, Nov 03, 2014 at 11:04:27AM -0800, Linus Torvalds wrote: > Oh, so you don't actually need any file contents at all? > > If that is actually a real usage, then maybe we should just say that > "O_TMPFILE|O_RDONLY" is fine, and remove the check that it has to be > writable. Wasn't this

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-11-05 Thread Christoph Hellwig
On Mon, Nov 03, 2014 at 02:07:07PM -0800, Jeremy Allison wrote: > Which we already do, actually.. Hmm, I didn't notice that part when implementing the atomic open, guess I need to dig deeper into the code. I still think supporting full open semantics with O_TMFILE + flink is useful, so that

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-11-05 Thread Christoph Hellwig
On Mon, Nov 03, 2014 at 02:07:07PM -0800, Jeremy Allison wrote: Which we already do, actually.. Hmm, I didn't notice that part when implementing the atomic open, guess I need to dig deeper into the code. I still think supporting full open semantics with O_TMFILE + flink is useful, so that

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-11-05 Thread Christoph Hellwig
On Mon, Nov 03, 2014 at 11:04:27AM -0800, Linus Torvalds wrote: Oh, so you don't actually need any file contents at all? If that is actually a real usage, then maybe we should just say that O_TMPFILE|O_RDONLY is fine, and remove the check that it has to be writable. Wasn't this disallowed

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-11-05 Thread Eric Rannaud
On Wed, Nov 5, 2014 at 12:27 AM, Christoph Hellwig h...@infradead.org wrote: On Mon, Nov 03, 2014 at 11:04:27AM -0800, Linus Torvalds wrote: Oh, so you don't actually need any file contents at all? If that is actually a real usage, then maybe we should just say that O_TMPFILE|O_RDONLY is

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-11-05 Thread Andy Lutomirski
On Wed, Nov 5, 2014 at 7:21 AM, Eric Rannaud e...@nanocritical.com wrote: On Wed, Nov 5, 2014 at 12:27 AM, Christoph Hellwig h...@infradead.org wrote: On Mon, Nov 03, 2014 at 11:04:27AM -0800, Linus Torvalds wrote: Oh, so you don't actually need any file contents at all? If that is actually a

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-11-05 Thread Linus Torvalds
On Wed, Nov 5, 2014 at 7:32 AM, Andy Lutomirski l...@amacapital.net wrote: I have no idea whether it's a security risk, but it's a serious wtf?!? risk. Yeah. And considering that the only apparently user of this read-only O_TMPFILE interface doesn't actually need it anyway because it carries

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-11-03 Thread Jeremy Allison
On Mon, Nov 03, 2014 at 10:49:24AM -0800, Eric Rannaud wrote: > On Mon, Nov 3, 2014 at 9:06 AM, Andy Lutomirski wrote: > >> That doesn't help because we explicitly reject O_RDONLY when combined > >> with O_TMPFILE. > > > > I think I'm missing something. How is an O_RDONLY temporary file > >

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-11-03 Thread Linus Torvalds
On Mon, Nov 3, 2014 at 10:49 AM, Eric Rannaud wrote: > > Isn't it because they are essentially emulating an atomic open() > capable of creating a file with inherited ACLs, according to > relatively complex rules? open *can* be used with O_CREAT|O_RDONLY > (touch(1) might do that), which would

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-11-03 Thread Eric Rannaud
On Mon, Nov 3, 2014 at 9:06 AM, Andy Lutomirski wrote: >> That doesn't help because we explicitly reject O_RDONLY when combined >> with O_TMPFILE. > > I think I'm missing something. How is an O_RDONLY temporary file > useful? Wouldn't you want an O_RDWR tempfile with mode 0400 or > something

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-11-03 Thread Andy Lutomirski
On Nov 3, 2014 12:34 AM, "Christoph Hellwig" wrote: > > On Fri, Oct 31, 2014 at 11:53:09AM -0700, Linus Torvalds wrote: > > On Fri, Oct 31, 2014 at 11:44 AM, Andy Lutomirski > > wrote: > > > > > > Does the patch in this thread not fix that? > > > > It should. Modulo the glibc problem that makes

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-11-03 Thread Linus Torvalds
On Mon, Nov 3, 2014 at 12:34 AM, Christoph Hellwig wrote: > > That doesn't help because we explicitly reject O_RDONLY when combined > with O_TMPFILE. You obviously cannot actually have a read-only file descriptor with O_TMPFILE, unless all you care about is a zero-sized file with no contents.

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-11-03 Thread Christoph Hellwig
On Fri, Oct 31, 2014 at 11:53:09AM -0700, Linus Torvalds wrote: > On Fri, Oct 31, 2014 at 11:44 AM, Andy Lutomirski wrote: > > > > Does the patch in this thread not fix that? > > It should. Modulo the glibc problem that makes it hard to actually set > the mode properly. That doesn't help

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-11-03 Thread Christoph Hellwig
On Fri, Oct 31, 2014 at 11:53:09AM -0700, Linus Torvalds wrote: On Fri, Oct 31, 2014 at 11:44 AM, Andy Lutomirski l...@amacapital.net wrote: Does the patch in this thread not fix that? It should. Modulo the glibc problem that makes it hard to actually set the mode properly. That doesn't

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-11-03 Thread Linus Torvalds
On Mon, Nov 3, 2014 at 12:34 AM, Christoph Hellwig h...@infradead.org wrote: That doesn't help because we explicitly reject O_RDONLY when combined with O_TMPFILE. You obviously cannot actually have a read-only file descriptor with O_TMPFILE, unless all you care about is a zero-sized file with

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-11-03 Thread Andy Lutomirski
On Nov 3, 2014 12:34 AM, Christoph Hellwig h...@infradead.org wrote: On Fri, Oct 31, 2014 at 11:53:09AM -0700, Linus Torvalds wrote: On Fri, Oct 31, 2014 at 11:44 AM, Andy Lutomirski l...@amacapital.net wrote: Does the patch in this thread not fix that? It should. Modulo the glibc

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-11-03 Thread Eric Rannaud
On Mon, Nov 3, 2014 at 9:06 AM, Andy Lutomirski l...@amacapital.net wrote: That doesn't help because we explicitly reject O_RDONLY when combined with O_TMPFILE. I think I'm missing something. How is an O_RDONLY temporary file useful? Wouldn't you want an O_RDWR tempfile with mode 0400 or

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-11-03 Thread Linus Torvalds
On Mon, Nov 3, 2014 at 10:49 AM, Eric Rannaud e...@nanocritical.com wrote: Isn't it because they are essentially emulating an atomic open() capable of creating a file with inherited ACLs, according to relatively complex rules? open *can* be used with O_CREAT|O_RDONLY (touch(1) might do that),

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-11-03 Thread Jeremy Allison
On Mon, Nov 03, 2014 at 10:49:24AM -0800, Eric Rannaud wrote: On Mon, Nov 3, 2014 at 9:06 AM, Andy Lutomirski l...@amacapital.net wrote: That doesn't help because we explicitly reject O_RDONLY when combined with O_TMPFILE. I think I'm missing something. How is an O_RDONLY temporary file

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-31 Thread Linus Torvalds
On Fri, Oct 31, 2014 at 11:44 AM, Andy Lutomirski wrote: > > Does the patch in this thread not fix that? It should. Modulo the glibc problem that makes it hard to actually set the mode properly. Linus -- To unsubscribe from this list: send the line "unsubscribe

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-31 Thread Andy Lutomirski
On Oct 31, 2014 1:42 AM, "Christoph Hellwig" wrote: > > On Thu, Oct 30, 2014 at 05:01:30PM -0700, Andy Lutomirski wrote: > > > flink()), the mode really matters. So this idiotic glibc behavior of > > > only forwarding the third argument if O_CREAT is set seems to be a > > > bug. > > > > We could

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-31 Thread Christoph Hellwig
On Thu, Oct 30, 2014 at 05:01:30PM -0700, Andy Lutomirski wrote: > > flink()), the mode really matters. So this idiotic glibc behavior of > > only forwarding the third argument if O_CREAT is set seems to be a > > bug. > > We could bite the bullet and add a tmpfile syscall. /me ducks I've got

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-31 Thread Christoph Hellwig
On Thu, Oct 30, 2014 at 05:01:30PM -0700, Andy Lutomirski wrote: flink()), the mode really matters. So this idiotic glibc behavior of only forwarding the third argument if O_CREAT is set seems to be a bug. We could bite the bullet and add a tmpfile syscall. /me ducks I've got another

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-31 Thread Andy Lutomirski
On Oct 31, 2014 1:42 AM, Christoph Hellwig h...@infradead.org wrote: On Thu, Oct 30, 2014 at 05:01:30PM -0700, Andy Lutomirski wrote: flink()), the mode really matters. So this idiotic glibc behavior of only forwarding the third argument if O_CREAT is set seems to be a bug. We could

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-31 Thread Linus Torvalds
On Fri, Oct 31, 2014 at 11:44 AM, Andy Lutomirski l...@amacapital.net wrote: Does the patch in this thread not fix that? It should. Modulo the glibc problem that makes it hard to actually set the mode properly. Linus -- To unsubscribe from this list: send the line

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-30 Thread Eric Rannaud
On Thu, Oct 30, 2014 at 6:04 PM, Linus Torvalds wrote: > On Thu, Oct 30, 2014 at 5:57 PM, Eric Rannaud wrote: >> Yes, there definitely is a glibc bug: a fix is being worked on and it >> looks like it will go in. The change replaces the test for O_CREAT by >> a test for either O_CREAT or

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-30 Thread Linus Torvalds
On Thu, Oct 30, 2014 at 5:57 PM, Eric Rannaud wrote: > > Yes, there definitely is a glibc bug: a fix is being worked on and it > looks like it will go in. The change replaces the test for O_CREAT by > a test for either O_CREAT or O_TMPFILE. Why not just do it unconditionally? There really is no

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-30 Thread Linus Torvalds
On Thu, Oct 30, 2014 at 5:01 PM, Andy Lutomirski wrote: > > Uh, because it's glibc? Yeah. Bloated, over-engineered, and stupid. > Or because it's trying not to screw up and on > some system where overrunning va_arg is terrible? No. On 99% of architectures the third argument is in a register

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-30 Thread Eric Rannaud
On Thu, Oct 30, 2014 at 3:58 PM, Linus Torvalds wrote: > On Thu, Oct 30, 2014 at 3:48 PM, Linus Torvalds > wrote: >> >> Agreed. Will apply and add the stable cc. > > Ho humm. Thinking about this some more, I'm starting to wonder. Not > about this patch per se (open on a newly created file should

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-30 Thread Andy Lutomirski
On Thu, Oct 30, 2014 at 3:58 PM, Linus Torvalds wrote: > On Thu, Oct 30, 2014 at 3:48 PM, Linus Torvalds > wrote: >> >> Agreed. Will apply and add the stable cc. > > Ho humm. Thinking about this some more, I'm starting to wonder. Not > about this patch per se (open on a newly created file should

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-30 Thread Linus Torvalds
On Thu, Oct 30, 2014 at 3:48 PM, Linus Torvalds wrote: > > Agreed. Will apply and add the stable cc. Ho humm. Thinking about this some more, I'm starting to wonder. Not about this patch per se (open on a newly created file should indeed succeed regardless), but about the horrible glibc behavior

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-30 Thread Linus Torvalds
On Thu, Oct 30, 2014 at 2:48 PM, Andy Lutomirski wrote: > > Looks sensible. Should this be Cc: stable? Agreed. Will apply and add the stable cc. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-30 Thread Andy Lutomirski
On 10/30/2014 02:08 AM, Eric Rannaud wrote: > The man page for open(2) indicates that when O_CREAT is specified, the > 'mode' argument applies only to future accesses to the file: > > Note that this mode applies only to future accesses of the newly > created file; the open() call that

[RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-30 Thread Eric Rannaud
The man page for open(2) indicates that when O_CREAT is specified, the 'mode' argument applies only to future accesses to the file: Note that this mode applies only to future accesses of the newly created file; the open() call that creates a read-only file may well return

[RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-30 Thread Eric Rannaud
The man page for open(2) indicates that when O_CREAT is specified, the 'mode' argument applies only to future accesses to the file: Note that this mode applies only to future accesses of the newly created file; the open() call that creates a read-only file may well return

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-30 Thread Andy Lutomirski
On 10/30/2014 02:08 AM, Eric Rannaud wrote: The man page for open(2) indicates that when O_CREAT is specified, the 'mode' argument applies only to future accesses to the file: Note that this mode applies only to future accesses of the newly created file; the open() call that

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-30 Thread Linus Torvalds
On Thu, Oct 30, 2014 at 2:48 PM, Andy Lutomirski l...@amacapital.net wrote: Looks sensible. Should this be Cc: stable? Agreed. Will apply and add the stable cc. Linus -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-30 Thread Linus Torvalds
On Thu, Oct 30, 2014 at 3:48 PM, Linus Torvalds torva...@linux-foundation.org wrote: Agreed. Will apply and add the stable cc. Ho humm. Thinking about this some more, I'm starting to wonder. Not about this patch per se (open on a newly created file should indeed succeed regardless), but about

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-30 Thread Andy Lutomirski
On Thu, Oct 30, 2014 at 3:58 PM, Linus Torvalds torva...@linux-foundation.org wrote: On Thu, Oct 30, 2014 at 3:48 PM, Linus Torvalds torva...@linux-foundation.org wrote: Agreed. Will apply and add the stable cc. Ho humm. Thinking about this some more, I'm starting to wonder. Not about this

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-30 Thread Eric Rannaud
On Thu, Oct 30, 2014 at 3:58 PM, Linus Torvalds torva...@linux-foundation.org wrote: On Thu, Oct 30, 2014 at 3:48 PM, Linus Torvalds torva...@linux-foundation.org wrote: Agreed. Will apply and add the stable cc. Ho humm. Thinking about this some more, I'm starting to wonder. Not about this

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-30 Thread Linus Torvalds
On Thu, Oct 30, 2014 at 5:01 PM, Andy Lutomirski l...@amacapital.net wrote: Uh, because it's glibc? Yeah. Bloated, over-engineered, and stupid. Or because it's trying not to screw up and on some system where overrunning va_arg is terrible? No. On 99% of architectures the third argument is

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-30 Thread Linus Torvalds
On Thu, Oct 30, 2014 at 5:57 PM, Eric Rannaud e...@nanocritical.com wrote: Yes, there definitely is a glibc bug: a fix is being worked on and it looks like it will go in. The change replaces the test for O_CREAT by a test for either O_CREAT or O_TMPFILE. Why not just do it unconditionally?

Re: [RFC PATCH] fs: allow open(dir, O_TMPFILE|..., 0) with mode 0

2014-10-30 Thread Eric Rannaud
On Thu, Oct 30, 2014 at 6:04 PM, Linus Torvalds torva...@linux-foundation.org wrote: On Thu, Oct 30, 2014 at 5:57 PM, Eric Rannaud e...@nanocritical.com wrote: Yes, there definitely is a glibc bug: a fix is being worked on and it looks like it will go in. The change replaces the test for