From patchwork Wed Jul 31 23:00:51 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Quentin Barnes X-Patchwork-Id: 2836610 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 BE203C0319 for ; Wed, 31 Jul 2013 23:00:57 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id D99F9200D4 for ; Wed, 31 Jul 2013 23:00:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 0D48B200C7 for ; Wed, 31 Jul 2013 23:00:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754550Ab3GaXAz (ORCPT ); Wed, 31 Jul 2013 19:00:55 -0400 Received: from mail-qe0-f53.google.com ([209.85.128.53]:48488 "EHLO mail-qe0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752067Ab3GaXAy (ORCPT ); Wed, 31 Jul 2013 19:00:54 -0400 Received: by mail-qe0-f53.google.com with SMTP id f6so745050qej.12 for ; Wed, 31 Jul 2013 16:00:53 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:mime-version:content-type :content-disposition:user-agent; bh=BdSY9mNF3KPoTwLTRcKVNoH5Gmhowg3H4ThP2DtMFP0=; b=lDUK57q/cjYfG8h0vpsJYmqCn258Wy4yLonYYvKSlEVqSDaB/RUmu57xNLxsHivhw6 9ei1btzWsS6rI7A4bdyeyvSKBa+iUTavH0PoFzFfX5AEK0KE3YbiCykQaRpQF1MetHfR PiCaVEB2Wia+ZnVIO3P1O19P7qtRMMWyGGgXQAB9ROIBLfefBuZKXOosSknM6ffI90rz eIqijQXmFegWuoCw7PVld5rjKdBku4hWcc5shAeUiC6bflzK7Vc7aAxctHbDaRq25nQV 7v3b0thzeCvBRrix1YiK71YnJuohMbfEiEXJ6rzPEmDN59vcH5OcFOciTj7z+ALW5mMn sqwg== X-Received: by 10.49.59.109 with SMTP id y13mr5725172qeq.71.1375311653745; Wed, 31 Jul 2013 16:00:53 -0700 (PDT) Received: from gmail.com (lo4.cfw-a-gci.champ.corp.yahoo.com. [204.11.79.48]) by mx.google.com with ESMTPSA id om8sm39698qeb.4.2013.07.31.16.00.52 for (version=TLSv1 cipher=RC4-SHA bits=128/128); Wed, 31 Jul 2013 16:00:53 -0700 (PDT) Date: Wed, 31 Jul 2013 18:00:51 -0500 From: Quentin Barnes To: "Myklebust, Trond" Cc: Jeff Layton , "linux-nfs@vger.kernel.org" Subject: Re: [PATCH] nfs: don't allow nfs_find_actor to match inodes of the wrong type Message-ID: <20130731230051.GB31859@gmail.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.20 (2009-12-10) 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, FAKE_REPLY_C, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID,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 > Quite frankly, all I care about in a situation like this is that the > client doesn't Oops. Certainly, and his patch does do that, but it's also pointing out there's another bug running around. And once you fix that bug, the original patch is no longer needed. > If a server is really this utterly broken, then there is no way we can > fix it on the client, and we're not even going to try. Of course. But you also don't want to unnecessarily leave the client with an invalid inode that's not properly flagged and possibly leave another bug unfixed. Here is a example patch that I feel better addresses the problem: commit 2d6b411eea04ae4398707b584b8d9e552606aaf7 Author: Quentin Barnes Date: Wed Jul 31 17:50:35 2013 -0500 Have nfs_refresh_inode_locked() ensure that it doesn't return without flagging invalid inodes (ones that don't match its fattr type). nfs_refresh_inode() already does this, so we need to check the return status from nfs_check_inode_attributes() before returning from nfs_refresh_inode_locked(). Once this hole is plugged, there will be no invalid inode references returned by nfs_fhget(), so no need to check in nfs_find_actor(). --- To unsubscribe from this list: send the line "unsubscribe linux-nfs" 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/nfs/inode.c b/fs/nfs/inode.c index af6e806..d2263a5 100644 --- a/fs/nfs/inode.c +++ b/fs/nfs/inode.c @@ -244,8 +244,6 @@ nfs_find_actor(struct inode *inode, void *opaque) if (NFS_FILEID(inode) != fattr->fileid) return 0; - if ((S_IFMT & inode->i_mode) != (S_IFMT & fattr->mode)) - return 0; if (nfs_compare_fh(NFS_FH(inode), fh)) return 0; if (is_bad_inode(inode) || NFS_STALE(inode)) @@ -1269,9 +1267,16 @@ static int nfs_inode_attrs_need_update(const struct inode *inode, const struct n static int nfs_refresh_inode_locked(struct inode *inode, struct nfs_fattr *fattr) { + int status; + if (nfs_inode_attrs_need_update(inode, fattr)) return nfs_update_inode(inode, fattr); - return nfs_check_inode_attributes(inode, fattr); + + status = nfs_check_inode_attributes(inode, fattr); + if (status) + nfs_invalidate_inode(inode); + + return status; } /**