From patchwork Fri Apr 17 21:16:23 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Steve French X-Patchwork-Id: 18756 Received: from lists.samba.org (mail.samba.org [66.70.73.150]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n3HLGbSa026561 for ; Fri, 17 Apr 2009 21:16:37 GMT Received: from dp.samba.org (localhost [127.0.0.1]) by lists.samba.org (Postfix) with ESMTP id A2CA5163C0C for ; Fri, 17 Apr 2009 21:16:16 +0000 (GMT) X-Spam-Checker-Version: SpamAssassin 3.1.7 (2006-10-05) on dp.samba.org X-Spam-Level: X-Spam-Status: No, score=-3.3 required=3.8 tests=AWL,BAYES_00, DNS_FROM_RFC_POST,SPF_PASS autolearn=no version=3.1.7 X-Original-To: linux-cifs-client@lists.samba.org Delivered-To: linux-cifs-client@lists.samba.org Received: from an-out-0708.google.com (an-out-0708.google.com [209.85.132.244]) by lists.samba.org (Postfix) with ESMTP id 288EB163B26 for ; Fri, 17 Apr 2009 21:16:03 +0000 (GMT) Received: by an-out-0708.google.com with SMTP id c2so110460anc.35 for ; Fri, 17 Apr 2009 14:16:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:in-reply-to:references :date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=t749PnsPDuV0uSGablXm7LruObC6drZK74g7aRXQeas=; b=gXIECmAcsjE6wt1Lt2nserPBc74Yv3HQXSEjztGQk4AKCcl+PMIA7KEWR5Q7IoG6py 0QFV/7gqwwQpxUkdUbJfZMHQ6n29PK9fW9sEm+aMTvV9tHd3RxYB4CUWaY0jgd1kQKC4 olJva8AI7g1LDljyr7HMy1eSTe2HI+UVzg8U8= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=W8phtPCkGHAyCwmOZ3gDZ/+5wWKPXA3LH89pFUHCT+60u8JRdPSA78WMgGT33nolP4 PEAqn2P+wiGwwi8aBwDPZ4PC0+DSCcL+pKLGNZeG07V6D/m4qxed2ypa02zfScq230X6 l/6NgutBxT5h/cJkkGvnNCF5gSGLPN3M9hQ+E= MIME-Version: 1.0 Received: by 10.100.227.18 with SMTP id z18mr4400472ang.49.1240002983935; Fri, 17 Apr 2009 14:16:23 -0700 (PDT) In-Reply-To: <1239983130-4341-1-git-send-email-jlayton@redhat.com> References: <1239983130-4341-1-git-send-email-jlayton@redhat.com> Date: Fri, 17 Apr 2009 16:16:23 -0500 Message-ID: <524f69650904171416y3807d6d2w7de57a711c5e5428@mail.gmail.com> From: Steve French To: Jeff Layton Cc: galquezar@tv-wan.es, linux-cifs-client@lists.samba.org, shirishp@us.ibm.com Subject: [linux-cifs-client] Re: [PATCH] cifs: when renaming don't try to unlink negative dentry X-BeenThere: linux-cifs-client@lists.samba.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: The Linux CIFS VFS client List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: linux-cifs-client-bounces+patchwork-cifs-client=patchwork.kernel.org@lists.samba.org Errors-To: linux-cifs-client-bounces+patchwork-cifs-client=patchwork.kernel.org@lists.samba.org I merged this, adding the CC: stable, but think we need to also consistently check for inode == NULL in cifs_unlink (we only check in two branches now) Any objections if I also add the following check: On Fri, Apr 17, 2009 at 10:45 AM, Jeff Layton wrote: > When attempting to rename a file on a read-only share, the kernel can > call cifs_unlink on a negative dentry, which causes an oops. Only try > to unlink the file if it's a positive dentry. > > Signed-off-by: Jeff Layton > Tested-by: Shirish Pargaonkar > --- >  fs/cifs/inode.c |    3 ++- >  1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c > index 09082ac..f36b4e4 100644 > --- a/fs/cifs/inode.c > +++ b/fs/cifs/inode.c > @@ -1453,7 +1453,8 @@ int cifs_rename(struct inode *source_dir, struct dentry *source_dentry, >                     checking the UniqueId via FILE_INTERNAL_INFO */ > >  unlink_target: > -       if ((rc == -EACCES) || (rc == -EEXIST)) { > +       /* Try unlinking the target dentry if it's not negative */ > +       if (target_dentry->d_inode && (rc == -EACCES || rc == -EEXIST)) { >                tmprc = cifs_unlink(target_dir, target_dentry); >                if (tmprc) >                        goto cifs_rename_exit; > -- > 1.6.0.6 > > diff --git a/fs/cifs/inode.c b/fs/cifs/inode.c index 09082ac..42572b7 100644 --- a/fs/cifs/inode.c +++ b/fs/cifs/inode.c @@ -968,7 +968,7 @@ int cifs_unlink(struct inode *dir, struct dentry *dentry) int xid; char *full_path = NULL; struct inode *inode = dentry->d_inode; - struct cifsInodeInfo *cifsInode = CIFS_I(inode); + struct cifsInodeInfo *cifs_inode; struct super_block *sb = dir->i_sb; struct cifs_sb_info *cifs_sb = CIFS_SB(sb); struct cifsTconInfo *tcon = cifs_sb->tcon; @@ -1012,7 +1012,7 @@ psx_del_no_retry: rc = cifs_rename_pending_delete(full_path, dentry, xid); if (rc == 0) drop_nlink(inode); - } else if (rc == -EACCES && dosattr == 0) { + } else if ((rc == -EACCES) && (dosattr == 0) && inode) { attrs = kzalloc(sizeof(*attrs), GFP_KERNEL); if (attrs == NULL) { rc = -ENOMEM; @@ -1020,7 +1020,8 @@ psx_del_no_retry: } /* try to reset dos attributes */ - origattr = cifsInode->cifsAttrs; + cifs_inode = CIFS_I(inode); + origattr = cifs_inode->cifsAttrs; if (origattr == 0) origattr |= ATTR_NORMAL; dosattr = origattr & ~ATTR_READONLY; @@ -1041,13 +1042,13 @@ psx_del_no_retry: out_reval: if (inode) { - cifsInode = CIFS_I(inode); - cifsInode->time = 0; /* will force revalidate to get info + cifs_inode = CIFS_I(inode); + cifs_inode->time = 0; /* will force revalidate to get info when needed */ inode->i_ctime = current_fs_time(sb); } dir->i_ctime = dir->i_mtime = current_fs_time(sb); - cifsInode = CIFS_I(dir); + cifs_inode = CIFS_I(dir); CIFS_I(dir)->time = 0; /* force revalidate of dir as well */ kfree(full_path);