From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Lutomirski Subject: Re: [RFC PATCH] fs: Add user_file_or_path_at and use it for truncate Date: Tue, 27 Aug 2013 16:13:12 -0700 Message-ID: References: <7d1419dda1da70a8ad915f85b093a58b86bcaf3b.1377630856.git.luto@amacapital.net> <20130827230827.GI27005@ZenIV.linux.org.uk> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Cc: Linus Torvalds , Willy Tarreau , Ingo Molnar , "security@kernel.org" , Linux Kernel Mailing List , Oleg Nesterov , Linux FS Devel , Brad Spengler To: Al Viro Return-path: Received: from mail-vc0-f172.google.com ([209.85.220.172]:54657 "EHLO mail-vc0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752490Ab3H0XNd (ORCPT ); Tue, 27 Aug 2013 19:13:33 -0400 Received: by mail-vc0-f172.google.com with SMTP id m17so3521467vca.17 for ; Tue, 27 Aug 2013 16:13:32 -0700 (PDT) In-Reply-To: <20130827230827.GI27005@ZenIV.linux.org.uk> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Tue, Aug 27, 2013 at 4:08 PM, Al Viro wrote: > On Tue, Aug 27, 2013 at 12:16:34PM -0700, Andy Lutomirski wrote: >> This is an experiment to see if we can get nice semantics for all syscalls >> that either follow symlinks or allow AT_EMPTY_PATH without jumping through >> enormous hoops. This converts truncate (although you can't tell using >> truncate from coreutils, because it actually uses open + ftruncate). >> >> The basic idea is that there's a new helper function >> user_file_or_path_at. It takes an fd and a path and, depending on >> flags, the emptiness of the path, and whether path is a magic /proc >> symlink (or a symlink to a magic /proc/symlink), it returns either a >> struct path or a struct file *. > > No. I'm curious what's wrong with the general concept. (I agree that the implementation is heinous.) If I ever wanted to add a new *at syscall, I'd like to be able to do: int sys_whateverat(int dfd, const char __user *name, int flags) { resolve_the_thing(dfd, name, flags); if (it's a file) { check fmode; } else { inode_permission(); } actually_do_something(inode); unreference_whatever_i_got(); return ret; } thereby killing the fwhatever and whateverat birds with one stone. > >> + path_get(&nd->path); >> + if (nd->flags & LOOKUP_FILE) { >> + if (nd->last_symlink_file) >> + fput(nd->last_symlink_file); >> + nd->last_symlink_file = file; > > This is ugly (and costs quite a bit of overhead) > >> -static int proc_cwd_link(struct dentry *dentry, struct path *path) >> +static int proc_cwd_link(struct dentry *dentry, struct file_or_path *link) > > ... and this is even more vile. Vetoed, for being too ugly to live. ...phew. I wasn't looking forward to testing and debugging my crap :) --Andy