Re: [PATCH] mmc: block: ensure error propagation for non-blk

Re: [PATCH] mmc: block: ensure error propagation for non-blk

[gregkh]

On Tue, May 30, 2023 at 07:58:01AM +0000, Christian Loehle wrote:
> commit 003fb0a51162d940f25fc35e70b0996a12c9e08a upstream.

<snip>

You sent this, and the other commit, in html format, which made it
impossible to apply (and the mailing lists rejected your change as
well.)

Please fix up your email client and resend in non-html format and I will
be glad to queue this up.

thanks,

greg k-h

[PATCH] mmc: block: ensure error propagation for non-blk

[Christian Loehle]

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

commit 003fb0a51162d940f25fc35e70b0996a12c9e08a upstream.

Requests to the mmc layer usually come through a block device IO.
The exceptions are the ioctl interface, RPMB chardev ioctl
and debugfs, which issue their own blk_mq requests through
blk_execute_rq and do not query the BLK_STS error but the
mmcblk-internal drv_op_result. This patch ensures that drv_op_result
defaults to an error and has to be overwritten by the operation
to be considered successful.

The behavior leads to a bug where the request never propagates
the error, e.g. by directly erroring out at mmc_blk_mq_issue_rq if
mmc_blk_part_switch fails. The ioctl caller of the rpmb chardev then
can never see an error (BLK_STS_IOERR, but drv_op_result is unchanged)
and thus may assume that their call executed successfully when it did not.

While always checking the blk_execute_rq return value would be
advised, let's eliminate the error by always setting
drv_op_result as -EIO to be overwritten on success (or other error)

Fixes: 614f0388f580 ("mmc: block: move single ioctl() commands to block requests")
Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
---
 drivers/mmc/core/block.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index ed034b93cb25..0b72096f10e6 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -265,6 +265,7 @@ static ssize_t power_ro_lock_store(struct device *dev,
 		goto out_put;
 	}
 	req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_BOOT_WP;
+	req_to_mmc_queue_req(req)->drv_op_result = -EIO;
 	blk_execute_rq(NULL, req, 0);
 	ret = req_to_mmc_queue_req(req)->drv_op_result;
 	blk_put_request(req);
@@ -656,6 +657,7 @@ static int mmc_blk_ioctl_cmd(struct mmc_blk_data *md,
 	idatas[0] = idata;
 	req_to_mmc_queue_req(req)->drv_op =
 		rpmb ? MMC_DRV_OP_IOCTL_RPMB : MMC_DRV_OP_IOCTL;
+	req_to_mmc_queue_req(req)->drv_op_result = -EIO;
 	req_to_mmc_queue_req(req)->drv_op_data = idatas;
 	req_to_mmc_queue_req(req)->ioc_count = 1;
 	blk_execute_rq(NULL, req, 0);
@@ -725,6 +727,7 @@ static int mmc_blk_ioctl_multi_cmd(struct mmc_blk_data *md,
 	}
 	req_to_mmc_queue_req(req)->drv_op =
 		rpmb ? MMC_DRV_OP_IOCTL_RPMB : MMC_DRV_OP_IOCTL;
+	req_to_mmc_queue_req(req)->drv_op_result = -EIO;
 	req_to_mmc_queue_req(req)->drv_op_data = idata;
 	req_to_mmc_queue_req(req)->ioc_count = num_of_cmds;
 	blk_execute_rq(NULL, req, 0);
@@ -2784,6 +2787,7 @@ static int mmc_dbg_card_status_get(void *data, u64 *val)
 	if (IS_ERR(req))
 		return PTR_ERR(req);
 	req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_GET_CARD_STATUS;
+	req_to_mmc_queue_req(req)->drv_op_result = -EIO;
 	blk_execute_rq(NULL, req, 0);
 	ret = req_to_mmc_queue_req(req)->drv_op_result;
 	if (ret >= 0) {
@@ -2822,6 +2826,7 @@ static int mmc_ext_csd_open(struct inode *inode, struct file *filp)
 		goto out_free;
 	}
 	req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_GET_EXT_CSD;
+	req_to_mmc_queue_req(req)->drv_op_result = -EIO;
 	req_to_mmc_queue_req(req)->drv_op_data = &ext_csd;
 	blk_execute_rq(NULL, req, 0);
 	err = req_to_mmc_queue_req(req)->drv_op_result;
--
2.37.3

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 10302 bytes --]

