glibc detected freeing invalid pointer on ash... might be a bug, but I'm not sure

[Denis Vlasenko]

Hi.

First, let me thank you for excellent bug report!

On Monday 12 March 2007 03:09, Franklin wrote:
> I tried to hacked into shells/ash.c, added some debug messages and found that, 
> in the popstackmark() in ash.c (about line 8315+), 
> 
> 	while (stackp != mark->stackp) {
> 		sp = stackp;
> 		stackp = sp->prev;
> 		ckfree(sp);
> 	}
> 
> in some cases the mark->stackp became NULL, so the stackp would never be equal 
> to mark->stackp, then finally it will try to free the stackbase, which is a 
> static non-pointer struct variable, and glibc complains about it.
> 
> I don't know what cases cause the mark->stackp become NULL. I think that the 
> mark itself might be invalid too.  I did not dig too much, I just added a 
> 
> if (!mark->stackp) return;

I'm no expert in ash too (we currently have that position vacant).
I will add this as a stopgap measure.

Meanwhile, if you feel so inclined, you may try to dig a bit deeper.
mark->stackp is set only in two places. Here:

static void
setstackmark(struct stackmark *mark)
{
        mark->stackp = stackp;
        mark->stacknxt = stacknxt;
        mark->stacknleft = stacknleft;
        mark->marknext = markp;
        markp = mark;
}

and here:

                while (xmark != NULL && xmark->stackp == oldstackp) {
                        xmark->stackp = stackp;
                        xmark->stacknxt = stacknxt;
                        xmark->stacknleft = stacknleft;
                        xmark = xmark->marknext;
                }

Can you add debug prints there like this?

if (!stackp) fprintf(stderr, "<func>: xmark->stackp set to NULL, xmark=%p\n", xmark);

and of course

static void
popstackmark(struct stackmark *mark)
{
        if (!mark->stackp) {
                fprintf(stderr, "popstackmark: mark->stackp is NULL, mark=%p\n", mark);
                return;
        }

Let's try to catch that NULL assignment, or determine that
it is done somewhere else (e.g. memset(0)).

Thanks!
--
vda