From patchwork Tue May 11 19:21:49 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Layton X-Patchwork-Id: 52311 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.samba.org (fn.samba.org [216.83.154.106]) by ozlabs.org (Postfix) with ESMTP id D169AB7DAB for ; Wed, 12 May 2010 05:21:59 +1000 (EST) Received: from fn.samba.org (localhost [127.0.0.1]) by lists.samba.org (Postfix) with ESMTP id 33FCCAD115; Tue, 11 May 2010 13:22:00 -0600 (MDT) X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on fn.samba.org X-Spam-Level: X-Spam-Status: No, score=-9.7 required=3.8 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.2.5 X-Original-To: linux-cifs-client@lists.samba.org Delivered-To: linux-cifs-client@lists.samba.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by lists.samba.org (Postfix) with ESMTP id 93D24ACFAB for ; Tue, 11 May 2010 13:21:55 -0600 (MDT) Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id o4BJLpMd004114 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 11 May 2010 15:21:51 -0400 Received: from tlielax.poochiereds.net (vpn-10-199.rdu.redhat.com [10.11.10.199]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id o4BJLoqO009245; Tue, 11 May 2010 15:21:50 -0400 Date: Tue, 11 May 2010 15:21:49 -0400 From: Jeff Layton To: Steve French Message-ID: <20100511152149.55240df2@tlielax.poochiereds.net> In-Reply-To: References: <1273241032-6404-1-git-send-email-jlayton@redhat.com> <4BE42D04.7020806@suse.de> <4BE43DAA.7030804@suse.de> <20100511130954.35267194@tlielax.poochiereds.net> <20100511141539.0ba62265@tlielax.poochiereds.net> Mime-Version: 1.0 X-Scanned-By: MIMEDefang 2.67 on 10.5.11.12 Cc: linux-cifs-client@lists.samba.org Subject: Re: [linux-cifs-client] [PATCH] cifs: guard against hardlinking directories X-BeenThere: linux-cifs-client@lists.samba.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: The Linux CIFS VFS client List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-cifs-client-bounces@lists.samba.org Errors-To: linux-cifs-client-bounces@lists.samba.org On Tue, 11 May 2010 13:43:27 -0500 Steve French wrote: > On Tue, May 11, 2010 at 1:15 PM, Jeff Layton wrote: > > On Tue, 11 May 2010 12:15:51 -0500 > > Steve French wrote: > > > >> Merged into cifs-2.6.git - if the branch for-linus does not look > >> weird/broken, plan to request upstream tonight. > >> > >> > >> > > > > I just made some comments on this to the bug: > > > > https://bugzilla.samba.org/show_bug.cgi?id=7407#c13 > > > > ...I wonder whether this patch may be too aggressive about disabling > > serverino. It seems like we ought to be OK with finding an inode that > > is "floating", just not one that has a dentry already attached. > > Thoughts? > > > > -- > > Jeff Layton > > > > I agree - but have to deal with the oops first ASAP - narrow it later. > I tend to agree -- disabling server inode numbers unnecessarily is better than oopsing at umount. I did just test the attached modified patch however and it fixes the reproducer I have for this. Thoughts on going with this instead? From 08bfd493bed6e217cfb10c9f4337ebbf834d4c6e Mon Sep 17 00:00:00 2001 From: Jeff Layton Date: Tue, 11 May 2010 14:59:55 -0400 Subject: [PATCH] cifs: guard against hardlinking directories When we made serverino the default, we trusted that the field sent by the server in the "uniqueid" field was actually unique. It turns out that it isn't reliably so. Samba, in particular, will just put the st_ino in the uniqueid field when unix extensions are enabled. When a share spans multiple filesystems, it's quite possible that there will be collisions. This is a server bug, but when the inodes in question are a directory (as is often the case) and there is a collision with the root inode of the mount, the result is a kernel panic on umount. Fix this by checking explicitly for directory inodes with the same uniqueid. If that is the case, then we can assume that using server inode numbers will be a problem and that they should be disabled. Signed-off-by: Jeff Layton --- fs/cifs/cifsglob.h | 1 + fs/cifs/inode.c | 21 +++++++++++++++++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index ecf0ffb..0c2fd17 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -502,6 +502,7 @@ struct dfs_info3_param { #define CIFS_FATTR_DFS_REFERRAL 0x1 #define CIFS_FATTR_DELETE_PENDING 0x2 #define CIFS_FATTR_NEED_REVAL 0x4 +#define CIFS_FATTR_INO_COLLISION 0x8 struct cifs_fattr { u32 cf_flags; diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index 35ec117..29b9ea2 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c @@ -715,6 +715,16 @@ cifs_find_inode(struct inode *inode, void *opaque) if (CIFS_I(inode)->uniqueid != fattr->cf_uniqueid) return 0; + /* + * uh oh -- it's a directory. We can't use it since hardlinked dirs are + * verboten. Disable serverino and return it as if it were found, the + * caller can discard it, generate a uniqueid and retry the find + */ + if (S_ISDIR(inode->i_mode) && !list_empty(&inode->i_dentry)) { + fattr->cf_flags |= CIFS_FATTR_INO_COLLISION; + cifs_autodisable_serverino(CIFS_SB(inode->i_sb)); + } + return 1; } @@ -734,15 +744,22 @@ cifs_iget(struct super_block *sb, struct cifs_fattr *fattr) unsigned long hash; struct inode *inode; +retry_iget5_locked: cFYI(1, ("looking for uniqueid=%llu", fattr->cf_uniqueid)); /* hash down to 32-bits on 32-bit arch */ hash = cifs_uniqueid_to_ino_t(fattr->cf_uniqueid); inode = iget5_locked(sb, hash, cifs_find_inode, cifs_init_inode, fattr); - - /* we have fattrs in hand, update the inode */ if (inode) { + /* was there a problematic inode number collision? */ + if (fattr->cf_flags & CIFS_FATTR_INO_COLLISION) { + iput(inode); + fattr->cf_uniqueid = iunique(sb, ROOT_I); + fattr->cf_flags &= ~CIFS_FATTR_INO_COLLISION; + goto retry_iget5_locked; + } + cifs_fattr_to_inode(inode, fattr); if (sb->s_flags & MS_NOATIME) inode->i_flags |= S_NOATIME | S_NOCMTIME; -- 1.6.6.1