Re: [Qemu-devel] [PULL 3/3] contrib: add vhost-user-input

[Peter Maydell]

On Wed, 22 May 2019 at 09:29, Gerd Hoffmann <address@hidden> wrote:
>
> From: Marc-André Lureau <address@hidden>
>
> Add a vhost-user input backend example, based on virtio-input-host
> device. It takes an evdev path as argument, and can be associated with
> a vhost-user-input device via a UNIX socket:
>
> $ vhost-user-input -p /dev/input/eventX -s /tmp/vui.sock
>
> $ qemu ... -chardev socket,id=vuic,path=/tmp/vui.sock
>   -device vhost-user-input-pci,chardev=vuic
>
> This example is intentionally not included in $TOOLS, and not
> installed by default.
>
> Signed-off-by: Marc-André Lureau <address@hidden>
> Message-id: address@hidden
> Signed-off-by: Gerd Hoffmann <address@hidden>

Hi; Coverity spotted a problem with this patch:

> +int
> +main(int argc, char *argv[])
> +{
> +    GMainLoop *loop = NULL;
> +    VuInput vi = { 0, };
> +    int rc, ver, fd;
> +    virtio_input_config id;
> +    struct input_id ids;
> +    GError *error = NULL;
> +    GOptionContext *context;
> +
> +    context = g_option_context_new(NULL);
> +    g_option_context_add_main_entries(context, entries, NULL);
> +    if (!g_option_context_parse(context, &argc, &argv, &error)) {
> +        g_printerr("Option parsing failed: %s\n", error->message);
> +        exit(EXIT_FAILURE);
> +    }
> +    if (opt_print_caps) {
> +        g_print("{\n");
> +        g_print("  \"type\": \"input\",\n");
> +        g_print("  \"features\": [\n");
> +        g_print("    \"evdev-path\",\n");
> +        g_print("    \"no-grab\"\n");
> +        g_print("  ]\n");
> +        g_print("}\n");
> +        exit(EXIT_SUCCESS);
> +    }
> +    if (!opt_evdev) {
> +        g_printerr("Please specify an evdev path\n");
> +        exit(EXIT_FAILURE);
> +    }
> +    if ((!!opt_socket_path + (opt_fdnum != -1)) != 1) {
> +        g_printerr("Please specify either --fd or --socket-path\n");
> +        exit(EXIT_FAILURE);
> +    }
> +
> +    vi.evdevfd = open(opt_evdev, O_RDWR);
> +    if (vi.evdevfd < 0) {
> +        g_printerr("Failed to open evdev: %s\n", g_strerror(errno));
> +        exit(EXIT_FAILURE);
> +    }
> +
> +    rc = ioctl(vi.evdevfd, EVIOCGVERSION, &ver);
> +    if (rc < 0) {
> +        g_printerr("%s: is not an evdev device\n", argv[1]);
> +        exit(EXIT_FAILURE);
> +    }
> +
> +    if (!opt_nograb) {
> +        rc = ioctl(vi.evdevfd, EVIOCGRAB, 1);
> +        if (rc < 0) {
> +            g_printerr("Failed to grab device\n");
> +            exit(EXIT_FAILURE);
> +        }
> +    }
> +
> +    vi.config = g_array_new(false, false, sizeof(virtio_input_config));
> +    memset(&id, 0, sizeof(id));
> +    ioctl(vi.evdevfd, EVIOCGNAME(sizeof(id.u.string) - 1), id.u.string);

CID 1401704 -- we don't check the return value from ioctl().

> +    id.select = VIRTIO_INPUT_CFG_ID_NAME;
> +    id.size = strlen(id.u.string);
> +    g_array_append_val(vi.config, id);
> +
> +    if (ioctl(vi.evdevfd, EVIOCGID, &ids) == 0) {
> +        memset(&id, 0, sizeof(id));
> +        id.select = VIRTIO_INPUT_CFG_ID_DEVIDS;
> +        id.size = sizeof(struct virtio_input_devids);
> +        id.u.ids.bustype = cpu_to_le16(ids.bustype);
> +        id.u.ids.vendor  = cpu_to_le16(ids.vendor);
> +        id.u.ids.product = cpu_to_le16(ids.product);
> +        id.u.ids.version = cpu_to_le16(ids.version);
> +        g_array_append_val(vi.config, id);
> +    }
> +
> +    vi_bits_config(&vi, EV_KEY, KEY_CNT);
> +    vi_bits_config(&vi, EV_REL, REL_CNT);
> +    vi_bits_config(&vi, EV_ABS, ABS_CNT);
> +    vi_bits_config(&vi, EV_MSC, MSC_CNT);
> +    vi_bits_config(&vi, EV_SW,  SW_CNT);
> +    g_debug("config length: %u", vi.config->len);
> +
> +    if (opt_socket_path) {
> +        int lsock = unix_listen(opt_socket_path, &error_fatal);
> +        fd = accept(lsock, NULL, NULL);
> +        close(lsock);

This is CID 1401705 -- failure to check return value from
unix_listen() -- which I just realised I probably replied
to the wrong version of the patch to point out, so I mention
it again here.

> +    } else {
> +        fd = opt_fdnum;
> +    }

thanks
-- PMM