linux-kernel - Re: [PATCH] 8250_pci.c: Update NI specific devices class to multi serial

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190122115508.GB22960@kuha.fi.intel.com>
Date:   Tue, 22 Jan 2019 13:55:08 +0200
From:   Heikki Krogerus <heikki.krogerus@...ux.intel.com>
To:     Guan Yung Tseng <guan.yung.tseng@...com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     gregkh@...uxfoundation.org, linux-serial@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] 8250_pci.c: Update NI specific devices class to multi
 serial

+Andy

On Mon, Jan 14, 2019 at 10:10:05PM +0800, Guan Yung Tseng wrote:
> Modified NI devices class to PCI_CLASS_COMMUNICATION_MULTISERIAL.
> The reason of doing this is because all NI multi port serial cards
> use PCI_CLASS_COMMUNICATION_OTHER class and thus fail the
> serial_pci_is_class_communication test added in the commit 7d8905d06405
> ("serial: 8250_pci: Enable device after we check black list").

OK, so commit 7d8905d06405 ("serial: 8250_pci: Enable device after we
check black list") has created a regression. If the device does not
use PCI_CLASS_COMMUNICATION*SERIAL class, probe will fail, and I
don't think that is how the driver should function.

If the device id is listed in serial_pci_tbl, we need to probe the
device, regardless of the class id.

> Signed-off-by: Guan Yung Tseng <guan.yung.tseng@...com>
> ---
>  drivers/tty/serial/8250/8250_pci.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
> index 4986b4a..0949db1 100644
> --- a/drivers/tty/serial/8250/8250_pci.c
> +++ b/drivers/tty/serial/8250/8250_pci.c
> @@ -663,6 +663,13 @@ static int pci_xircom_init(struct pci_dev *dev)
>  	return 0;
>  }
>  
> +static int pci_ni_probe(struct pci_dev *dev)
> +{
> +	dev->class = PCI_CLASS_COMMUNICATION_MULTISERIAL << 8 |
> +			(dev->class & 0xff);
> +	return 0;
> +}

This is only working around the regression that 7d8905d064058 created,
and only with your UART. There may be others.

We need to fix the regression, not work around it. How about something
like the attached diff?


thanks,

-- 
heikki

View attachment "improved_7d8905d064058.diff" of type "text/plain" (2361 bytes)