[PATCH v2 0/2] spi: spi-qpic-snand: avoid memory corruption

[PATCH v2 0/2] spi: spi-qpic-snand: avoid memory corruption

[Gabor Juhos]

The 'spi-qpic-nand' driver may cause memory corruption under some
circumstances. The first patch in the series changes the driver to
avoid that, whereas the second adds some sanity checks to the common
QPIC code in order to make detecting such errors easier in the future.

Preferably, the two patches should go along in via the SPI tree.
It is not a strict requirement though, in the case the second patch
gets included separately through the MTD tree it reveals the bug
which is fixed in the first patch.

Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
---
Changes in v2:
  - collect offered tags
  - reduce kernel log spam in commit description of patch 1
  - remove inline error printing function from patch 2, and adjust the
    commit message of the patch
  - Link to v1: https://lore.kernel.org/r/20250525-qpic-snand-avoid-mem-corruption-v1-0-5fe528def7fb@gmail.com

---
Gabor Juhos (2):
      spi: spi-qpic-snand: reallocate BAM transactions
      mtd: nand: qpic_common: prevent out of bounds access of BAM arrays

 drivers/mtd/nand/qpic_common.c       | 30 ++++++++++++++++++++++++++----
 drivers/spi/spi-qpic-snand.c         | 16 ++++++++++++++++
 include/linux/mtd/nand-qpic-common.h |  8 ++++++++
 3 files changed, 50 insertions(+), 4 deletions(-)
---
base-commit: b00d6864a4c948529dc6ddd2df76bf175bf27c63
change-id: 20250523-qpic-snand-avoid-mem-corruption-301afabeb0eb

Best regards,
-- 
Gabor Juhos <j4g8y7@gmail.com>

[PATCH v2 1/2] spi: spi-qpic-snand: reallocate BAM transactions

[Gabor Juhos]

Using the mtd_nandbiterrs module for testing the driver occasionally
results in weird things like below.

1. swiotlb mapping fails with the following message:

  [   85.926216] qcom_snand 79b0000.spi: swiotlb buffer is full (sz: 4294967294 bytes), total 512 (slots), used 0 (slots)
  [   85.932937] qcom_snand 79b0000.spi: failure in mapping desc
  [   87.999314] qcom_snand 79b0000.spi: failure to write raw page
  [   87.999352] mtd_nandbiterrs: error: write_oob failed (-110)

  Rebooting the board after this causes a panic due to a NULL pointer
  dereference.

2. If the swiotlb mapping does not fail, rebooting the board may result
   in a different panic due to a bad spinlock magic:

  [  256.104459] BUG: spinlock bad magic on CPU#3, procd/2241
  [  256.104488] Unable to handle kernel paging request at virtual address ffffffff0000049b
  ...

Investigating the issue revealed that these symptoms are results of
memory corruption which is caused by out of bounds access within the
driver.

The driver uses a dynamically allocated structure for BAM transactions,
which structure must have enough space for all possible variations of
different flash operations initiated by the driver. The required space
heavily depends on the actual number of 'codewords' which is calculated
from the pagesize of the actual NAND chip.

Although the qcom_nandc_alloc() function allocates memory for the BAM
transactions during probe, but since the actual number of 'codewords'
is not yet know the allocation is done for one 'codeword' only.

Because of this, whenever the driver does a flash operation, and the
number of the required transactions exceeds the size of the allocated
arrays the driver accesses memory out of the allocated range.

To avoid this, change the code to free the initially allocated BAM
transactions memory, and allocate a new one once the actual number of
'codewords' required for a given NAND chip is known.

Fixes: 7304d1909080 ("spi: spi-qpic: add driver for QCOM SPI NAND flash Interface")
Reviewed-by: Md Sadre Alam <quic_mdalam@quicinc.com>
Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
---
Changes in v2:
  - add 'Reviewed-by' tag from Alam
  - reduce kernel log spam in the commit message
---
 drivers/spi/spi-qpic-snand.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/spi/spi-qpic-snand.c b/drivers/spi/spi-qpic-snand.c
