netdev - Re: [PATCH net-next] net: sfp: Stop SFP polling and interrupt handling during shutdown

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: <20190607104251.opqdsgjbyweb2rfg@shell.armlinux.org.uk>
Date:   Fri, 7 Jun 2019 11:42:51 +0100
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Robert Hancock <hancock@...systems.ca>
Cc:     netdev@...r.kernel.org, andrew@...n.ch, f.fainelli@...il.com,
        hkallweit1@...il.com
Subject: Re: [PATCH net-next] net: sfp: Stop SFP polling and interrupt
 handling during shutdown

On Thu, Jun 06, 2019 at 02:57:22PM -0600, Robert Hancock wrote:
> The idea there was to deal with the case where GPIO interrupts were
> previously raised before shutdown and not yet handled by the threaded
> interrupt handler by the time shutdown is called. After shutdown on the
> SFP completes, the bus the GPIO stuff is on could potentially be shut
> down at any moment, so we really don't want to be digging into the GPIO
> states after that. Locking the mutex there ensures that we don't read a
> stale value for the shutdown flag in the interrupt handler, since AFAIK
> there's no other synchronization around that value.

There are two cases:

1) The interrupt is raised just as sfp_shutdown() is called but before
   the mutex is taken.  We will get the full processing in this case.

2) The interrupt is raised during the mutex-locked bit of sfp_shutdown()
   or after the mutex in sfp_shutdown() is released.  We will get the
   abbreviated processing.

This means that the mutex doesn't provide any protection against full
interrupt processing if it occurs just prior to or during the initial
execution of sfp_shutdown().

All that we need to ensure is that the state of sfp->shutdown is
visible by the time sfp_shutdown() returns, and that none of the
interrupt and worker functions are executing.  We have the worker
functions covered by the synchronous cancelling of them, but not the
interrupts, and as Florian points out, it's probably better to disable
the interrupts, and again, that can be done synchronously to ensure
that the handlers are not running.

If the workers and interrupt handlers are synchronously disabled, we
can be sure by the end of sfp_shutdown() that none of those paths are
executing, so the next time something attempts to trigger them, they
will see sfp->shutdown is set.

I'm not convinced that we even need sfp->shutdown if we have cancelled
the workers and disabled the interrupts.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up