On Sat, Apr 10, 2021 at 12:56:34PM +0300, Vladimir Sementsov-Ogievskiy wrote: > 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<rvka...@yandex-team.ru> > > > > > > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@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.
The test didn't crash at me somehow but you're right there's bug here. > 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. I agree it turned out more complex than apporpriate for 6.0. Let's proceed with the one you've posted. Thanks, Roman.
