From owner-autofs@linux.kernel.org Fri Oct 16 11:53:45 1998 Return-Path: Received: from linux.kernel.org (majordom@linux.kernel.org [206.184.214.34]) by mail.Linux-Consulting.com (8.8.7/8.8.7) with ESMTP id LAA11322 for ; Fri, 16 Oct 1998 11:53:44 -0700 Received: (from majordom@localhost) by linux.kernel.org (8.9.1/8.9.1) id KAA03643 for autofs-list; Fri, 16 Oct 1998 10:49:45 -0700 Received: from neon.transmeta.com (neon-best.transmeta.com [206.184.214.10]) by linux.kernel.org (8.9.1/8.9.1) with ESMTP id KAA03640 for ; Fri, 16 Oct 1998 10:49:43 -0700 Received: from deepthought.transmeta.com (mailhost.transmeta.com [10.1.1.15]) by neon.transmeta.com (8.9.1/8.9.1) with ESMTP id KAA01801 for ; Fri, 16 Oct 1998 10:46:32 -0700 Received: from cesium.transmeta.com (hpa@cesium.transmeta.com [10.1.2.55]) by deepthought.transmeta.com (8.8.8+spamcan/8.8.5) with ESMTP id KAA00744; Fri, 16 Oct 1998 10:49:39 -0700 (PDT) From: "H. Peter Anvin" Received: (from hpa@localhost) by cesium.transmeta.com (8.8.4/8.7.3) id KAA01600; Fri, 16 Oct 1998 10:49:39 -0700 Message-Id: <199810161749.KAA01600@cesium.transmeta.com> Subject: Patch to autofs for 2.1.125 To: autofs@linux.kernel.org (autofs mailing list), mis@transmeta.com (MIS Account) Date: Fri, 16 Oct 1998 10:49:39 -0700 (PDT) X-Mailer: ELM [version 2.4ME+ PL38 (25)] MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: owner-autofs@linux.kernel.org Precedence: bulk Status: RO Hello folks, I have a patch for the autofs kernel code against 2.1.125. I *believe* this fixes the currently known bugs in autofs; it also implements code to hide automount directories that don't have anything mounted on them from readdir(). Bugs *hopefully* addressed: - Expire can cause oops - "negative dentry on expire queue" - cd /autofs/deadserver lands in empty directory Please try it out and let me know. -hpa From owner-autofs@linux.kernel.org Fri Oct 23 12:01:44 1998 Return-Path: Received: from linux.kernel.org (majordom@linux.kernel.org [206.184.214.34]) by mail.Linux-Consulting.com (8.8.7/8.8.7) with ESMTP id MAA02691 for ; Fri, 23 Oct 1998 12:01:43 -0700 Received: (from majordom@localhost) by linux.kernel.org (8.9.1/8.9.1) id SAA04626 for autofs-list; Fri, 23 Oct 1998 18:14:53 -0700 Received: from neon.transmeta.com (neon-best.transmeta.com [206.184.214.10]) by linux.kernel.org (8.9.1/8.9.1) with ESMTP id SAA04622 for ; Fri, 23 Oct 1998 18:14:51 -0700 Received: from deepthought.transmeta.com (mailhost.transmeta.com [10.1.1.15]) by neon.transmeta.com (8.9.1/8.9.1) with ESMTP id SAA25009 for ; Fri, 23 Oct 1998 18:14:50 -0700 Received: from cesium.transmeta.com (hpa@cesium.transmeta.com [10.1.2.55]) by deepthought.transmeta.com (8.8.8+spamcan/8.8.5) with ESMTP id SAA02957; Fri, 23 Oct 1998 18:14:49 -0700 (PDT) From: "H. Peter Anvin" Received: (from hpa@localhost) by cesium.transmeta.com (8.8.4/8.7.3) id SAA02101; Fri, 23 Oct 1998 18:14:48 -0700 Message-Id: <199810240114.SAA02101@cesium.transmeta.com> Subject: Another autofs debugging patch To: autofs@linux.kernel.org (autofs mailing list) Date: Fri, 23 Oct 1998 18:14:48 -0700 (PDT) Cc: quinlan@transmeta.com (Daniel Quinlan) X-Mailer: ELM [version 2.4ME+ PL38 (25)] MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: owner-autofs@linux.kernel.org Precedence: bulk Status: RO Hi folks, The previously posted autofs patch for the 2.1.125 kernel didn't turn out so well, so here is a second attempt. Note that this patch contains extra debugging code that isn't likely to be in the distributed kernel. Please test it, and let me know how it turns out. The patch is against 2.1.125, but it should apply cleanly to pretty much any 2.1.12x kernel. -hpa diff -ur stock/linux-2.1.125/fs/autofs/autofs_i.h linux-2.1.125-smp/fs/autofs/autofs_i.h --- stock/linux-2.1.125/fs/autofs/autofs_i.h Thu Oct 8 21:32:25 1998 +++ linux-2.1.125-smp/fs/autofs/autofs_i.h Fri Oct 16 10:33:38 1998 @@ -47,11 +47,13 @@ struct autofs_dir_ent { int hash; - struct autofs_dir_ent *next; - struct autofs_dir_ent **back; char *name; int len; ino_t ino; + struct dentry *dentry; + /* Linked list of entries */ + struct autofs_dir_ent *next; + struct autofs_dir_ent **back; /* The following entries are for the expiry system */ unsigned long last_usage; struct autofs_dir_ent *exp_next; @@ -123,7 +125,7 @@ struct autofs_dir_ent *autofs_hash_lookup(const struct autofs_dirhash *,struct qstr *); void autofs_hash_insert(struct autofs_dirhash *,struct autofs_dir_ent *); void autofs_hash_delete(struct autofs_dir_ent *); -struct autofs_dir_ent *autofs_hash_enum(const struct autofs_dirhash *,off_t *); +struct autofs_dir_ent *autofs_hash_enum(const struct autofs_dirhash *,off_t *,struct autofs_dir_ent *); void autofs_hash_nuke(struct autofs_dirhash *); /* Expiration-handling functions */ diff -ur stock/linux-2.1.125/fs/autofs/dirhash.c linux-2.1.125-smp/fs/autofs/dirhash.c --- stock/linux-2.1.125/fs/autofs/dirhash.c Tue Aug 18 13:12:17 1998 +++ linux-2.1.125-smp/fs/autofs/dirhash.c Tue Oct 20 15:31:00 1998 @@ -30,9 +30,50 @@ ent->exp_next->exp_prev = ent->exp_prev; } +static int autofs_verify_expiry_list_sane(struct autofs_dirhash *dh, + struct autofs_dir_ent *ent) +{ + /* Sanity check: verify the integrity of the expiry list + and make sure "ent" is a member */ + int found_ent, timeout; + struct autofs_dir_ent *ptr; + + ptr = dh->expiry_head.exp_next; + found_ent = 0; + timeout = 64; + + while ( ptr != &dh->expiry_head ) { + if ( ptr == ent ) { + if ( found_ent ) { + printk("autofs: entry %s on expiry queue twice - circular list?\n", ent->name); + return 1; + } else + found_ent = 1; + } + if ( ! --timeout ) { + printk("autofs: expiry queue doesn't appear to end\n"); + return 1; + } + ptr = ptr->exp_next; + } + if ( !found_ent ) { + printk("autofs: expiry queue doesn't contain expected entry: %s\n", ent->name); + printk("queue contents: "); + ptr = dh->expiry_head.exp_next; + while ( ptr != &dh->expiry_head ) { + printk("%s ", ptr->name); + ptr = ptr->exp_next; + } + } + return 0; +} + void autofs_update_usage(struct autofs_dirhash *dh, struct autofs_dir_ent *ent) { + if ( autofs_verify_expiry_list_sane(dh,ent) ) + return; + autofs_delete_usage(ent); /* Unlink from current position */ autofs_init_usage(dh,ent); /* Relink at queue tail */ } @@ -59,13 +100,11 @@ return ent; /* Symlinks are always expirable */ /* Get the dentry for the autofs subdirectory */ - dentry = lookup_dentry(ent->name, dget(sb->s_root), 0); + dentry = ent->dentry; - if ( IS_ERR(dentry) ) { - printk("autofs: no such dentry on expiry queue: %s\n", - ent->name); + if ( !dentry ) { + printk("autofs: dentry == NULL but inode range is directory, entry %s\n", ent->name); autofs_delete_usage(ent); - continue; } if ( !dentry->d_inode ) { @@ -79,24 +118,12 @@ /* Make sure entry is mounted and unused; note that dentry will point to the mounted-on-top root. */ if ( !S_ISDIR(dentry->d_inode->i_mode) - || dentry->d_covers == dentry ) { - dput(dentry); + || dentry->d_mounts == dentry ) { DPRINTK(("autofs: not expirable (not a mounted directory): %s\n", ent->name)); continue; } - /* - * Now, this is known to be a mount point; therefore the dentry - * will be held by the superblock. is_root_busy() will break if - * we hold a use count here, so we have to dput() it before calling - * is_root_busy(). However, since it is a mount point (already - * verified), dput() will be a nonblocking operation and the use - * count will not go to zero; therefore the call to is_root_busy() - * here is legal. - */ - dput(dentry); - - if ( !is_root_busy(dentry) ) { + if ( !is_root_busy(dentry->d_mounts) ) { DPRINTK(("autofs: signaling expire on %s\n", ent->name)); return ent; /* Expirable! */ } @@ -136,6 +163,8 @@ autofs_say(ent->name,ent->len); autofs_init_usage(dh,ent); + if ( ent->dentry ) + ent->dentry->d_count++; dhnp = &dh->h[(unsigned) ent->hash % AUTOFS_HASH_SIZE]; ent->next = *dhnp; @@ -153,6 +182,8 @@ autofs_delete_usage(ent); + if ( ent->dentry ) + dput(ent->dentry); kfree(ent->name); kfree(ent); } @@ -161,8 +192,12 @@ * Used by readdir(). We must validate "ptr", so we can't simply make it * a pointer. Values below 0xffff are reserved; calling with any value * <= 0x10000 will return the first entry found. + * + * "last" can be NULL or the value returned by the last search *if* we + * want the next sequential entry. */ -struct autofs_dir_ent *autofs_hash_enum(const struct autofs_dirhash *dh, off_t *ptr) +struct autofs_dir_ent *autofs_hash_enum(const struct autofs_dirhash *dh, + off_t *ptr, struct autofs_dir_ent *last) { int bucket, ecount, i; struct autofs_dir_ent *ent; @@ -176,19 +211,23 @@ DPRINTK(("autofs_hash_enum: bucket %d, entry %d\n", bucket, ecount)); - ent = NULL; - - while ( bucket < AUTOFS_HASH_SIZE ) { - ent = dh->h[bucket]; - for ( i = ecount ; ent && i ; i-- ) - ent = ent->next; + ent = last ? last->next : NULL; - if (ent) { - ecount++; /* Point to *next* entry */ - break; + if ( ent ) { + ecount++; + } else { + while ( bucket < AUTOFS_HASH_SIZE ) { + ent = dh->h[bucket]; + for ( i = ecount ; ent && i ; i-- ) + ent = ent->next; + + if (ent) { + ecount++; /* Point to *next* entry */ + break; + } + + bucket++; ecount = 0; } - - bucket++; ecount = 0; } #ifdef DEBUG @@ -214,6 +253,8 @@ for ( i = 0 ; i < AUTOFS_HASH_SIZE ; i++ ) { for ( ent = dh->h[i] ; ent ; ent = nent ) { nent = ent->next; + if ( ent->dentry ) + dput(ent->dentry); kfree(ent->name); kfree(ent); } diff -ur stock/linux-2.1.125/fs/autofs/root.c linux-2.1.125-smp/fs/autofs/root.c --- stock/linux-2.1.125/fs/autofs/root.c Mon Aug 24 13:14:10 1998 +++ linux-2.1.125-smp/fs/autofs/root.c Fri Oct 23 15:41:35 1998 @@ -66,9 +66,10 @@ static int autofs_root_readdir(struct file *filp, void *dirent, filldir_t filldir) { - struct autofs_dir_ent *ent; + struct autofs_dir_ent *ent = NULL; struct autofs_dirhash *dirhash; struct inode * inode = filp->f_dentry->d_inode; + void *where = NULL; off_t onr, nr; if (!inode || !S_ISDIR(inode->i_mode)) @@ -90,10 +91,12 @@ filp->f_pos = ++nr; /* fall through */ default: - while ( onr = nr, ent = autofs_hash_enum(dirhash,&nr) ) { - if (filldir(dirent,ent->name,ent->len,onr,ent->ino) < 0) - return 0; - filp->f_pos = nr; + while ( onr = nr, ent = autofs_hash_enum(dirhash,&nr,ent) ) { + if ( !ent->dentry || ent->dentry->d_mounts != ent->dentry ) { + if (filldir(dirent,ent->name,ent->len,onr,ent->ino) < 0) + return 0; + filp->f_pos = nr; + } } break; } @@ -110,8 +113,8 @@ if ( !(ent = autofs_hash_lookup(&sbi->dirhash, &dentry->d_name)) ) { do { if ( status && dentry->d_inode ) { - printk("autofs warning: lookup failure on existing dentry, status = %d, name = %s\n", status, dentry->d_name.name); - break; + printk("autofs warning: lookup failure on positive dentry, status = %d, name = %s\n", status, dentry->d_name.name); + return 0; /* Try to get the kernel to invalidate this dentry */ } /* Turn this into a real negative dentry? */ @@ -148,8 +151,9 @@ /* We don't update the usages for the autofs daemon itself, this is necessary for recursive autofs mounts */ - if ( !autofs_oz_mode(sbi) ) + if ( !autofs_oz_mode(sbi) ) { autofs_update_usage(&sbi->dirhash,ent); + } dentry->d_flags &= ~DCACHE_AUTOFS_PENDING; return 1; @@ -193,7 +197,8 @@ /* Update the usage list */ if ( !autofs_oz_mode(sbi) ) { ent = (struct autofs_dir_ent *) dentry->d_time; - autofs_update_usage(&sbi->dirhash,ent); + if ( ent ) + autofs_update_usage(&sbi->dirhash,ent); } return 1; } @@ -207,6 +212,7 @@ static int autofs_root_lookup(struct inode *dir, struct dentry * dentry) { struct autofs_sb_info *sbi; + struct autofs_dir_ent *ent; struct inode *res; int oz_mode; @@ -249,6 +255,15 @@ return -ERESTARTNOINTR; } + /* + * If this dentry is unhashed, then we shouldn't honour this + * lookup even if the dentry is positive. Returning ENOENT here + * doesn't do the right thing for all system calls, but it should + * be OK for the operations we permit from an autofs. + */ + if ( dentry->d_inode && list_empty(&dentry->d_hash) ) + return -ENOENT; + return 0; } @@ -304,6 +319,7 @@ ent->ino = AUTOFS_FIRST_SYMLINK + n; ent->hash = dentry->d_name.hash; memcpy(ent->name, dentry->d_name.name, 1+(ent->len = dentry->d_name.len)); + ent->dentry = NULL; /* We don't keep the dentry for symlinks */ autofs_hash_insert(dh,ent); d_instantiate(dentry, iget(dir->i_sb,ent->ino)); @@ -339,7 +355,8 @@ n = ent->ino - AUTOFS_FIRST_SYMLINK; if ( n >= AUTOFS_MAX_SYMLINKS || !test_bit(n,sbi->symlink_bitmap) ) return -EINVAL; /* Not a symlink inode, can't unlink */ - + + dentry->d_time = (unsigned long)(struct autofs_dirhash *)NULL; autofs_hash_delete(ent); clear_bit(n,sbi->symlink_bitmap); kfree(sbi->symlink[n].data); @@ -353,6 +370,7 @@ struct autofs_sb_info *sbi = (struct autofs_sb_info *) dir->i_sb->u.generic_sbp; struct autofs_dirhash *dh = &sbi->dirhash; struct autofs_dir_ent *ent; + struct dentry *odentry; if ( !autofs_oz_mode(sbi) ) return -EPERM; @@ -364,6 +382,12 @@ if ( (unsigned int)ent->ino < AUTOFS_FIRST_DIR_INO ) return -ENOTDIR; /* Not a directory */ + if ( ent->dentry != dentry ) { + printk("autofs_rmdir: odentry != dentry for entry %s\n", + dentry->d_name); + } + + dentry->d_time = (unsigned long)(struct autofs_dir_ent *)NULL; autofs_hash_delete(ent); dir->i_nlink--; d_drop(dentry); @@ -399,12 +423,14 @@ return -ENOSPC; } + dir->i_nlink++; + d_instantiate(dentry, iget(dir->i_sb,ent->ino)); + ent->hash = dentry->d_name.hash; memcpy(ent->name, dentry->d_name.name, 1+(ent->len = dentry->d_name.len)); ent->ino = sbi->next_dir_ino++; + ent->dentry = dentry; autofs_hash_insert(dh,ent); - dir->i_nlink++; - d_instantiate(dentry, iget(dir->i_sb,ent->ino)); return 0; }