| Message ID | 20240326022325.1785-1-greg.malysa@timesys.com |
|---|---|
| State | Accepted |
| Delegated to: | Jaehoon Chung |
| Headers | show |
| Series | mmc: sdhci: Fix potential ADMA descriptor table overflow | expand |
> -----Original Message----- > From: Greg Malysa <greg.malysa@timesys.com> > Sent: Tuesday, March 26, 2024 11:23 AM > To: u-boot@lists.denx.de; Peng Fan <peng.fan@nxp.com>; Jaehoon Chung <jh80.chung@samsung.com> > Cc: Ian Roberts <ian.roberts@timesys.com>; Nathan Barrett-Morrison <nathan.morrison@timesys.com>; Greg > Malysa <greg.malysa@timesys.com>; Sean Anderson <sean.anderson@seco.com>; Tom Rini > <trini@konsulko.com> > Subject: [PATCH] mmc: sdhci: Fix potential ADMA descriptor table overflow > > From: Ian Roberts <ian.roberts@timesys.com> > > Change ADMA_TABLE_NO_ENTRIES to round the division up to fully > contain CONFIG_SYS_MMC_MAX_BLK_COUNT, fixing potential buffer overflow > of the ADMA descriptor table. > > sdhci_prepare_adma_table() expecitily states it does _not_ check for > overflow as the descriptor table size is dependent on > CONFIG_SYS_MMC_MAX_BLK_COUNT. However, the ADMA_TABLE_NO_ENTRIES > calculation does not round up the divison, so with the current u-boot > defaults: > max_mmc_transfer = (CONFIG_SYS_MMC_MAX_BLK_COUNT * MMC_MAX_BLOCK_LEN) = > 65535 * 512 = 33553920 bytes. > ADMA_TABLE_NO_ENTRIES = max_mmc_transfer / ADMA_MAX_LEN = > 33553920 / 65532, which does not divide cleanly. > actual_max_transfer = ADMA_TABLE_NO_ENTRIES * ADMA_MAX_LEN = 512 * > 65532 = 33552384, which is smaller than max_mmc_transfer. > This can cause sdhci_prepare_adma_table() to write one extra > descriptor, overflowing the table when a transaction larger than > actual_max_transfer is issued. > > Co-developed-by: Nathan Barrett-Morrison <nathan.morrison@timesys.com> > Signed-off-by: Nathan Barrett-Morrison <nathan.morrison@timesys.com> > Signed-off-by: Greg Malysa <greg.malysa@timesys.com> > Signed-off-by: Ian Roberts <ian.roberts@timesys.com> Reviewed-by: Jaehoon Chung <jh80.chung@samsung.com> Best Regards, Jaehoon Chung > > --- > > > --- > include/sdhci.h | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/include/sdhci.h b/include/sdhci.h > index a1b74e3bd7..fbc0f0391c 100644 > --- a/include/sdhci.h > +++ b/include/sdhci.h > @@ -11,6 +11,7 @@ > > #include <linux/bitops.h> > #include <linux/types.h> > +#include <linux/kernel.h> > #include <asm/io.h> > #include <mmc.h> > #include <asm/gpio.h> > @@ -299,8 +300,8 @@ struct sdhci_ops { > #else > #define ADMA_DESC_LEN 8 > #endif > -#define ADMA_TABLE_NO_ENTRIES (CONFIG_SYS_MMC_MAX_BLK_COUNT * \ > - MMC_MAX_BLOCK_LEN) / ADMA_MAX_LEN > +#define ADMA_TABLE_NO_ENTRIES DIV_ROUND_UP(CONFIG_SYS_MMC_MAX_BLK_COUNT * \ > + MMC_MAX_BLOCK_LEN, ADMA_MAX_LEN) > > #define ADMA_TABLE_SZ (ADMA_TABLE_NO_ENTRIES * ADMA_DESC_LEN) > > -- > 2.43.2
diff --git a/include/sdhci.h b/include/sdhci.h index a1b74e3bd7..fbc0f0391c 100644 --- a/include/sdhci.h +++ b/include/sdhci.h @@ -11,6 +11,7 @@ #include <linux/bitops.h> #include <linux/types.h> +#include <linux/kernel.h> #include <asm/io.h> #include <mmc.h> #include <asm/gpio.h> @@ -299,8 +300,8 @@ struct sdhci_ops { #else #define ADMA_DESC_LEN 8 #endif -#define ADMA_TABLE_NO_ENTRIES (CONFIG_SYS_MMC_MAX_BLK_COUNT * \ - MMC_MAX_BLOCK_LEN) / ADMA_MAX_LEN +#define ADMA_TABLE_NO_ENTRIES DIV_ROUND_UP(CONFIG_SYS_MMC_MAX_BLK_COUNT * \ + MMC_MAX_BLOCK_LEN, ADMA_MAX_LEN) #define ADMA_TABLE_SZ (ADMA_TABLE_NO_ENTRIES * ADMA_DESC_LEN)