[PATCH] mmc: block: ensure error propagation for non-blk

[Christian Loehle]

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

commit 003fb0a51162d940f25fc35e70b0996a12c9e08a upstream.

Requests to the mmc layer usually come through a block device IO.
The exceptions are the ioctl interface, RPMB chardev ioctl
and debugfs, which issue their own blk_mq requests through
blk_execute_rq and do not query the BLK_STS error but the
mmcblk-internal drv_op_result. This patch ensures that drv_op_result
defaults to an error and has to be overwritten by the operation
to be considered successful.

The behavior leads to a bug where the request never propagates
the error, e.g. by directly erroring out at mmc_blk_mq_issue_rq if
mmc_blk_part_switch fails. The ioctl caller of the rpmb chardev then
can never see an error (BLK_STS_IOERR, but drv_op_result is unchanged)
and thus may assume that their call executed successfully when it did not.

While always checking the blk_execute_rq return value would be
advised, let's eliminate the error by always setting
drv_op_result as -EIO to be overwritten on success (or other error)

Fixes: 614f0388f580 ("mmc: block: move single ioctl() commands to block requests")
Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
---
 drivers/mmc/core/block.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 79e5acc6e964..a6228bfdf3ea 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -243,6 +243,7 @@ static ssize_t power_ro_lock_store(struct device *dev,
 		goto out_put;
 	}
 	req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_BOOT_WP;
+	req_to_mmc_queue_req(req)->drv_op_result = -EIO;
 	blk_execute_rq(mq->queue, NULL, req, 0);
 	ret = req_to_mmc_queue_req(req)->drv_op_result;
 	blk_put_request(req);
@@ -671,6 +672,7 @@ static int mmc_blk_ioctl_cmd(struct mmc_blk_data *md,
 	idatas[0] = idata;
 	req_to_mmc_queue_req(req)->drv_op =
 		rpmb ? MMC_DRV_OP_IOCTL_RPMB : MMC_DRV_OP_IOCTL;
+	req_to_mmc_queue_req(req)->drv_op_result = -EIO;
 	req_to_mmc_queue_req(req)->drv_op_data = idatas;
 	req_to_mmc_queue_req(req)->ioc_count = 1;
 	blk_execute_rq(mq->queue, NULL, req, 0);
@@ -741,6 +743,7 @@ static int mmc_blk_ioctl_multi_cmd(struct mmc_blk_data *md,
 	}
 	req_to_mmc_queue_req(req)->drv_op =
 		rpmb ? MMC_DRV_OP_IOCTL_RPMB : MMC_DRV_OP_IOCTL;
+	req_to_mmc_queue_req(req)->drv_op_result = -EIO;
 	req_to_mmc_queue_req(req)->drv_op_data = idata;
 	req_to_mmc_queue_req(req)->ioc_count = num_of_cmds;
 	blk_execute_rq(mq->queue, NULL, req, 0);
@@ -2590,6 +2593,7 @@ static int mmc_dbg_card_status_get(void *data, u64 *val)
 	if (IS_ERR(req))
 		return PTR_ERR(req);
 	req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_GET_CARD_STATUS;
+	req_to_mmc_queue_req(req)->drv_op_result = -EIO;
 	blk_execute_rq(mq->queue, NULL, req, 0);
 	ret = req_to_mmc_queue_req(req)->drv_op_result;
 	if (ret >= 0) {
@@ -2628,6 +2632,7 @@ static int mmc_ext_csd_open(struct inode *inode, struct file *filp)
 		goto out_free;
 	}
 	req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_GET_EXT_CSD;
+	req_to_mmc_queue_req(req)->drv_op_result = -EIO;
 	req_to_mmc_queue_req(req)->drv_op_data = &ext_csd;
 	blk_execute_rq(mq->queue, NULL, req, 0);
 	err = req_to_mmc_queue_req(req)->drv_op_result;
--
2.37.3

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 10302 bytes --]

Re: [PATCH] mmc: block: ensure error propagation for non-blk

[gregkh]

