public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [PATCH] mmc: arm_pl180: Limit data transfer to U16_MAX
@ 2024-02-27 20:18 ` cmax
  2024-03-04 15:39   ` Lean Sheng Tan
                     ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: cmax @ 2024-02-27 20:18 UTC (permalink / raw)
  To: u-boot; +Cc: max, Peng Fan, Jaehoon Chung

From: max <cmax@mailbox.org>

Currently fetching files bigger that cause a data transfer greater than
U16_MAX fails.

The reason is that the specification defines the datalength register
as a 16 bit wide register, but in u-boot it is used as if it is an
32 bit register. Therefore values greater than U16_MAX cause an
infinite loop inside u-boot. U-boot expects to get more data from
interface/hardware then it will ever get and therefore inifintely waits
for more data that will never come.

Signed-off-by: max <cmax@mailbox.org>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/arm_pl180_mmci.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/mmc/arm_pl180_mmci.c b/drivers/mmc/arm_pl180_mmci.c
index 5cf5502ed5..af2f9a5a84 100644
--- a/drivers/mmc/arm_pl180_mmci.c
+++ b/drivers/mmc/arm_pl180_mmci.c
@@ -231,6 +231,7 @@ static int do_data_transfer(struct mmc *dev,
 	u32 blksz = 0;
 	u32 data_ctrl = 0;
 	u32 data_len = (u32) (data->blocks * data->blocksize);
+	assert(data_len < U16_MAX); // should be ensured by arm_pl180_get_b_max
 
 	if (!host->version2) {
 		blksz = (ffs(data->blocksize) - 1);
@@ -358,6 +359,14 @@ static int  host_set_ios(struct mmc *dev)
 	return 0;
 }
 
+static int arm_pl180_get_b_max(struct udevice *dev, void *dst, lbaint_t blkcnt)
+{
+	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
+	struct mmc *mmc = upriv->mmc;
+
+	return U16_MAX / mmc->read_bl_len;
+}
+
 #ifndef CONFIG_DM_MMC
 /* MMC uses open drain drivers in the enumeration phase */
 static int mmc_host_reset(struct mmc *dev)
@@ -373,6 +382,7 @@ static const struct mmc_ops arm_pl180_mmci_ops = {
 	.send_cmd = host_request,
 	.set_ios = host_set_ios,
 	.init = mmc_host_reset,
+	.get_b_max = arm_pl180_get_b_max,
 };
 
 /*
@@ -531,6 +541,7 @@ static const struct dm_mmc_ops arm_pl180_dm_mmc_ops = {
 	.send_cmd = dm_host_request,
 	.set_ios = dm_host_set_ios,
 	.get_cd = dm_mmc_getcd,
+	.get_b_max = arm_pl180_get_b_max,
 };
 
 static int arm_pl180_mmc_of_to_plat(struct udevice *dev)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] mmc: arm_pl180: Limit data transfer to U16_MAX
  2024-02-27 20:18 ` [PATCH] mmc: arm_pl180: Limit data transfer to U16_MAX cmax
@ 2024-03-04 15:39   ` Lean Sheng Tan
  2024-03-15 18:45     ` Lean Sheng Tan
  2024-04-03  1:24   ` Jaehoon Chung
                     ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Lean Sheng Tan @ 2024-03-04 15:39 UTC (permalink / raw)
  To: cmax; +Cc: u-boot, Peng Fan, Jaehoon Chung

Quick reminder:
Can anyone help to review this?
Thanks!

Best Regards,
*Lean Sheng Tan*



9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany
Email: sheng.tan@9elements.com
Phone: *+49 234 68 94 188 <+492346894188>*
Mobile: *+49 176 76 113842 <+4917676113842>*

Registered office: Bochum
Commercial register: Amtsgericht Bochum, HRB 17519
Management: Sebastian German, Eray Bazaar

Data protection information according to Art. 13 GDPR
<https://9elements.com/privacy>


On Tue, 27 Feb 2024 at 22:02, <cmax@mailbox.org> wrote:

> From: max <cmax@mailbox.org>
>
> Currently fetching files bigger that cause a data transfer greater than
> U16_MAX fails.
>
> The reason is that the specification defines the datalength register
> as a 16 bit wide register, but in u-boot it is used as if it is an
> 32 bit register. Therefore values greater than U16_MAX cause an
> infinite loop inside u-boot. U-boot expects to get more data from
> interface/hardware then it will ever get and therefore inifintely waits
> for more data that will never come.
>
> Signed-off-by: max <cmax@mailbox.org>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  drivers/mmc/arm_pl180_mmci.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/mmc/arm_pl180_mmci.c b/drivers/mmc/arm_pl180_mmci.c
> index 5cf5502ed5..af2f9a5a84 100644
> --- a/drivers/mmc/arm_pl180_mmci.c
> +++ b/drivers/mmc/arm_pl180_mmci.c
> @@ -231,6 +231,7 @@ static int do_data_transfer(struct mmc *dev,
>         u32 blksz = 0;
>         u32 data_ctrl = 0;
>         u32 data_len = (u32) (data->blocks * data->blocksize);
> +       assert(data_len < U16_MAX); // should be ensured by
> arm_pl180_get_b_max
>
>         if (!host->version2) {
>                 blksz = (ffs(data->blocksize) - 1);
> @@ -358,6 +359,14 @@ static int  host_set_ios(struct mmc *dev)
>         return 0;
>  }
>
> +static int arm_pl180_get_b_max(struct udevice *dev, void *dst, lbaint_t
> blkcnt)
> +{
> +       struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
> +       struct mmc *mmc = upriv->mmc;
> +
> +       return U16_MAX / mmc->read_bl_len;
> +}
> +
>  #ifndef CONFIG_DM_MMC
>  /* MMC uses open drain drivers in the enumeration phase */
>  static int mmc_host_reset(struct mmc *dev)
> @@ -373,6 +382,7 @@ static const struct mmc_ops arm_pl180_mmci_ops = {
>         .send_cmd = host_request,
>         .set_ios = host_set_ios,
>         .init = mmc_host_reset,
> +       .get_b_max = arm_pl180_get_b_max,
>  };
>
>  /*
> @@ -531,6 +541,7 @@ static const struct dm_mmc_ops arm_pl180_dm_mmc_ops = {
>         .send_cmd = dm_host_request,
>         .set_ios = dm_host_set_ios,
>         .get_cd = dm_mmc_getcd,
> +       .get_b_max = arm_pl180_get_b_max,
>  };
>
>  static int arm_pl180_mmc_of_to_plat(struct udevice *dev)
> --
> 2.43.0
>
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mmc: arm_pl180: Limit data transfer to U16_MAX
  2024-03-04 15:39   ` Lean Sheng Tan
@ 2024-03-15 18:45     ` Lean Sheng Tan
  0 siblings, 0 replies; 8+ messages in thread
From: Lean Sheng Tan @ 2024-03-15 18:45 UTC (permalink / raw)
  To: cmax; +Cc: Jaehoon Chung, Peng Fan, Simon Glass, u-boot

+ @Simon

Best Regards,
*Lean Sheng Tan*



9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany
Email: sheng.tan@9elements.com
Phone: *+49 234 68 94 188 <+492346894188>*
Mobile: *+49 176 76 113842 <+4917676113842>*

Registered office: Bochum
Commercial register: Amtsgericht Bochum, HRB 17519
Management: Sebastian German, Eray Bazaar

Data protection information according to Art. 13 GDPR
<https://9elements.com/privacy>


On Mon 4. Mar 2024 at 16:39, Lean Sheng Tan <sheng.tan@9elements.com> wrote:

> Quick reminder:
> Can anyone help to review this?
> Thanks!
>
> Best Regards,
> *Lean Sheng Tan*
>
>
>
> 9elements GmbH, Kortumstraße 19-21, 44787 Bochum, Germany
> Email: sheng.tan@9elements.com
> Phone: *+49 234 68 94 188 <+492346894188>*
> Mobile: *+49 176 76 113842 <+4917676113842>*
>
> Registered office: Bochum
> Commercial register: Amtsgericht Bochum, HRB 17519
> Management: Sebastian German, Eray Bazaar
>
> Data protection information according to Art. 13 GDPR
> <https://9elements.com/privacy>
>
>
> On Tue, 27 Feb 2024 at 22:02, <cmax@mailbox.org> wrote:
>
>> From: max <cmax@mailbox.org>
>>
>> Currently fetching files bigger that cause a data transfer greater than
>> U16_MAX fails.
>>
>> The reason is that the specification defines the datalength register
>> as a 16 bit wide register, but in u-boot it is used as if it is an
>> 32 bit register. Therefore values greater than U16_MAX cause an
>> infinite loop inside u-boot. U-boot expects to get more data from
>> interface/hardware then it will ever get and therefore inifintely waits
>> for more data that will never come.
>>
>> Signed-off-by: max <cmax@mailbox.org>
>> Cc: Peng Fan <peng.fan@nxp.com>
>> Cc: Jaehoon Chung <jh80.chung@samsung.com>
>> ---
>>  drivers/mmc/arm_pl180_mmci.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/mmc/arm_pl180_mmci.c b/drivers/mmc/arm_pl180_mmci.c
>> index 5cf5502ed5..af2f9a5a84 100644
>> --- a/drivers/mmc/arm_pl180_mmci.c
>> +++ b/drivers/mmc/arm_pl180_mmci.c
>> @@ -231,6 +231,7 @@ static int do_data_transfer(struct mmc *dev,
>>         u32 blksz = 0;
>>         u32 data_ctrl = 0;
>>         u32 data_len = (u32) (data->blocks * data->blocksize);
>> +       assert(data_len < U16_MAX); // should be ensured by
>> arm_pl180_get_b_max
>>
>>         if (!host->version2) {
>>                 blksz = (ffs(data->blocksize) - 1);
>> @@ -358,6 +359,14 @@ static int  host_set_ios(struct mmc *dev)
>>         return 0;
>>  }
>>
>> +static int arm_pl180_get_b_max(struct udevice *dev, void *dst, lbaint_t
>> blkcnt)
>> +{
>> +       struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
>> +       struct mmc *mmc = upriv->mmc;
>> +
>> +       return U16_MAX / mmc->read_bl_len;
>> +}
>> +
>>  #ifndef CONFIG_DM_MMC
>>  /* MMC uses open drain drivers in the enumeration phase */
>>  static int mmc_host_reset(struct mmc *dev)
>> @@ -373,6 +382,7 @@ static const struct mmc_ops arm_pl180_mmci_ops = {
>>         .send_cmd = host_request,
>>         .set_ios = host_set_ios,
>>         .init = mmc_host_reset,
>> +       .get_b_max = arm_pl180_get_b_max,
>>  };
>>
>>  /*
>> @@ -531,6 +541,7 @@ static const struct dm_mmc_ops arm_pl180_dm_mmc_ops =
>> {
>>         .send_cmd = dm_host_request,
>>         .set_ios = dm_host_set_ios,
>>         .get_cd = dm_mmc_getcd,
>> +       .get_b_max = arm_pl180_get_b_max,
>>  };
>>
>>  static int arm_pl180_mmc_of_to_plat(struct udevice *dev)
>> --
>> 2.43.0
>>
>>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mmc: arm_pl180: Limit data transfer to U16_MAX
  2024-02-27 20:18 ` [PATCH] mmc: arm_pl180: Limit data transfer to U16_MAX cmax
  2024-03-04 15:39   ` Lean Sheng Tan
@ 2024-04-03  1:24   ` Jaehoon Chung
  2024-04-04  6:58   ` [PATCH v2] " cmax
  2024-04-15  9:53   ` cmax
  3 siblings, 0 replies; 8+ messages in thread
From: Jaehoon Chung @ 2024-04-03  1:24 UTC (permalink / raw)
  To: cmax, u-boot; +Cc: Peng Fan

Hi,

On 2/28/24 05:18, cmax@mailbox.org wrote:
> From: max <cmax@mailbox.org>
> 
> Currently fetching files bigger that cause a data transfer greater than
> U16_MAX fails.
> 
> The reason is that the specification defines the datalength register
> as a 16 bit wide register, but in u-boot it is used as if it is an
> 32 bit register. Therefore values greater than U16_MAX cause an
> infinite loop inside u-boot. U-boot expects to get more data from
> interface/hardware then it will ever get and therefore inifintely waits
> for more data that will never come.
> 
> Signed-off-by: max <cmax@mailbox.org>

Could you add your full name as Signed-off's tag?

> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  drivers/mmc/arm_pl180_mmci.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/mmc/arm_pl180_mmci.c b/drivers/mmc/arm_pl180_mmci.c
> index 5cf5502ed5..af2f9a5a84 100644
> --- a/drivers/mmc/arm_pl180_mmci.c
> +++ b/drivers/mmc/arm_pl180_mmci.c
> @@ -231,6 +231,7 @@ static int do_data_transfer(struct mmc *dev,
>  	u32 blksz = 0;
>  	u32 data_ctrl = 0;
>  	u32 data_len = (u32) (data->blocks * data->blocksize);
> +	assert(data_len < U16_MAX); // should be ensured by arm_pl180_get_b_max

Add the comment at above with  "/* ... */"

/* Should be ensured by arm_pl180_get_b_max */
assert(data_len < U16_MAX);

Best Regards,
Jaehoon Chung

>  
>  	if (!host->version2) {
>  		blksz = (ffs(data->blocksize) - 1);
> @@ -358,6 +359,14 @@ static int  host_set_ios(struct mmc *dev)
>  	return 0;
>  }
>  
> +static int arm_pl180_get_b_max(struct udevice *dev, void *dst, lbaint_t blkcnt)
> +{
> +	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
> +	struct mmc *mmc = upriv->mmc;
> +
> +	return U16_MAX / mmc->read_bl_len;
> +}
> +
>  #ifndef CONFIG_DM_MMC
>  /* MMC uses open drain drivers in the enumeration phase */
>  static int mmc_host_reset(struct mmc *dev)
> @@ -373,6 +382,7 @@ static const struct mmc_ops arm_pl180_mmci_ops = {
>  	.send_cmd = host_request,
>  	.set_ios = host_set_ios,
>  	.init = mmc_host_reset,
> +	.get_b_max = arm_pl180_get_b_max,
>  };
>  
>  /*
> @@ -531,6 +541,7 @@ static const struct dm_mmc_ops arm_pl180_dm_mmc_ops = {
>  	.send_cmd = dm_host_request,
>  	.set_ios = dm_host_set_ios,
>  	.get_cd = dm_mmc_getcd,
> +	.get_b_max = arm_pl180_get_b_max,
>  };
>  
>  static int arm_pl180_mmc_of_to_plat(struct udevice *dev)


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] mmc: arm_pl180: Limit data transfer to U16_MAX
  2024-02-27 20:18 ` [PATCH] mmc: arm_pl180: Limit data transfer to U16_MAX cmax
  2024-03-04 15:39   ` Lean Sheng Tan
  2024-04-03  1:24   ` Jaehoon Chung
@ 2024-04-04  6:58   ` cmax
  2024-04-15  7:07     ` Jaehoon Chung
  2024-04-15  9:53   ` cmax
  3 siblings, 1 reply; 8+ messages in thread
From: cmax @ 2024-04-04  6:58 UTC (permalink / raw)
  To: u-boot; +Cc: Maximilian Brune, Peng Fan, Jaehoon Chung

From: Maximilian Brune <maximilian.brune@9elements.com>

Currently fetching files bigger that cause a data transfer greater than
U16_MAX fails.

The reason is that the specification defines the datalength register
as a 16 bit wide register, but in u-boot it is used as if it is an
32 bit register. Therefore values greater than U16_MAX cause an
infinite loop inside u-boot. U-boot expects to get more data from
interface/hardware then it will ever get and therefore inifintely waits
for more data that will never come.

Signed-off-by: Maximilian Brune <maximilian.brune@9elements.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/arm_pl180_mmci.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/mmc/arm_pl180_mmci.c b/drivers/mmc/arm_pl180_mmci.c
index 5cf5502ed5..cad73ea106 100644
--- a/drivers/mmc/arm_pl180_mmci.c
+++ b/drivers/mmc/arm_pl180_mmci.c
@@ -231,6 +231,7 @@ static int do_data_transfer(struct mmc *dev,
 	u32 blksz = 0;
 	u32 data_ctrl = 0;
 	u32 data_len = (u32) (data->blocks * data->blocksize);
+	assert(data_len < U16_MAX); /* should be ensured by arm_pl180_get_b_max */
 
 	if (!host->version2) {
 		blksz = (ffs(data->blocksize) - 1);
@@ -358,6 +359,14 @@ static int  host_set_ios(struct mmc *dev)
 	return 0;
 }
 
+static int arm_pl180_get_b_max(struct udevice *dev, void *dst, lbaint_t blkcnt)
+{
+	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
+	struct mmc *mmc = upriv->mmc;
+
+	return U16_MAX / mmc->read_bl_len;
+}
+
 #ifndef CONFIG_DM_MMC
 /* MMC uses open drain drivers in the enumeration phase */
 static int mmc_host_reset(struct mmc *dev)
@@ -373,6 +382,7 @@ static const struct mmc_ops arm_pl180_mmci_ops = {
 	.send_cmd = host_request,
 	.set_ios = host_set_ios,
 	.init = mmc_host_reset,
+	.get_b_max = arm_pl180_get_b_max,
 };
 
 /*
@@ -531,6 +541,7 @@ static const struct dm_mmc_ops arm_pl180_dm_mmc_ops = {
 	.send_cmd = dm_host_request,
 	.set_ios = dm_host_set_ios,
 	.get_cd = dm_mmc_getcd,
+	.get_b_max = arm_pl180_get_b_max,
 };
 
 static int arm_pl180_mmc_of_to_plat(struct udevice *dev)
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* RE: [PATCH v2] mmc: arm_pl180: Limit data transfer to U16_MAX
  2024-04-04  6:58   ` [PATCH v2] " cmax
@ 2024-04-15  7:07     ` Jaehoon Chung
  0 siblings, 0 replies; 8+ messages in thread
From: Jaehoon Chung @ 2024-04-15  7:07 UTC (permalink / raw)
  To: cmax, u-boot; +Cc: 'Maximilian Brune', 'Peng Fan'

Hi,

> -----Original Message-----
> From: cmax@mailbox.org <cmax@mailbox.org>
> Sent: Thursday, April 4, 2024 3:58 PM
> To: u-boot@lists.denx.de
> Cc: Maximilian Brune <maximilian.brune@9elements.com>; Peng Fan <peng.fan@nxp.com>; Jaehoon Chung
> <jh80.chung@samsung.com>
> Subject: [PATCH v2] mmc: arm_pl180: Limit data transfer to U16_MAX
> 
> From: Maximilian Brune <maximilian.brune@9elements.com>
> 
> Currently fetching files bigger that cause a data transfer greater than
> U16_MAX fails.
> 
> The reason is that the specification defines the datalength register
> as a 16 bit wide register, but in u-boot it is used as if it is an
> 32 bit register. Therefore values greater than U16_MAX cause an
> infinite loop inside u-boot. U-boot expects to get more data from
> interface/hardware then it will ever get and therefore inifintely waits
> for more data that will never come.
> 
> Signed-off-by: Maximilian Brune <maximilian.brune@9elements.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  drivers/mmc/arm_pl180_mmci.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/mmc/arm_pl180_mmci.c b/drivers/mmc/arm_pl180_mmci.c
> index 5cf5502ed5..cad73ea106 100644
> --- a/drivers/mmc/arm_pl180_mmci.c
> +++ b/drivers/mmc/arm_pl180_mmci.c
> @@ -231,6 +231,7 @@ static int do_data_transfer(struct mmc *dev,
>  	u32 blksz = 0;
>  	u32 data_ctrl = 0;
>  	u32 data_len = (u32) (data->blocks * data->blocksize);
> +	assert(data_len < U16_MAX); /* should be ensured by arm_pl180_get_b_max */
> 
>  	if (!host->version2) {
>  		blksz = (ffs(data->blocksize) - 1);
> @@ -358,6 +359,14 @@ static int  host_set_ios(struct mmc *dev)
>  	return 0;
>  }
> 
> +static int arm_pl180_get_b_max(struct udevice *dev, void *dst, lbaint_t blkcnt)
> +{
> +	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
> +	struct mmc *mmc = upriv->mmc;
> +
> +	return U16_MAX / mmc->read_bl_len;
> +}
> +
>  #ifndef CONFIG_DM_MMC
>  /* MMC uses open drain drivers in the enumeration phase */
>  static int mmc_host_reset(struct mmc *dev)
> @@ -373,6 +382,7 @@ static const struct mmc_ops arm_pl180_mmci_ops = {
>  	.send_cmd = host_request,
>  	.set_ios = host_set_ios,
>  	.init = mmc_host_reset,
> +	.get_b_max = arm_pl180_get_b_max,
>  };
> 
>  /*
> @@ -531,6 +541,7 @@ static const struct dm_mmc_ops arm_pl180_dm_mmc_ops = {
>  	.send_cmd = dm_host_request,
>  	.set_ios = dm_host_set_ios,
>  	.get_cd = dm_mmc_getcd,
> +	.get_b_max = arm_pl180_get_b_max,
>  };
> 
>  static int arm_pl180_mmc_of_to_plat(struct udevice *dev)
> --
> 2.44.0



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] mmc: arm_pl180: Limit data transfer to U16_MAX
  2024-02-27 20:18 ` [PATCH] mmc: arm_pl180: Limit data transfer to U16_MAX cmax
                     ` (2 preceding siblings ...)
  2024-04-04  6:58   ` [PATCH v2] " cmax
@ 2024-04-15  9:53   ` cmax
  2024-04-15 11:01     ` Jaehoon Chung
  3 siblings, 1 reply; 8+ messages in thread
From: cmax @ 2024-04-15  9:53 UTC (permalink / raw)
  To: u-boot; +Cc: Maximilian Brune, Peng Fan, Jaehoon Chung

From: Maximilian Brune <maximilian.brune@9elements.com>

Currently fetching files bigger that cause a data transfer greater than
U16_MAX fails.

The reason is that the specification defines the datalength register
as a 16 bit wide register, but in u-boot it is used as if it is an
32 bit register. Therefore values greater than U16_MAX cause an
infinite loop inside u-boot. U-boot expects to get more data from
interface/hardware then it will ever get and therefore inifintely waits
for more data that will never come.

Signed-off-by: Maximilian Brune <maximilian.brune@9elements.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Jaehoon Chung <jh80.chung@samsung.com>
---
 drivers/mmc/arm_pl180_mmci.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/mmc/arm_pl180_mmci.c b/drivers/mmc/arm_pl180_mmci.c
index 2666b65362..cecc7ad783 100644
--- a/drivers/mmc/arm_pl180_mmci.c
+++ b/drivers/mmc/arm_pl180_mmci.c
@@ -229,6 +229,7 @@ static int do_data_transfer(struct mmc *dev,
 	u32 blksz = 0;
 	u32 data_ctrl = 0;
 	u32 data_len = (u32) (data->blocks * data->blocksize);
+	assert(data_len < U16_MAX); /* should be ensured by arm_pl180_get_b_max */
 
 	if (!host->version2) {
 		blksz = (ffs(data->blocksize) - 1);
@@ -356,6 +357,14 @@ static int  host_set_ios(struct mmc *dev)
 	return 0;
 }
 
+static int arm_pl180_get_b_max(struct udevice *dev, void *dst, lbaint_t blkcnt)
+{
+	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
+	struct mmc *mmc = upriv->mmc;
+
+	return U16_MAX / mmc->read_bl_len;
+}
+
 static void arm_pl180_mmc_init(struct pl180_mmc_host *host)
 {
 	u32 sdi_u32;
@@ -470,6 +479,7 @@ static const struct dm_mmc_ops arm_pl180_dm_mmc_ops = {
 	.send_cmd = dm_host_request,
 	.set_ios = dm_host_set_ios,
 	.get_cd = dm_mmc_getcd,
+	.get_b_max = arm_pl180_get_b_max,
 };
 
 static int arm_pl180_mmc_of_to_plat(struct udevice *dev)
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* RE: [PATCH v2] mmc: arm_pl180: Limit data transfer to U16_MAX
  2024-04-15  9:53   ` cmax
@ 2024-04-15 11:01     ` Jaehoon Chung
  0 siblings, 0 replies; 8+ messages in thread
From: Jaehoon Chung @ 2024-04-15 11:01 UTC (permalink / raw)
  To: cmax, u-boot; +Cc: 'Maximilian Brune', 'Peng Fan'



> -----Original Message-----
> From: cmax@mailbox.org <cmax@mailbox.org>
> Sent: Monday, April 15, 2024 6:53 PM
> To: u-boot@lists.denx.de
> Cc: Maximilian Brune <maximilian.brune@9elements.com>; Peng Fan <peng.fan@nxp.com>; Jaehoon Chung
> <jh80.chung@samsung.com>
> Subject: [PATCH v2] mmc: arm_pl180: Limit data transfer to U16_MAX
> 
> From: Maximilian Brune <maximilian.brune@9elements.com>
> 
> Currently fetching files bigger that cause a data transfer greater than
> U16_MAX fails.
> 
> The reason is that the specification defines the datalength register
> as a 16 bit wide register, but in u-boot it is used as if it is an
> 32 bit register. Therefore values greater than U16_MAX cause an
> infinite loop inside u-boot. U-boot expects to get more data from
> interface/hardware then it will ever get and therefore inifintely waits
> for more data that will never come.
> 
> Signed-off-by: Maximilian Brune <maximilian.brune@9elements.com>

Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Jaehoon Chung <jh80.chung@samsung.com>
> ---
>  drivers/mmc/arm_pl180_mmci.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/mmc/arm_pl180_mmci.c b/drivers/mmc/arm_pl180_mmci.c
> index 2666b65362..cecc7ad783 100644
> --- a/drivers/mmc/arm_pl180_mmci.c
> +++ b/drivers/mmc/arm_pl180_mmci.c
> @@ -229,6 +229,7 @@ static int do_data_transfer(struct mmc *dev,
>  	u32 blksz = 0;
>  	u32 data_ctrl = 0;
>  	u32 data_len = (u32) (data->blocks * data->blocksize);
> +	assert(data_len < U16_MAX); /* should be ensured by arm_pl180_get_b_max */
> 
>  	if (!host->version2) {
>  		blksz = (ffs(data->blocksize) - 1);
> @@ -356,6 +357,14 @@ static int  host_set_ios(struct mmc *dev)
>  	return 0;
>  }
> 
> +static int arm_pl180_get_b_max(struct udevice *dev, void *dst, lbaint_t blkcnt)
> +{
> +	struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
> +	struct mmc *mmc = upriv->mmc;
> +
> +	return U16_MAX / mmc->read_bl_len;
> +}
> +
>  static void arm_pl180_mmc_init(struct pl180_mmc_host *host)
>  {
>  	u32 sdi_u32;
> @@ -470,6 +479,7 @@ static const struct dm_mmc_ops arm_pl180_dm_mmc_ops = {
>  	.send_cmd = dm_host_request,
>  	.set_ios = dm_host_set_ios,
>  	.get_cd = dm_mmc_getcd,
> +	.get_b_max = arm_pl180_get_b_max,
>  };
> 
>  static int arm_pl180_mmc_of_to_plat(struct udevice *dev)
> --
> 2.44.0



^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-04-15 11:02 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CGME20240403012406epcas1p2c14e01c3092a280bd459ea75bd2b6704@epcas1p2.samsung.com>
2024-02-27 20:18 ` [PATCH] mmc: arm_pl180: Limit data transfer to U16_MAX cmax
2024-03-04 15:39   ` Lean Sheng Tan
2024-03-15 18:45     ` Lean Sheng Tan
2024-04-03  1:24   ` Jaehoon Chung
2024-04-04  6:58   ` [PATCH v2] " cmax
2024-04-15  7:07     ` Jaehoon Chung
2024-04-15  9:53   ` cmax
2024-04-15 11:01     ` Jaehoon Chung

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox