[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 3/3] contrib: add vhost-user-input
From: |
Marc-André Lureau |
Subject: |
Re: [Qemu-devel] [PULL 3/3] contrib: add vhost-user-input |
Date: |
Wed, 5 Jun 2019 15:49:41 +0200 |
Hi
On Thu, May 30, 2019 at 1:17 PM Peter Maydell <address@hidden> wrote:
>
> 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().
ok, I'll send a fix
>
> > + 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.
coverity should realize that passing &error_fatal will not return, no?
Can we mark this as false-positive?
>
> > + } else {
> > + fd = opt_fdnum;
> > + }
>
> thanks
> -- PMM
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PULL 3/3] contrib: add vhost-user-input,
Marc-André Lureau <=