[PATCH] nvme: Do not allocate 8kB buffer on stack

[PATCH] nvme: Do not allocate 8kB buffer on stack

[Pali Rohár]

Calling 'nvme scan' followed by 'nvme detail' crashes U-Boot on Turris
Omnia with the following error:

  undefined instruction
  pc : [<0a000000>]          lr : [<7ff80bfc>]
  reloc pc : [<8a8c0000>]    lr : [<00840bfc>]
  sp : 7fb2b908  ip : 0000002a     fp : 02000000
  r10: 04000000  r9 : 7fb2fed0     r8 : e1000000
  r7 : 0c000000  r6 : 03000000     r5 : 06000000  r4 : 01000000
  r3 : 7fb30928  r2 : 7fb30928     r1 : 00000000  r0 : 00000000
  Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
  Code: 0f0fb4f0 0f0fb4f0 0f0fb4f0 0f0fb4f0 (f0f04b0f)
  Resetting CPU ...

This happens when nvme_print_info() tries to return to the caller. It
looks like this error is caused by trying to allocate 8 KiB of memory
on the stack by the two uses of ALLOC_CACHE_ALIGN_BUFFER().

Use malloc_cache_aligned() to allocate this memory dynamically instead.

This fixes 'nvme detail' on Turris Omnia.

Note that similar change was applied to file drivers/nvme/nvme.c in past by
commit 2f83481dff9c ("nvme: use page-aligned buffer for identify command").

Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 drivers/nvme/nvme_show.c | 35 ++++++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 9 deletions(-)

diff --git a/drivers/nvme/nvme_show.c b/drivers/nvme/nvme_show.c
index 15e459da1acd..72cbac82bcca 100644
--- a/drivers/nvme/nvme_show.c
+++ b/drivers/nvme/nvme_show.c
@@ -106,24 +106,41 @@ int nvme_print_info(struct udevice *udev)
 {
 	struct nvme_ns *ns = dev_get_priv(udev);
 	struct nvme_dev *dev = ns->dev;
-	ALLOC_CACHE_ALIGN_BUFFER(char, buf_ns, sizeof(struct nvme_id_ns));
-	struct nvme_id_ns *id = (struct nvme_id_ns *)buf_ns;
-	ALLOC_CACHE_ALIGN_BUFFER(char, buf_ctrl, sizeof(struct nvme_id_ctrl));
-	struct nvme_id_ctrl *ctrl = (struct nvme_id_ctrl *)buf_ctrl;
+	struct nvme_id_ctrl *ctrl;
+	struct nvme_id_ns *id;
+	int ret = 0;
 
-	if (nvme_identify(dev, 0, 1, (dma_addr_t)(long)ctrl))
-		return -EIO;
+	ctrl = memalign(dev->page_size, sizeof(struct nvme_id_ctrl));
+	if (!ctrl)
+		return -ENOMEM;
+
+	if (nvme_identify(dev, 0, 1, (dma_addr_t)(long)ctrl)) {
+		ret = -EIO;
+		goto free_ctrl;
+	}
 
 	print_optional_admin_cmd(le16_to_cpu(ctrl->oacs), ns->devnum);
 	print_optional_nvm_cmd(le16_to_cpu(ctrl->oncs), ns->devnum);
 	print_format_nvme_attributes(ctrl->fna, ns->devnum);
 
-	if (nvme_identify(dev, ns->ns_id, 0, (dma_addr_t)(long)id))
-		return -EIO;
+	id = memalign(dev->page_size, sizeof(struct nvme_id_ns));
+	if (!id) {
+		ret = -ENOMEM;
+		goto free_ctrl;
+	}
+
+	if (nvme_identify(dev, ns->ns_id, 0, (dma_addr_t)(long)id)) {
+		ret = -EIO;
+		goto free_id;
+	}
 
 	print_formats(id, ns);
 	print_data_protect_cap(id->dpc, ns->devnum);
 	print_metadata_cap(id->mc, ns->devnum);
 
-	return 0;
+free_id:
+	free(id);
+free_ctrl:
+	free(ctrl);
+	return ret;
 }
-- 
2.20.1

Re: [PATCH] nvme: Do not allocate 8kB buffer on stack

[Pali Rohár]

Hello! Could you please review this patch?

