[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-6.0 2/2] block/nbd: ensure ->connection_thread is always
From: |
Roman Kagan |
Subject: |
Re: [PATCH for-6.0 2/2] block/nbd: ensure ->connection_thread is always valid |
Date: |
Sat, 10 Apr 2021 15:20:51 +0300 |
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<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.
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.