[Spice-devel] [PATCH] channel-webdav: avoid possible crash

[Frediano Ziglio]

> 
> From: Victor Toso <me at victortoso.com>
> 
> In case PhodavServer or SoupServer are NULL, we should stop and
> return.
> 
> Signed-off-by: Victor Toso <victortoso at redhat.com>
> ---
> 
> Rebased. Patch was related user report of crash in FlexVDI client [0].
> Being careful in this not-hot path should be okay IMHO.
> 
> [0] https://lists.freedesktop.org/archives/spice-devel/2019-May/049070.html
> 
>  src/channel-webdav.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/channel-webdav.c b/src/channel-webdav.c
> index fb25084..de2843e 100644
> --- a/src/channel-webdav.c
> +++ b/src/channel-webdav.c
> @@ -361,12 +361,17 @@ static void start_client(SpiceWebdavChannel *self)
>      GIOStream *peer = NULL;
>      SpiceSession *session;
>      SoupServer *server;
> +    PhodavServer* phodav_server;
>      GSocketAddress *addr;
>      GError *error = NULL;
>      bool started;
>  
>      session = spice_channel_get_session(SPICE_CHANNEL(self));
> -    server =
> phodav_server_get_soup_server(spice_session_get_webdav_server(session));
> +    phodav_server = spice_session_get_webdav_server(session);
> +    g_return_if_fail(phodav_server != NULL);
> +

As this will be always NULL we'll have a critical for each message sent by
the server.
g_return_if_fail documentation states:

"If expr evaluates to FALSE, the current function should be considered to have
undefined behaviour (a programmer error). The only correct solution to such
an error is to change the module that is calling the current function,
so that it avoids this incorrect call."

So this is a wrong usage of g_return_if_fail.

> +    server = phodav_server_get_soup_server(phodav_server);
> +    g_return_if_fail(server != NULL);
>  
>      CHANNEL_DEBUG(self, "starting client %" G_GINT64_FORMAT,
>      c->demux.client);
>  

Frediano