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] [day] [month] [year] [list]
Message-ID: <a6b50b85-63c1-e464-f758-eee3168048c1@huawei.com>
Date:   Sat, 24 Sep 2022 10:21:07 +0800
From:   Ruan Jinjie <ruanjinjie@...wei.com>
To:     Greg KH <gregkh@...uxfoundation.org>
CC:     <jirislaby@...nel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH -next] tty: moxa: add missing pci_disable_device()



On 2022/9/23 19:25, Greg KH wrote:
> On Fri, Sep 23, 2022 at 06:12:03PM +0800, Ruan Jinjie wrote:
>>
>>
>> On 2022/9/23 17:56, Greg KH wrote:
>>> On Fri, Sep 23, 2022 at 05:25:30PM +0800, ruanjinjie wrote:
>>>> Driver should call pci_disable_device() if it returns from
>>>> moxa_pci_probe() with error.
>>>
>>> Why?
>>>
>>> That is not a normal thing to do, as you can disable other PCI devices
>>> attached to it, right?

I think pci_enable_device and pci_disable_device should be paired. If
there are other PCI devices, they will not be disabled because they are
disabled only when reference counting enable_cnt is reduced to 0.

When these devices are initialized, pci_enable_device is called and
reference counting enable_cnt is incremented. Therefore, when the
current device invokes pci_disable_device, other devices are working
normally and have not invoke pci_disable_device. As a result, the value
of enable_cnt is not 0, and the device will not be really disabled.
There's no problem with that.

On the other hand, if there are no other PCI devices attached to it, and
this driver do not call pci_disable_device() if it returns from
moxa_pci_probe() with error or during removal, the device will never be
disabled.

>>>
>>>> Meanwhile, the driver calls pci_enable_device() in
>>>> moxa_pci_probe(), but never calls pci_disable_device() during removal.
>>>
>>> And is that really a problem?
>>>
>>>>
>>>> Signed-off-by: ruanjinjie <ruanjinjie@...wei.com>
>>>> ---
>>>>  drivers/tty/moxa.c | 4 +++-
>>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> Do you have this hardware to test with?
>>>
>>> How did you find this issue?
>>>
>>
>> We use static analysis via coccinelle to find the above issue. The
>> command we use is below:
>>
>> spatch -I include -timeout 60 -very_quiet -sp_file
>> pci_disable_device_missing.cocci drivers/tty/moxa.c
> 
> Please test with real hardware and look up what pci_disable_device()
> does.
> 
> greg k-h

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