From mboxrd@z Thu Jan 1 00:00:00 1970 From: Linus Torvalds Subject: Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH) Date: Thu, 22 Aug 2013 11:48:10 -0700 Message-ID: References: Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Cc: "security@kernel.org" , Ingo Molnar , Willy Tarreau , Linux Kernel Mailing List , Oleg Nesterov , Al Viro , Linux FS Devel , Brad Spengler To: Andy Lutomirski Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org 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 fundamentally checks that one special case in a place that is impossible to check for the /proc case, so the proc case remains totally separate. Which just bothers me. I think we could easily at least allow the "file->f_creds == current->creds" case (and yes, I literally mean comparing the pointers - not only is it cheaper, but it literally means "nothing odd has happened in between opening and the lookup"). And I'm wondering if we shouldn't actually do that at "path_init" time. Right now the code says: /* Caller must check execute permissions on the starting path component */ struct fd f = fdget_raw(dfd); and then uses the struct file mindlessly. I'm wondering if we should just do some validation in that place, and say: - for directories, we require exec permissions here - for everything else, we require that f->f_cred == current->cred check. I dunno. But that I_LINKABLE thing just bothers me. It screams "I'm hacky" to me. Linus