[PATCH 1/4] mmc: sdio: Fix potential NULL pointer error in mmc_sdio_init_card()

[PATCH 1/4] mmc: sdio: Fix potential NULL pointer error in mmc_sdio_init_card()

[Ulf Hansson]

During some scenarios mmc_sdio_init_card() runs a retry path for the UHS-I
specific initialization, which leads to removal of the previously allocated
card. A new card is then re-allocated while retrying.

However, in one of the corresponding error paths we may end up to remove an
already removed card, which likely leads to a NULL pointer exception. So,
let's fix this.

Fixes: 5fc3d80ef496 ("mmc: sdio: don't use rocr to check if the card could support UHS mode")
Cc: <stable@vger.kernel.org>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/core/sdio.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index ebb387aa5158..d35679e6e6aa 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -718,9 +718,8 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
 			/* Retry init sequence, but without R4_18V_PRESENT. */
 			retries = 0;
 			goto try_again;
-		} else {
-			goto remove;
 		}
+		return err;
 	}
 
 	/*
-- 
2.20.1

[PATCH 2/4] mmc: sdio: Fix several potential memory leaks in mmc_sdio_init_card()

[Ulf Hansson]

Over the years, the code in mmc_sdio_init_card() has grown to become quite
messy. Unfortunate this has also lead to that several paths are leaking
memory in form of an allocated struct mmc_card, which includes additional
data, such as initialized struct device for example.

Unfortunate, it's a too complex task find each offending commit. Therefore,
this change fixes all memory leaks at once.

Cc: <stable@vger.kernel.org>
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/core/sdio.c | 58 +++++++++++++++++++----------------------
 1 file changed, 27 insertions(+), 31 deletions(-)

diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index d35679e6e6aa..20eed28ea60d 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -584,7 +584,7 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
 	 */
 	err = mmc_send_io_op_cond(host, ocr, &rocr);
 	if (err)
-		goto err;
+		return err;
 
 	/*
 	 * For SPI, enable CRC as appropriate.
@@ -592,17 +592,15 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
 	if (mmc_host_is_spi(host)) {
 		err = mmc_spi_set_crc(host, use_spi_crc);
 		if (err)
-			goto err;
+			return err;
 	}
 
 	/*
 	 * Allocate card structure.
 	 */
 	card = mmc_alloc_card(host, NULL);
-	if (IS_ERR(card)) {
-		err = PTR_ERR(card);
-		goto err;
-	}
+	if (IS_ERR(card))
+		return PTR_ERR(card);
 
 	if ((rocr & R4_MEMORY_PRESENT) &&
 	    mmc_sd_get_cid(host, ocr & rocr, card->raw_cid, NULL) == 0) {
@@ -610,19 +608,15 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
 
 		if (oldcard && (oldcard->type != MMC_TYPE_SD_COMBO ||
 		    memcmp(card->raw_cid, oldcard->raw_cid, sizeof(card->raw_cid)) != 0)) {
-			mmc_remove_card(card);
-			pr_debug("%s: Perhaps the card was replaced\n",
-				mmc_hostname(host));
-			return -ENOENT;
+			err = -ENOENT;
+			goto mismatch;
 		}
 	} else {
 		card->type = MMC_TYPE_SDIO;
 
 		if (oldcard && oldcard->type != MMC_TYPE_SDIO) {
-			mmc_remove_card(card);
-			pr_debug("%s: Perhaps the card was replaced\n",
-				mmc_hostname(host));
-			return -ENOENT;
+			err = -ENOENT;
+			goto mismatch;
 		}
 	}
 
@@ -677,7 +671,7 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
 	if (!oldcard && card->type == MMC_TYPE_SD_COMBO) {
 		err = mmc_sd_get_csd(host, card);
 		if (err)
-			return err;
+			goto remove;
 
 		mmc_decode_cid(card);
 	}
@@ -704,7 +698,12 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
 			mmc_set_timing(card->host, MMC_TIMING_SD_HS);
 		}
 