index fd129650434f0129e24d3bdac7e7c4d5542627e6..c552cb7aa80e368e193d71e1826b2cc005571a9c 100644
--- a/drivers/spi/spi-qpic-snand.c
+++ b/drivers/spi/spi-qpic-snand.c
@@ -315,6 +315,22 @@ static int qcom_spi_ecc_init_ctx_pipelined(struct nand_device *nand)
 
 	mtd_set_ooblayout(mtd, &qcom_spi_ooblayout);
 
+	/*
+	 * Free the temporary BAM transaction allocated initially by
+	 * qcom_nandc_alloc(), and allocate a new one based on the
+	 * updated max_cwperpage value.
+	 */
+	qcom_free_bam_transaction(snandc);
+
+	snandc->max_cwperpage = cwperpage;
+
+	snandc->bam_txn = qcom_alloc_bam_transaction(snandc);
+	if (!snandc->bam_txn) {
+		dev_err(snandc->dev, "failed to allocate BAM transaction\n");
+		ret = -ENOMEM;
+		goto err_free_ecc_cfg;
+	}
+
 	ecc_cfg->cfg0 = FIELD_PREP(CW_PER_PAGE_MASK, (cwperpage - 1)) |
 			FIELD_PREP(UD_SIZE_BYTES_MASK, ecc_cfg->cw_data) |
 			FIELD_PREP(DISABLE_STATUS_AFTER_WRITE, 1) |

-- 
2.49.0

[PATCH v2 2/2] mtd: nand: qpic_common: prevent out of bounds access of BAM arrays

[Gabor Juhos]

The common QPIC code does not do any boundary checking when it handles
the command elements and scatter gater list arrays of a BAM transaction,
thus it allows to access out of bounds elements in those.

Although it is the responsibility of the given driver to allocate enough
space for all possible BAM transaction variations, however there can be
mistakes in the driver code which can lead to hidden memory corruption
issues which are hard to debug.

This kind of problem has been observed during testing the 'spi-qpic-snand'
driver. Although the driver has been fixed with a preceding patch, but it
still makes sense to reduce the chance of having such errors again later.

In order to prevent such errors, change the qcom_alloc_bam_transaction()
function to store the number of elements of the arrays in the
'bam_transaction' strucutre during allocation. Also, add sanity checks to
the qcom_prep_bam_dma_desc_{cmd,data}() functions to avoid using out of
bounds indices for the arrays.

Tested-by: Lakshmi Sowjanya D <quic_laksd@quicinc.com>     # on SDX75
Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
---
Changes in v2:
  - remove the inline qcom_err_bam_array_full() function and print the error
    messages directly from the respective functions instead
  - add 'Tested-by' tag from Lakshmi Sowjanya D, and remove the
    "Tested with the 'spi-qpic-snand' driver only." line from the
    commit message as SDX75 uses the qcom_nandc driver
  - move the note about of the preferred merging order into the cover letter
---
 drivers/mtd/nand/qpic_common.c       | 30 ++++++++++++++++++++++++++----
 include/linux/mtd/nand-qpic-common.h |  8 ++++++++
 2 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/nand/qpic_common.c b/drivers/mtd/nand/qpic_common.c
index e0ed25b5afea9b289b767cd3d9c2d7572ed52008..30f17d959300cc7448d0c2e9e2516c52655494f0 100644
--- a/drivers/mtd/nand/qpic_common.c
+++ b/drivers/mtd/nand/qpic_common.c
@@ -57,14 +57,15 @@ qcom_alloc_bam_transaction(struct qcom_nand_controller *nandc)
 	bam_txn_buf += sizeof(*bam_txn);
 
 	bam_txn->bam_ce = bam_txn_buf;
-	bam_txn_buf +=
-		sizeof(*bam_txn->bam_ce) * QPIC_PER_CW_CMD_ELEMENTS * num_cw;
+	bam_txn->bam_ce_nitems = QPIC_PER_CW_CMD_ELEMENTS * num_cw;
+	bam_txn_buf += sizeof(*bam_txn->bam_ce) * bam_txn->bam_ce_nitems;
 
 	bam_txn->cmd_sgl = bam_txn_buf;
