linux-kernel - Re: [PATCH 22/24] thunderbolt: Add support for host and device NVM firmware upgrade

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:   Thu, 25 May 2017 15:28:34 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     Mika Westerberg <mika.westerberg@...ux.intel.com>
Cc:     Andreas Noever <andreas.noever@...il.com>,
        Michael Jamet <michael.jamet@...el.com>,
        Yehezkel Bernat <yehezkel.bernat@...el.com>,
        Lukas Wunner <lukas@...ner.de>,
        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 22/24] thunderbolt: Add support for host and device NVM
 firmware upgrade

On Thu, May 18, 2017 at 05:39:12PM +0300, Mika Westerberg wrote:
> +static int tb_switch_nvm_add(struct tb_switch *sw)
> +{
> +	struct nvmem_device *nvm_dev;
> +	struct tb_switch_nvm *nvm;
> +	u32 val;
> +	int ret;
> +
> +	if (!sw->dma_port)
> +		return 0;
> +
> +	nvm = kzalloc(sizeof(*nvm), GFP_KERNEL);
> +	if (!nvm)
> +		return -ENOMEM;
> +
> +	nvm->id = ida_simple_get(&nvm_ida, 0, 0, GFP_KERNEL);
> +
> +	/*
> +	 * If the switch is in safe-mode the only accessible portion of
> +	 * the NVM is the non-active one where userspace is expected to
> +	 * write new functional NVM.
> +	 */
> +	if (!sw->safe_mode) {
> +		u32 nvm_size, hdr_size;
> +
> +		ret = dma_port_flash_read(sw->dma_port, NVM_FLASH_SIZE, &val,
> +					  sizeof(val));
> +		if (ret)
> +			goto err_ida;
> +
> +		hdr_size = sw->generation < 3 ? SZ_8K : SZ_16K;
> +		nvm_size = (SZ_1M << (val & 7)) / 8;
> +		nvm_size = (nvm_size - hdr_size) / 2;
> +
> +		ret = dma_port_flash_read(sw->dma_port, NVM_VERSION, &val,
> +					  sizeof(val));
> +		if (ret)
> +			goto err_ida;
> +
> +		nvm->major = val >> 16 & 0xff;
> +		nvm->minor = val >> 8 & 0xff;
> +
> +		nvm_dev = register_nvmem(sw, nvm->id, nvm_size, true);
> +		if (IS_ERR(nvm_dev)) {
> +			ret = PTR_ERR(nvm_dev);
> +			goto err_ida;
> +		}
> +		nvm->active = nvm_dev;
> +	}
> +
> +	nvm_dev = register_nvmem(sw, nvm->id, NVM_MAX_SIZE, false);
> +	if (IS_ERR(nvm_dev)) {
> +		ret = PTR_ERR(nvm_dev);
> +		goto err_nvm_active;
> +	}
> +	nvm->non_active = nvm_dev;
> +
> +	sw->nvm = nvm;
> +
> +	ret = sysfs_create_group(&sw->dev.kobj, &nvm_group);

Why are you adding this to the sw device?  And doing so _after_ it was
announced to userspace?  Why can't you make it part of the device's
default groups so that the driver core can handle it properly?

Hint, if you ever have to call sysfs_* from within a driver, something
might really be wrong :)

thanks,

greg k-h