qemu-block
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 1/3] nbd: CVE-XXX: Use cookie to track generation of nbd-s


From: Eric Blake
Subject: Re: [PATCH v2 1/3] nbd: CVE-XXX: Use cookie to track generation of nbd-server
Date: Mon, 5 Aug 2024 21:28:44 -0500
User-agent: NeoMutt/20240425

On Fri, Aug 02, 2024 at 06:00:32PM GMT, Vladimir Sementsov-Ogievskiy wrote:
> > -static void nbd_blockdev_client_closed(NBDClient *client, bool ignored)
> > +static void nbd_blockdev_client_closed(NBDClient *client, uint32_t cookie,
> > +                                       bool ignored)
> >   {
> >       nbd_client_put(client);
> > -    assert(nbd_server->connections > 0);
> > -    nbd_server->connections--;
> > -    nbd_update_server_watch(nbd_server);
> > +    /* Ignore any (late) connection made under a previous server */
> > +    if (cookie == nbd_cookie) {
> 
> creating a getter nbd_client_get_cookie(client), and use it instead of 
> passing together with client, will simplify the patch a lot. [*]
> 
> Hmm.. don't we need some atomic accessors for nbd_cookie? and for 
> nbs_server->connections.. The function is called from client, which live in 
> coroutine and maybe in another thread? At least client code do atomic 
> accesses of client->refcount..
> 
> > +        assert(nbd_server->connections > 0);
> > +        nbd_server->connections--;
> > +        nbd_update_server_watch(nbd_server);

The client code already jumps through hoops to make sure
nbd_blockdev_client_closed() is the last thing called, and that
nbd_client_put() is only reached from the main thread (any coroutines
fired off a one-shot bh); but I added asserts in v3 to make it clear
I'm relying on the synchronous nature of coroutines yielding only at
known points and the code executing only in the main thread as the
reason why we don't need explicit locking here.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org




reply via email to

[Prev in Thread] Current Thread [Next in Thread]