From mboxrd@z Thu Jan 1 00:00:00 1970 From: Oleg Nesterov Subject: Re: /proc/pid/fd && anon_inode_fops Date: Sun, 25 Aug 2013 17:45:56 +0200 Message-ID: <20130825154556.GA25922@redhat.com> References: <20130822185317.GI31117@1wt.eu> <20130822201530.GL31117@1wt.eu> <20130824182939.GA23630@redhat.com> <20130824212432.GA9299@1wt.eu> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Andy Lutomirski , Linus Torvalds , "security@kernel.org" , Ingo Molnar , Linux Kernel Mailing List , Al Viro , Linux FS Devel , Brad Spengler To: Willy Tarreau Return-path: Content-Disposition: inline In-Reply-To: <20130824212432.GA9299@1wt.eu> Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org Hi Willy, On 08/24, Willy Tarreau wrote: > > On Sat, Aug 24, 2013 at 08:29:39PM +0200, Oleg Nesterov wrote: > > > > On 08/22, Willy Tarreau wrote: > > > > > > It's not only that, it also supports sockets and pipes that you can access > > > via /proc/pid/fd and not via a real symlink which would try to open eg > > > "pipe:[23456]" instead of the real file. > > > > But sock_no_open() disallows this, and for good reason I guess. > > Hmmm not exactly, it works for a pipe but not for a socket. But this is what I meant, sorry for confusion. Let me try to explain. Just in case, this has nothing to do with security and I do not see any problem, still I think there is something wrong (but harmless). Suppose that you are trying to open(/proc/pid/$pipe-or-socket-fd). nd_jump_link() sets nd->inode correctly, then dentry_open() does the rest. Everything is correct at this stage, the new file gets the correct f_inode/f_op. However, unlike fifo_open(), socket_file_ops->open() can not actually create the file/sock connection, so sock_no_open() just fails and nothing bad happens. But if you open an anon_inodefs file via proc, you get the "bogus" file. There is a single anon_inode_inode, its ->i_fop points to the empty anon_inode_fops, this has nothing to do with ->f_op of the actual file you tried to open. Nothing bad happens, still I think this is simply wrong and misleading, and thus I think it would be better to disallow this via anon_open(). Oleg.