d_op->d_delete race problem

Bill Hawes (whawes@star.net)
Sun, 21 Sep 1997 20:58:39 -0400


This is a multi-part message in MIME format.
--------------61E1A298ADD2F7873BEA59E1
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

I've run into a potential problem in dput() with the call to
d_op->d_delete. It's called with the dentry d_count at 0 but still
hashed, so if the d_delete op blocks, there's a chance the dentry could
go back into use. Since d_count isn't checked again after
d_op->d_delete returns, this could lead to memory being freed while a
reference is still active.

My usage of d_delete in smbfs needs to block, as I'm using this to close
the network filehandle, so the dcache layer can cache the file without
tying up filehandles on the server.

The attached patch seems like a reasonable solution, as it allows for
the dentry to go back into use with no problems, except that the fs has
to be careful with whatever operations it's doing in the call. Another
possibility might be to use a busy flag to hold off lookups while the
operation is in progress, but this approach would have to be careful not
to reference the dentry after waking up.

Any other ideas for handling this?

Regards,
Bill
--------------61E1A298ADD2F7873BEA59E1
Content-Type: text/plain; charset=us-ascii; name="dput_56-patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="dput_56-patch"

--- fs/dcache.c.old Sat Sep 20 08:16:14 1997
+++ fs/dcache.c Sun Sep 21 13:15:09 1997
@@ -43,6 +43,7 @@
kfree(dentry);
}

+#define TEMP_SMBFS_HACK 1
/*
* dput()
*
@@ -64,6 +65,15 @@
if (dentry) {
int count;
repeat:
+ if (dentry->d_count == 1) {
+ /*
+ * Note that if d_op->d_delete blocks,
+ * the dentry could go back in use.
+ * Each fs will have to watch for this.
+ */
+ if (dentry->d_op && dentry->d_op->d_delete)
+ dentry->d_op->d_delete(dentry);
+ }
count = dentry->d_count-1;
if (count < 0) {
printk("Negative d_count (%d) for %s/%s\n",
@@ -75,13 +85,15 @@
dentry->d_count = count;
if (!count) {
list_del(&dentry->d_lru);
- if (dentry->d_op && dentry->d_op->d_delete)
- dentry->d_op->d_delete(dentry);
if (list_empty(&dentry->d_hash)) {
struct inode *inode = dentry->d_inode;
struct dentry * parent;
- if (inode)
+ if (inode) {
+#ifdef TEMP_SMBFS_HACK
+ dentry->d_inode = NULL;
+#endif
iput(inode);
+ }
parent = dentry->d_parent;
d_free(dentry);
if (dentry == parent)

--------------61E1A298ADD2F7873BEA59E1--