From patchwork Fri May 8 22:37:57 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Andreas_Gr=C3=BCnbacher?= X-Patchwork-Id: 6367931 Return-Path: X-Original-To: patchwork-linux-nfs@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 8E7D6BEEE1 for ; Fri, 8 May 2015 22:38:30 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 811B2202FF for ; Fri, 8 May 2015 22:38:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 82FD8202EC for ; Fri, 8 May 2015 22:38:28 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751568AbbEHWi1 (ORCPT ); Fri, 8 May 2015 18:38:27 -0400 Received: from mail-wi0-f171.google.com ([209.85.212.171]:35348 "EHLO mail-wi0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751536AbbEHWi1 (ORCPT ); Fri, 8 May 2015 18:38:27 -0400 Received: by widdi4 with SMTP id di4so45487847wid.0 for ; Fri, 08 May 2015 15:38:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=SEmvyEfbcEQNnTWAJGAXdXflvlRYNCcO5U7GTJHGBYQ=; b=T8FuwpVll6Rzl9WEpUjjianjvhT+2ry9apfFInnMOPAp+A/YePKxapBbkaReL2bw2G 2E8GhgWq71Zhwtyx7EZt8WmPa2GdZNRpEE7Ox6YzNHnuBls3DbmiuvhZosYUFS8yeyTy MIhA/CBFcYAfOGFjk6tLbYV8uKxk4nYt782IblWm3JaN0f2E5GAe+4KXrHIpAi/00gdD 05G3+SGEt4gVPWaKvkiGumx8IVJ+J5cGqGVRwJTiMScfywTuWzOPviSUe6PRNcTrPVWh YgCX1Wd2ez6UjkbdZZG/zVqisYbw6ddSN+X+nUbeVHXJPLhLLQy867XQbWoufZtbaVJo qw0Q== X-Received: by 10.195.18.103 with SMTP id gl7mr433591wjd.34.1431124705562; Fri, 08 May 2015 15:38:25 -0700 (PDT) Received: from nuc.home.com (80-110-112-232.cgn.dynamic.surfer.at. [80.110.112.232]) by mx.google.com with ESMTPSA id di9sm843338wib.16.2015.05.08.15.38.23 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 08 May 2015 15:38:24 -0700 (PDT) From: Andreas Gruenbacher To: "J. Bruce Fields" , linux-nfs@vger.kernel.org, Christoph Hellwig Cc: Andreas Gruenbacher Subject: [PATCH v2] nfsd: Disable NFSv2 timestamp workaround for NFSv3+ Date: Sat, 9 May 2015 00:37:57 +0200 Message-Id: <1431124677-13388-1-git-send-email-andreas.gruenbacher@gmail.com> X-Mailer: git-send-email 2.1.0 In-Reply-To: <20150507140814.GF27106@fieldses.org> References: <20150507140814.GF27106@fieldses.org> Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, T_DKIM_INVALID, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=ham 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 NFSv2 can set the atime and/or mtime of a file to specific timestamps but not to the server's current time. To implement the equivalent of utimes("file", NULL), it uses a heuristic. NFSv3 and later do support setting the atime and/or mtime to the server's current time directly. The NFSv2 heuristic is still enabled, and causes timestamps to be set wrong sometimes. Fix this by moving the heuristic into the NFSv2 specific code. We can leave it out of the create code path: the owner can always set timestamps arbitrarily, and the workaround would never trigger. Signed-off-by: Andreas Gruenbacher Reviewed-by: Christoph Hellwig --- fs/nfsd/nfsproc.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++-- fs/nfsd/vfs.c | 36 ------------------------------------ 2 files changed, 50 insertions(+), 38 deletions(-) diff --git a/fs/nfsd/nfsproc.c b/fs/nfsd/nfsproc.c index aecbcd3..4cd78ef 100644 --- a/fs/nfsd/nfsproc.c +++ b/fs/nfsd/nfsproc.c @@ -59,13 +59,61 @@ static __be32 nfsd_proc_setattr(struct svc_rqst *rqstp, struct nfsd_sattrargs *argp, struct nfsd_attrstat *resp) { + struct iattr *iap = &argp->attrs; + struct svc_fh *fhp; __be32 nfserr; + dprintk("nfsd: SETATTR %s, valid=%x, size=%ld\n", SVCFH_fmt(&argp->fh), argp->attrs.ia_valid, (long) argp->attrs.ia_size); - fh_copy(&resp->fh, &argp->fh); - nfserr = nfsd_setattr(rqstp, &resp->fh, &argp->attrs,0, (time_t)0); + fhp = fh_copy(&resp->fh, &argp->fh); + + /* + * NFSv2 does not differentiate between "set-[ac]time-to-now" + * which only requires access, and "set-[ac]time-to-X" which + * requires ownership. + * So if it looks like it might be "set both to the same time which + * is close to now", and if inode_change_ok fails, then we + * convert to "set to now" instead of "set to explicit time" + * + * We only call inode_change_ok as the last test as technically + * it is not an interface that we should be using. + */ +#define BOTH_TIME_SET (ATTR_ATIME_SET | ATTR_MTIME_SET) +#define MAX_TOUCH_TIME_ERROR (30*60) + if ((iap->ia_valid & BOTH_TIME_SET) == BOTH_TIME_SET && + iap->ia_mtime.tv_sec == iap->ia_atime.tv_sec) { + /* + * Looks probable. + * + * Now just make sure time is in the right ballpark. + * Solaris, at least, doesn't seem to care what the time + * request is. We require it be within 30 minutes of now. + */ + time_t delta = iap->ia_atime.tv_sec - get_seconds(); + struct inode *inode; + + nfserr = fh_verify(rqstp, fhp, 0, NFSD_MAY_NOP); + if (nfserr) + goto done; + inode = d_inode(fhp->fh_dentry); + + if (delta < 0) + delta = -delta; + if (delta < MAX_TOUCH_TIME_ERROR && + inode_change_ok(inode, iap) != 0) { + /* + * Turn off ATTR_[AM]TIME_SET but leave ATTR_[AM]TIME. + * This will cause notify_change to set these times + * to "now" + */ + iap->ia_valid &= ~BOTH_TIME_SET; + } + } + + nfserr = nfsd_setattr(rqstp, fhp, iap, 0, (time_t)0); +done: return nfsd_return_attrs(nfserr, resp); } diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c index 84d770b..92de374 100644 --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -302,42 +302,6 @@ commit_metadata(struct svc_fh *fhp) static void nfsd_sanitize_attrs(struct inode *inode, struct iattr *iap) { - /* - * NFSv2 does not differentiate between "set-[ac]time-to-now" - * which only requires access, and "set-[ac]time-to-X" which - * requires ownership. - * So if it looks like it might be "set both to the same time which - * is close to now", and if inode_change_ok fails, then we - * convert to "set to now" instead of "set to explicit time" - * - * We only call inode_change_ok as the last test as technically - * it is not an interface that we should be using. - */ -#define BOTH_TIME_SET (ATTR_ATIME_SET | ATTR_MTIME_SET) -#define MAX_TOUCH_TIME_ERROR (30*60) - if ((iap->ia_valid & BOTH_TIME_SET) == BOTH_TIME_SET && - iap->ia_mtime.tv_sec == iap->ia_atime.tv_sec) { - /* - * Looks probable. - * - * Now just make sure time is in the right ballpark. - * Solaris, at least, doesn't seem to care what the time - * request is. We require it be within 30 minutes of now. - */ - time_t delta = iap->ia_atime.tv_sec - get_seconds(); - if (delta < 0) - delta = -delta; - if (delta < MAX_TOUCH_TIME_ERROR && - inode_change_ok(inode, iap) != 0) { - /* - * Turn off ATTR_[AM]TIME_SET but leave ATTR_[AM]TIME. - * This will cause notify_change to set these times - * to "now" - */ - iap->ia_valid &= ~BOTH_TIME_SET; - } - } - /* sanitize the mode change */ if (iap->ia_valid & ATTR_MODE) { iap->ia_mode &= S_IALLUGO;