-	bam_txn_buf +=
-		sizeof(*bam_txn->cmd_sgl) * QPIC_PER_CW_CMD_SGL * num_cw;
+	bam_txn->cmd_sgl_nitems = QPIC_PER_CW_CMD_SGL * num_cw;
+	bam_txn_buf += sizeof(*bam_txn->cmd_sgl) * bam_txn->cmd_sgl_nitems;
 
 	bam_txn->data_sgl = bam_txn_buf;
+	bam_txn->data_sgl_nitems = QPIC_PER_CW_DATA_SGL * num_cw;
 
 	init_completion(&bam_txn->txn_done);
 
@@ -237,6 +238,11 @@ int qcom_prep_bam_dma_desc_cmd(struct qcom_nand_controller *nandc, bool read,
 	struct bam_cmd_element *bam_ce_buffer;
 	struct bam_transaction *bam_txn = nandc->bam_txn;
 
+	if (bam_txn->bam_ce_pos + size > bam_txn->bam_ce_nitems) {
+		dev_err(nandc->dev, "BAM %s array is full\n", "CE");
+		return -EINVAL;
+	}
+
 	bam_ce_buffer = &bam_txn->bam_ce[bam_txn->bam_ce_pos];
 
 	/* fill the command desc */
@@ -258,6 +264,12 @@ int qcom_prep_bam_dma_desc_cmd(struct qcom_nand_controller *nandc, bool read,
 
 	/* use the separate sgl after this command */
 	if (flags & NAND_BAM_NEXT_SGL) {
+		if (bam_txn->cmd_sgl_pos >= bam_txn->cmd_sgl_nitems) {
+			dev_err(nandc->dev, "BAM %s array is full\n",
+				"CMD sgl");
+			return -EINVAL;
+		}
+
 		bam_ce_buffer = &bam_txn->bam_ce[bam_txn->bam_ce_start];
 		bam_ce_size = (bam_txn->bam_ce_pos -
 				bam_txn->bam_ce_start) *
@@ -297,10 +309,20 @@ int qcom_prep_bam_dma_desc_data(struct qcom_nand_controller *nandc, bool read,
 	struct bam_transaction *bam_txn = nandc->bam_txn;
 
 	if (read) {
+		if (bam_txn->rx_sgl_pos >= bam_txn->data_sgl_nitems) {
+			dev_err(nandc->dev, "BAM %s array is full\n", "RX sgl");
+			return -EINVAL;
+		}
+
 		sg_set_buf(&bam_txn->data_sgl[bam_txn->rx_sgl_pos],
 			   vaddr, size);
 		bam_txn->rx_sgl_pos++;
 	} else {
+		if (bam_txn->tx_sgl_pos >= bam_txn->data_sgl_nitems) {
+			dev_err(nandc->dev, "BAM %s array is full\n", "TX sgl");
+			return -EINVAL;
+		}
+
 		sg_set_buf(&bam_txn->data_sgl[bam_txn->tx_sgl_pos],
 			   vaddr, size);
 		bam_txn->tx_sgl_pos++;
diff --git a/include/linux/mtd/nand-qpic-common.h b/include/linux/mtd/nand-qpic-common.h
index cd7172e6c1bbffeee0363a14044980a72ea17723..3ca4073a496b8fd2a99112a9caefd3f110260568 100644
--- a/include/linux/mtd/nand-qpic-common.h
+++ b/include/linux/mtd/nand-qpic-common.h
@@ -240,6 +240,9 @@
  * @last_data_desc - last DMA desc in data channel (tx/rx).
  * @last_cmd_desc - last DMA desc in command channel.
  * @txn_done - completion for NAND transfer.
+ * @bam_ce_nitems - the number of elements in the @bam_ce array
+ * @cmd_sgl_nitems - the number of elements in the @cmd_sgl array
+ * @data_sgl_nitems - the number of elements in the @data_sgl array
  * @bam_ce_pos - the index in bam_ce which is available for next sgl
  * @bam_ce_start - the index in bam_ce which marks the start position ce
  *		   for current sgl. It will be used for size calculation
@@ -258,6 +261,11 @@ struct bam_transaction {
 	struct dma_async_tx_descriptor *last_data_desc;
 	struct dma_async_tx_descriptor *last_cmd_desc;
 	struct completion txn_done;
+
+	unsigned int bam_ce_nitems;
+	unsigned int cmd_sgl_nitems;
+	unsigned int data_sgl_nitems;
+
 	struct_group(bam_positions,
 		u32 bam_ce_pos;
 		u32 bam_ce_start;

-- 
2.49.0

Re: [PATCH v2 2/2] mtd: nand: qpic_common: prevent out of bounds access of BAM arrays

[Bryan O'Donoghue]

On 29/05/2025 18:25, Gabor Juhos wrote:
> The common QPIC code does not do any boundary checking when it handles
> the command elements and scatter gater list arrays of a BAM transaction,
> thus it allows to access out of bounds elements in those.
> 
> Although it is the responsibility of the given driver to allocate enough
> space for all possible BAM transaction variations, however there can be
> mistakes in the driver code which can lead to hidden memory corruption
> issues which are hard to debug.
> 
> This kind of problem has been observed during testing the 'spi-qpic-snand'
> driver. Although the driver has been fixed with a preceding patch, but it
> still makes sense to reduce the chance of having such errors again later.
> 
> In order to prevent such errors, change the qcom_alloc_bam_transaction()
> function to store the number of elements of the arrays in the
> 'bam_transaction' strucutre during allocation. Also, add sanity checks to
> the qcom_prep_bam_dma_desc_{cmd,data}() functions to avoid using out of
> bounds indices for the arrays.
> 
> Tested-by: Lakshmi Sowjanya D <quic_laksd@quicinc.com>     # on SDX75
> Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>
> ---
> Changes in v2:
>    - remove the inline qcom_err_bam_array_full() function and print the error
>      messages directly from the respective functions instead
>    - add 'Tested-by' tag from Lakshmi Sowjanya D, and remove the
>      "Tested with the 'spi-qpic-snand' driver only." line from the
>      commit message as SDX75 uses the qcom_nandc driver
>    - move the note about of the preferred merging order into the cover letter
> ---
>   drivers/mtd/nand/qpic_common.c       | 30 ++++++++++++++++++++++++++----
>   include/linux/mtd/nand-qpic-common.h |  8 ++++++++
>   2 files changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/nand/qpic_common.c b/drivers/mtd/nand/qpic_common.c
> index e0ed25b5afea9b289b767cd3d9c2d7572ed52008..30f17d959300cc7448d0c2e9e2516c52655494f0 100644
> --- a/drivers/mtd/nand/qpic_common.c
> +++ b/drivers/mtd/nand/qpic_common.c
> @@ -57,14 +57,15 @@ qcom_alloc_bam_transaction(struct qcom_nand_controller *nandc)
>   	bam_txn_buf += sizeof(*bam_txn);
> 
>   	bam_txn->bam_ce = bam_txn_buf;
> -	bam_txn_buf +=
> -		sizeof(*bam_txn->bam_ce) * QPIC_PER_CW_CMD_ELEMENTS * num_cw;
> +	bam_txn->bam_ce_nitems = QPIC_PER_CW_CMD_ELEMENTS * num_cw;
> +	bam_txn_buf += sizeof(*bam_txn->bam_ce) * bam_txn->bam_ce_nitems;
> 
>   	bam_txn->cmd_sgl = bam_txn_buf;
> -	bam_txn_buf +=
> -		sizeof(*bam_txn->cmd_sgl) * QPIC_PER_CW_CMD_SGL * num_cw;
> +	bam_txn->cmd_sgl_nitems = QPIC_PER_CW_CMD_SGL * num_cw;
> +	bam_txn_buf += sizeof(*bam_txn->cmd_sgl) * bam_txn->cmd_sgl_nitems;
> 
>   	bam_txn->data_sgl = bam_txn_buf;
> +	bam_txn->data_sgl_nitems = QPIC_PER_CW_DATA_SGL * num_cw;
> 
>   	init_completion(&bam_txn->txn_done);
> 
> @@ -237,6 +238,11 @@ int qcom_prep_bam_dma_desc_cmd(struct qcom_nand_controller *nandc, bool read,
>   	struct bam_cmd_element *bam_ce_buffer;
>   	struct bam_transaction *bam_txn = nandc->bam_txn;
> 
> +	if (bam_txn->bam_ce_pos + size > bam_txn->bam_ce_nitems) {
> +		dev_err(nandc->dev, "BAM %s array is full\n", "CE");
> +		return -EINVAL;
> +	}
> +
>   	bam_ce_buffer = &bam_txn->bam_ce[bam_txn->bam_ce_pos];
> 
>   	/* fill the command desc */
> @@ -258,6 +264,12 @@ int qcom_prep_bam_dma_desc_cmd(struct qcom_nand_controller *nandc, bool read,
> 
>   	/* use the separate sgl after this command */
>   	if (flags & NAND_BAM_NEXT_SGL) {
> +		if (bam_txn->cmd_sgl_pos >= bam_txn->cmd_sgl_nitems) {
> +			dev_err(nandc->dev, "BAM %s array is full\n",
> +				"CMD sgl");
> +			return -EINVAL;
> +		}
> +
>   		bam_ce_buffer = &bam_txn->bam_ce[bam_txn->bam_ce_start];
>   		bam_ce_size = (bam_txn->bam_ce_pos -
>   				bam_txn->bam_ce_start) *
> @@ -297,10 +309,20 @@ int qcom_prep_bam_dma_desc_data(struct qcom_nand_controller *nandc, bool read,
>   	struct bam_transaction *bam_txn = nandc->bam_txn;
> 
>   	if (read) {
> +		if (bam_txn->rx_sgl_pos >= bam_txn->data_sgl_nitems) {
> +			dev_err(nandc->dev, "BAM %s array is full\n", "RX sgl");
> +			return -EINVAL;
> +		}
> +
>   		sg_set_buf(&bam_txn->data_sgl[bam_txn->rx_sgl_pos],
>   			   vaddr, size);
>   		bam_txn->rx_sgl_pos++;
>   	} else {
> +		if (bam_txn->tx_sgl_pos >= bam_txn->data_sgl_nitems) {
> +			dev_err(nandc->dev, "BAM %s array is full\n", "TX sgl");
> +			return -EINVAL;
> +		}
> +
>   		sg_set_buf(&bam_txn->data_sgl[bam_txn->tx_sgl_pos],
>   			   vaddr, size);
>   		bam_txn->tx_sgl_pos++;
> diff --git a/include/linux/mtd/nand-qpic-common.h b/include/linux/mtd/nand-qpic-common.h
> index cd7172e6c1bbffeee0363a14044980a72ea17723..3ca4073a496b8fd2a99112a9caefd3f110260568 100644
> --- a/include/linux/mtd/nand-qpic-common.h
> +++ b/include/linux/mtd/nand-qpic-common.h
> @@ -240,6 +240,9 @@
>    * @last_data_desc - last DMA desc in data channel (tx/rx).
>    * @last_cmd_desc - last DMA desc in command channel.
>    * @txn_done - completion for NAND transfer.
> + * @bam_ce_nitems - the number of elements in the @bam_ce array
> + * @cmd_sgl_nitems - the number of elements in the @cmd_sgl array
> + * @data_sgl_nitems - the number of elements in the @data_sgl array
>    * @bam_ce_pos - the index in bam_ce which is available for next sgl
>    * @bam_ce_start - the index in bam_ce which marks the start position ce
>    *		   for current sgl. It will be used for size calculation
> @@ -258,6 +261,11 @@ struct bam_transaction {
>   	struct dma_async_tx_descriptor *last_data_desc;
>   	struct dma_async_tx_descriptor *last_cmd_desc;
>   	struct completion txn_done;
> +
> +	unsigned int bam_ce_nitems;
> +	unsigned int cmd_sgl_nitems;
> +	unsigned int data_sgl_nitems;
> +
>   	struct_group(bam_positions,
>   		u32 bam_ce_pos;
>   		u32 bam_ce_start;
> 
> --
> 2.49.0
> 
> 

This one doesn't apply to -next

deckard 
{~/Development/worktree/reviews/linux-next-25-05-30-daily-reviews}±(linux-next-25-05-30-daily-reviews); 
greetings, earthling [1.052Mb]$ ☞ b4 shazam 
20250529-qpic-snand-avoid-mem-corruption-v2-0-2f0d13afc7d2@gm
Grabbing thread from 
lore.kernel.org/all/20250529-qpic-snand-avoid-mem-corruption-v2-0-2f0d13afc7d2@gmail.com/t.mbox.gz
Checking for newer revisions
Grabbing search results from lore.kernel.org
Analyzing 3 messages in the thread
Analyzing 12 code-review messages
Checking attestation on all messages, may take a moment...
---
   ✓ [PATCH v2 1/2] spi: spi-qpic-snand: reallocate BAM transactions
   ✓ [PATCH v2 2/2] mtd: nand: qpic_common: prevent out of bounds access 
of BAM arrays
   ---
   ✓ Signed: DKIM/gmail.com
---
Total patches: 2
---
  Base: using specified base-commit b00d6864a4c948529dc6ddd2df76bf175bf27c63
Applying: spi: spi-qpic-snand: reallocate BAM transactions
Applying: mtd: nand: qpic_common: prevent out of bounds access of BAM arrays
Patch failed at 0002 mtd: nand: qpic_common: prevent out of bounds 
access of BAM arrays
error: patch failed: drivers/mtd/nand/qpic_common.c:237
error: drivers/mtd/nand/qpic_common.c: patch does not apply
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am 
--abort".
hint: Disable this message with "git config set advice.mergeConflict false"
deckard 
{~/Development/worktree/reviews/linux-next-25-05-30-daily-reviews}±(linux-next-25-05-30-daily-reviews); 
greetings, earthling [1.052Mb]$ ☞ git-log-graph
* 4ae57ce867d8f - (HEAD -> linux-next-25-05-30-daily-reviews) spi: 
spi-qpic-snand: reallocate BAM transactions (8 seconds ago)

Re: [PATCH v2 2/2] mtd: nand: qpic_common: prevent out of bounds access of BAM arrays

[Gabor Juhos]

2025. 05. 30. 10:56 keltezéssel, Bryan O'Donoghue írta:

...

> This one doesn't apply to -next

It is because the series is based on the SPI tree, and -next contains another
change for 'drivers/mtd/nand/qpic_common.c' which comes from the MTD tree.

It can be applied by specifying the 'M' switch for b4 shazam.


$ git reset --hard next-20250530
HEAD is now at 3a83b350b5be Add linux-next specific files for 20250530
$ b4 shazam -M 20250529-qpic-snand-avoid-mem-corruption-v2-0-2f0d13afc7d2@gmail.com
Grabbing thread from lore.kernel.org/all/20250529-qpic-snand-avoid-mem-corruption-v2-0-2f0d13afc7d2@gmail.com/t.mbox.gz
Checking for newer revisions
Grabbing search results from lore.kernel.org
Analyzing 4 messages in the thread
Analyzing 12 code-review messages
Checking attestation on all messages, may take a moment...
---
  ? [PATCH v2 1/2] spi: spi-qpic-snand: reallocate BAM transactions
  ? [PATCH v2 2/2] mtd: nand: qpic_common: prevent out of bounds access of BAM arrays
  ---
  ? Signed: DKIM/gmail.com
---
Total patches: 2
---
 Base: using specified base-commit b00d6864a4c948529dc6ddd2df76bf175bf27c63
Magic: Preparing a sparse worktree
---
Applying: spi: spi-qpic-snand: reallocate BAM transactions
Applying: mtd: nand: qpic_common: prevent out of bounds access of BAM arrays
---
Fetching into FETCH_HEAD
Will exec: git merge --no-ff -F /home/juhosg/devel/linux-ipq95xx/.git/b4-cover --edit FETCH_HEAD --signoff
Press Enter to continue or Ctrl-C to abort
Auto-merging drivers/mtd/nand/qpic_common.c
Auto-merging drivers/spi/spi-qpic-snand.c
Auto-merging include/linux/mtd/nand-qpic-common.h
Merge made by the 'ort' strategy.
 drivers/mtd/nand/qpic_common.c       | 30 ++++++++++++++++++++++++++----
 drivers/spi/spi-qpic-snand.c         | 16 ++++++++++++++++
 include/linux/mtd/nand-qpic-common.h |  8 ++++++++
 3 files changed, 50 insertions(+), 4 deletions(-)
$

Re: [PATCH v2 2/2] mtd: nand: qpic_common: prevent out of bounds access of BAM arrays

[Miquel Raynal]

Hello,

On 29/05/2025 at 19:25:11 +02, Gabor Juhos <j4g8y7@gmail.com> wrote:

> The common QPIC code does not do any boundary checking when it handles
> the command elements and scatter gater list arrays of a BAM transaction,
> thus it allows to access out of bounds elements in those.
>
> Although it is the responsibility of the given driver to allocate enough
> space for all possible BAM transaction variations, however there can be
> mistakes in the driver code which can lead to hidden memory corruption
> issues which are hard to debug.
>
> This kind of problem has been observed during testing the 'spi-qpic-snand'
> driver. Although the driver has been fixed with a preceding patch, but it
> still makes sense to reduce the chance of having such errors again later.
>
> In order to prevent such errors, change the qcom_alloc_bam_transaction()
> function to store the number of elements of the arrays in the
> 'bam_transaction' strucutre during allocation. Also, add sanity checks to
> the qcom_prep_bam_dma_desc_{cmd,data}() functions to avoid using out of
> bounds indices for the arrays.
>
> Tested-by: Lakshmi Sowjanya D <quic_laksd@quicinc.com>     # on SDX75
> Signed-off-by: Gabor Juhos <j4g8y7@gmail.com>

I'm fine with this patch going through spi,

Acked-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

Re: [PATCH v2 2/2] mtd: nand: qpic_common: prevent out of bounds access of BAM arrays

[Miquel Raynal]

On 30/05/2025 at 13:07:35 +02, Gabor Juhos <j4g8y7@gmail.com> wrote:

> 2025. 05. 30. 10:56 keltezéssel, Bryan O'Donoghue írta:
>
> ...
>
>> This one doesn't apply to -next
>
> It is because the series is based on the SPI tree, and -next contains another
> change for 'drivers/mtd/nand/qpic_common.c' which comes from the MTD tree.
>
> It can be applied by specifying the 'M' switch for b4 shazam.

I suggest you rebase on -rc1 (when it's out) and resend to simplify
Mark's life.

Re: [PATCH v2 2/2] mtd: nand: qpic_common: prevent out of bounds access of BAM arrays

[Gabor Juhos]

2025. 06. 02. 15:54 keltezéssel, Miquel Raynal írta:
> 
> On 30/05/2025 at 13:07:35 +02, Gabor Juhos <j4g8y7@gmail.com> wrote:
> 
>> 2025. 05. 30. 10:56 keltezéssel, Bryan O'Donoghue írta:
>>
>> ...
>>
>>> This one doesn't apply to -next
>>
>> It is because the series is based on the SPI tree, and -next contains another
>> change for 'drivers/mtd/nand/qpic_common.c' which comes from the MTD tree.
>>
>> It can be applied by specifying the 'M' switch for b4 shazam.
> 
> I suggest you rebase on -rc1 (when it's out) and resend to simplify
> Mark's life.

I will do that, thanks!

Regards,
Gabor