All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Laight <david.laight.linux@gmail.com>
To: Bryam Vargas via B4 Relay <devnull+hexlabsecurity.proton.me@kernel.org>
Cc: hexlabsecurity@proton.me, Dave Jiang <dave.jiang@intel.com>,
	Dan Williams <djbw@kernel.org>, Ira Weiny <ira.weiny@intel.com>,
	Vishal Verma <vishal.l.verma@intel.com>,
	linux-kernel@vger.kernel.org, nvdimm@lists.linux.dev
Subject: Re: [PATCH] libnvdimm/labels: Prevent integer overflow in __nd_label_validate()
Date: Sun, 21 Jun 2026 11:23:57 +0100	[thread overview]
Message-ID: <20260621112357.56a290bc@pumpkin> (raw)
In-Reply-To: <20260620-b4-disp-7f43b155-v1-1-0cfd8017f7a0@proton.me>

On Sat, 20 Jun 2026 15:54:39 -0500
Bryam Vargas via B4 Relay <devnull+hexlabsecurity.proton.me@kernel.org> wrote:

> From: Bryam Vargas <hexlabsecurity@proton.me>
> 
> The on-media namespace index field nslot is a u32 read from the DIMM
> label storage area.  __nd_label_validate() bounds it against the config
> area size, but sizeof_namespace_label() returns unsigned, so the product
> nslot * label_size is evaluated in 32-bit and wraps modulo 2^32 before
> the comparison.  A crafted nslot passes the bound and is then used as the
> loop trip count in nd_label_data_init(), whose memset() walks off the end
> of the config_size buffer: an out-of-bounds write.
> 
> The field is not trusted -- it comes from the medium, or from userspace
> via ND_CMD_SET_CONFIG_DATA.  Evaluate the product in 64-bit so the bound
> check is exact; conforming labels are unaffected.

Is this enough and/or a sane way to stop the overflow.
AFAICT label_size is either 128 or 258.
But I can't see where nsarea.config_size is set.
If it comes from a user ioctl there should be some associated sanity limits.
The same could be done for nslot - any value above 64k is pretty much
guaranteed to be garbage - I'd bet valid values are actually very small
integers.

	David

> 
> Fixes: 564e871aa66f ("libnvdimm, label: add v1.2 nvdimm label definitions")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
> ---
> The check was safe when introduced: 4a826c83db4e ("libnvdimm: namespace
> indices: read and validate") multiplied by sizeof(struct
> nd_namespace_label), a size_t, so the product was 64-bit.  564e871aa66f
> replaced that with sizeof_namespace_label(), which returns unsigned, when
> the label size became a runtime value -- narrowing the product to 32 bits.
> 
> The sibling multiply in sizeof_namespace_index() uses an nslot derived
> from config_size (nvdimm_num_label_slots()), not the on-media field, so it
> cannot overflow and is left unchanged.
> 
> Reproduced with an out-of-tree module that mirrors nd_label_data_init() --
> kvzalloc(config_size), the __nd_label_validate() bound check, and the
> memset loop -- since the defect is the wrapped arithmetic into the memset,
> not the DIMM-probe plumbing:
> 
> Build A (without this patch), nslot = 0x02000000, 128-byte labels:
>     the u32 product wraps to 0, the index is accepted, and the loop's
>     memset() writes past the kvzalloc'd buffer ->
>       right of the config_size region -> panic.
>   Build B (with this patch): the 64-bit product exceeds config_size, the
>     index is rejected, the loop never runs -> clean.
>   Control (legitimate small nslot): writes stay in bounds -> clean.
> 
> BUG: KASAN: slab-out-of-bounds, Write of size 128, 0 bytes to the
> ---
>  drivers/nvdimm/label.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/nvdimm/label.c b/drivers/nvdimm/label.c
> index 4218e3ac4a2a..ec12ce72cfe2 100644
> --- a/drivers/nvdimm/label.c
> +++ b/drivers/nvdimm/label.c
> @@ -202,7 +202,7 @@ static int __nd_label_validate(struct nvdimm_drvdata *ndd)
>  		}
>  
>  		nslot = __le32_to_cpu(nsindex[i]->nslot);
> -		if (nslot * sizeof_namespace_label(ndd)
> +		if ((u64)nslot * sizeof_namespace_label(ndd)
>  				+ 2 * sizeof_namespace_index(ndd)
>  				> ndd->nsarea.config_size) {  
>  			dev_dbg(dev, "nsindex%d nslot: %u invalid, config_size: %#x\n",
> 
> ---
> base-commit: 8e65320d91cdc3b241d4b94855c88459b91abf66
> change-id: 20260620-b4-disp-7f43b155-92b84c904c08
> 
> Best regards,


      reply	other threads:[~2026-06-21 10:24 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-20 20:54 [PATCH] libnvdimm/labels: Prevent integer overflow in __nd_label_validate() Bryam Vargas
2026-06-20 20:54 ` Bryam Vargas via B4 Relay
2026-06-21 10:23 ` David Laight [this message]

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=20260621112357.56a290bc@pumpkin \
    --to=david.laight.linux@gmail.com \
    --cc=dave.jiang@intel.com \
    --cc=devnull+hexlabsecurity.proton.me@kernel.org \
    --cc=djbw@kernel.org \
    --cc=hexlabsecurity@proton.me \
    --cc=ira.weiny@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nvdimm@lists.linux.dev \
    --cc=vishal.l.verma@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.