From: keith.busch@intel.com (Keith Busch)
Subject: [PATCH] nvme: initialize hostid uuid in nvmf_host_default to not leak kernel memory
Date: Tue, 9 Jan 2018 09:42:52 -0700 [thread overview]
Message-ID: <20180109164252.GD15154@localhost.localdomain> (raw)
In-Reply-To: <20180109152043.30422-1-jthumshirn@suse.de>
On Tue, Jan 09, 2018@04:20:43PM +0100, Johannes Thumshirn wrote:
> Alexander reports:
> according to KMSAN (and common sense as well) the following code in
> drivers/nvme/host/fabrics.c
> (http://elixir.free-electrons.com/linux/latest/source/drivers/nvme/host/fabrics.c#L68):
>
> 72 host = kmalloc(sizeof(*host), GFP_KERNEL);
> 73 if (!host)
> 74 return NULL;
> 75
> 76 kref_init(&host->ref);
> 77 snprintf(host->nqn, NVMF_NQN_SIZE,
> 78 "nqn.2014-08.org.nvmexpress:uuid:%pUb", &host->id);
>
> uses uninitialized heap memory to generate the unique id for the NVMF host.
> If I'm understanding correctly, it can be then passed to the
> userspace, so the contents of the uninitialized chunk may potentially
> leak.
> If the specification doesn't rely on this UID to be random or unique,
> I suggest using kzalloc() here, otherwise it might be a good idea to
> use a real RNG.
>
> this assumption is correct so initialize the host->id using uuid_gen() as
> it was done before commit 6bfe04255d5e ("nvme: add hostid token to fabric
> options").
>
> Fixes: 6bfe04255d5e ("nvme: add hostid token to fabric options")
> Reported-by: Alexander Potapenko <glider at google.com>
> Signed-off-by: Johannes Thumshirn <jthumshirn at suse.de>
Thanks for the report and the fix. It'd still be good to use the kzalloc
variant in addition to this.
Reviewed-by: Keith Busch <keith.busch at intel.com>
WARNING: multiple messages have this Message-ID (diff)
From: Keith Busch <keith.busch@intel.com>
To: Johannes Thumshirn <jthumshirn@suse.de>
Cc: Christoph Hellwig <hch@lst.de>, Sagi Grimberg <sagi@grimberg.me>,
Linux Kernel Mailinglist <linux-kernel@vger.kernel.org>,
Linux NVMe Mailinglist <linux-nvme@lists.infradead.org>,
Alexander Potapenko <glider@google.com>
Subject: Re: [PATCH] nvme: initialize hostid uuid in nvmf_host_default to not leak kernel memory
Date: Tue, 9 Jan 2018 09:42:52 -0700 [thread overview]
Message-ID: <20180109164252.GD15154@localhost.localdomain> (raw)
In-Reply-To: <20180109152043.30422-1-jthumshirn@suse.de>
On Tue, Jan 09, 2018 at 04:20:43PM +0100, Johannes Thumshirn wrote:
> Alexander reports:
> according to KMSAN (and common sense as well) the following code in
> drivers/nvme/host/fabrics.c
> (http://elixir.free-electrons.com/linux/latest/source/drivers/nvme/host/fabrics.c#L68):
>
> 72 host = kmalloc(sizeof(*host), GFP_KERNEL);
> 73 if (!host)
> 74 return NULL;
> 75
> 76 kref_init(&host->ref);
> 77 snprintf(host->nqn, NVMF_NQN_SIZE,
> 78 "nqn.2014-08.org.nvmexpress:uuid:%pUb", &host->id);
>
> uses uninitialized heap memory to generate the unique id for the NVMF host.
> If I'm understanding correctly, it can be then passed to the
> userspace, so the contents of the uninitialized chunk may potentially
> leak.
> If the specification doesn't rely on this UID to be random or unique,
> I suggest using kzalloc() here, otherwise it might be a good idea to
> use a real RNG.
>
> this assumption is correct so initialize the host->id using uuid_gen() as
> it was done before commit 6bfe04255d5e ("nvme: add hostid token to fabric
> options").
>
> Fixes: 6bfe04255d5e ("nvme: add hostid token to fabric options")
> Reported-by: Alexander Potapenko <glider@google.com>
> Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
Thanks for the report and the fix. It'd still be good to use the kzalloc
variant in addition to this.
Reviewed-by: Keith Busch <keith.busch@intel.com>
next prev parent reply other threads:[~2018-01-09 16:42 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-09 15:20 [PATCH] nvme: initialize hostid uuid in nvmf_host_default to not leak kernel memory Johannes Thumshirn
2018-01-09 15:20 ` Johannes Thumshirn
2018-01-09 16:42 ` Keith Busch [this message]
2018-01-09 16:42 ` Keith Busch
2018-01-09 16:42 ` Christoph Hellwig
2018-01-09 16:42 ` Christoph Hellwig
2018-01-10 19:39 ` Ewan D. Milne
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180109164252.GD15154@localhost.localdomain \
--to=keith.busch@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.