linux-kernel - Re: [PATCH v5 9/9] nvmet: allow overriding the NVMe VS via configfs

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: <1e6c06b6-4aec-d2bf-5361-7b656b64dc22@huawei.com>
Date:   Wed, 7 Jun 2017 12:47:09 +0800
From:   Guan Junxiong <guanjunxiong@...wei.com>
To:     Johannes Thumshirn <jthumshirn@...e.de>,
        Christoph Hellwig <hch@....de>,
        Sagi Grimberg <sagi@...mberg.me>,
        Keith Busch <keith.busch@...el.com>
CC:     <maxg@...lanox.com>, Hannes Reinecke <hare@...e.de>,
        "Linux NVMe Mailinglist" <linux-nvme@...ts.infradead.org>,
        Linux Kernel Mailinglist <linux-kernel@...r.kernel.org>,
        <shenhong09@...wei.com>, "Hege (A)" <hege09@...wei.com>,
        Yang Feng <philip.yang@...wei.com>
Subject: Re: [PATCH v5 9/9] nvmet: allow overriding the NVMe VS via configfs

Hi,Johannes

On 2017/6/6 22:36, Johannes Thumshirn wrote:
> Allow overriding the announced NVMe Version of a via configfs.

> +static ssize_t nvmet_subsys_version_store(struct config_item *item,
> +					       const char *page, size_t count)
> +{
> +	struct nvmet_subsys *subsys = to_subsys(item);
> +	int major, minor, tertiary;
> +	int ret;
> +
> +
> +	ret = sscanf(page, "%d.%d.%d\n", &major, &minor, &tertiary);
> +	if (ret != 2 && ret != 3)
> +		return -EINVAL;
> +
> +	down_write(&nvmet_config_sem);
> +	subsys->ver = NVME_VS(major, minor, tertiary);

tertiary variable is not initialized, which could induce garbage info if ret = 2

> +++ b/include/linux/nvme.h
> @@ -1070,4 +1070,8 @@ struct nvme_completion {
>  #define NVME_VS(major, minor, tertiary) \
>  	(((major) << 16) | ((minor) << 8) | (tertiary))
>  
> +#define NVME_MAJOR(ver)		((ver) >> 16)
> +#define NVME_MINOR(ver)		(((ver) >> 8) & 0xff)
> +#define NVME_TERTIARY(ver)	((ver) & 0xff)the above line could be aligned with the preceding two lines

Except for those above, others looks good for me.

Reviewed-by: Guan Junxiong <guanjunxiong@...wei.com>