On Mon, Jun 05, 2023 at 01:46:16PM +0000, Christian Loehle wrote:
> commit 003fb0a51162d940f25fc35e70b0996a12c9e08a upstream.
> 
> Requests to the mmc layer usually come through a block device IO.
> The exceptions are the ioctl interface, RPMB chardev ioctl
> and debugfs, which issue their own blk_mq requests through
> blk_execute_rq and do not query the BLK_STS error but the
> mmcblk-internal drv_op_result. This patch ensures that drv_op_result
> defaults to an error and has to be overwritten by the operation
> to be considered successful.
> 
> The behavior leads to a bug where the request never propagates
> the error, e.g. by directly erroring out at mmc_blk_mq_issue_rq if
> mmc_blk_part_switch fails. The ioctl caller of the rpmb chardev then
> can never see an error (BLK_STS_IOERR, but drv_op_result is unchanged)
> and thus may assume that their call executed successfully when it did not.
> 
> While always checking the blk_execute_rq return value would be
> advised, let's eliminate the error by always setting
> drv_op_result as -EIO to be overwritten on success (or other error)
> 
> Fixes: 614f0388f580 ("mmc: block: move single ioctl() commands to block requests")
> Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
> ---
>  drivers/mmc/core/block.c | 5 +++++
>  1 file changed, 5 insertions(+)

What stable tree(s) do you want this applied to?

thanks,

greg k-h

RE: [PATCH] mmc: block: ensure error propagation for non-blk

[Christian Loehle]

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

Hey Greg,
The fix should be applied to all stable trees.
I backported one that works for
4.14
4.19
5.4
5.10
And one for
5.15

Regards,
Christian

-----Original Message-----
From: gregkh@linuxfoundation.org <gregkh@linuxfoundation.org>
Sent: Monday, June 5, 2023 9:50 PM
To: Christian Loehle <CLoehle@hyperstone.com>
Cc: stable@vger.kernel.org
Subject: Re: [PATCH] mmc: block: ensure error propagation for non-blk

CAUTION: this mail comes from external!/ACHTUNG: Diese Mail kommt von extern!

On Mon, Jun 05, 2023 at 01:46:16PM +0000, Christian Loehle wrote:
> commit 003fb0a51162d940f25fc35e70b0996a12c9e08a upstream.
>
> Requests to the mmc layer usually come through a block device IO.
> The exceptions are the ioctl interface, RPMB chardev ioctl and
> debugfs, which issue their own blk_mq requests through blk_execute_rq
> and do not query the BLK_STS error but the mmcblk-internal
> drv_op_result. This patch ensures that drv_op_result defaults to an
> error and has to be overwritten by the operation to be considered
> successful.
>
> The behavior leads to a bug where the request never propagates the
> error, e.g. by directly erroring out at mmc_blk_mq_issue_rq if
> mmc_blk_part_switch fails. The ioctl caller of the rpmb chardev then
> can never see an error (BLK_STS_IOERR, but drv_op_result is unchanged)
> and thus may assume that their call executed successfully when it did not.
>
> While always checking the blk_execute_rq return value would be
> advised, let's eliminate the error by always setting drv_op_result as
> -EIO to be overwritten on success (or other error)
>
> Fixes: 614f0388f580 ("mmc: block: move single ioctl() commands to
> block requests")
> Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
> ---
>  drivers/mmc/core/block.c | 5 +++++
>  1 file changed, 5 insertions(+)

What stable tree(s) do you want this applied to?

thanks,

greg k-h

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 10302 bytes --]

Re: [PATCH] mmc: block: ensure error propagation for non-blk

[gregkh]

On Tue, Jun 06, 2023 at 12:58:39PM +0000, Christian Loehle wrote:
> Hey Greg,
> The fix should be applied to all stable trees.
> I backported one that works for
> 4.14
> 4.19
> 5.4
> 5.10
> And one for
> 5.15

How can I know which from which?  Please give me us a hint somehow to
know this...

Can you resend these, with that hint either in the subject line (i.e.
[PATCH 5.15]), or below the --- line.

thanks,

greg k-h

[PATCH] mmc: block: ensure error propagation for non-blk

[Christian Loehle]

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

commit 003fb0a51162d940f25fc35e70b0996a12c9e08a upstream.

Requests to the mmc layer usually come through a block device IO.
The exceptions are the ioctl interface, RPMB chardev ioctl
and debugfs, which issue their own blk_mq requests through
blk_execute_rq and do not query the BLK_STS error but the
mmcblk-internal drv_op_result. This patch ensures that drv_op_result
defaults to an error and has to be overwritten by the operation
to be considered successful.

