[PATCH] usbredirhost: fix null dereference

[Uri Lublin]

Hi Dmitry,

Thanks for testing and sending this patch.
One comment below.

On Wed, Apr 9, 2025 at 12:07 AM Dmitry Frolov <frolov at swemel.ru> wrote:

> In function usbredirhost_open_full(), when host->parser==NULL,
> the call sequence:
> "usbredirhost_close() --->
>     usbredirhost_clear_device() --->
>         usbredirhost_handle_disconnect()"
> is called, where host->parser is being derefferenced without a check.
>

Can this really happen ?
In usbredirhost_open_full() if host->parser is NULL then host->dev
is NULL -- being set only later in the function, with a call to
usbredirhost_set_device().

In usbredirhost_clear_device() if host->dev is NULL the function returns
immediately and does not call usbredirhost_handle_disconnect().

Thanks,
    Uri.


> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Signed-off-by: Dmitry Frolov <frolov at swemel.ru>
> ---
>  usbredirhost/usbredirhost.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/usbredirhost/usbredirhost.c b/usbredirhost/usbredirhost.c
> index 03c56e9..ad4c09b 100644
> --- a/usbredirhost/usbredirhost.c
> +++ b/usbredirhost/usbredirhost.c
> @@ -269,6 +269,7 @@ static int usbredirhost_write(void *priv, uint8_t
> *data, int count)
>     packet completion callbacks */
>  static void usbredirhost_handle_disconnect(struct usbredirhost *host)
>  {
> +    if(!host->parser) return;
>      /* Disconnect uses its own lock to avoid needing nesting capable
> locks */
>      if (host->disconnect_lock) {
>          host->parser->lock_func(host->disconnect_lock);
> --
> 2.34.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20250411/30e05bb6/attachment.htm>