-		goto finish;
+		if (oldcard)
+			mmc_remove_card(card);
+		else
+			host->card = card;
+
+		return 0;
 	}
 
 	/*
@@ -730,16 +729,14 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
 		goto remove;
 
 	if (oldcard) {
-		int same = (card->cis.vendor == oldcard->cis.vendor &&
-			    card->cis.device == oldcard->cis.device);
-		mmc_remove_card(card);
-		if (!same) {
-			pr_debug("%s: Perhaps the card was replaced\n",
-				mmc_hostname(host));
-			return -ENOENT;
+		if (card->cis.vendor == oldcard->cis.vendor &&
+		    card->cis.device == oldcard->cis.device) {
+			mmc_remove_card(card);
+			card = oldcard;
+		} else {
+			err = -ENOENT;
+			goto mismatch;
 		}
-
-		card = oldcard;
 	}
 	card->ocr = ocr_card;
 	mmc_fixup_device(card, sdio_fixup_methods);
@@ -800,16 +797,15 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
 		err = -EINVAL;
 		goto remove;
 	}
-finish:
-	if (!oldcard)
-		host->card = card;
+
+	host->card = card;
 	return 0;
 
+mismatch:
+	pr_debug("%s: Perhaps the card was replaced\n", mmc_hostname(host));
 remove:
-	if (!oldcard)
+	if (oldcard != card)
 		mmc_remove_card(card);
-
-err:
 	return err;
 }
 
-- 
2.20.1

Re: [PATCH 2/4] mmc: sdio: Fix several potential memory leaks in mmc_sdio_init_card()

[Sasha Levin]

Hi

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v5.6.7, v5.4.35, v4.19.118, v4.14.177, v4.9.220, v4.4.220.

v5.6.7: Build OK!
v5.4.35: Build OK!
v4.19.118: Failed to apply! Possible dependencies:
    099b64811609 ("mmc: core: Add a debug print when the card may have been replaced")
    3c30e73977e5 ("mmc: sdio: Drop unused in-parameter to mmc_sdio_reinit_card()")
    4aaaf3ab1509 ("mmc: sdio: Drop unused in-parameter from mmc_sdio_init_card()")
    6ebc581c3f9e ("mmc: sdio: Don't re-initialize powered-on removable SDIO cards at resume")
    7fbbe725378d ("mmc: sdio: Drop powered-on re-init at runtime resume and HW reset")

v4.14.177: Failed to apply! Possible dependencies:
    099b64811609 ("mmc: core: Add a debug print when the card may have been replaced")
    247cfe535575 ("mmc: core: Add capability to avoid 3.3V signaling")
    3a3db6030b64 ("mmc: core: Rename ->reset() bus ops to ->hw_reset()")
    3c30e73977e5 ("mmc: sdio: Drop unused in-parameter to mmc_sdio_reinit_card()")
    4aaaf3ab1509 ("mmc: sdio: Drop unused in-parameter from mmc_sdio_init_card()")
    6a11fc47f175 ("mmc: sd: Fix signal voltage when there is no power cycle")
    6ebc581c3f9e ("mmc: sdio: Don't re-initialize powered-on removable SDIO cards at resume")
    7405df4c79cd ("mmc: core: Implement ->sw_reset bus ops for SDIO")
    7fbbe725378d ("mmc: sdio: Drop powered-on re-init at runtime resume and HW reset")
    f690f4409ddd ("mmc: mmc: Enable CQE's")
    fb09f44e290b ("mmc: core: Re-factor some code for SDIO re-initialization")

v4.9.220: Failed to apply! Possible dependencies:
    066185d69063 ("mmc: core: First step in cleaning up private mmc header files")
    099b64811609 ("mmc: core: Add a debug print when the card may have been replaced")
    20348d1981da ("mmc: core: Make mmc_switch_status() available for mmc core")
    247cfe535575 ("mmc: core: Add capability to avoid 3.3V signaling")
    2ed573b603f7 ("mmc: core: Clarify usage of mmc_set_signal_voltage()")
    3a3db6030b64 ("mmc: core: Rename ->reset() bus ops to ->hw_reset()")
    3c30e73977e5 ("mmc: sdio: Drop unused in-parameter to mmc_sdio_reinit_card()")
    437590a123b6 ("mmc: core: Retry instead of ignore at CRC errors when polling for busy")
    4aaaf3ab1509 ("mmc: sdio: Drop unused in-parameter from mmc_sdio_init_card()")
    4facdde11394 ("mmc: core: Move public functions from card.h to private headers")
    55244c5659b5 ("mmc: core: Move public functions from core.h to private headers")
    625228fa3e01 ("mmc: core: Rename ignore_crc to retry_crc_err to reflect its purpose")
    6ebc581c3f9e ("mmc: sdio: Don't re-initialize powered-on removable SDIO cards at resume")
    70562644f4ee ("mmc: core: Don't use ->card_busy() and CMD13 in combination when polling")
    716bdb8953c7 ("mmc: core: Factor out code related to polling in __mmc_switch()")
    7405df4c79cd ("mmc: core: Implement ->sw_reset bus ops for SDIO")
    7fbbe725378d ("mmc: sdio: Drop powered-on re-init at runtime resume and HW reset")
    9d4579a85c84 ("mmc: mmc_test: Disable Command Queue while mmc_test is used")
    aa33ce3c411a ("mmc: core: Enable __mmc_switch() to change bus speed timing for the host")
    cb26ce069ffa ("mmc: core: Clarify code which deals with polling in __mmc_switch()")
    f690f4409ddd ("mmc: mmc: Enable CQE's")
    fb09f44e290b ("mmc: core: Re-factor some code for SDIO re-initialization")

v4.4.220: Failed to apply! Possible dependencies:
    066185d69063 ("mmc: core: First step in cleaning up private mmc header files")
    099b64811609 ("mmc: core: Add a debug print when the card may have been replaced")
    247cfe535575 ("mmc: core: Add capability to avoid 3.3V signaling")
    29eb7bd01e80 ("mmc: card: do away with indirection pointer")
    2ed573b603f7 ("mmc: core: Clarify usage of mmc_set_signal_voltage()")
    3a3db6030b64 ("mmc: core: Rename ->reset() bus ops to ->hw_reset()")
    3c30e73977e5 ("mmc: sdio: Drop unused in-parameter to mmc_sdio_reinit_card()")
    48ab086d262e ("mmc: block: add missing header dependencies")
    4aaaf3ab1509 ("mmc: sdio: Drop unused in-parameter from mmc_sdio_init_card()")
    4e6c71788d6b ("mmc: core: Do regular power cycle when lacking eMMC HW reset support")
    4facdde11394 ("mmc: core: Move public functions from card.h to private headers")
    55244c5659b5 ("mmc: core: Move public functions from core.h to private headers")
    5b96fea730ab ("mmc: pwrseq_simple: add to_pwrseq_simple() macro")
    6ebc581c3f9e ("mmc: sdio: Don't re-initialize powered-on removable SDIO cards at resume")
    7405df4c79cd ("mmc: core: Implement ->sw_reset bus ops for SDIO")
    7fbbe725378d ("mmc: sdio: Drop powered-on re-init at runtime resume and HW reset")
    81f351615772 ("xen/blkfront: separate per ring information out of device info")
    9d4579a85c84 ("mmc: mmc_test: Disable Command Queue while mmc_test is used")
    c2df40dfb8c0 ("drivers: use req op accessor")
    d97a1e5d7cd2 ("mmc: pwrseq: convert to proper platform device")
    f01b72d0fd53 ("mmc: pwrseq_emmc: add to_pwrseq_emmc() macro")
    f690f4409ddd ("mmc: mmc: Enable CQE's")
    fb09f44e290b ("mmc: core: Re-factor some code for SDIO re-initialization")
    ffedbd2210f2 ("mmc: pwrseq: constify mmc_pwrseq_ops structures")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

-- 
Thanks
Sasha