[PATCH 0/5] memstick: fixes for mspro_block and ms_block

[PATCH 0/5] memstick: fixes for mspro_block and ms_block

[Ho Jie Feng]

This series fixes bugs that I found in the memstick core while writing
the bcm577x5_ms host driver for the Broadcom BCM57765/57785 card reader.

Some of these bugs were really rare, so I used Claude to automatically
fuzz access patterns/events to reproduce some of these issues.

Should I put this with Cc: stable?

Ho Jie Feng (5):
  memstick: mspro_block: don't hand a finished request back to the host
  memstick: mspro_block: wait for running dispatches in
    mspro_block_stop()
  memstick: ms_block: don't erase the old block of an unmapped LBA
  memstick: ms_block: keep the power off during a full msb_reset()
  memstick: ms_block: add the missing newline to the parallel switch
    error

 drivers/memstick/core/ms_block.c    | 11 ++++++++---
 drivers/memstick/core/mspro_block.c |  9 ++++++---
 2 files changed, 14 insertions(+), 6 deletions(-)


base-commit: 461294b587f6fa5bb17d5856e4404b67d666086d
-- 
2.56.0

[PATCH 1/5] memstick: mspro_block: don't hand a finished request back to the host

[Ho Jie Feng]

When a block request ends, mspro_block_complete_req() clears
msb->block_req, completes card->mrq_complete and returns 0.
Host drivers take a 0 return from memstick_next_req() to mean that
*mrq holds a new request, so the host issues card->current_mrq again
even though it has already finished.

At the same time, msb->block_req being NULL allows mspro_queue_rq() to
pass the check and queue another request, causing a race condition
that rewrites card->current_mrq while the host is executing it.

Return -EAGAIN instead, as the function already does when there is no
block request.

Fixes: d0be12274dad ("mspro_block: convert to blk-mq")
Signed-off-by: Ho Jie Feng <hjf3108@gmail.com>
---
 drivers/memstick/core/mspro_block.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/memstick/core/mspro_block.c b/drivers/memstick/core/mspro_block.c
index 4a52830026c50..fe38b42431219 100644
--- a/drivers/memstick/core/mspro_block.c
+++ b/drivers/memstick/core/mspro_block.c
@@ -706,6 +706,8 @@ static int mspro_block_complete_req(struct memstick_dev *card, int error)
 			__blk_mq_end_request(msb->block_req,
 						errno_to_blk_status(error));
 			msb->block_req = NULL;
