From patchwork Wed Apr 26 13:47:31 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Miklos Szeredi X-Patchwork-Id: 9701407 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 4A9E460245 for ; Wed, 26 Apr 2017 13:47:45 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3916B28660 for ; Wed, 26 Apr 2017 13:47:45 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2A72B2865D; Wed, 26 Apr 2017 13:47:45 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id DD8362865D for ; Wed, 26 Apr 2017 13:47:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1947660AbdDZNrm (ORCPT ); Wed, 26 Apr 2017 09:47:42 -0400 Received: from mail-wm0-f67.google.com ([74.125.82.67]:36086 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1433376AbdDZNrj (ORCPT ); Wed, 26 Apr 2017 09:47:39 -0400 Received: by mail-wm0-f67.google.com with SMTP id u65so1207781wmu.3 for ; Wed, 26 Apr 2017 06:47:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=szeredi.hu; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=vrHqjD2fc4qRgnzggRUWFCkYEDRSiA9EI5MSzUGIy5g=; b=m72XBA9O1VcjFGXyUdQ4yyBIU6N4axb0oADpvK3a1s7KFoA5sLk6EDI/yXKpfOVIs9 3IpWIb6f1BZkMnfPW4YFT56S7S5RcCrKYglLqCVlFFNWp9Kfm3+IbBIKUks6Ih0pgu+P gyrOAoBE/0zTMFStDWJXmW1IDcqUGOGyAeJ50= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=vrHqjD2fc4qRgnzggRUWFCkYEDRSiA9EI5MSzUGIy5g=; b=KRKidFQ5/1pkbyZ/KpbH8IpkY4l48vvcA0+t3sIpgxc+XH08qNGII3xTYmYPKCO3+W p6TanO+7tf2Y95Zw8Yr1DcXP2GgKO+NnM+kX7sbGnmLH/DlLsr7tHjhlZZ4hae8JAgbP gKQE7+dDbK25q+hlWK+xQ0keOoxH7t8r9rnpSeEtI6h0UQHDCNW1GB73obWP8rUMeOec 0IuROqNaiYLXe4Er+flaw1sJdXwZEM3RcRtiQPwL8Qy10ZQFfxseiQ8UyMScQoXULac/ c7A9J3UtCBJAwjDJuxKbn8TIaoKSXu7G4tdD100lAth3oJv8aFhGm1D6GTKYn0VZOrD0 pB/g== X-Gm-Message-State: AN3rC/7rH5OzJb7e/G6VeleOQ/oYlTq6FSKhCcNX1EsrhR45OjoTbcP/ ZetnSUPQOaxTrw== X-Received: by 10.28.129.65 with SMTP id c62mr943347wmd.79.1493214455521; Wed, 26 Apr 2017 06:47:35 -0700 (PDT) Received: from veci.piliscsaba.szeredi.hu (catv-176-63-54-97.catv.broadband.hu. [176.63.54.97]) by smtp.gmail.com with ESMTPSA id x20sm266134wrd.63.2017.04.26.06.47.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 26 Apr 2017 06:47:34 -0700 (PDT) Date: Wed, 26 Apr 2017 15:47:31 +0200 From: Miklos Szeredi To: Amir Goldstein Cc: Trond Myklebust , Jeff Layton , "J . Bruce Fields" , Tyler Hicks , Al Viro , linux-unionfs@vger.kernel.org, linux-fsdevel@vger.kernel.org, stable@vger.kernel.org Subject: Re: [PATCH] ovl: don't expose EOPENSTALE to userspace Message-ID: <20170426134731.GB5214@veci.piliscsaba.szeredi.hu> References: <1493128675-24331-1-git-send-email-amir73il@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1493128675-24331-1-git-send-email-amir73il@gmail.com> User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Apr 25, 2017 at 04:57:55PM +0300, Amir Goldstein wrote: > An overlay dentry holds a lower dentry that may belong to an NFS mount. > > Overlayfs calls ovl_path_open() to get a file descriptor of a lower file > for copying up its data and for a lower directory for listing its > content. > > If that lower dentry gets invalidated after ovl_dentry_revalidate() and > before ovl_path_open(), the internal error code 518 (EOPENSTALE), which > is not a POSIX error code, will be exposed to the user. > > Check the internal return value -EOPENSTALE in ovl_path_open(), just > the same as it is checked in path_openat() and replace it with the POSIX > error code ESTALE. I'm looking at the EOPENSTALE story and it very much looks like we can just replace the single use with ESTALE and handle the lookup retry logic nuances inside the lookup code... Below is untested patch. Thanks, Miklos diff --git a/Documentation/filesystems/path-lookup.md b/Documentation/filesystems/path-lookup.md index 1b39e084a2b2..41f2b25b6ac3 100644 --- a/Documentation/filesystems/path-lookup.md +++ b/Documentation/filesystems/path-lookup.md @@ -1,4 +1,4 @@ - +\ @@ -1150,11 +1150,10 @@ code. created file will be performed by `vfs_open()`, just as if the name were found in the dcache. -2. `vfs_open()` can fail with `-EOPENSTALE` if the cached information - wasn't quite current enough. Rather than restarting the lookup from - the top with `LOOKUP_REVAL` set, `lookup_open()` is called instead, - giving the filesystem a chance to resolve small inconsistencies. - If that doesn't work, only then is the lookup restarted from the top. +2. `vfs_open()` can fail with `-ESTALE` if the cached information wasn't + quite current enough. The lookup will be restarted first without + LOOKUP_REVAL and then with. + 3. An open with O_CREAT **does** follow a symlink in the final component, unlike other creation system calls (like `mkdir`). So the sequence: diff --git a/fs/namei.c b/fs/namei.c index d41fab78798b..af0b31340ada 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -3187,7 +3187,7 @@ static int lookup_open(struct nameidata *nd, struct path *path, */ static int do_last(struct nameidata *nd, struct file *file, const struct open_flags *op, - int *opened) + int *opened, bool firstpass) { struct dentry *dir = nd->path.dentry; int open_flag = op->open_flag; @@ -3342,8 +3342,16 @@ static int do_last(struct nameidata *nd, goto out; BUG_ON(*opened & FILE_OPENED); /* once it's opened, it's opened */ error = vfs_open(&nd->path, file, current_cred()); - if (error) + if (error) { + /* + * If we got ESTALE here it likely means that only need to + * revalidate the last component. So first restart without + * LOOKUP_REVAL. + */ + if (error == -ESTALE && firstpass) + error = -ECHILD; goto out; + } *opened |= FILE_OPENED; opened: error = open_check_o_direct(file); @@ -3458,6 +3466,7 @@ static struct file *path_openat(struct nameidata *nd, struct file *file; int opened = 0; int error; + bool firstpass = flags & LOOKUP_RCU; file = get_empty_filp(); if (IS_ERR(file)) @@ -3483,7 +3492,7 @@ static struct file *path_openat(struct nameidata *nd, return ERR_CAST(s); } while (!(error = link_path_walk(s, nd)) && - (error = do_last(nd, file, op, &opened)) > 0) { + (error = do_last(nd, file, op, &opened, firstpass)) > 0) { nd->flags &= ~(LOOKUP_OPEN|LOOKUP_CREATE|LOOKUP_EXCL); s = trailing_symlink(nd); if (IS_ERR(s)) { @@ -3497,15 +3506,9 @@ static struct file *path_openat(struct nameidata *nd, BUG_ON(!error); put_filp(file); } - if (unlikely(error)) { - if (error == -EOPENSTALE) { - if (flags & LOOKUP_RCU) - error = -ECHILD; - else - error = -ESTALE; - } + if (unlikely(error)) file = ERR_PTR(error); - } + return file; } diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c index 0efba77789b9..ecd2e1b165b1 100644 --- a/fs/nfs/nfs4file.c +++ b/fs/nfs/nfs4file.c @@ -39,7 +39,7 @@ nfs4_file_open(struct inode *inode, struct file *filp) * * We only get this far for a cached positive dentry. We skipped * revalidation, so handle it here by dropping the dentry and returning - * -EOPENSTALE. The VFS will retry the lookup/create/open. + * -ESTALE. The VFS will retry the lookup/create/open. */ dprintk("NFS: open file(%pd2)\n", dentry); @@ -99,7 +99,7 @@ nfs4_file_open(struct inode *inode, struct file *filp) out_drop: d_drop(dentry); - err = -EOPENSTALE; + err = -ESTALE; goto out_put_ctx; } diff --git a/include/linux/errno.h b/include/linux/errno.h index 7ce9fb1b7d28..28a979008af2 100644 --- a/include/linux/errno.h +++ b/include/linux/errno.h @@ -16,7 +16,6 @@ #define ENOIOCTLCMD 515 /* No ioctl command */ #define ERESTART_RESTARTBLOCK 516 /* restart by calling sys_restart_syscall */ #define EPROBE_DEFER 517 /* Driver requests probe retry */ -#define EOPENSTALE 518 /* open found a stale dentry */ /* Defined for the NFSv3 protocol */ #define EBADHANDLE 521 /* Illegal NFS file handle */