[vlc-devel] [PATCH] Support for alternative libv4l2 library (userspace drivers)

[Markus Rechberger]

On Fri, Aug 1, 2014 at 6:32 PM, Rémi Denis-Courmont <remi at remlab.net> wrote:
> Le 2014-08-01 19:07, Markus Rechberger a écrit :
>
>> libv4l2 accesses device nodes, our one communicates via unix domain
>> sockets to a local server the v4l2 API is fully compatible but the
>> interface is different of course. I just mean that the API calls are
>> fully compatible. The underlying architecture is completely different.
>
>
> As far as I understand, v4l2_fd_open() does not really care what type of
> file descriptor you feed it. As long as the file path can be opened in
> read/write mode, it should work just fine. Of course a socket cannot be
> opened, at least not with open/openat, but that is an internal
> implementation issue. You can still get the application to pass a regular
> file or a named pipe descriptor to v4l2_fd_open() and then switch to a
> socket in the V4L plugin initialization.
>
>
>>> +        int fd;
>>> +        char *buf=NULL;
>>> +        int buflen=0;
>>> +        int nread;
>>> +        char rbuf[10240];
>>> +        int rv;
>>> +        struct stat statbuf;
>>> +
>>> +        const char *sockpath="@/de/sundtek/mediasocket";
>>> +
>>> +        /* FreeBSD, Solaris, MacOSX */
>>> +        rv = stat("/tmp/.mediasocket", &statbuf);
>>> +        if (rv == 0)
>>> +                return 0;
>>>
>>> Looks very much like a TOCTOU race.
>>>
>>> Ditto for the procfs check.
>>>
>>> +
>>> +        fd = open("/proc/net/unix", O_RDONLY);
>>>
>>> Missing close-on-exec.
>>>
>>
>> close-on-exec is handled in the library already.
>
>
> Uh? open() does not set close-on-exec by default, and it would be a serious
> problem if the run-time did it, since it would break those programs that do
> need to preserve file descriptors across exec().
>
>
>>> +                        free(buf);
>>> +                       close(fd);
>>> +                        return 0;
>>> +                }
>>> +               close(fd);
>>> +                free(buf);
>>> +        }
>>> +        return -1;
>>> +}
>>> +
>>> +static int v4l2_net_open (char *path) {
>>> +    void *h;
>>> +    int fd;
>>> +
>>> +    h = dlopen ("libmcsimple.so", RTLD_LAZY | RTLD_LOCAL);
>>> +    if (h == NULL) {
>>> +       /* this is the default driver location */
>>> +       h = dlopen("/opt/lib/libmcsimple.so", RTLD_LAZY | RTLD_LOCAL);
>>>
>>> From a quick glance, that library is not reentrant.
>>>
>>
>> the library has locking inside to avoid race conditions.
>
>
> I have to doubt that. For one thing, a quick objdump shows strerror() calls
> and strerror() is not reentrant at least not on GNU/Linux.
>
>

it's a good argument though, I'll replace it for our next release.
Just because it didn't hit someone yet doesn't mean that it's good to keep it.


thanks :-)


>>> +       if (h == NULL)
>>> +            goto fallback;
>>> +    }
>>> +    /* no need for that one */
>>> +    v4l2_open_direct = dlsym (h, "net_open");
>>> +    v4l2_close = dlsym (h, "net_close");
>>> +    v4l2_ioctl = dlsym (h, "net_ioctl");
>>> +    v4l2_read = dlsym (h, "__net_read");
>>> +    v4l2_mmap = dlsym (h, "net_mmap");
>>> +    v4l2_munmap = dlsym (h, "net_munmap");
>>>
>>> This breaks reentrancy and the memory model.
>>> The reast of the assumes that the back-end library is constant throughout
>>> the lifetime of the process.
>>>
>>
>> I see what you mean here, I'll do some further tests.
>> But I guess it should be more dynamic and just be checked when the
>> user tries to open a node.
>
>
> In general, we do not support updating VLC underlying libraries live; we
> just keep the old library version in memory. I cannot conceive any other
> corner case.
>
> Unloading and reloading the library everytime would be a waste, not to
> mention the increased risk of memory leaks (should the library have any
> static data).
>
>
>> Aside of that I was wondering about the pthread_once inside the
>> original source, can you elaborate what the purpose
>> of that is instead of doing those things directly without pthread_once?
>
>
> Reentrancy is the whole point. While unlikely, nothing (else) truly prevents
> two concurrent calls.
>
> --
> Rémi Denis-Courmont
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel