Date: Wed, 28 Sep 2005 23:24:51 +0100 From: Peter Edwards <peadar.edwards@gmail.com> To: freebsd-current@freebsd.org Subject: biodone panics Message-ID: <34cb7c8405092815247dc89bf6@mail.gmail.com>
next in thread | raw e-mail | index | archive | help
There's a few people complaining about double-frees (and other
crashes) in biodone(), and I just banged my head off it too: They
generally look a little like this:
> panic(c0870cfb,c1b0c4a4,c143f000,c0850ced,c0870cdf) at panic+0x127
(where c0870cfb =3D=3D "duplicate free of item %p from zone %p(%s)"
> uma_dbg_free(c143f000,0,c1b0c4a4) at uma_dbg_free+0x110
> uma_zfree_arg(c143f000,c1b0c4a4,0) at uma_zfree_arg+0x66
> g_destroy_bio(c1b0c4a4) at g_destroy_bio+0x13
> g_vfs_done(c1b0c4a4) at g_vfs_done+0x5a
> biodone(c1b0c4a4,ca0bccc4,0,c0850cb0,1e4) at biodone+0x57
> g_io_schedule_up(c1833300) at g_io_schedule_up+0xb5
> g_up_procbody(0,ca0bcd38,0,c05fed08,0) at g_up_procbody+0x5a
> fork_exit(c05fed08,0,ca0bcd38) at fork_exit+0xa0
> fork_trampoline() at fork_trampoline+0x8
Am I just being dense, or is there a race condition in biodone():
> void
> biodone(struct bio *bp)
> {
>
> mtx_lock(&bdonelock);
> bp->bio_flags |=3D BIO_DONE;
> if (bp->bio_done =3D=3D NULL)
> wakeup(bp);
> mtx_unlock(&bdonelock);
> if (bp->bio_done !=3D NULL)
> bp->bio_done(bp);
> }
the call to wakeup may set in motion some events that cause the bio to
be freed. By the time the mtx_unlock completes, "bp" could point to an
invalid bio, and the "if (bp->bio_done !=3D NULL)" is bogus. True?
Shouldn't it be
> biodone(struct bio *bp)
> {
> void (*done)(struct bio *);
>
> mtx_lock(&bdonelock);
> bp->bio_flags |=3D BIO_DONE;
> done =3D bp->bio_done
> if (done =3D=3D NULL)
> wakeup(bp);
> mtx_unlock(&bdonelock);
> if (done !=3D NULL)
> bp->bio_done(bp);
> }
Anyone agree?
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?34cb7c8405092815247dc89bf6>