From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754050Ab3HWBHi (ORCPT ); Thu, 22 Aug 2013 21:07:38 -0400 Received: from zeniv.linux.org.uk ([195.92.253.2]:36030 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752965Ab3HWBHh (ORCPT ); Thu, 22 Aug 2013 21:07:37 -0400 Date: Fri, 23 Aug 2013 02:07:26 +0100 From: Al Viro To: Linus Torvalds Cc: Andy Lutomirski , Willy Tarreau , "security@kernel.org" , Ingo Molnar , Linux Kernel Mailing List , Oleg Nesterov , Linux FS Devel , Brad Spengler Subject: Re: [PATCH v2] vfs: Tighten up linkat(..., AT_EMPTY_PATH) Message-ID: <20130823010726.GP27005@ZenIV.linux.org.uk> References: <20130822185317.GI31117@1wt.eu> <20130822201530.GL31117@1wt.eu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 there are many > levels of nesting outside of it. > > > With the current code structure, trying to enforce some kind of > > security restriction in the middle of lookup seems really unpleasant. > > If it's conditional (ie "linkat behaves differently from openat"), it > certainly means that we'd have to pass in that info in annoying ways. Nope. All we need to pass is one more LOOKUP_... Add if (unlikely(nd->last_type == LAST_BIND)) { if ((nd->flags & LOOKUP_BLAH) && !may_flink(...)) { terminate_walk(nd); return -EINVAL; } } in the beginning of lookup_last() and pass LOOKUP_BLAH in flags when linkat() calls user_path_at(). That will affect *only* the terminal symlinks and cost nothing in all normal cases. The same check can bloody well go into path_init() - take if (*name) { if (!can_lookup(dentry->d_inode)) { fdput(f); return -ENOTDIR; } } in there and slap else { if ((flags & LOOKUP_BLAH) && !may_flink(...)) { fdput(f); return -EINVAL; } } after it.