On Thursday 09 December 2021 11:06:39 Pali Rohár wrote:
> Calling 'nvme scan' followed by 'nvme detail' crashes U-Boot on Turris
> Omnia with the following error:
> 
>   undefined instruction
>   pc : [<0a000000>]          lr : [<7ff80bfc>]
>   reloc pc : [<8a8c0000>]    lr : [<00840bfc>]
>   sp : 7fb2b908  ip : 0000002a     fp : 02000000
>   r10: 04000000  r9 : 7fb2fed0     r8 : e1000000
>   r7 : 0c000000  r6 : 03000000     r5 : 06000000  r4 : 01000000
>   r3 : 7fb30928  r2 : 7fb30928     r1 : 00000000  r0 : 00000000
>   Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
>   Code: 0f0fb4f0 0f0fb4f0 0f0fb4f0 0f0fb4f0 (f0f04b0f)
>   Resetting CPU ...
> 
> This happens when nvme_print_info() tries to return to the caller. It
> looks like this error is caused by trying to allocate 8 KiB of memory
> on the stack by the two uses of ALLOC_CACHE_ALIGN_BUFFER().
> 
> Use malloc_cache_aligned() to allocate this memory dynamically instead.
> 
> This fixes 'nvme detail' on Turris Omnia.
> 
> Note that similar change was applied to file drivers/nvme/nvme.c in past by
> commit 2f83481dff9c ("nvme: use page-aligned buffer for identify command").
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> ---
>  drivers/nvme/nvme_show.c | 35 ++++++++++++++++++++++++++---------
>  1 file changed, 26 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/nvme/nvme_show.c b/drivers/nvme/nvme_show.c
> index 15e459da1acd..72cbac82bcca 100644
> --- a/drivers/nvme/nvme_show.c
> +++ b/drivers/nvme/nvme_show.c
> @@ -106,24 +106,41 @@ int nvme_print_info(struct udevice *udev)
>  {
>  	struct nvme_ns *ns = dev_get_priv(udev);
>  	struct nvme_dev *dev = ns->dev;
> -	ALLOC_CACHE_ALIGN_BUFFER(char, buf_ns, sizeof(struct nvme_id_ns));
> -	struct nvme_id_ns *id = (struct nvme_id_ns *)buf_ns;
> -	ALLOC_CACHE_ALIGN_BUFFER(char, buf_ctrl, sizeof(struct nvme_id_ctrl));
> -	struct nvme_id_ctrl *ctrl = (struct nvme_id_ctrl *)buf_ctrl;
> +	struct nvme_id_ctrl *ctrl;
> +	struct nvme_id_ns *id;
> +	int ret = 0;
>  
> -	if (nvme_identify(dev, 0, 1, (dma_addr_t)(long)ctrl))
> -		return -EIO;
> +	ctrl = memalign(dev->page_size, sizeof(struct nvme_id_ctrl));
> +	if (!ctrl)
> +		return -ENOMEM;
> +
> +	if (nvme_identify(dev, 0, 1, (dma_addr_t)(long)ctrl)) {
> +		ret = -EIO;
> +		goto free_ctrl;
> +	}
>  
>  	print_optional_admin_cmd(le16_to_cpu(ctrl->oacs), ns->devnum);
>  	print_optional_nvm_cmd(le16_to_cpu(ctrl->oncs), ns->devnum);
>  	print_format_nvme_attributes(ctrl->fna, ns->devnum);
>  
> -	if (nvme_identify(dev, ns->ns_id, 0, (dma_addr_t)(long)id))
> -		return -EIO;
> +	id = memalign(dev->page_size, sizeof(struct nvme_id_ns));
> +	if (!id) {
> +		ret = -ENOMEM;
> +		goto free_ctrl;
> +	}
> +
> +	if (nvme_identify(dev, ns->ns_id, 0, (dma_addr_t)(long)id)) {
> +		ret = -EIO;
> +		goto free_id;
> +	}
>  
>  	print_formats(id, ns);
>  	print_data_protect_cap(id->dpc, ns->devnum);
>  	print_metadata_cap(id->mc, ns->devnum);
>  
> -	return 0;
> +free_id:
> +	free(id);
> +free_ctrl:
> +	free(ctrl);
> +	return ret;
>  }
> -- 
> 2.20.1
> 

Re: [PATCH] nvme: Do not allocate 8kB buffer on stack

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1326 bytes --]

On Thu, Dec 09, 2021 at 11:06:39AM +0100, Pali Rohár wrote:

> Calling 'nvme scan' followed by 'nvme detail' crashes U-Boot on Turris
> Omnia with the following error:
> 
>   undefined instruction
>   pc : [<0a000000>]          lr : [<7ff80bfc>]
>   reloc pc : [<8a8c0000>]    lr : [<00840bfc>]
>   sp : 7fb2b908  ip : 0000002a     fp : 02000000
>   r10: 04000000  r9 : 7fb2fed0     r8 : e1000000
>   r7 : 0c000000  r6 : 03000000     r5 : 06000000  r4 : 01000000
>   r3 : 7fb30928  r2 : 7fb30928     r1 : 00000000  r0 : 00000000
>   Flags: nZCv  IRQs off  FIQs off  Mode SVC_32
>   Code: 0f0fb4f0 0f0fb4f0 0f0fb4f0 0f0fb4f0 (f0f04b0f)
>   Resetting CPU ...
> 
> This happens when nvme_print_info() tries to return to the caller. It
> looks like this error is caused by trying to allocate 8 KiB of memory
> on the stack by the two uses of ALLOC_CACHE_ALIGN_BUFFER().
> 
> Use malloc_cache_aligned() to allocate this memory dynamically instead.
> 
> This fixes 'nvme detail' on Turris Omnia.
> 
> Note that similar change was applied to file drivers/nvme/nvme.c in past by
> commit 2f83481dff9c ("nvme: use page-aligned buffer for identify command").
> 
> Signed-off-by: Pali Rohár <pali@kernel.org>
> Signed-off-by: Marek Behún <marek.behun@nic.cz>

Applied to u-boot/master, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]