The behavior leads to a bug where the request never propagates
the error, e.g. by directly erroring out at mmc_blk_mq_issue_rq if
mmc_blk_part_switch fails. The ioctl caller of the rpmb chardev then
can never see an error (BLK_STS_IOERR, but drv_op_result is unchanged)
and thus may assume that their call executed successfully when it did not.

While always checking the blk_execute_rq return value would be
advised, let's eliminate the error by always setting
drv_op_result as -EIO to be overwritten on success (or other error)

Fixes: 614f0388f580 ("mmc: block: move single ioctl() commands to block requests")
Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
---
This is for the following stable trees:
4.14
4.19
5.4
5.10
 drivers/mmc/core/block.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 79e5acc6e964..a6228bfdf3ea 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -243,6 +243,7 @@ static ssize_t power_ro_lock_store(struct device *dev,
 		goto out_put;
 	}
 	req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_BOOT_WP;
+	req_to_mmc_queue_req(req)->drv_op_result = -EIO;
 	blk_execute_rq(mq->queue, NULL, req, 0);
 	ret = req_to_mmc_queue_req(req)->drv_op_result;
 	blk_put_request(req);
@@ -671,6 +672,7 @@ static int mmc_blk_ioctl_cmd(struct mmc_blk_data *md,
 	idatas[0] = idata;
 	req_to_mmc_queue_req(req)->drv_op =
 		rpmb ? MMC_DRV_OP_IOCTL_RPMB : MMC_DRV_OP_IOCTL;
+	req_to_mmc_queue_req(req)->drv_op_result = -EIO;
 	req_to_mmc_queue_req(req)->drv_op_data = idatas;
 	req_to_mmc_queue_req(req)->ioc_count = 1;
 	blk_execute_rq(mq->queue, NULL, req, 0);
@@ -741,6 +743,7 @@ static int mmc_blk_ioctl_multi_cmd(struct mmc_blk_data *md,
 	}
 	req_to_mmc_queue_req(req)->drv_op =
 		rpmb ? MMC_DRV_OP_IOCTL_RPMB : MMC_DRV_OP_IOCTL;
+	req_to_mmc_queue_req(req)->drv_op_result = -EIO;
 	req_to_mmc_queue_req(req)->drv_op_data = idata;
 	req_to_mmc_queue_req(req)->ioc_count = num_of_cmds;
 	blk_execute_rq(mq->queue, NULL, req, 0);
@@ -2590,6 +2593,7 @@ static int mmc_dbg_card_status_get(void *data, u64 *val)
 	if (IS_ERR(req))
 		return PTR_ERR(req);
 	req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_GET_CARD_STATUS;
+	req_to_mmc_queue_req(req)->drv_op_result = -EIO;
 	blk_execute_rq(mq->queue, NULL, req, 0);
 	ret = req_to_mmc_queue_req(req)->drv_op_result;
 	if (ret >= 0) {
@@ -2628,6 +2632,7 @@ static int mmc_ext_csd_open(struct inode *inode, struct file *filp)
 		goto out_free;
 	}
 	req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_GET_EXT_CSD;
+	req_to_mmc_queue_req(req)->drv_op_result = -EIO;
 	req_to_mmc_queue_req(req)->drv_op_data = &ext_csd;
 	blk_execute_rq(mq->queue, NULL, req, 0);
 	err = req_to_mmc_queue_req(req)->drv_op_result;
--
2.37.3

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 10302 bytes --]

[PATCH] mmc: block: ensure error propagation for non-blk

[Christian Loehle]

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

commit 003fb0a51162d940f25fc35e70b0996a12c9e08a upstream.

Requests to the mmc layer usually come through a block device IO.
The exceptions are the ioctl interface, RPMB chardev ioctl
and debugfs, which issue their own blk_mq requests through
blk_execute_rq and do not query the BLK_STS error but the
mmcblk-internal drv_op_result. This patch ensures that drv_op_result
defaults to an error and has to be overwritten by the operation
to be considered successful.

The behavior leads to a bug where the request never propagates
the error, e.g. by directly erroring out at mmc_blk_mq_issue_rq if
mmc_blk_part_switch fails. The ioctl caller of the rpmb chardev then
can never see an error (BLK_STS_IOERR, but drv_op_result is unchanged)
and thus may assume that their call executed successfully when it did not.

While always checking the blk_execute_rq return value would be
advised, let's eliminate the error by always setting
drv_op_result as -EIO to be overwritten on success (or other error)

