[Spice-devel] [spice-gtk v1] channel-webdav: Do not crash if no PhodavServer is found

[Victor Toso]

Hi,

On Mon, Apr 03, 2017 at 09:13:58AM -0400, Marc-André Lureau wrote:
> Hi
>
> ----- Original Message -----
> > From: Victor Toso <me at victortoso.com>
> >
> > This fixes a crash when spice_session_get_webdav_server() returns NULL
> > which can easily happen if no shared folder is set.
> >
>
> Can you start the phodav server without shared folder?
>
> How did you reproduce?

Running as root, with sudo ./tools/spicy ...

>
> > We still lack a way to tell the Guest about the failure as it waits
> > the data to mount the shared folder.
> > 
> > Log:
> > > (spicy:20825): phodav-CRITICAL **: phodav_server_get_soup_server: assertion
> > > 'PHODAV_IS_SERVER (self)' failed
> > > (spicy:20825): GSpice-DEBUG: channel-webdav.c:368 webdav-11:0: starting
> > > client
> > > -1812687888
> > >
> > > (spicy:20825): GLib-GObject-CRITICAL **: g_type_instance_get_private:
> > > assertion
> > > 'instance != NULL && instance->g_class != NULL' failed
> > >
> > > Thread 1 "spicy" received signal SIGSEGV, Segmentation fault.
> > > soup_server_accept_socket (server=server at entry=0x0,
> > > sock=sock at entry=0x7f7670
> > >
> > > [SoupSocket]) at soup-server.c:1468
> > > 1468            priv->clients = g_slist_prepend (priv->clients, client);
> > 
> > Backtrace:
>
> Hmm, looks like libsoup could use a g_return_if_fail

Yes!

> 
> > > (gdb) bt
> > > #0  in soup_server_accept_socket (server=server at entry=0x0,
> > >     sock=sock at entry=0x7f7670[SoupSocket]) at soup-server.c:1468
> > > #1  in soup_server_accept_iostream (server=0x0,
> > >     stream=0xb77890 [GSimpleIOStream], local_addr=<optimized out>,
> > >     remote_addr=<optimized out>, error=<optimized out>) at
> > >     soup-server.c:1515
> > > #2  in start_client (self=0xa0a6e0 [SpiceWebdavChannel]) at
> > > channel-webdav.c:382
> > > #3  in data_read_cb (source_object=0x9d2e60 [SpiceVmcInputStream],
> > >     res=0x9d0e80, user_data=0xa0a6e0) at channel-webdav.c:429
> > > #4  in g_task_return_now (task=0x9d0e80 [GTask]) at gtask.c:1121
> > > #5  in g_task_return (task=0x9d0e80 [GTask], type=<optimized out>) at
> > > gtask.c:1179
> > > #6  in complete_in_idle_cb (user_data=0x1233070) at vmcstream.c:110
> > 
> > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > ---
> >  src/channel-webdav.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/channel-webdav.c b/src/channel-webdav.c
> > index 4a246b5..38de2fb 100644
> > --- a/src/channel-webdav.c
> > +++ b/src/channel-webdav.c
> > @@ -361,9 +361,15 @@ static void start_client(SpiceWebdavChannel *self)
> >      SoupServer *server;
> >      GSocketAddress *addr;
> >      GError *error = NULL;
> > +    PhodavServer *phodav;
> >  
> >      session = spice_channel_get_session(SPICE_CHANNEL(self));
> > -    server =
> > phodav_server_get_soup_server(spice_session_get_webdav_server(session));
> > +    phodav = spice_session_get_webdav_server(session);
> > +    if (phodav == NULL) {
> > +        CHANNEL_DEBUG(self, "failed to start client: no Phodav Server");
> > +        return;
> > +    }
>
> g_return_if_fail() enough as well, there is already a clear DEBUG in
> spice-session.c, that seems redundant.

True, it is redundant. Logging a critical seems better indeed.

I'll send a followup patch soon.

>
> > +    server = phodav_server_get_soup_server(phodav);
> >  
> >      CHANNEL_DEBUG(self, "starting client %" G_GINT64_FORMAT,
> >      c->demux.client);
> >  
> > --
> > 2.9.3
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170403/4c1c5ff5/attachment.sig>