diff mbox series

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

Message ID 20240313133744.2405325-1-mikko.rapeli@linaro.org
State New
Headers show
Series [1/2] mmc core block.c: initialize mmc_blk_ioc_data | expand

Commit Message

Mikko Rapeli March 13, 2024, 1:37 p.m. UTC 
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(-)

Comments

Avri Altman March 13, 2024, 2:11 p.m. UTC | #1 
> 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
Avri Altman March 13, 2024, 2:12 p.m. UTC | #2 
> -----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
Mikko Rapeli March 13, 2024, 2:18 p.m. UTC | #3 
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
>
Adrian Hunter March 13, 2024, 2:23 p.m. UTC | #4 
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;
Avri Altman March 13, 2024, 2:24 p.m. UTC | #5 
> 
> 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
> >
Francesco Dolcini March 25, 2024, 9:30 a.m. UTC | #6 
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:
Ulf Hansson March 25, 2024, 1:18 p.m. UTC | #7 
+ 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
>
diff mbox series

Patch

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;