Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
In-Reply-To: <20180723140801.GA4835@kroah.com>
References: <20180723140220.7166-1-romain.izard.pro@gmail.com> <20180723140801.GA4835@kroah.com>
From:   Romain Izard <romain.izard.pro@gmail.com>
Date:   Mon, 23 Jul 2018 18:45:22 +0200
Message-ID: <CAGkQfmPbuK7-f=9GcvQPH6mkHXwsaGpjqv40DLgFUJmbOCpbgQ@mail.gmail.com>
Subject: Re: [PATCH] option: Do not try to bind to ADB interfaces
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:     Johan Hovold <johan@kernel.org>, linux-usb@vger.kernel.org,
        LKML <linux-kernel@vger.kernel.org>,
        stable <stable@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

2018-07-23 16:08 GMT+02:00 Greg Kroah-Hartman <gregkh@linuxfoundation.org>:
> On Mon, Jul 23, 2018 at 04:02:20PM +0200, Romain Izard wrote:
>> Some modems now use the Android Debug Bridge to provide a debugging
>> interface, and some phones can also export serial ports managed by the
>> "option" driver.
>>
>> The ADB daemon running in userspace tries to use USB interfaces with
>> bDeviceClass=0xFF, bDeviceSubClass=0x42, bDeviceProtocol=1
>>
>> Prevent the option driver from binding to those interfaces, as they
>> will not be serial ports.
>>
>> This can fix issues like:
>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=781256
>>
>> Signed-off-by: Romain Izard <romain.izard.pro@gmail.com>
>> Cc: stable <stable@vger.kernel.org>
>> ---
>>  drivers/usb/serial/option.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
>> index 664e61f16b6a..f98943a57ff0 100644
>> --- a/drivers/usb/serial/option.c
>> +++ b/drivers/usb/serial/option.c
>> @@ -1987,6 +1987,12 @@ static int option_probe(struct usb_serial *serial,
>>       if (iface_desc->bInterfaceClass == USB_CLASS_MASS_STORAGE)
>>               return -ENODEV;
>>
>> +     /* Do not bind Android Debug Bridge interfaces */
>> +     if (iface_desc->bInterfaceClass == USB_CLASS_VENDOR_SPEC &&
>> +             iface_desc->bInterfaceSubClass == 0x42 &&
>> +             iface_desc->bInterfaceProtocol == 1)
>> +             return -ENODEV;
>
> Shouldn't you also check the vendor/product id as well?  Otherwise this
> has the potential to match random devices that are not really adb
> devices.

The only random devices are those that already match with the option driver,
either with the whole device or the whole reserved class. It reduces the
amount of potentially affected devices.

Among those, I do not expect any of them to use 0xff,0x42,0x01 for a
serial port. But if it occurred, it would be necessary to revert this change as
no userspace hack would allow to rebind the interface.

As this is only an intuition, please discard this patch if you have any doubt
about this.

Best regards,
-- 
Romain Izard