From patchwork Tue Jul 9 16:16:45 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nadav Shemer X-Patchwork-Id: 2825365 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.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id B835EC0AB2 for ; Tue, 9 Jul 2013 16:17:01 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id A5D76201EE for ; Tue, 9 Jul 2013 16:17:00 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 3A777201EF for ; Tue, 9 Jul 2013 16:16:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752421Ab3GIQQv (ORCPT ); Tue, 9 Jul 2013 12:16:51 -0400 Received: from mail-we0-f181.google.com ([74.125.82.181]:61261 "EHLO mail-we0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752386Ab3GIQQt (ORCPT ); Tue, 9 Jul 2013 12:16:49 -0400 Received: by mail-we0-f181.google.com with SMTP id p58so4849726wes.40 for ; Tue, 09 Jul 2013 09:16:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=from:to:cc:subject:date:message-id:x-mailer:x-gm-message-state; bh=DmyEdGCEWGNkDKyQi6OvSnTGvZVCd8FiMJFzZ6/UBBk=; b=YjEcSXXuKYW5FJY8FWjowx4xkS2j8fXHyMfd2swIBNVtvYNvuvhYeXuFxLj96wmE9+ iglxascjMFQJGKel9UbzQ+XJIMkLW2CP3yECW/V1UW9fNX0nD7+5/uOoSjrRqCjFJAfl yhXbL+i//0+Cqv4Do3vHvkt+u7uyIBNVrXvLqiOvi3U+btKvNG/woOnpzfGBLtdIRsOr UlCp21t7rvMreHCu/RNE5uvKXGKtl129671nD++m9ftOeHxfs4A00gJ9HQhJNDbxt19+ WHWnw5VteUI6ga3RvhQsuATnji4z0or3COCJs2CzjMXNzwNTwNIi1nirjm8c4aKCBuo0 /wYg== X-Received: by 10.180.182.229 with SMTP id eh5mr14678058wic.63.1373386607422; Tue, 09 Jul 2013 09:16:47 -0700 (PDT) Received: from tonian.com (bzq-80-49-130.static.bezeqint.net. [82.80.49.130]) by mx.google.com with ESMTPSA id p1sm62154088wix.9.2013.07.09.09.16.46 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Tue, 09 Jul 2013 09:16:46 -0700 (PDT) From: Nadav Shemer To: Trond.Myklebust@netapp.com Cc: linux-nfs@vger.kernel.org Subject: [PATCH] NFSv4: fix open(O_RDONLY|O_TRUNC) returning EBADF Date: Tue, 9 Jul 2013 19:16:45 +0300 Message-Id: <1373386605-18684-1-git-send-email-nadav@tonian.com> X-Mailer: git-send-email 1.7.11.7 X-Gm-Message-State: ALoCoQnKuJxQ/JU2WpbhRi9pOLqZfJVrlt7roqCryvGWBzUv8jkrQcyiZg8BNsKEKzKNOvvol2S1 Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org X-Spam-Status: No, score=-7.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, 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 Move ATTR_OPEN handling from nfs4_proc_setattr into nfs4_do_setattr Signed-off-by: Nadav Shemer Acked-by: Jeff Layton --- Hello. I've come across an oddity while testing filesystem coverage My test creates a non-empty file without write permissions and tries to open it with O_RDONLY|O_TRUNC It expects EACCES (and gets that for local filesystems and NFSv3) but gets EBADF on NFSv4 (and v4.1) I found some history on this: In a previous kernel it would just hang due to mishandling the NFS4ERR_OPENMODE exception http://www.spinics.net/lists/linux-nfs/msg28881.html A fix for this was introduced (it specifically tests for NFS4ERR_OPENMODE and returns EACCES for the open() case, EBADF otherwise) http://www.spinics.net/linux/fedora/fedora-kernel/msg03736.html but another patch was also introduced in the same set which seems to break it (it optimizes away the time modification and removes ATTR_OPEN in nfs4_proc_setattr) http://www.spinics.net/linux/fedora/fedora-kernel/msg03732.html By moving the 'Deal with open(O_TRUNC)' bit inside (into nfs4_do_setattr), I got it working again (with no other functional change, as far as I can see) fs/nfs/nfs4proc.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 8fbc100..17b9f32 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -2180,6 +2180,10 @@ static int nfs4_do_setattr(struct inode *inode, struct rpc_cred *cred, .inode = inode, }; int err; + int is_o_trunc = sattr->ia_valid & ATTR_OPEN; + /* Deal with open(O_TRUNC) */ + if (sattr->ia_valid & ATTR_OPEN) + sattr->ia_valid &= ~(ATTR_MTIME|ATTR_CTIME|ATTR_OPEN); do { err = _nfs4_do_setattr(inode, cred, fattr, sattr, state); switch (err) { @@ -2193,7 +2197,7 @@ static int nfs4_do_setattr(struct inode *inode, struct rpc_cred *cred, } if (state && !(state->state & FMODE_WRITE)) { err = -EBADF; - if (sattr->ia_valid & ATTR_OPEN) + if (is_o_trunc) err = -EACCES; goto out; } @@ -2774,10 +2778,6 @@ nfs4_proc_setattr(struct dentry *dentry, struct nfs_fattr *fattr, nfs_fattr_init(fattr); - /* Deal with open(O_TRUNC) */ - if (sattr->ia_valid & ATTR_OPEN) - sattr->ia_valid &= ~(ATTR_MTIME|ATTR_CTIME|ATTR_OPEN); - /* Optimization: if the end result is no change, don't RPC */ if ((sattr->ia_valid & ~(ATTR_FILE)) == 0) return 0;