+			if (!error)
+				error = -EAGAIN;
 		}
 	} else {
 		if (!error)
-- 
2.56.0

[PATCH 2/5] memstick: mspro_block: wait for running dispatches in mspro_block_stop()

[Ho Jie Feng]

mspro_block_stop() only calls blk_mq_stop_hw_queues() once it sees that
msb->block_req is NULL. That marks the queues stopped so that no new
dispatch starts, but does not wait for dispatches that are already
running. In this case, a request can start just after
mspro_block_stop() has returned, causing a race condition.

Stop the queues before checking msb->block_req, and use
blk_mq_wait_quiesce_done() to wait for running ->queue_rq() calls to
return. The existing loop then waits for the current request to finish
before mspro_block_stop() returns.

Fixes: d0be12274dad ("mspro_block: convert to blk-mq")
Signed-off-by: Ho Jie Feng <hjf3108@gmail.com>
---
 drivers/memstick/core/mspro_block.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/memstick/core/mspro_block.c b/drivers/memstick/core/mspro_block.c
index fe38b42431219..215f4c3759c41 100644
--- a/drivers/memstick/core/mspro_block.c
+++ b/drivers/memstick/core/mspro_block.c
@@ -727,12 +727,13 @@ static void mspro_block_stop(struct memstick_dev *card)
 	int rc = 0;
 	unsigned long flags;
 
+	blk_mq_stop_hw_queues(msb->queue);
+	blk_mq_wait_quiesce_done(&msb->tag_set);
+
 	while (1) {
 		spin_lock_irqsave(&msb->q_lock, flags);
-		if (!msb->block_req) {
-			blk_mq_stop_hw_queues(msb->queue);
+		if (!msb->block_req)
 			rc = 1;
-		}
 		spin_unlock_irqrestore(&msb->q_lock, flags);
 
 		if (rc)
-- 
2.56.0

[PATCH 3/5] memstick: ms_block: don't erase the old block of an unmapped LBA

[Ho Jie Feng]

msb_update_block() checks for MS_BLOCK_INVALID before it sets the update
flag on the old block, but then erases the old block unconditionally. For
an LBA without a block, this tries to erase block 65535.

msb_erase_block() then calls msb_mark_block_unused() and sets the bit in
erased_blocks_bitmap with pba 65535, so used_blocks_bitmap,
free_block_count[] and erased_blocks_bitmap are accessed out of bounds.

UBSAN reported free_block_count[127] on the int[16] array.

Only erase the old block if there is one.

Fixes: 0ab30494bc4f ("memstick: add support for legacy memorysticks")
Signed-off-by: Ho Jie Feng <hjf3108@gmail.com>
---
 drivers/memstick/core/ms_block.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/memstick/core/ms_block.c b/drivers/memstick/core/ms_block.c
index cd4ff1b79a6da..e373eaf92530f 100644
--- a/drivers/memstick/core/ms_block.c
+++ b/drivers/memstick/core/ms_block.c
@@ -1149,8 +1149,10 @@ static int msb_update_block(struct msb_data *msb, u16 lba,
 		if (error)
 			goto out;
 
-		dbg_verbose("block update: erasing the old block");
-		msb_erase_block(msb, pba);
+		if (pba != MS_BLOCK_INVALID) {
+			dbg_verbose("block update: erasing the old block");
+			msb_erase_block(msb, pba);
+		}
 		msb->lba_to_pba_table[lba] = new_pba;
 		return 0;
 	}
-- 
2.56.0

[PATCH 4/5] memstick: ms_block: keep the power off during a full msb_reset()

[Ho Jie Feng]

Keep the power off for long enough so that the card properly
power cycles.

Fixes: 0ab30494bc4f ("memstick: add support for legacy memorysticks")
Signed-off-by: Ho Jie Feng <hjf3108@gmail.com>
---
 drivers/memstick/core/ms_block.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/memstick/core/ms_block.c b/drivers/memstick/core/ms_block.c
index e373eaf92530f..66672235a4584 100644
--- a/drivers/memstick/core/ms_block.c
+++ b/drivers/memstick/core/ms_block.c
@@ -740,6 +740,9 @@ static int msb_reset(struct msb_data *msb, bool full)
 
 		msb_invalidate_reg_window(msb);
 
+		/* Allow enough time for the card to fully power off */
+		msleep(20);
+
 		error = host->set_param(host,
 					MEMSTICK_POWER, MEMSTICK_POWER_ON);
 		if (error)
-- 
2.56.0

[PATCH 5/5] memstick: ms_block: add the missing newline to the parallel switch error

[Ho Jie Feng]

Allow printk to finalize the record immediately instead of waiting
for the next message with a newline.

Fixes: 0ab30494bc4f ("memstick: add support for legacy memorysticks")
Signed-off-by: Ho Jie Feng <hjf3108@gmail.com>
---
 drivers/memstick/core/ms_block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/memstick/core/ms_block.c b/drivers/memstick/core/ms_block.c
index 66672235a4584..7237a6789e0ca 100644
--- a/drivers/memstick/core/ms_block.c
+++ b/drivers/memstick/core/ms_block.c
@@ -778,7 +778,7 @@ static int msb_switch_to_parallel(struct msb_data *msb)
 
 	error = msb_run_state_machine(msb, h_msb_parallel_switch);
 	if (error) {
-		pr_err("Switch to parallel failed");
+		pr_err("Switch to parallel failed\n");
 		msb->regs.param.system &= ~MEMSTICK_SYS_PAM;
 		msb_reset(msb, true);
 		return -EFAULT;
-- 
2.56.0