netdev - Re: [PATCH net-next 08/14] ice: protect init and calibrating fields with spinlock

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: <Y4yg/91T+r4+XdSd@unreal>
Date:   Sun, 4 Dec 2022 15:30:39 +0200
From:   Leon Romanovsky <leon@...nel.org>
To:     Jacob Keller <jacob.e.keller@...el.com>
Cc:     Tony Nguyen <anthony.l.nguyen@...el.com>, davem@...emloft.net,
        kuba@...nel.org, pabeni@...hat.com, edumazet@...gle.com,
        netdev@...r.kernel.org, richardcochran@...il.com,
        Gurucharan G <gurucharanx.g@...el.com>
Subject: Re: [PATCH net-next 08/14] ice: protect init and calibrating fields
 with spinlock

On Fri, Dec 02, 2022 at 03:07:16PM -0800, Jacob Keller wrote:
> 
> 
> On 12/2/2022 10:36 AM, Jacob Keller wrote:
> > I am not sure what the best way to fix c) is. Perhaps an additional flag
> > of some sort which indicates that the timestamp thread function is
> > processing so that tear down can wait until after the interrupt function
> > completes. Once init is cleared, the function will stop re-executing,
> > but we need a way to wait until it has stopped. That method in tear down
> > can't hold the lock or else we'd potentially deadlock... Maybe an
> > additional "processing" flag which is set under lock only if the init
> > flag is set? and then cleared when the function exits. Then the tear
> > down can check and wait for the processing to be complete? Hmm.
> > 
> 
> For what its worth, I think this is an issue regardless of whether this
> series is applied, since the change from kthread to threaded IRQ was done a
> while ago.

Sorry, I can't say anything clear without deep dive into ice code. But this specific
patch is not correct.

Thanks