Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750813AbVKWOQB (ORCPT ); Wed, 23 Nov 2005 09:16:01 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1750814AbVKWOQB (ORCPT ); Wed, 23 Nov 2005 09:16:01 -0500 Received: from ms-smtp-02.nyroc.rr.com ([24.24.2.56]:60562 "EHLO ms-smtp-02.nyroc.rr.com") by vger.kernel.org with ESMTP id S1750813AbVKWOQA (ORCPT ); Wed, 23 Nov 2005 09:16:00 -0500 Subject: Re: What protection does sysfs_readdir have with SMP/Preemption? From: Steven Rostedt To: maneesh@in.ibm.com Cc: Greg KH , LKML , Ingo Molnar In-Reply-To: <20051123135847.GF22714@in.ibm.com> References: <1132695202.13395.15.camel@localhost.localdomain> <20051122213947.GB8575@kroah.com> <20051123045049.GA22714@in.ibm.com> <20051123135847.GF22714@in.ibm.com> Content-Type: text/plain Date: Wed, 23 Nov 2005 09:15:44 -0500 Message-Id: <1132755344.13395.32.camel@localhost.localdomain> Mime-Version: 1.0 X-Mailer: Evolution 2.2.3 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2157 Lines: 58 On Wed, 2005-11-23 at 19:28 +0530, Maneesh Soni wrote: > > hmm looks like we got some situation which is not desirable and could lead > to bogus sysfs_dirent in the parent list. It may not be the exact problem > in this case though, but needs fixing IMO. > > After sysfs_make_dirent(), the ref count for sysfs dirent will be 2. > (one from allocation, and after linking the new dentry to it). On > error from sysfs_create(), we do sysfs_put() once, decrementing the > ref count to 1. And again when the new dentry for which we couldn't > allocate the d_inode, is d_drop()'ed. In sysfs_d_iput() we again > sysfs_put(), and decrement the sysfs dirent's ref count to 0, which will > be the final sysfs_put(), and it will free the sysfs_dirent but never > unlinks it from the parent list. So, parent list could still will having > links to the freed sysfs_dirent in its s_children list. > > so basically list_del_init(&sd->s_sibling) should be done in error path > in create_dir(). > > Could you also put the appended patch in your trial runs.. > I'm already playing around with this. You might want this patch instead. I noticed that if sysfs_make_dirent fails to allocate the sd, then a null will be passed to sysfs_put. But this is not the end of the problems. I'll follow up on that comment right after this. -- Steve Signed-off-by: Steven Rostedt Index: linux-2.6.15-rc2-git2/fs/sysfs/dir.c =================================================================== --- linux-2.6.15-rc2-git2.orig/fs/sysfs/dir.c 2005-11-23 08:40:33.000000000 -0500 +++ linux-2.6.15-rc2-git2/fs/sysfs/dir.c 2005-11-23 08:52:57.000000000 -0500 @@ -112,7 +112,11 @@ } } if (error && (error != -EEXIST)) { - sysfs_put((*d)->d_fsdata); + struct sysfs_dirent *sd = (*d)->d_fsdata; + if (sd) { + list_del_init(&sd->s_sibling); + sysfs_put(sd); + } d_drop(*d); } dput(*d); - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/