linux-kernel - Re: [PATCH v2 15/28] thunderbolt: Deactivate all paths before restarting them

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: <20190211085203.GV7875@lahna.fi.intel.com>
Date:   Mon, 11 Feb 2019 10:52:03 +0200
From:   Mika Westerberg <mika.westerberg@...ux.intel.com>
To:     Lukas Wunner <lukas@...ner.de>
Cc:     linux-kernel@...r.kernel.org,
        Michael Jamet <michael.jamet@...el.com>,
        Yehezkel Bernat <YehezkelShB@...il.com>,
        Andreas Noever <andreas.noever@...il.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Subject: Re: [PATCH v2 15/28] thunderbolt: Deactivate all paths before
 restarting them

On Mon, Feb 11, 2019 at 07:35:55AM +0100, Lukas Wunner wrote:
> On Wed, Feb 06, 2019 at 04:17:25PM +0300, Mika Westerberg wrote:
> > We can't be sure the paths are actually properly deactivated when a
> > tunnel is restarted after resume.
> 
> Why can't we be sure?  Please provide proper reasoning.

IIRC the reason was that when you suspend, then reconfigure parts of the
topology and resume, establishing the tunnel again went wrong. I'll
update the changelog with a better reasoning.

> > So instead of marking all paths as
> > inactive we go ahead and deactivate them explicitly.
> 
> This seems like a bad idea if the root partition is on a Thunderbolt-
> attached drive, the system is waking from hibernate and the EFI NHI
> driver has already established a tunnel to that drive.  It would seem
> more appropriate to discover tunnels already existing on resume from
> system sleep and then attempt to establish any others that might be
> missing.

That's what we do in the patch following, no? We discover the EFI
created paths and use that information to re-establish tunnels upon S3
resume and also when they are torn down.

> > @@ -183,8 +183,15 @@ int tb_tunnel_restart(struct tb_tunnel *tunnel)
> >  
> >  	tb_tunnel_info(tunnel, "activating\n");
> >  
> > +	/* Make sure all paths are properly disabled before enable them again */
> 
> This isn't proper English, s/enable/enabling/.

Thanks, I'll fix it up.