Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 23 Oct 1995 10:52:48 -0700 (MST)
From:      Terry Lambert <terry@lambert.org>
To:        phk@freefall.freebsd.org (Poul-Henning Kamp)
Cc:        terry@lambert.org, current@freebsd.org
Subject:   Re: fs layering pathces doesn't work.
Message-ID:  <199510231752.KAA11278@phaeton.artisoft.com>
In-Reply-To: <15680.814315774@freefall.freebsd.org> from "Poul-Henning Kamp" at Oct 21, 95 03:49:34 pm

next in thread | previous in thread | raw e-mail | index | archive | help
> Hi Terry,
> 
> I have tried your fs_layering stuff again, and they still doesn't work
> for me.

Sorry for not getting back to you sooner on this -- I've been deluged
at work.  8-(.

> Here's what I did:
> 
> I checked out a -current /sys.
> applied the two files from freefall:~terry
> config -g'ed this file:
> (Notice: No NFS (and not as a LKM either :-), No DIAGNOSTICS, as I have 
> already told you several times, though you don't seem to want to listen :-)

My reluctance is purely from the panic message you originally reported
existing only in #ifdef DIAGNOSTIC code.  8-).

It's pretty obvious that either a patch has failed or something else
has gone wrong.

I gave you a "hack" to get around the "multiple free" problem: in the
nfs_nameifree() routine in /sys/nfs/nfs_subs.c and in the nameifree()
routine in /sys/kern/vfs_lookup.c, clear the "HASBUF" bit when the
path name buffer is freed.

But it turns out that that is insufficient:

] cd /a/fs_layer/sys
] find . -name '*.[ch]' -type f -print | xargs grep -i 'free.*cn_pnbuf'
] 
] ./kern/vfs_lookup.c:		free(cnp->cn_pnbuf, M_NAMEI);
] ./kern/vfs_lookup.c:			FREE(cnp->cn_pnbuf, M_NAMEI);
] ./kern/vfs_lookup.c:				FREE(cnp->cn_pnbuf, M_NAMEI);
] ./kern/vfs_lookup.c:			FREE(cnp->cn_pnbuf, M_NAMEI);
] ./kern/vfs_lookup.c:	FREE(cnp->cn_pnbuf, M_NAMEI);
] ./kern/vfs_lookup.c:		FREE(cnp->cn_pnbuf, M_NAMEI);
] ./nfs/nfs_subs.c:	FREE(cnp->cn_pnbuf, M_NAMEI);
] ./nfs/nfs_subs.c:		FREE(cnp->cn_pnbuf, M_NAMEI);

All of the above are correct.  All of the following are *BOGUS*:

] ./miscfs/devfs/devfs_vnops.c:		FREE(ap->a_cnp->cn_pnbuf, M_NAMEI);
] ./miscfs/union/union_subr.c:	free(cn.cn_pnbuf, M_NAMEI);
] ./miscfs/union/union_subr.c:	free(cn.cn_pnbuf, M_NAMEI);
] ./ufs/ufs/ufs_vnops.c:	FREE(cnp->cn_pnbuf, M_NAMEI);
] ./ufs/ufs/ufs_vnops.c:		free(cnp->cn_pnbuf, M_NAMEI);
] ./ufs/ufs/ufs_vnops.c:	FREE(cnp->cn_pnbuf, M_NAMEI);
] 
] Hope you can find it from this.

Yep.  The pathname buffer is only allowed to be allocated/freed in
either /sys/kern/vfs_lookup.c or in /sys/nfs/nfs_subs.c.  This is
the main fix, in a nutshell.

Your problem is clearly the remaining free()'s in ufs_vnops.c.  If
you used the unionfs or devfs, you'd have a problem there, too.

I don't know why you have these, unless you are running a different
ufs_vnops.c from -current, since my patches are supposed to remove
all non-opaque reference to the path name buffer (this is prepatory
to going to parsed path structures, something I've done locally).

The devfs I understand, since it is under active developement, but
the union and ufs free()'s of cn_pnbuf are *definitely* broken (the
"HASBUF" flag hack, which shouldn't be necessary, would not save
you from a multiple free in this case because the flag is not
correctly unset in these instances in the orginal code).

Remove the free()'s, and your problem should go away.  If you can
grep them out in context, I can provide the fixes (or email me the
post patch-file, or put it on freefall, and I'll hand-correct it/them).

Thanks for sticking with this so hard; I appreciate the effort!  It
really will be worth it in the end.


> I don't give you too high ratings for the regression testing you
> claim to have done though... :-)


I can't explain why the extra layering violations survive in your tree
but not in mine, but they are *definitely* the problem.  Check for
patch rejects or non-standard branch tags (?).


					Regards,
					Terry Lambert
					terry@lambert.org
---
Any opinions in this posting are my own and not those of my present
or previous employers.



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?199510231752.KAA11278>