linux-kernel - Re: [PATCH] thunderbolt: Do not enumerate more ports from DROM than the controller has

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] [day] [month] [year] [list]

Date:   Tue, 25 Jul 2017 16:17:42 +0200
From:   Lukas Wunner <lukas@...ner.de>
To:     Mika Westerberg <mika.westerberg@...ux.intel.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Christian Kellner <ckellner@...hat.com>,
        Andreas Noever <andreas.noever@...il.com>,
        Michael Jamet <michael.jamet@...el.com>,
        Yehezkel Bernat <yehezkel.bernat@...el.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] thunderbolt: Do not enumerate more ports from DROM than
 the controller has

On Tue, Jul 25, 2017 at 03:06:47PM +0300, Mika Westerberg wrote:
> +	/*
> +	 * Some DROMs list more ports than the controller actually has
> +	 * so we skip those but allow the parser to continue.
> +	 */
> +	if (header->index > sw->config.max_port_number) {
> +		dev_warn_once(&sw->dev, "DROM has too many port entries %u (expected %u)\n",
> +			      header->index, sw->config.max_port_number);
> +		return 0;
> +	}
> +

I wouldn't have gotten into bikeshedding here but since Greg is
indicating he'd like a repost:

Could you tone down the error to KERN_INFO, it seems harmless and
the user will see this on every boot even though they may not be able
to do anything about it, short of flashing the EEPROM on the
Thunderbolt controller which may not be supported by the vendor.

Also, you're now only reporting the first index of additional
unwanted entries, which isn't really helpful.  And max_port_number
is already reported upon allocation of the switch.  I suggest removing
the two %u and just reporting the existence of additional
superfluous port entries in the Device ROM and that they're being
ignored (e.g. "ignoring unnecessary extra entries in DROM").

Apart from these nits,
Reviewed-by: Lukas Wunner <lukas@...ner.de>

Thanks for the report and quick fix!

Lukas