Fixes: 614f0388f580 ("mmc: block: move single ioctl() commands to block requests")
Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
---
This is for the 5.15. stable tree
 drivers/mmc/core/block.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index ed034b93cb25..0b72096f10e6 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -265,6 +265,7 @@ static ssize_t power_ro_lock_store(struct device *dev,
 		goto out_put;
 	}
 	req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_BOOT_WP;
+	req_to_mmc_queue_req(req)->drv_op_result = -EIO;
 	blk_execute_rq(NULL, req, 0);
 	ret = req_to_mmc_queue_req(req)->drv_op_result;
 	blk_put_request(req);
@@ -656,6 +657,7 @@ static int mmc_blk_ioctl_cmd(struct mmc_blk_data *md,
 	idatas[0] = idata;
 	req_to_mmc_queue_req(req)->drv_op =
 		rpmb ? MMC_DRV_OP_IOCTL_RPMB : MMC_DRV_OP_IOCTL;
+	req_to_mmc_queue_req(req)->drv_op_result = -EIO;
 	req_to_mmc_queue_req(req)->drv_op_data = idatas;
 	req_to_mmc_queue_req(req)->ioc_count = 1;
 	blk_execute_rq(NULL, req, 0);
@@ -725,6 +727,7 @@ static int mmc_blk_ioctl_multi_cmd(struct mmc_blk_data *md,
 	}
 	req_to_mmc_queue_req(req)->drv_op =
 		rpmb ? MMC_DRV_OP_IOCTL_RPMB : MMC_DRV_OP_IOCTL;
+	req_to_mmc_queue_req(req)->drv_op_result = -EIO;
 	req_to_mmc_queue_req(req)->drv_op_data = idata;
 	req_to_mmc_queue_req(req)->ioc_count = num_of_cmds;
 	blk_execute_rq(NULL, req, 0);
@@ -2784,6 +2787,7 @@ static int mmc_dbg_card_status_get(void *data, u64 *val)
 	if (IS_ERR(req))
 		return PTR_ERR(req);
 	req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_GET_CARD_STATUS;
+	req_to_mmc_queue_req(req)->drv_op_result = -EIO;
 	blk_execute_rq(NULL, req, 0);
 	ret = req_to_mmc_queue_req(req)->drv_op_result;
 	if (ret >= 0) {
@@ -2822,6 +2826,7 @@ static int mmc_ext_csd_open(struct inode *inode, struct file *filp)
 		goto out_free;
 	}
 	req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_GET_EXT_CSD;
+	req_to_mmc_queue_req(req)->drv_op_result = -EIO;
 	req_to_mmc_queue_req(req)->drv_op_data = &ext_csd;
 	blk_execute_rq(NULL, req, 0);
 	err = req_to_mmc_queue_req(req)->drv_op_result;
--
2.37.3

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 10302 bytes --]

Re: [PATCH] mmc: block: ensure error propagation for non-blk

[Ulf Hansson]

On Tue, 13 Jun 2023 at 14:37, Christian Loehle <CLoehle@hyperstone.com> wrote:
>
> commit 003fb0a51162d940f25fc35e70b0996a12c9e08a upstream.
>
> Requests to the mmc layer usually come through a block device IO.
> The exceptions are the ioctl interface, RPMB chardev ioctl
> and debugfs, which issue their own blk_mq requests through
> blk_execute_rq and do not query the BLK_STS error but the
> mmcblk-internal drv_op_result. This patch ensures that drv_op_result
> defaults to an error and has to be overwritten by the operation
> to be considered successful.
>
> The behavior leads to a bug where the request never propagates
> the error, e.g. by directly erroring out at mmc_blk_mq_issue_rq if
> mmc_blk_part_switch fails. The ioctl caller of the rpmb chardev then
> can never see an error (BLK_STS_IOERR, but drv_op_result is unchanged)
> and thus may assume that their call executed successfully when it did not.
>
> While always checking the blk_execute_rq return value would be
> advised, let's eliminate the error by always setting
> drv_op_result as -EIO to be overwritten on success (or other error)
>
> Fixes: 614f0388f580 ("mmc: block: move single ioctl() commands to block requests")
> Signed-off-by: Christian Loehle <cloehle@hyperstone.com>

Acked-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

