linux-kernel - Re: staging driver s626 clashes with philips SAA7146 media/dvb based cards

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: <4A3A7A2F.3020800@mev.co.uk>
Date:	Thu, 18 Jun 2009 18:32:31 +0100
From:	Ian Abbott <abbotti@....co.uk>
To:	Herton Ronaldo Krzesinski <herton@...driva.com.br>
CC:	"fmhess@...rs.sourceforge.net" <fmhess@...rs.sourceforge.net>,
	Ian Abbott <ian.abbott@....co.uk>, Greg KH <gregkh@...e.de>,
	LKML <linux-kernel@...r.kernel.org>,
	Gianluca Palli <gpalli@...s.unibo.it>,
	David Schleef <ds@...leef.org>
Subject: Re: staging driver s626 clashes with philips SAA7146 media/dvb based
 cards

On 18/06/09 18:23, Herton Ronaldo Krzesinski wrote:
> Let me know if there is any problem remaining with this version:
> 
> Staging: comedi: s626: use subvendor:subdevice ids for SAA7146 board
> 
> The current s626 comedi driver in staging conflicts with philips SAA7146
> media/dvb based cards, because it claims the same vendor:device pci id
> for all subdevice/subvendor ids. What happens is that for people that have a
> philips SAA7146 media/dvb based card, s626 if available gets loaded by udev
> and makes system freeze (https://qa.mandriva.com/show_bug.cgi?id=51445).
> 
> The s626 driver shouldn't claim all 1131:7146 devices. Fix this by
> specifying specific known subvendor:subdevice ids in its pci id table
> list.
> 
> Also s626_attach is modified to use now pci_get_subsys instead of
> pci_get_device as reported by Ian Abbott, and now we loop over pci id
> table entries in case more ids are added in the future.
> 
> Reference: http://lkml.org/lkml/2009/6/16/552
> 
> Signed-off-by: Herton Ronaldo Krzesinski <herton@...driva.com.br>
> 
> diff --git a/drivers/staging/comedi/drivers/s626.c b/drivers/staging/comedi/drivers/s626.c
> index 30dec9d..4210590 100644
> --- a/drivers/staging/comedi/drivers/s626.c
> +++ b/drivers/staging/comedi/drivers/s626.c
> @@ -110,9 +110,13 @@ static const struct s626_board s626_boards[] = {
>  #define PCI_VENDOR_ID_S626 0x1131
>  #define PCI_DEVICE_ID_S626 0x7146
> 
> +/*
> + * For devices with vendor:device id == 0x1131:0x7146 you must specify
> + * also subvendor:subdevice ids, because otherwise it will conflict with
> + * Philips SAA7146 media/dvb based cards.
> + */
>  static DEFINE_PCI_DEVICE_TABLE(s626_pci_table) = {
> -       {PCI_VENDOR_ID_S626, PCI_DEVICE_ID_S626, PCI_ANY_ID, PCI_ANY_ID, 0, 0,
> -               0},
> +       {PCI_VENDOR_ID_S626, PCI_DEVICE_ID_S626, 0x6000, 0x0272, 0, 0, 0},
>         {0}
>  };
> 
> @@ -498,25 +502,26 @@ static int s626_attach(struct comedi_device *dev, struct comedi_devconfig *it)
>         resource_size_t resourceStart;
>         dma_addr_t appdma;
>         struct comedi_subdevice *s;
> -       struct pci_dev *pdev;
> +       const struct pci_device_id *ids;
> +       struct pci_dev *pdev = NULL;
> 
>         if (alloc_private(dev, sizeof(struct s626_private)) < 0)
>                 return -ENOMEM;
> 
> -       for (pdev = pci_get_device(PCI_VENDOR_ID_S626, PCI_DEVICE_ID_S626,
> -                       NULL); pdev != NULL;
> -               pdev = pci_get_device(PCI_VENDOR_ID_S626,
> -                       PCI_DEVICE_ID_S626, pdev)) {
> -               if (it->options[0] || it->options[1]) {
> -                       if (pdev->bus->number == it->options[0] &&
> -                               PCI_SLOT(pdev->devfn) == it->options[1]) {
> +       for (i = 0; i < ARRAY_SIZE(s626_pci_table) && !pdev; i++) {
> +               ids = &s626_pci_table[i];
> +               do {
> +                       pdev = pci_get_subsys(ids->vendor, ids->device, ids->subvendor,
> +                                             ids->subdevice, pdev);
> +
> +                       if ((it->options[0] || it->options[1]) && pdev) {
>                                 /* matches requested bus/slot */
> +                               if (pdev->bus->number == it->options[0] &&
> +                                   PCI_SLOT(pdev->devfn) == it->options[1])
> +                                       break;
> +                       } else
>                                 break;
> -                       }
> -               } else {
> -                       /* no bus/slot specified */
> -                       break;
> -               }
> +               } while (1);
>         }
>         devpriv->pdev = pdev;

The outer for loop iterates once too often - it doesn't need to iterate 
over the sentinel at the end of the id table as that shouldn't match any 
PCI device.

-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti@....co.uk>        )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587         )=-
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/