linux-kernel - Re: [PATCH 10/24] thunderbolt: Read vendor and device name from DROM

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]

Date:   Sun, 21 May 2017 11:33:16 +0200
From:   Lukas Wunner <lukas@...ner.de>
To:     Mika Westerberg <mika.westerberg@...ux.intel.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Andreas Noever <andreas.noever@...il.com>,
        Michael Jamet <michael.jamet@...el.com>,
        Yehezkel Bernat <yehezkel.bernat@...el.com>,
        Amir Levy <amir.jer.levy@...el.com>,
        Andy Lutomirski <luto@...nel.org>, Mario.Limonciello@...l.com,
        Jared.Dominguez@...l.com,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 10/24] thunderbolt: Read vendor and device name from DROM

On Sun, May 21, 2017 at 10:48:19AM +0300, Mika Westerberg wrote:
> On Sun, May 21, 2017 at 07:31:14AM +0200, Lukas Wunner wrote:
> > On Fri, May 19, 2017 at 01:28:36PM +0300, Mika Westerberg wrote:
> > > On Fri, May 19, 2017 at 12:07:10PM +0200, Lukas Wunner wrote:
> > > > If there can be many attributes, should they be stored in a list
> > > > rather than adding a char* pointer for each one to struct tb_switch?
> > > > The latter doesn't scale.
> > > 
> > > I don't think we need other attributes (well, at least right now). The
> > > device/vendor name is useful because that's what we expose to the
> > > userspace for device identification along with the device/vendor ID.
> > 
> > Okay.  It might be worth to log additional attributes with info level.
> 
> I don't think we want to log anything with info level to be honest. The
> driver currently already is pretty noisy so adding even more information
> there just makes it worse ;-)
> 
> I would rather convert debugging information to use tracepoints and get
> rid of the tb_*_info() things completely.

The noisiness has value in that it helps with reverse-engineering:
Just google for dmesg output and check what other machines are
reporting for unknown registers. :-)

If there was public documentation available or Intel would be okay
with answering specific questions (as you've done with the 0x30
attribute id), then the value obviously diminishes.

Can't say anything about converting to tracepoints, that's Andreas'
call.


> The whole DROM content is already available through nvm_active/nvmem
> file under each device (well starting with Alpine Ridge) so the
> userspace can investigate it as much as it likes without spamming the
> kernel dmesg :)

Okay, fair enough, that should indeed suffice.

Thanks,

Lukas