> ---
> This is for the following stable trees:
> 4.14
> 4.19
> 5.4
> 5.10
>  drivers/mmc/core/block.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 79e5acc6e964..a6228bfdf3ea 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -243,6 +243,7 @@ static ssize_t power_ro_lock_store(struct device *dev,
>                 goto out_put;
>         }
>         req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_BOOT_WP;
> +       req_to_mmc_queue_req(req)->drv_op_result = -EIO;
>         blk_execute_rq(mq->queue, NULL, req, 0);
>         ret = req_to_mmc_queue_req(req)->drv_op_result;
>         blk_put_request(req);
> @@ -671,6 +672,7 @@ static int mmc_blk_ioctl_cmd(struct mmc_blk_data *md,
>         idatas[0] = idata;
>         req_to_mmc_queue_req(req)->drv_op =
>                 rpmb ? MMC_DRV_OP_IOCTL_RPMB : MMC_DRV_OP_IOCTL;
> +       req_to_mmc_queue_req(req)->drv_op_result = -EIO;
>         req_to_mmc_queue_req(req)->drv_op_data = idatas;
>         req_to_mmc_queue_req(req)->ioc_count = 1;
>         blk_execute_rq(mq->queue, NULL, req, 0);
> @@ -741,6 +743,7 @@ static int mmc_blk_ioctl_multi_cmd(struct mmc_blk_data *md,
>         }
>         req_to_mmc_queue_req(req)->drv_op =
>                 rpmb ? MMC_DRV_OP_IOCTL_RPMB : MMC_DRV_OP_IOCTL;
> +       req_to_mmc_queue_req(req)->drv_op_result = -EIO;
>         req_to_mmc_queue_req(req)->drv_op_data = idata;
>         req_to_mmc_queue_req(req)->ioc_count = num_of_cmds;
>         blk_execute_rq(mq->queue, NULL, req, 0);
> @@ -2590,6 +2593,7 @@ static int mmc_dbg_card_status_get(void *data, u64 *val)
>         if (IS_ERR(req))
>                 return PTR_ERR(req);
>         req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_GET_CARD_STATUS;
> +       req_to_mmc_queue_req(req)->drv_op_result = -EIO;
>         blk_execute_rq(mq->queue, NULL, req, 0);
>         ret = req_to_mmc_queue_req(req)->drv_op_result;
>         if (ret >= 0) {
> @@ -2628,6 +2632,7 @@ static int mmc_ext_csd_open(struct inode *inode, struct file *filp)
>                 goto out_free;
>         }
>         req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_GET_EXT_CSD;
> +       req_to_mmc_queue_req(req)->drv_op_result = -EIO;
>         req_to_mmc_queue_req(req)->drv_op_data = &ext_csd;
>         blk_execute_rq(mq->queue, NULL, req, 0);
>         err = req_to_mmc_queue_req(req)->drv_op_result;
> --
> 2.37.3

Re: [PATCH] mmc: block: ensure error propagation for non-blk

[Ulf Hansson]

On Tue, 13 Jun 2023 at 14:43, Christian Loehle <CLoehle@hyperstone.com> wrote:
>
> commit 003fb0a51162d940f25fc35e70b0996a12c9e08a upstream.
>
> Requests to the mmc layer usually come through a block device IO.
> The exceptions are the ioctl interface, RPMB chardev ioctl
> and debugfs, which issue their own blk_mq requests through
> blk_execute_rq and do not query the BLK_STS error but the
> mmcblk-internal drv_op_result. This patch ensures that drv_op_result
> defaults to an error and has to be overwritten by the operation
> to be considered successful.
>
> The behavior leads to a bug where the request never propagates
> the error, e.g. by directly erroring out at mmc_blk_mq_issue_rq if
> mmc_blk_part_switch fails. The ioctl caller of the rpmb chardev then
> can never see an error (BLK_STS_IOERR, but drv_op_result is unchanged)
> and thus may assume that their call executed successfully when it did not.
>
> While always checking the blk_execute_rq return value would be
> advised, let's eliminate the error by always setting
> drv_op_result as -EIO to be overwritten on success (or other error)
>
> Fixes: 614f0388f580 ("mmc: block: move single ioctl() commands to block requests")
> Signed-off-by: Christian Loehle <cloehle@hyperstone.com>

Acked-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe

> ---
> This is for the 5.15. stable tree
>  drivers/mmc/core/block.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index ed034b93cb25..0b72096f10e6 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -265,6 +265,7 @@ static ssize_t power_ro_lock_store(struct device *dev,
>                 goto out_put;
>         }
>         req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_BOOT_WP;
> +       req_to_mmc_queue_req(req)->drv_op_result = -EIO;
>         blk_execute_rq(NULL, req, 0);
>         ret = req_to_mmc_queue_req(req)->drv_op_result;
>         blk_put_request(req);
> @@ -656,6 +657,7 @@ static int mmc_blk_ioctl_cmd(struct mmc_blk_data *md,
>         idatas[0] = idata;
>         req_to_mmc_queue_req(req)->drv_op =
>                 rpmb ? MMC_DRV_OP_IOCTL_RPMB : MMC_DRV_OP_IOCTL;
> +       req_to_mmc_queue_req(req)->drv_op_result = -EIO;
>         req_to_mmc_queue_req(req)->drv_op_data = idatas;
>         req_to_mmc_queue_req(req)->ioc_count = 1;
>         blk_execute_rq(NULL, req, 0);
> @@ -725,6 +727,7 @@ static int mmc_blk_ioctl_multi_cmd(struct mmc_blk_data *md,
>         }
>         req_to_mmc_queue_req(req)->drv_op =
>                 rpmb ? MMC_DRV_OP_IOCTL_RPMB : MMC_DRV_OP_IOCTL;
> +       req_to_mmc_queue_req(req)->drv_op_result = -EIO;
>         req_to_mmc_queue_req(req)->drv_op_data = idata;
>         req_to_mmc_queue_req(req)->ioc_count = num_of_cmds;
>         blk_execute_rq(NULL, req, 0);
> @@ -2784,6 +2787,7 @@ static int mmc_dbg_card_status_get(void *data, u64 *val)
>         if (IS_ERR(req))
>                 return PTR_ERR(req);
>         req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_GET_CARD_STATUS;
> +       req_to_mmc_queue_req(req)->drv_op_result = -EIO;
>         blk_execute_rq(NULL, req, 0);
>         ret = req_to_mmc_queue_req(req)->drv_op_result;
>         if (ret >= 0) {
> @@ -2822,6 +2826,7 @@ static int mmc_ext_csd_open(struct inode *inode, struct file *filp)
>                 goto out_free;
>         }
>         req_to_mmc_queue_req(req)->drv_op = MMC_DRV_OP_GET_EXT_CSD;
> +       req_to_mmc_queue_req(req)->drv_op_result = -EIO;
>         req_to_mmc_queue_req(req)->drv_op_data = &ext_csd;
>         blk_execute_rq(NULL, req, 0);
>         err = req_to_mmc_queue_req(req)->drv_op_result;
> --
> 2.37.3

Re: [PATCH] mmc: block: ensure error propagation for non-blk

[gregkh]

On Wed, Jun 14, 2023 at 12:20:39PM +0200, Ulf Hansson wrote:
> On Tue, 13 Jun 2023 at 14:43, Christian Loehle <CLoehle@hyperstone.com> wrote:
> >
> > commit 003fb0a51162d940f25fc35e70b0996a12c9e08a upstream.
> >
> > Requests to the mmc layer usually come through a block device IO.
> > The exceptions are the ioctl interface, RPMB chardev ioctl
> > and debugfs, which issue their own blk_mq requests through
> > blk_execute_rq and do not query the BLK_STS error but the
> > mmcblk-internal drv_op_result. This patch ensures that drv_op_result
> > defaults to an error and has to be overwritten by the operation
> > to be considered successful.
> >
> > The behavior leads to a bug where the request never propagates
> > the error, e.g. by directly erroring out at mmc_blk_mq_issue_rq if
> > mmc_blk_part_switch fails. The ioctl caller of the rpmb chardev then
> > can never see an error (BLK_STS_IOERR, but drv_op_result is unchanged)
> > and thus may assume that their call executed successfully when it did not.
> >
> > While always checking the blk_execute_rq return value would be
> > advised, let's eliminate the error by always setting
> > drv_op_result as -EIO to be overwritten on success (or other error)
> >
> > Fixes: 614f0388f580 ("mmc: block: move single ioctl() commands to block requests")
> > Signed-off-by: Christian Loehle <cloehle@hyperstone.com>
> 
> Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
> 

All now queued up, thanks.

greg k-h