[PATCH 1/2] mmc core block.c: initialize mmc_blk_ioc_data

[PATCH 1/2] mmc core block.c: initialize mmc_blk_ioc_data

[mikko.rapeli]

From: Mikko Rapeli <mikko.rapeli@linaro.org>

Commit "mmc: core: Use mrq.sbc in close-ended ffu" adds flags uint to
struct mmc_blk_ioc_data but it does not get initialized for RPMB ioctls
which now fail.

Fix this by always initializing the struct and flags to zero.

Fixes access to RPMB storage.

Fixes: 4d0c8d0aef63 ("mmc: core: Use mrq.sbc in close-ended ffu")

Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218587

Link: https://lore.kernel.org/all/20231129092535.3278-1-avri.altman@wdc.com/

Cc: Avri Altman <avri.altman@wdc.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: linux-mmc@vger.kernel.org
Cc: stable@vger.kernel.org
Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
---
 drivers/mmc/core/block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 32d49100dff5..0df627de9cee 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -413,7 +413,7 @@ static struct mmc_blk_ioc_data *mmc_blk_ioctl_copy_from_user(
 	struct mmc_blk_ioc_data *idata;
 	int err;
 
-	idata = kmalloc(sizeof(*idata), GFP_KERNEL);
+	idata = kzalloc(sizeof(*idata), GFP_KERNEL);
 	if (!idata) {
 		err = -ENOMEM;
 		goto out;
-- 
2.34.1

[PATCH 2/2] mmc core block.c: avoid negative index with array access

[mikko.rapeli]

From: Mikko Rapeli <mikko.rapeli@linaro.org>

Commit "mmc: core: Use mrq.sbc in close-ended ffu" assigns
prev_idata = idatas[i - 1] but doesn't check that int iterator
i is greater than zero. Add the check.

Fixes: 4d0c8d0aef63 ("mmc: core: Use mrq.sbc in close-ended ffu")

Link: https://lore.kernel.org/all/20231129092535.3278-1-avri.altman@wdc.com/

Cc: Avri Altman <avri.altman@wdc.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: linux-mmc@vger.kernel.org
Cc: stable@vger.kernel.org
Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
---
 drivers/mmc/core/block.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 0df627de9cee..7f275b4ca9fa 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -488,7 +488,7 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card *card, struct mmc_blk_data *md,
 	if (idata->flags & MMC_BLK_IOC_DROP)
 		return 0;
 
-	if (idata->flags & MMC_BLK_IOC_SBC)
+	if (idata->flags & MMC_BLK_IOC_SBC && i > 0)
 		prev_idata = idatas[i - 1];
 
 	/*
-- 
2.34.1

RE: [PATCH 1/2] mmc core block.c: initialize mmc_blk_ioc_data

[Avri Altman]

> From: Mikko Rapeli <mikko.rapeli@linaro.org>
> 
> Commit "mmc: core: Use mrq.sbc in close-ended ffu" adds flags uint to struct
> mmc_blk_ioc_data but it does not get initialized for RPMB ioctls which now fail.
> 
> Fix this by always initializing the struct and flags to zero.
> 
> Fixes access to RPMB storage.
> 
> Fixes: 4d0c8d0aef63 ("mmc: core: Use mrq.sbc in close-ended ffu")
> 
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218587
> 
> Link: https://lore.kernel.org/all/20231129092535.3278-1-
> avri.altman@wdc.com/
> 
> Cc: Avri Altman <avri.altman@wdc.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: linux-mmc@vger.kernel.org
> Cc: stable@vger.kernel.org
> Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
Reviewed-by: Avri Altman <avri.altman@wdc.com>


> ---
>  drivers/mmc/core/block.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index
> 32d49100dff5..0df627de9cee 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -413,7 +413,7 @@ static struct mmc_blk_ioc_data
> *mmc_blk_ioctl_copy_from_user(
>         struct mmc_blk_ioc_data *idata;
>         int err;
> 
> -       idata = kmalloc(sizeof(*idata), GFP_KERNEL);
> +       idata = kzalloc(sizeof(*idata), GFP_KERNEL);
>         if (!idata) {
>                 err = -ENOMEM;
>                 goto out;
> --
> 2.34.1

RE: [PATCH 2/2] mmc core block.c: avoid negative index with array access

[Avri Altman]

> -----Original Message-----
> From: mikko.rapeli@linaro.org <mikko.rapeli@linaro.org>
> Sent: Wednesday, March 13, 2024 3:38 PM
> To: linux-mmc@vger.kernel.org
> Cc: Mikko Rapeli <mikko.rapeli@linaro.org>; Avri Altman
> <Avri.Altman@wdc.com>; Ulf Hansson <ulf.hansson@linaro.org>; Adrian Hunter
> <adrian.hunter@intel.com>; stable@vger.kernel.org
> Subject: [PATCH 2/2] mmc core block.c: avoid negative index with array access
> 
> CAUTION: This email originated from outside of Western Digital. Do not click
> on links or open attachments unless you recognize the sender and know that the
> content is safe.
> 
> 
> From: Mikko Rapeli <mikko.rapeli@linaro.org>
> 
> Commit "mmc: core: Use mrq.sbc in close-ended ffu" assigns prev_idata =
> idatas[i - 1] but doesn't check that int iterator i is greater than zero. Add the
> check.
I don't think this is even possible given 1/2.

Thanks,
Avri

> 
> Fixes: 4d0c8d0aef63 ("mmc: core: Use mrq.sbc in close-ended ffu")
> 
> Link: https://lore.kernel.org/all/20231129092535.3278-1-
> avri.altman@wdc.com/
> 
> Cc: Avri Altman <avri.altman@wdc.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: linux-mmc@vger.kernel.org
> Cc: stable@vger.kernel.org
> Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
> ---
>  drivers/mmc/core/block.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index
> 0df627de9cee..7f275b4ca9fa 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -488,7 +488,7 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card
> *card, struct mmc_blk_data *md,
>         if (idata->flags & MMC_BLK_IOC_DROP)
>                 return 0;
> 
> -       if (idata->flags & MMC_BLK_IOC_SBC)
> +       if (idata->flags & MMC_BLK_IOC_SBC && i > 0)
>                 prev_idata = idatas[i - 1];
> 
>         /*
> --
> 2.34.1

Re: [PATCH 2/2] mmc core block.c: avoid negative index with array access

[Mikko Rapeli]

On Wed, Mar 13, 2024 at 02:12:52PM +0000, Avri Altman wrote:
> > -----Original Message-----
> > From: mikko.rapeli@linaro.org <mikko.rapeli@linaro.org>
> > Sent: Wednesday, March 13, 2024 3:38 PM
> > To: linux-mmc@vger.kernel.org
> > Cc: Mikko Rapeli <mikko.rapeli@linaro.org>; Avri Altman
> > <Avri.Altman@wdc.com>; Ulf Hansson <ulf.hansson@linaro.org>; Adrian Hunter
> > <adrian.hunter@intel.com>; stable@vger.kernel.org
> > Subject: [PATCH 2/2] mmc core block.c: avoid negative index with array access
> > 
> > CAUTION: This email originated from outside of Western Digital. Do not click
> > on links or open attachments unless you recognize the sender and know that the
> > content is safe.
> > 
> > 
> > From: Mikko Rapeli <mikko.rapeli@linaro.org>
> > 
> > Commit "mmc: core: Use mrq.sbc in close-ended ffu" assigns prev_idata =
> > idatas[i - 1] but doesn't check that int iterator i is greater than zero. Add the
> > check.
> I don't think this is even possible given 1/2.

With RPMB ioctl:

        case MMC_DRV_OP_IOCTL_RPMB:
                idata = mq_rq->drv_op_data;
                for (i = 0, ret = 0; i < mq_rq->ioc_count; i++) {
                        ret = __mmc_blk_ioctl_cmd(card, md, idata, i);
                        if (ret)
                                break;
                }

First call is with i = 0?

Cheers,

-Mikko

> Thanks,
> Avri
> 
> > 
> > Fixes: 4d0c8d0aef63 ("mmc: core: Use mrq.sbc in close-ended ffu")
> > 
> > Link: https://lore.kernel.org/all/20231129092535.3278-1-
> > avri.altman@wdc.com/
> > 
> > Cc: Avri Altman <avri.altman@wdc.com>
> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > Cc: Adrian Hunter <adrian.hunter@intel.com>
> > Cc: linux-mmc@vger.kernel.org
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
> > ---
> >  drivers/mmc/core/block.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index
> > 0df627de9cee..7f275b4ca9fa 100644
> > --- a/drivers/mmc/core/block.c
> > +++ b/drivers/mmc/core/block.c
> > @@ -488,7 +488,7 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card
> > *card, struct mmc_blk_data *md,
> >         if (idata->flags & MMC_BLK_IOC_DROP)
> >                 return 0;
> > 
> > -       if (idata->flags & MMC_BLK_IOC_SBC)
> > +       if (idata->flags & MMC_BLK_IOC_SBC && i > 0)
> >                 prev_idata = idatas[i - 1];
> > 
> >         /*
> > --
> > 2.34.1
> 

Re: [PATCH 1/2] mmc core block.c: initialize mmc_blk_ioc_data

[Adrian Hunter]

On 13/03/24 15:37, mikko.rapeli@linaro.org wrote:
> From: Mikko Rapeli <mikko.rapeli@linaro.org>
> 
> Commit "mmc: core: Use mrq.sbc in close-ended ffu" adds flags uint to
> struct mmc_blk_ioc_data but it does not get initialized for RPMB ioctls
> which now fail.
> 
> Fix this by always initializing the struct and flags to zero.
> 
> Fixes access to RPMB storage.
> 
> Fixes: 4d0c8d0aef63 ("mmc: core: Use mrq.sbc in close-ended ffu")
> 
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218587
> 
> Link: https://lore.kernel.org/all/20231129092535.3278-1-avri.altman@wdc.com/
> 
> Cc: Avri Altman <avri.altman@wdc.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: linux-mmc@vger.kernel.org
> Cc: stable@vger.kernel.org
> Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>

Not used to seeing blank lines after Fixes:, Closes, Link: tags,
nevertheless:

Acked-by: Adrian Hunter <adrian.hunter@intel.com>

> ---
>  drivers/mmc/core/block.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 32d49100dff5..0df627de9cee 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -413,7 +413,7 @@ static struct mmc_blk_ioc_data *mmc_blk_ioctl_copy_from_user(
>  	struct mmc_blk_ioc_data *idata;
>  	int err;
>  
> -	idata = kmalloc(sizeof(*idata), GFP_KERNEL);
> +	idata = kzalloc(sizeof(*idata), GFP_KERNEL);
>  	if (!idata) {
>  		err = -ENOMEM;
>  		goto out;

RE: [PATCH 2/2] mmc core block.c: avoid negative index with array access

[Avri Altman]

> 
> On Wed, Mar 13, 2024 at 02:12:52PM +0000, Avri Altman wrote:
> > > -----Original Message-----
> > > From: mikko.rapeli@linaro.org <mikko.rapeli@linaro.org>
> > > Sent: Wednesday, March 13, 2024 3:38 PM
> > > To: linux-mmc@vger.kernel.org
> > > Cc: Mikko Rapeli <mikko.rapeli@linaro.org>; Avri Altman
> > > <Avri.Altman@wdc.com>; Ulf Hansson <ulf.hansson@linaro.org>; Adrian
> > > Hunter <adrian.hunter@intel.com>; stable@vger.kernel.org
> > > Subject: [PATCH 2/2] mmc core block.c: avoid negative index with
> > > array access
> > >
> > > CAUTION: This email originated from outside of Western Digital. Do
> > > not click on links or open attachments unless you recognize the
> > > sender and know that the content is safe.
> > >
> > >
> > > From: Mikko Rapeli <mikko.rapeli@linaro.org>
> > >
> > > Commit "mmc: core: Use mrq.sbc in close-ended ffu" assigns
> > > prev_idata = idatas[i - 1] but doesn't check that int iterator i is
> > > greater than zero. Add the check.
> > I don't think this is even possible given 1/2.
> 
> With RPMB ioctl:
> 
>         case MMC_DRV_OP_IOCTL_RPMB:
>                 idata = mq_rq->drv_op_data;
>                 for (i = 0, ret = 0; i < mq_rq->ioc_count; i++) {
>                         ret = __mmc_blk_ioctl_cmd(card, md, idata, i);
>                         if (ret)
>                                 break;
>                 }
> 
> First call is with i = 0?
I meant bogus MMC_BLK_IOC_SBC should not happened any more.
Anyway, that's fine - let's keep it also.

> 
> Cheers,
> 
> -Mikko
> 
> > Thanks,
> > Avri
> >
> > >
> > > Fixes: 4d0c8d0aef63 ("mmc: core: Use mrq.sbc in close-ended ffu")
> > >
> > > Link: https://lore.kernel.org/all/20231129092535.3278-1-
> > > avri.altman@wdc.com/
> > >
> > > Cc: Avri Altman <avri.altman@wdc.com>
> > > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > > Cc: Adrian Hunter <adrian.hunter@intel.com>
> > > Cc: linux-mmc@vger.kernel.org
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
Reviewed-by: Avri Altman <avri.altman@wdc.com>


> > > ---
> > >  drivers/mmc/core/block.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> > > index 0df627de9cee..7f275b4ca9fa 100644
> > > --- a/drivers/mmc/core/block.c
> > > +++ b/drivers/mmc/core/block.c
> > > @@ -488,7 +488,7 @@ static int __mmc_blk_ioctl_cmd(struct mmc_card
> > > *card, struct mmc_blk_data *md,
> > >         if (idata->flags & MMC_BLK_IOC_DROP)
> > >                 return 0;
> > >
> > > -       if (idata->flags & MMC_BLK_IOC_SBC)
> > > +       if (idata->flags & MMC_BLK_IOC_SBC && i > 0)
> > >                 prev_idata = idatas[i - 1];
> > >
> > >         /*
> > > --
> > > 2.34.1
> >

Re: [PATCH 2/2] mmc core block.c: avoid negative index with array access

[Francesco Dolcini]

Hello Mikko and Avri,

On Wed, Mar 13, 2024 at 03:37:44PM +0200, mikko.rapeli@linaro.org wrote:
> From: Mikko Rapeli <mikko.rapeli@linaro.org>
> 
> Commit "mmc: core: Use mrq.sbc in close-ended ffu" assigns
> prev_idata = idatas[i - 1] but doesn't check that int iterator
> i is greater than zero. Add the check.
> 
> Fixes: 4d0c8d0aef63 ("mmc: core: Use mrq.sbc in close-ended ffu")
> 
> Link: https://lore.kernel.org/all/20231129092535.3278-1-avri.altman@wdc.com/
> 
> Cc: Avri Altman <avri.altman@wdc.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: linux-mmc@vger.kernel.org
> Cc: stable@vger.kernel.org
> Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>

I just had the following Oops

[   31.377291] Unable to handle kernel paging request at virtual address 0000fffffc386a14
[   31.385348] Mem abort info:
[   31.388136]   ESR = 0x0000000096000006
[   31.392338]   EC = 0x25: DABT (current EL), IL = 32 bits
[   31.397681]   SET = 0, FnV = 0
[   31.400730]   EA = 0, S1PTW = 0
[   31.405397]   FSC = 0x06: level 2 translation fault
[   31.410355] Data abort info:
[   31.413245]   ISV = 0, ISS = 0x00000006
[   31.417086]   CM = 0, WnR = 0
[   31.420049] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000084f89000
[   31.426552] [0000fffffc386a14] pgd=0800000084af2003, p4d=0800000084af2003, pud=0800000083ec0003, pmd=0000000000000000
[   31.437393] Internal error: Oops: 0000000096000006 [#1] PREEMPT SMP
[   31.443657] Modules linked in: crct10dif_ce ti_k3_r5_remoteproc virtio_rpmsg_bus rpmsg_ns rtc_ti_k3 ti_k3_m4_remoteproc ti_k3_common tidss drm_dma_helper mcrc sa2ul lontium_lt8912b tc358768 display_connector drm_kms_helper ina2xx syscopyarea sysfillrect sysimgblt fb_sys_fops spi_omap2_mcspi pwm_tiehrpwm drm lm75 drm_panel_orientation_quirks optee_rng rng_core
[   31.475530] CPU: 0 PID: 8 Comm: kworker/0:0H Not tainted 6.1.80+git.ba628d222cde #1
[   31.483179] Hardware name: Toradex Verdin AM62 on Verdin Development Board (DT)
[   31.490480] Workqueue: kblockd blk_mq_run_work_fn
[   31.495216] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   31.502172] pc : __mmc_blk_ioctl_cmd+0x12c/0x590
[   31.506795] lr : __mmc_blk_ioctl_cmd+0x2cc/0x590
[   31.511408] sp : ffff8000092a39e0
[   31.514717] x29: ffff8000092a3b50 x28: ffff8000092a3d28 x27: 0000000000000000
[   31.521853] x26: ffff80000a5a3cf0 x25: ffff000018bbb400 x24: 0000fffffc386a08
[   31.528989] x23: ffff000018a8b808 x22: 0000000000000000 x21: 00000000ffffffff
[   31.536124] x20: ffff000018a8b800 x19: ffff0000048c6680 x18: 0000000000000000
[   31.543260] x17: 0000000000000000 x16: 0000000000000000 x15: 0000146d78b52ba4
[   31.550394] x14: 0000000000000206 x13: 0000000000000001 x12: 0000000000000000
[   31.557529] x11: 0000000000000000 x10: 00000000000009b0 x9 : 0000000000000651
[   31.564664] x8 : ffff8000092a3ad8 x7 : 0000000000000000 x6 : 0000000000000000
[   31.571800] x5 : 0000000000000200 x4 : 0000000000000000 x3 : 00000000000003e8
[   31.578935] x2 : 0000000000000000 x1 : 000000000000001d x0 : 0000000000000017
[   31.586071] Call trace:
[   31.588513]  __mmc_blk_ioctl_cmd+0x12c/0x590
[   31.592782]  mmc_blk_mq_issue_rq+0x50c/0x920
[   31.597049]  mmc_mq_queue_rq+0x118/0x2ac
[   31.600970]  blk_mq_dispatch_rq_list+0x1a8/0x8b0
[   31.605588]  __blk_mq_sched_dispatch_requests+0xb8/0x164
[   31.610898]  blk_mq_sched_dispatch_requests+0x3c/0x80
[   31.615946]  __blk_mq_run_hw_queue+0x68/0xa0
[   31.620215]  blk_mq_run_work_fn+0x20/0x30
[   31.624223]  process_one_work+0x1d0/0x320
[   31.628238]  worker_thread+0x14c/0x444
[   31.631989]  kthread+0x10c/0x110
[   31.635219]  ret_from_fork+0x10/0x20
[   31.638801] Code: 12010000 2a010000 b90137e0 b4000078 (b9400f00)
[   31.644888] ---[ end trace 0000000000000000 ]---

From a quick look I assume that this is the exact same issue you are
fixing here, correct?

Francesco

RE: [PATCH 2/2] mmc core block.c: avoid negative index with array access

[Avri Altman]

> 
> Hello Mikko and Avri,
> 
> On Wed, Mar 13, 2024 at 03:37:44PM +0200, mikko.rapeli@linaro.org wrote:
> > From: Mikko Rapeli <mikko.rapeli@linaro.org>
> >
> > Commit "mmc: core: Use mrq.sbc in close-ended ffu" assigns prev_idata
> > = idatas[i - 1] but doesn't check that int iterator i is greater than
> > zero. Add the check.
> >
> > Fixes: 4d0c8d0aef63 ("mmc: core: Use mrq.sbc in close-ended ffu")
> >
> > Link:
> > https://lore.kernel.org/all/20231129092535.3278-1-avri.altman@wdc.com/
> >
> > Cc: Avri Altman <avri.altman@wdc.com>
> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > Cc: Adrian Hunter <adrian.hunter@intel.com>
> > Cc: linux-mmc@vger.kernel.org
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
> 
> I just had the following Oops
> 
> [   31.377291] Unable to handle kernel paging request at virtual address
> 0000fffffc386a14
> [   31.385348] Mem abort info:
> [   31.388136]   ESR = 0x0000000096000006
> [   31.392338]   EC = 0x25: DABT (current EL), IL = 32 bits
> [   31.397681]   SET = 0, FnV = 0
> [   31.400730]   EA = 0, S1PTW = 0
> [   31.405397]   FSC = 0x06: level 2 translation fault
> [   31.410355] Data abort info:
> [   31.413245]   ISV = 0, ISS = 0x00000006
> [   31.417086]   CM = 0, WnR = 0
> [   31.420049] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000084f89000
> [   31.426552] [0000fffffc386a14] pgd=0800000084af2003,
> p4d=0800000084af2003, pud=0800000083ec0003, pmd=0000000000000000
> [   31.437393] Internal error: Oops: 0000000096000006 [#1] PREEMPT SMP
> [   31.443657] Modules linked in: crct10dif_ce ti_k3_r5_remoteproc
> virtio_rpmsg_bus rpmsg_ns rtc_ti_k3 ti_k3_m4_remoteproc ti_k3_common
> tidss drm_dma_helper mcrc sa2ul lontium_lt8912b tc358768 display_connector
> drm_kms_helper ina2xx syscopyarea sysfillrect sysimgblt fb_sys_fops
> spi_omap2_mcspi pwm_tiehrpwm drm lm75 drm_panel_orientation_quirks
> optee_rng rng_core
> [   31.475530] CPU: 0 PID: 8 Comm: kworker/0:0H Not tainted
> 6.1.80+git.ba628d222cde #1
> [   31.483179] Hardware name: Toradex Verdin AM62 on Verdin Development
> Board (DT)
> [   31.490480] Workqueue: kblockd blk_mq_run_work_fn
> [   31.495216] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--
> )
> [   31.502172] pc : __mmc_blk_ioctl_cmd+0x12c/0x590
> [   31.506795] lr : __mmc_blk_ioctl_cmd+0x2cc/0x590
> [   31.511408] sp : ffff8000092a39e0
> [   31.514717] x29: ffff8000092a3b50 x28: ffff8000092a3d28 x27:
> 0000000000000000
> [   31.521853] x26: ffff80000a5a3cf0 x25: ffff000018bbb400 x24:
> 0000fffffc386a08
> [   31.528989] x23: ffff000018a8b808 x22: 0000000000000000 x21:
> 00000000ffffffff
> [   31.536124] x20: ffff000018a8b800 x19: ffff0000048c6680 x18:
> 0000000000000000
> [   31.543260] x17: 0000000000000000 x16: 0000000000000000 x15:
> 0000146d78b52ba4
> [   31.550394] x14: 0000000000000206 x13: 0000000000000001 x12:
> 0000000000000000
> [   31.557529] x11: 0000000000000000 x10: 00000000000009b0 x9 :
> 0000000000000651
> [   31.564664] x8 : ffff8000092a3ad8 x7 : 0000000000000000 x6 :
> 0000000000000000
> [   31.571800] x5 : 0000000000000200 x4 : 0000000000000000 x3 :
> 00000000000003e8
> [   31.578935] x2 : 0000000000000000 x1 : 000000000000001d x0 :
> 0000000000000017
> [   31.586071] Call trace:
> [   31.588513]  __mmc_blk_ioctl_cmd+0x12c/0x590
> [   31.592782]  mmc_blk_mq_issue_rq+0x50c/0x920
> [   31.597049]  mmc_mq_queue_rq+0x118/0x2ac
> [   31.600970]  blk_mq_dispatch_rq_list+0x1a8/0x8b0
> [   31.605588]  __blk_mq_sched_dispatch_requests+0xb8/0x164
> [   31.610898]  blk_mq_sched_dispatch_requests+0x3c/0x80
> [   31.615946]  __blk_mq_run_hw_queue+0x68/0xa0
> [   31.620215]  blk_mq_run_work_fn+0x20/0x30
> [   31.624223]  process_one_work+0x1d0/0x320
> [   31.628238]  worker_thread+0x14c/0x444
> [   31.631989]  kthread+0x10c/0x110
> [   31.635219]  ret_from_fork+0x10/0x20
> [   31.638801] Code: 12010000 2a010000 b90137e0 b4000078 (b9400f00)
> [   31.644888] ---[ end trace 0000000000000000 ]---
> 
> From a quick look I assume that this is the exact same issue you are fixing here,
> correct?
Probably.  Did you applied the patch and the issue persists?

Thanks,
Avri

> 
> Francesco

RE: [PATCH 2/2] mmc core block.c: avoid negative index with array access

[Francesco Dolcini]

Il 24 marzo 2024 19:51:19 CET, Avri Altman <Avri.Altman@wdc.com> ha scritto:
>> 
>> Hello Mikko and Avri,
>> 
>> On Wed, Mar 13, 2024 at 03:37:44PM +0200, mikko.rapeli@linaro.org wrote:
>> > From: Mikko Rapeli <mikko.rapeli@linaro.org>
>> >
>> > Commit "mmc: core: Use mrq.sbc in close-ended ffu" assigns prev_idata
>> > = idatas[i - 1] but doesn't check that int iterator i is greater than
>> > zero. Add the check.
>> >
>> > Fixes: 4d0c8d0aef63 ("mmc: core: Use mrq.sbc in close-ended ffu")
>> >
>> > Link:
>> > https://lore.kernel.org/all/20231129092535.3278-1-avri.altman@wdc.com/
>> >
>> > Cc: Avri Altman <avri.altman@wdc.com>
>> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
>> > Cc: Adrian Hunter <adrian.hunter@intel.com>
>> > Cc: linux-mmc@vger.kernel.org
>> > Cc: stable@vger.kernel.org
>> > Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
>> 
>> I just had the following Oops
>> 
>> [   31.377291] Unable to handle kernel paging request at virtual address
>> 0000fffffc386a14
>> [   31.385348] Mem abort info:
>> [   31.388136]   ESR = 0x0000000096000006
>> [   31.392338]   EC = 0x25: DABT (current EL), IL = 32 bits
>> [   31.397681]   SET = 0, FnV = 0
>> [   31.400730]   EA = 0, S1PTW = 0
>> [   31.405397]   FSC = 0x06: level 2 translation fault
>> [   31.410355] Data abort info:
>> [   31.413245]   ISV = 0, ISS = 0x00000006
>> [   31.417086]   CM = 0, WnR = 0
>> [   31.420049] user pgtable: 4k pages, 48-bit VAs, pgdp=0000000084f89000
>> [   31.426552] [0000fffffc386a14] pgd=0800000084af2003,
>> p4d=0800000084af2003, pud=0800000083ec0003, pmd=0000000000000000
>> [   31.437393] Internal error: Oops: 0000000096000006 [#1] PREEMPT SMP
>> [   31.443657] Modules linked in: crct10dif_ce ti_k3_r5_remoteproc
>> virtio_rpmsg_bus rpmsg_ns rtc_ti_k3 ti_k3_m4_remoteproc ti_k3_common
>> tidss drm_dma_helper mcrc sa2ul lontium_lt8912b tc358768 display_connector
>> drm_kms_helper ina2xx syscopyarea sysfillrect sysimgblt fb_sys_fops
>> spi_omap2_mcspi pwm_tiehrpwm drm lm75 drm_panel_orientation_quirks
>> optee_rng rng_core
>> [   31.475530] CPU: 0 PID: 8 Comm: kworker/0:0H Not tainted
>> 6.1.80+git.ba628d222cde #1
>> [   31.483179] Hardware name: Toradex Verdin AM62 on Verdin Development
>> Board (DT)
>> [   31.490480] Workqueue: kblockd blk_mq_run_work_fn
>> [   31.495216] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--
>> )
>> [   31.502172] pc : __mmc_blk_ioctl_cmd+0x12c/0x590
>> [   31.506795] lr : __mmc_blk_ioctl_cmd+0x2cc/0x590
>> [   31.511408] sp : ffff8000092a39e0
>> [   31.514717] x29: ffff8000092a3b50 x28: ffff8000092a3d28 x27:
>> 0000000000000000
>> [   31.521853] x26: ffff80000a5a3cf0 x25: ffff000018bbb400 x24:
>> 0000fffffc386a08
>> [   31.528989] x23: ffff000018a8b808 x22: 0000000000000000 x21:
>> 00000000ffffffff
>> [   31.536124] x20: ffff000018a8b800 x19: ffff0000048c6680 x18:
>> 0000000000000000
>> [   31.543260] x17: 0000000000000000 x16: 0000000000000000 x15:
>> 0000146d78b52ba4
>> [   31.550394] x14: 0000000000000206 x13: 0000000000000001 x12:
>> 0000000000000000
>> [   31.557529] x11: 0000000000000000 x10: 00000000000009b0 x9 :
>> 0000000000000651
>> [   31.564664] x8 : ffff8000092a3ad8 x7 : 0000000000000000 x6 :
>> 0000000000000000
>> [   31.571800] x5 : 0000000000000200 x4 : 0000000000000000 x3 :
>> 00000000000003e8
>> [   31.578935] x2 : 0000000000000000 x1 : 000000000000001d x0 :
>> 0000000000000017
>> [   31.586071] Call trace:
>> [   31.588513]  __mmc_blk_ioctl_cmd+0x12c/0x590
>> [   31.592782]  mmc_blk_mq_issue_rq+0x50c/0x920
>> [   31.597049]  mmc_mq_queue_rq+0x118/0x2ac
>> [   31.600970]  blk_mq_dispatch_rq_list+0x1a8/0x8b0
>> [   31.605588]  __blk_mq_sched_dispatch_requests+0xb8/0x164
>> [   31.610898]  blk_mq_sched_dispatch_requests+0x3c/0x80
>> [   31.615946]  __blk_mq_run_hw_queue+0x68/0xa0
>> [   31.620215]  blk_mq_run_work_fn+0x20/0x30
>> [   31.624223]  process_one_work+0x1d0/0x320
>> [   31.628238]  worker_thread+0x14c/0x444
>> [   31.631989]  kthread+0x10c/0x110
>> [   31.635219]  ret_from_fork+0x10/0x20
>> [   31.638801] Code: 12010000 2a010000 b90137e0 b4000078 (b9400f00)
>> [   31.644888] ---[ end trace 0000000000000000 ]---
>> 
>> From a quick look I assume that this is the exact same issue you are fixing here,
>> correct?
>Probably.  Did you applied the patch and the issue persists?

It's not systematic, probably it depends on what is in the memory at this negative indexed array.

I would try to confirm is the exact same issue tomorrow. Worth mentioning that on our system this leads to some pretty bad failures (data corruption).

Considering that the buggy commit was back ported to LTS kernel (6.1, in my case), we should have this into mainline as soon as possible, so that the fix can also be backported.

Francesco 

Re: [PATCH 1/2] mmc core block.c: initialize mmc_blk_ioc_data

[Francesco Dolcini]

On Wed, Mar 13, 2024 at 04:23:04PM +0200, Adrian Hunter wrote:
> On 13/03/24 15:37, mikko.rapeli@linaro.org wrote:
> > From: Mikko Rapeli <mikko.rapeli@linaro.org>
> > 
> > Commit "mmc: core: Use mrq.sbc in close-ended ffu" adds flags uint to
> > struct mmc_blk_ioc_data but it does not get initialized for RPMB ioctls
> > which now fail.
> > 
> > Fix this by always initializing the struct and flags to zero.
> > 
> > Fixes access to RPMB storage.
> > 
> > Fixes: 4d0c8d0aef63 ("mmc: core: Use mrq.sbc in close-ended ffu")
> > 
> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218587
> > 
> > Link: https://lore.kernel.org/all/20231129092535.3278-1-avri.altman@wdc.com/
> > 
> > Cc: Avri Altman <avri.altman@wdc.com>
> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > Cc: Adrian Hunter <adrian.hunter@intel.com>
> > Cc: linux-mmc@vger.kernel.org
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>
> 
> Not used to seeing blank lines after Fixes:, Closes, Link: tags,
> nevertheless:

From what I know no spaces in between the tags at the end of the commit
message is just required. Having empty line there might break some tooling and
automation.

> Acked-by: Adrian Hunter <adrian.hunter@intel.com>

Tested-by: Francesco Dolcini <francesco.dolcini@toradex.com>

Ulf, Adrian: the bug these 2 patches are fixing is now in LTS kernel, it
would be beneficial to have the fix in mainline ASAP.

Francesco

Re: [PATCH 2/2] mmc core block.c: avoid negative index with array access

[Francesco Dolcini]

On Wed, Mar 13, 2024 at 03:37:44PM +0200, mikko.rapeli@linaro.org wrote:
> From: Mikko Rapeli <mikko.rapeli@linaro.org>
> 
> Commit "mmc: core: Use mrq.sbc in close-ended ffu" assigns
> prev_idata = idatas[i - 1] but doesn't check that int iterator
> i is greater than zero. Add the check.
> 
> Fixes: 4d0c8d0aef63 ("mmc: core: Use mrq.sbc in close-ended ffu")
> 
No empty new line here.

> Link: https://lore.kernel.org/all/20231129092535.3278-1-avri.altman@wdc.com/
> 
No empty new line here.

> Cc: Avri Altman <avri.altman@wdc.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: linux-mmc@vger.kernel.org
> Cc: stable@vger.kernel.org
> Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>

Tested-by: Francesco Dolcini <francesco.dolcini@toradex.com>

Francesco

Re: [PATCH 1/2] mmc core block.c: initialize mmc_blk_ioc_data

[Ulf Hansson]

+ Francesco Dolcini

On Wed, 13 Mar 2024 at 14:57, <mikko.rapeli@linaro.org> wrote:
>
> From: Mikko Rapeli <mikko.rapeli@linaro.org>
>
> Commit "mmc: core: Use mrq.sbc in close-ended ffu" adds flags uint to
> struct mmc_blk_ioc_data but it does not get initialized for RPMB ioctls
> which now fail.
>
> Fix this by always initializing the struct and flags to zero.
>
> Fixes access to RPMB storage.
>
> Fixes: 4d0c8d0aef63 ("mmc: core: Use mrq.sbc in close-ended ffu")
>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218587
>
> Link: https://lore.kernel.org/all/20231129092535.3278-1-avri.altman@wdc.com/
>
> Cc: Avri Altman <avri.altman@wdc.com>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Cc: Adrian Hunter <adrian.hunter@intel.com>
> Cc: linux-mmc@vger.kernel.org
> Cc: stable@vger.kernel.org
> Signed-off-by: Mikko Rapeli <mikko.rapeli@linaro.org>

Both patch1 and patch2 applied fixes, thanks!

I took the liberty of updating the commit messages a bit and dropped
some of the unessarry newlines.

Kind regards
Uffe



> ---
>  drivers/mmc/core/block.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 32d49100dff5..0df627de9cee 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -413,7 +413,7 @@ static struct mmc_blk_ioc_data *mmc_blk_ioctl_copy_from_user(
>         struct mmc_blk_ioc_data *idata;
>         int err;
>
> -       idata = kmalloc(sizeof(*idata), GFP_KERNEL);
> +       idata = kzalloc(sizeof(*idata), GFP_KERNEL);
>         if (!idata) {
>                 err = -ENOMEM;
>                 goto out;
> --
> 2.34.1
>