> On 19 Feb 2020, at 20:51, Martin Pieuchot <m...@openbsd.org> wrote:
>
> On 19/02/20(Wed) 14:13, Vitaliy Makkoveev wrote:
>> On Wed, Jan 17, 2018 at 11:40:24AM +0100, Martin Pieuchot wrote:
>>> Hello Sebastien,
>>>
>>> On 17/01/18(Wed) 10:19, Sebastien Marie wrote:
>>>> [...]
>>>> kernel modification is desirable in some cases, at least for disabling
>>>> ulpt(4) when using cups with USB printer.
>>>
>>> Sorry to hijack your thread, but if somebody wants to fix this ulpt(4)
>>> problem permanently here's the plan:
>>>
>>> - Add the USBD_EXCLUSIVE_USE to usbd_open_pipe() in ulptopen().
>>> Actually this flag should be the default everywhere. This should
>>> prevent open(2) on /dev/ulpt? to work if a userland driver is using
>>> your printer.
>>>
>>> - Do some plumbing between libusb/ugen(4)/usb(4) to make it possible
>>> to submit bulk transfer via /dev/usb?. The logic in ugenopen()
>>> should also have the USBD_EXCLUSIVE_USE flag such that it will fail
>>> if the corresponding /dev/ultp? has already been opened.
>>>
>>> That should be enough to have CUPS work with GENERIC kernels without
>>> having to disable anything. I'm here to help/review diffs but since
>>> I don't have a printer myself, I can't do the work.
>>>
>>
>> If driver exists for USB device, generic driver for this device will
>> never be attached. So, if ulpt(4) is enabled and attached, corresponding
>> ugen(4) is missing and CUPS/libusb can't work with this printer. I
>> suppose, coexisting with ugen(4) should be allowed.
>
> Attaching two drivers to the same piece of hardware might confuse the
> stack and the hardware below it. I don't see anything in your diff
> taking care of the ownership of, at least, interfaces.
>
> I'd also be afraid that if userland tries to change the configuration of
> the device via the ugen(4) interface that might crash the kernel. Current
> drivers (or the stack) is written for that.
Original ugen(4) coexist with other drivers in composite USB device case. It
attached to the first
unsupported interface and it can’t change configuration or interface setting on
claimed interface.
>
>> Index: sys/dev/usb/ugen.c
>> ===================================================================
>> RCS file: /cvs/src/sys/dev/usb/ugen.c,v
>> retrieving revision 1.101
>> diff -u -p -r1.101 ugen.c
>> --- sys/dev/usb/ugen.c 4 Jan 2020 11:37:33 -0000 1.101
>> +++ sys/dev/usb/ugen.c 19 Feb 2020 11:07:01 -0000
>> @@ -222,7 +222,8 @@ ugen_set_config(struct ugen_softc *sc, i
>> memset(sc->sc_endpoints, 0, sizeof sc->sc_endpoints);
>> for (ifaceno = 0; ifaceno < cdesc->bNumInterface; ifaceno++) {
>> DPRINTFN(1,("ugen_set_config: ifaceno %d\n", ifaceno));
>> - if (usbd_iface_claimed(sc->sc_udev, ifaceno)) {
>> + if (!sc->sc_secondary &&
>> + usbd_iface_claimed(sc->sc_udev, ifaceno)) {
>> DPRINTF(("%s: iface %d not available\n", __func__,
>> ifaceno));
>> continue;
>
> This allows two pieces of code trying to use this interface at the same
> time, what can go wrong?
With my patch, some drivers can destroy endpoints stuff allocated for shared
interface and crash
kernel. Sorry :( But I suppose, coexisting is possible if ugen(4) will become
something like simple
wrapper around usbd_open_pipe() calls.
