From patchwork Sun Jan 31 13:17:53 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Konstantin Khlebnikov X-Patchwork-Id: 8174321 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 1633FBEEE5 for ; Sun, 31 Jan 2016 13:19:02 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 024B5202F0 for ; Sun, 31 Jan 2016 13:19:01 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0F15B20320 for ; Sun, 31 Jan 2016 13:19:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757345AbcAaNSC (ORCPT ); Sun, 31 Jan 2016 08:18:02 -0500 Received: from mail-lf0-f42.google.com ([209.85.215.42]:35090 "EHLO mail-lf0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757318AbcAaNSA (ORCPT ); Sun, 31 Jan 2016 08:18:00 -0500 Received: by mail-lf0-f42.google.com with SMTP id l143so14200904lfe.2; Sun, 31 Jan 2016 05:17:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=subject:from:to:cc:date:message-id:user-agent:mime-version :content-type:content-transfer-encoding; bh=hRM1yTXo2zycpmFuBxPm/dMq8O6887qrkPJ/MQD91/A=; b=iOZKpA8coZ/ccCCMiFxB0EZVE1RNOtvZTgH9Ojlrh/BoBv9uqgU0rYaDpJV0wzB1V/ CB+cqf0FzIWmUcVbRCk8hPwEF4aZ623LR1+oPWu3XDBQ231phiWQiWdsAiTDJKEI6w6q 7JkvteLBr00Dr50p92odnSIq0ZpWXWqlqI7G4xy828uRXLY4C/6i/oVbKsPoB3gMGGtX dmc6HyXHwAUNrRRUmFYIJkYco7kk4913nrqBUfaYXCx6Eeyheib07MnuSR6+Sz/NmVSx xLbWQfmGggeecG3+rw8aal0AAFVeHrDpmZn0u3ly/Ix+sMNRSTCwYjEBTtSpJ1E+d/Yu jZlQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:from:to:cc:date:message-id:user-agent :mime-version:content-type:content-transfer-encoding; bh=hRM1yTXo2zycpmFuBxPm/dMq8O6887qrkPJ/MQD91/A=; b=h6/fSP3Fa1in/Y+uMsLUnYo2uqMsxltYpcfYjFgQQnPBXkYKx/i5hvJPcltFTRBNvT KtPJX5kdW4WxVktHA0KWus9edHaKtKYaZmPmRsIBhT9WAYScJNtNNWxcYWCTwW4QZRPF bKSsK9/JdduqwLJ1vXGlMzi9rY2PmSzXEXR7mfaq5DThud7nHhUSCGRcw7hrOg1P6FJG uNLTVgW+uwpNN3lui7K4AT7hGqN5otzsiinqxLqKIbUprSImYcEOXWjSJmFV3yffeN6O 0NIAhjrzEWQR+nbUkgfj69e7OFCzCOF6Lpe2JYEurousyeD2l7mVz03vASjhWfy92TSZ Am+g== X-Gm-Message-State: AG10YOS63zXbCD7EdYrxXOrIruxZvlORydpWv5hWzvU9LZChqStIU/8o+gb3joozKs/Rrw== X-Received: by 10.25.23.24 with SMTP id n24mr6693661lfi.66.1454246277773; Sun, 31 Jan 2016 05:17:57 -0800 (PST) Received: from localhost (ppp79-139-147-94.pppoe.spdop.ru. [79.139.147.94]) by smtp.gmail.com with ESMTPSA id z193sm3418696lfd.0.2016.01.31.05.17.56 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 31 Jan 2016 05:17:56 -0800 (PST) Subject: [PATCH] ovl: ignore lower entries when checking purity of non-directory entries From: Konstantin Khlebnikov To: linux-fsdevel@vger.kernel.org, linux-unionfs@vger.kernel.org Cc: David Howells , linux-kernel@vger.kernel.org, Vivek Goyal Date: Sun, 31 Jan 2016 16:17:53 +0300 Message-ID: <145424627339.27058.6895511767458306300.stgit@zurg> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Spam-Status: No, score=-7.3 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP After rename file dentry still holds reference to lower dentry from previous location. This doesn't matter for data access because data cames from upper dentry. But this stale lower dentry taints dentry at new location and turns it into non-pure upper. Such file leaves visible whiteout entry after remove in directory which shouldn't have whiteouts at all. Overlayfs already tracks pureness of file location in oe->opaque. This patch just uses that for detecting actual path type. Comment from Vivek Goyal's patch: Here are the details of the problem. Do following. $ mkdir upper lower work merged upper/dir/ $ touch lower/test $ sudo mount -t overlay overlay -olowerdir=lower,upperdir=upper,workdir=work merged $ mv merged/test merged/dir/ $ rm merged/dir/test $ ls -l merged/dir/ /usr/bin/ls: cannot access merged/dir/test: No such file or directory total 0 c????????? ? ? ? ? ? test Basic problem seems to be that once a file has been unlinked, a whiteout has been left behind which was not needed and hence it becomes visible. whiteout is visible because parent dir is of not type MERGE, hence od->is_real is set during ovl_dir_open(). And that means ovl_iterate() passes on iterate handling directly to underlying fs. Underlying fs does not know/filter whiteouts so it becomes visible to user. Why did we leave a whiteout to begin with when we should not have. ovl_do_remove() checks for OVL_TYPE_PURE_UPPER() and does not leave whiteout if file is pure upper. In this case file is not found to be pure upper hence whiteout is left. So why file was not PURE_UPPER in this case? I think because dentry is still carrying some leftover state which was valid before rename. For example, od->numlower was set to 1 as it was a lower file. After rename, this state is not valid anymore as there is no such file in lower. Signed-off-by: Konstantin Khlebnikov Reported-by: Viktor Stanchev Diagnosed-by: Vivek Goyal Link: https://bugzilla.kernel.org/show_bug.cgi?id=109611 Acked-by: Vivek Goyal --- fs/overlayfs/dir.c | 7 +++++++ fs/overlayfs/super.c | 12 +++++++----- 2 files changed, 14 insertions(+), 5 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c index ed95272d57a6..edf83f325bca 100644 --- a/fs/overlayfs/dir.c +++ b/fs/overlayfs/dir.c @@ -903,6 +903,13 @@ static int ovl_rename2(struct inode *olddir, struct dentry *old, if (!overwrite && new_is_dir && !old_opaque && new_opaque) ovl_remove_opaque(newdentry); + /* + * Old dentry now lives in different location. Dentries in + * lowerstack are stale. We cannot drop them here because + * access to them is lockless. This could be only pure upper + * or opaque directory - numlower is zero. Or upper non-dir + * entry - its pureness is tracked by flag opaque. + */ if (old_opaque != new_opaque) { ovl_dentry_set_opaque(old, new_opaque); if (!overwrite) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 8d826bd56b26..ba28b007005e 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -76,12 +76,14 @@ enum ovl_path_type ovl_path_type(struct dentry *dentry) if (oe->__upperdentry) { type = __OVL_PATH_UPPER; - if (oe->numlower) { - if (S_ISDIR(dentry->d_inode->i_mode)) - type |= __OVL_PATH_MERGE; - } else if (!oe->opaque) { + /* + * Non-dir dentry can hold lower dentry from previous + * location. Its purity depends only on opaque flag. + */ + if (oe->numlower && S_ISDIR(dentry->d_inode->i_mode)) + type |= __OVL_PATH_MERGE; + else if (!oe->opaque) type |= __OVL_PATH_PURE; - } } else { if (oe->numlower > 1) type |= __OVL_PATH_MERGE;