linux-kernel - Re: [PATCH RFC v2 2/4] PCI: pciehp: Do not turn off slot if presence comes up after link

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: <dfeadc6798984cce965e2202e90fe0b0@ausx13mps321.AMER.DELL.COM>
Date:   Fri, 22 Feb 2019 19:56:28 +0000
From:   <Alex_Gagniuc@...lteam.com>
To:     <lukas@...ner.de>, <mr.nuke.me@...il.com>
Cc:     <bhelgaas@...gle.com>, <Austin.Bolen@...l.com>,
        <keith.busch@...el.com>, <Shyam.Iyer@...l.com>, <okaya@...nel.org>,
        <linux-pci@...r.kernel.org>, <gustavo@...eddedor.com>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RFC v2 2/4] PCI: pciehp: Do not turn off slot if presence
 comes up after link

On 2/21/19 1:36 AM, Lukas Wunner wrote:
> On Tue, Feb 19, 2019 at 07:20:28PM -0600, Alexandru Gagniuc wrote:
>>   	mutex_lock(&ctrl->state_lock);
>> +	present = pciehp_card_present(ctrl);
>> +	link_active = pciehp_check_link_active(ctrl);
>>   	switch (ctrl->state) {
> 
> These two assignments appear to be superfluous as you're also performing
> them in pciehp_check_link_active().

Not sure. Between the first check, and this check, you can have several 
seconds elapse depending on whether the driver's .probe()/remove() is 
invoked. Whatever you got at the beginning would be stale. If you had a 
picture dictionary and looked up 'bad idea', it would have a picture of 
the above code with the second check removed.

I've got all the other review comments addressed in my local branch. I'm 
waiting on Lord Helgass' decision on which solution is better.

Alex