Messages in this thread | | | Date | Wed, 28 Sep 2022 14:54:03 +0300 | From | Leon Romanovsky <> | Subject | Re: [syzbot] KASAN: use-after-free Read in rdma_close |
| |
On Wed, Sep 28, 2022 at 08:23:14PM +0900, asmadeus@codewreck.org wrote: > Leon Romanovsky wrote on Wed, Sep 28, 2022 at 01:49:19PM +0300: > > > But I agree I did get that wrong: trans_mod->close() wasn't called if > > > create failed. > > > We do want the idr_for_each_entry() that is in p9_client_destroy so > > > rather than revert the commit (fix a bug, create a new one..) I'd rather > > > split it out in an internal function that takes a 'bool close' or > > > something to not duplicate the rest. > > > (Bit of a nitpick, sure) > > > > Please do proper unwind without extra variable. > > > > Proper unwind means that you will call to symmetrical functions in > > destroy as you used in create: > > alloc -> free > > create -> close > > e.t.c > > > > When you use some global function like you did, there is huge chance > > to see unwind bugs. > > No.
Let's agree to disagree.
> > Duplicating complicated cleanup code leads to leaks like we used to > have; that destroy function already frees up things in the right order.
It is pretty straightforward code, nothing complex there.
Just pause for a minute, and ask yourself how totally random guy who looked on this syzbot bug just because RDMA name in it, found the issue so quickly.
I will give a hint, I saw not symmetrical error unwind in call trace.
Thanks
| |