Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-26 Thread Andy Lutomirski
On Mon, Aug 26, 2013 at 11:07 AM, Linus Torvalds wrote: > On Mon, Aug 26, 2013 at 10:37 AM, Linus Torvalds > wrote: >> >> I'm playing with a patch that then in addition to the ptrace check >> *also* requires that the file was opened with the same credentials as >> the follower _or_ the task

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-26 Thread Linus Torvalds
On Mon, Aug 26, 2013 at 10:37 AM, Linus Torvalds wrote: > > I'm playing with a patch that then in addition to the ptrace check > *also* requires that the file was opened with the same credentials as > the follower _or_ the task being followed. I'll see if that works out. Chrome sandboxing still

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-26 Thread Linus Torvalds
On Sun, Aug 25, 2013 at 1:23 PM, Linus Torvalds wrote: > > So I'll just go back to square one, and wonder if we could/should just > make the rule be that in order to be in that LAST_BIND case, you > really have to have f_cred match your own credentials. Or have > CAP_SEARCH. Nope. That doesn't

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-26 Thread Linus Torvalds
On Sun, Aug 25, 2013 at 1:23 PM, Linus Torvalds torva...@linux-foundation.org wrote: So I'll just go back to square one, and wonder if we could/should just make the rule be that in order to be in that LAST_BIND case, you really have to have f_cred match your own credentials. Or have

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-26 Thread Linus Torvalds
On Mon, Aug 26, 2013 at 10:37 AM, Linus Torvalds torva...@linux-foundation.org wrote: I'm playing with a patch that then in addition to the ptrace check *also* requires that the file was opened with the same credentials as the follower _or_ the task being followed. I'll see if that works out.

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-26 Thread Andy Lutomirski
On Mon, Aug 26, 2013 at 11:07 AM, Linus Torvalds torva...@linux-foundation.org wrote: On Mon, Aug 26, 2013 at 10:37 AM, Linus Torvalds torva...@linux-foundation.org wrote: I'm playing with a patch that then in addition to the ptrace check *also* requires that the file was opened with the same

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-25 Thread Linus Torvalds
On Sun, Aug 25, 2013 at 1:06 PM, Al Viro wrote: > > Timestamp updates, chmod/chown, xattr mess... Ok, so that's just too much details. So I'll just go back to square one, and wonder if we could/should just make the rule be that in order to be in that LAST_BIND case, you really have to have

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-25 Thread Al Viro
On Sun, Aug 25, 2013 at 12:57:25PM -0700, Linus Torvalds wrote: > Yes. I think we should do this, but I think we should also look at > what _other_ LOOKUP_xyz we should do for the /proc case. > > For the read-only fd case, we should have a LOOKUP_WRITE flag, and > return -EPERM if an operation

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-25 Thread Linus Torvalds
On Sat, Aug 24, 2013 at 8:37 PM, Al Viro wrote: > > FWIW, I'm tempted to try the following trick: > * introduce FMODE_FLINK in file->f_mode; O_TMPFILE would set it, > unless O_EXCL is present. > * introduce LOOKUP_LINK, to be passed by sys_linkat() when > resolving the target. [

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-25 Thread Andy Lutomirski
On Sun, Aug 25, 2013 at 7:23 AM, Al Viro wrote: > On Sun, Aug 25, 2013 at 12:26:34AM -0700, Andy Lutomirski wrote: > >> I think this is more screwed up than just flink and open. For example: >> >> $ echo 'WTF' >test >> $ truncate -s 1 /proc/self/fd/3 3> $ cat test >> W$ >> >> IMO that should

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-25 Thread Al Viro
On Sun, Aug 25, 2013 at 12:26:34AM -0700, Andy Lutomirski wrote: > I think this is more screwed up than just flink and open. For example: > > $ echo 'WTF' >test > $ truncate -s 1 /proc/self/fd/3 3 $ cat test > W$ > > IMO that should have failed. Why? truncate() always follows links, so

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-25 Thread Andy Lutomirski
On Sat, Aug 24, 2013 at 8:37 PM, Al Viro wrote: > On Fri, Aug 23, 2013 at 02:07:26AM +0100, Al Viro wrote: >> On Thu, Aug 22, 2013 at 01:54:15PM -0700, Linus Torvalds wrote: >> > On Thu, Aug 22, 2013 at 1:48 PM, Andy Lutomirski >> > wrote: >> > > >> > > Sure. But aren't they always last? >> >

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-25 Thread Andy Lutomirski
On Sat, Aug 24, 2013 at 8:37 PM, Al Viro v...@zeniv.linux.org.uk wrote: On Fri, Aug 23, 2013 at 02:07:26AM +0100, Al Viro wrote: On Thu, Aug 22, 2013 at 01:54:15PM -0700, Linus Torvalds wrote: On Thu, Aug 22, 2013 at 1:48 PM, Andy Lutomirski l...@amacapital.net wrote: Sure. But

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-25 Thread Al Viro
On Sun, Aug 25, 2013 at 12:26:34AM -0700, Andy Lutomirski wrote: I think this is more screwed up than just flink and open. For example: $ echo 'WTF' test $ truncate -s 1 /proc/self/fd/3 3test $ cat test W$ IMO that should have failed. Why? truncate() always follows links, so what's

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-25 Thread Andy Lutomirski
On Sun, Aug 25, 2013 at 7:23 AM, Al Viro v...@zeniv.linux.org.uk wrote: On Sun, Aug 25, 2013 at 12:26:34AM -0700, Andy Lutomirski wrote: I think this is more screwed up than just flink and open. For example: $ echo 'WTF' test $ truncate -s 1 /proc/self/fd/3 3test $ cat test W$ IMO that

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-25 Thread Linus Torvalds
On Sat, Aug 24, 2013 at 8:37 PM, Al Viro v...@zeniv.linux.org.uk wrote: FWIW, I'm tempted to try the following trick: * introduce FMODE_FLINK in file-f_mode; O_TMPFILE would set it, unless O_EXCL is present. * introduce LOOKUP_LINK, to be passed by sys_linkat() when resolving

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-25 Thread Al Viro
On Sun, Aug 25, 2013 at 12:57:25PM -0700, Linus Torvalds wrote: Yes. I think we should do this, but I think we should also look at what _other_ LOOKUP_xyz we should do for the /proc case. For the read-only fd case, we should have a LOOKUP_WRITE flag, and return -EPERM if an operation is a

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-25 Thread Linus Torvalds
On Sun, Aug 25, 2013 at 1:06 PM, Al Viro v...@zeniv.linux.org.uk wrote: Timestamp updates, chmod/chown, xattr mess... Ok, so that's just too much details. So I'll just go back to square one, and wonder if we could/should just make the rule be that in order to be in that LAST_BIND case, you

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-24 Thread Al Viro
On Fri, Aug 23, 2013 at 02:07:26AM +0100, Al Viro wrote: > On Thu, Aug 22, 2013 at 01:54:15PM -0700, Linus Torvalds wrote: > > On Thu, Aug 22, 2013 at 1:48 PM, Andy Lutomirski > > wrote: > > > > > > Sure. But aren't they always last? > > > > What do you mean? I'd say that the /proc lookup is

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-24 Thread Al Viro
On Fri, Aug 23, 2013 at 02:07:26AM +0100, Al Viro wrote: On Thu, Aug 22, 2013 at 01:54:15PM -0700, Linus Torvalds wrote: On Thu, Aug 22, 2013 at 1:48 PM, Andy Lutomirski l...@amacapital.net wrote: Sure. But aren't they always last? What do you mean? I'd say that the /proc lookup

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Al Viro
On Thu, Aug 22, 2013 at 01:54:15PM -0700, Linus Torvalds wrote: > On Thu, Aug 22, 2013 at 1:48 PM, Andy Lutomirski wrote: > > > > Sure. But aren't they always last? > > What do you mean? I'd say that the /proc lookup is always *innermost*. > Which means that it certainly cannot bail out, since

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Andy Lutomirski
On Thu, Aug 22, 2013 at 1:54 PM, Linus Torvalds wrote: > On Thu, Aug 22, 2013 at 1:48 PM, Andy Lutomirski wrote: >> >> Sure. But aren't they always last? > > What do you mean? I'd say that the /proc lookup is always *innermost*. > Which means that it certainly cannot bail out, since there are

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Linus Torvalds
On Thu, Aug 22, 2013 at 1:48 PM, Andy Lutomirski wrote: > > Sure. But aren't they always last? What do you mean? I'd say that the /proc lookup is always *innermost*. Which means that it certainly cannot bail out, since there are many levels of nesting outside of it. > With the current code

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Andy Lutomirski
On Thu, Aug 22, 2013 at 1:44 PM, Linus Torvalds wrote: > On Thu, Aug 22, 2013 at 1:22 PM, Andy Lutomirski wrote: >> >> Let me rephrase that: why do we allow these types of lookup to recurse >> like normal symlinks? I'm proposing that these links immediately >> terminate lookup [..] > > It can

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Linus Torvalds
On Thu, Aug 22, 2013 at 1:22 PM, Andy Lutomirski wrote: > > Let me rephrase that: why do we allow these types of lookup to recurse > like normal symlinks? I'm proposing that these links immediately > terminate lookup [..] It can nest *inside* a regular symlink. So there should not be any

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Andy Lutomirski
On Thu, Aug 22, 2013 at 1:15 PM, Willy Tarreau wrote: > On Thu, Aug 22, 2013 at 01:10:43PM -0700, Andy Lutomirski wrote: >> What's the point of nd_jump_link anyway? The only way I can think of >> for a magic symlink in /proc to point to another symlink is to open a >> symlink with O_PATH |

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Willy Tarreau
On Thu, Aug 22, 2013 at 01:10:43PM -0700, Andy Lutomirski wrote: > What's the point of nd_jump_link anyway? The only way I can think of > for a magic symlink in /proc to point to another symlink is to open a > symlink with O_PATH | O_NOFOLLOW. Actually trying to use the > resulting link in /proc

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Andy Lutomirski
On Thu, Aug 22, 2013 at 12:23 PM, Linus Torvalds wrote: > On Thu, Aug 22, 2013 at 12:05 PM, Andy Lutomirski wrote: >> >> Does this work for the procfs case? As far as I understand it (which >> isn't saying much), it goes through the symlink-following path. > > Right. The /proc case is still

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Willy Tarreau
On Thu, Aug 22, 2013 at 12:05:50PM -0700, Andy Lutomirski wrote: > On Thu, Aug 22, 2013 at 11:53 AM, Willy Tarreau wrote: > > On Thu, Aug 22, 2013 at 11:48:10AM -0700, Linus Torvalds wrote: > >> And I'm wondering if we shouldn't actually do that at "path_init" > >> time. Right now the code says:

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Linus Torvalds
On Thu, Aug 22, 2013 at 12:05 PM, Andy Lutomirski wrote: > > Does this work for the procfs case? As far as I understand it (which > isn't saying much), it goes through the symlink-following path. Right. The /proc case is still separate, and we really should do something about that too. But

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Andy Lutomirski
On Thu, Aug 22, 2013 at 11:53 AM, Willy Tarreau wrote: > On Thu, Aug 22, 2013 at 11:48:10AM -0700, Linus Torvalds wrote: >> On Wed, Aug 21, 2013 at 12:14 PM, Andy Lutomirski >> wrote: >> > >> > So let's be careful for now: only allow linkat(..., AT_EMPTY_PATH) >> > if the target is I_LINKABLE.

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Willy Tarreau
On Thu, Aug 22, 2013 at 11:48:10AM -0700, Linus Torvalds wrote: > On Wed, Aug 21, 2013 at 12:14 PM, Andy Lutomirski wrote: > > > > So let's be careful for now: only allow linkat(..., AT_EMPTY_PATH) > > if the target is I_LINKABLE. > > So I really detest this just because it's such a special

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Linus Torvalds
On Wed, Aug 21, 2013 at 12:14 PM, Andy Lutomirski wrote: > > So let's be careful for now: only allow linkat(..., AT_EMPTY_PATH) > if the target is I_LINKABLE. So I really detest this just because it's such a special case. Now this is only useful for that one special case, and the thing very

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Linus Torvalds
On Wed, Aug 21, 2013 at 12:14 PM, Andy Lutomirski l...@amacapital.net wrote: So let's be careful for now: only allow linkat(..., AT_EMPTY_PATH) if the target is I_LINKABLE. So I really detest this just because it's such a special case. Now this is only useful for that one special case, and the

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Willy Tarreau
On Thu, Aug 22, 2013 at 11:48:10AM -0700, Linus Torvalds wrote: On Wed, Aug 21, 2013 at 12:14 PM, Andy Lutomirski l...@amacapital.net wrote: So let's be careful for now: only allow linkat(..., AT_EMPTY_PATH) if the target is I_LINKABLE. So I really detest this just because it's such a

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Andy Lutomirski
On Thu, Aug 22, 2013 at 11:53 AM, Willy Tarreau w...@1wt.eu wrote: On Thu, Aug 22, 2013 at 11:48:10AM -0700, Linus Torvalds wrote: On Wed, Aug 21, 2013 at 12:14 PM, Andy Lutomirski l...@amacapital.net wrote: So let's be careful for now: only allow linkat(..., AT_EMPTY_PATH) if the target

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Linus Torvalds
On Thu, Aug 22, 2013 at 12:05 PM, Andy Lutomirski l...@amacapital.net wrote: Does this work for the procfs case? As far as I understand it (which isn't saying much), it goes through the symlink-following path. Right. The /proc case is still separate, and we really should do something about

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Willy Tarreau
On Thu, Aug 22, 2013 at 12:05:50PM -0700, Andy Lutomirski wrote: On Thu, Aug 22, 2013 at 11:53 AM, Willy Tarreau w...@1wt.eu wrote: On Thu, Aug 22, 2013 at 11:48:10AM -0700, Linus Torvalds wrote: And I'm wondering if we shouldn't actually do that at path_init time. Right now the code says:

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Andy Lutomirski
On Thu, Aug 22, 2013 at 12:23 PM, Linus Torvalds torva...@linux-foundation.org wrote: On Thu, Aug 22, 2013 at 12:05 PM, Andy Lutomirski l...@amacapital.net wrote: Does this work for the procfs case? As far as I understand it (which isn't saying much), it goes through the symlink-following

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Willy Tarreau
On Thu, Aug 22, 2013 at 01:10:43PM -0700, Andy Lutomirski wrote: What's the point of nd_jump_link anyway? The only way I can think of for a magic symlink in /proc to point to another symlink is to open a symlink with O_PATH | O_NOFOLLOW. Actually trying to use the resulting link in /proc

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Andy Lutomirski
On Thu, Aug 22, 2013 at 1:15 PM, Willy Tarreau w...@1wt.eu wrote: On Thu, Aug 22, 2013 at 01:10:43PM -0700, Andy Lutomirski wrote: What's the point of nd_jump_link anyway? The only way I can think of for a magic symlink in /proc to point to another symlink is to open a symlink with O_PATH |

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Linus Torvalds
On Thu, Aug 22, 2013 at 1:22 PM, Andy Lutomirski l...@amacapital.net wrote: Let me rephrase that: why do we allow these types of lookup to recurse like normal symlinks? I'm proposing that these links immediately terminate lookup [..] It can nest *inside* a regular symlink. So there should

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Andy Lutomirski
On Thu, Aug 22, 2013 at 1:44 PM, Linus Torvalds torva...@linux-foundation.org wrote: On Thu, Aug 22, 2013 at 1:22 PM, Andy Lutomirski l...@amacapital.net wrote: Let me rephrase that: why do we allow these types of lookup to recurse like normal symlinks? I'm proposing that these links

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Linus Torvalds
On Thu, Aug 22, 2013 at 1:48 PM, Andy Lutomirski l...@amacapital.net wrote: Sure. But aren't they always last? What do you mean? I'd say that the /proc lookup is always *innermost*. Which means that it certainly cannot bail out, since there are many levels of nesting outside of it. With the

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Andy Lutomirski
On Thu, Aug 22, 2013 at 1:54 PM, Linus Torvalds torva...@linux-foundation.org wrote: On Thu, Aug 22, 2013 at 1:48 PM, Andy Lutomirski l...@amacapital.net wrote: Sure. But aren't they always last? What do you mean? I'd say that the /proc lookup is always *innermost*. Which means that it

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-22 Thread Al Viro
On Thu, Aug 22, 2013 at 01:54:15PM -0700, Linus Torvalds wrote: On Thu, Aug 22, 2013 at 1:48 PM, Andy Lutomirski l...@amacapital.net wrote: Sure. But aren't they always last? What do you mean? I'd say that the /proc lookup is always *innermost*. Which means that it certainly cannot bail

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-21 Thread Willy Tarreau
On Wed, Aug 21, 2013 at 12:33:24PM -0700, Andy Lutomirski wrote: > On Wed, Aug 21, 2013 at 12:29 PM, Linus Torvalds > wrote: > > On my phone, sorry. Removed lists due to HTML crud.. OK no HTML from me here, so re-adding the 3 lists if that can help reaching an end faster on this stuff. > > If

[PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-21 Thread Andy Lutomirski
There have long been two ways to ask the kernel to create a new hardlink to the inode represented by an fd: linkat(..., AT_EMPTY_PATH) and AT_SYMLINK_FOLLOW on /proc/self/fd/N. The latter has no particular security restrictions, but the former required privilege until: commit

[PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-21 Thread Andy Lutomirski
There have long been two ways to ask the kernel to create a new hardlink to the inode represented by an fd: linkat(..., AT_EMPTY_PATH) and AT_SYMLINK_FOLLOW on /proc/self/fd/N. The latter has no particular security restrictions, but the former required privilege until: commit

Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH)

2013-08-21 Thread Willy Tarreau
On Wed, Aug 21, 2013 at 12:33:24PM -0700, Andy Lutomirski wrote: On Wed, Aug 21, 2013 at 12:29 PM, Linus Torvalds torva...@linux-foundation.org wrote: On my phone, sorry. Removed lists due to HTML crud.. OK no HTML from me here, so re-adding the 3 lists if that can help reaching an end faster