Re: [PATCH for-6.0 2/2] block/nbd: ensure ->connection_thread is always valid

[Vladimir Sementsov-Ogievskiy]

10.04.2021 11:38, Vladimir Sementsov-Ogievskiy wrote:
> 10.04.2021 11:06, Vladimir Sementsov-Ogievskiy wrote:
> > 09.04.2021 19:04, Roman Kagan wrote:
> > > Simplify lifetime management of BDRVNBDState->connection_thread by
> > > delaying the possible cleanup of it until the BDRVNBDState itself goes
> > > away.
> > >
> > > This also fixes possible use-after-free in nbd_co_establish_connection
> > > when it races with nbd_co_establish_connection_cancel.
> > >
> > > Signed-off-by: Roman Kagan<rvkagan@yandex-team.ru>
> >
> > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>
> Ha stop, it crashes iotest 51, as nbd_clear_bdrvstate is called also from 
> nbd_process_options.
>
> And this shows that we also do wrong thing when simply return from two ifs 
> pre-patch (and one after-patch). Yes, after successful nbd_process options we 
> should call nbd_clear_bdrvstate() on failure path.

And also it shows that patch is more difficult than it seems. I still think 
that for 6.0 we should take a simple use-after-free-only fix, and don't care 
about anything else.

--
Best regards,
Vladimir