* [PATCH 1/4] mmc: sdio: Fix potential NULL pointer error in mmc_sdio_init_card()
2020-04-30 9:16 [PATCH 0/4] mmc: sdio: Fix various issues in mmc_sdio_init_card() Ulf Hansson
@ 2020-04-30 9:16 ` Ulf Hansson
2020-04-30 9:16 ` [PATCH 2/4] mmc: sdio: Fix several potential memory leaks " Ulf Hansson
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Ulf Hansson @ 2020-04-30 9:16 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson
Cc: Adrian Hunter, Douglas Anderson, Matthias Kaehlcke, Shawn Lin,
Yong Mao, Chaotian Jing, stable
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/4] mmc: sdio: Fix several potential memory leaks in mmc_sdio_init_card()
2020-04-30 9:16 [PATCH 0/4] mmc: sdio: Fix various issues in mmc_sdio_init_card() Ulf Hansson
2020-04-30 9:16 ` [PATCH 1/4] mmc: sdio: Fix potential NULL pointer error " Ulf Hansson
@ 2020-04-30 9:16 ` Ulf Hansson
2020-05-01 2:55 ` Sasha Levin
2020-04-30 9:16 ` [PATCH 3/4] mmc: sdio: Re-use negotiated OCR mask when re-sending CMD8 Ulf Hansson
2020-04-30 9:16 ` [PATCH 4/4] mmc: sdio: Align the initialization commands in retry path for UHS-I Ulf Hansson
3 siblings, 1 reply; 6+ messages in thread
From: Ulf Hansson @ 2020-04-30 9:16 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson
Cc: Adrian Hunter, Douglas Anderson, Matthias Kaehlcke, Shawn Lin,
Yong Mao, Chaotian Jing, stable
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/4] mmc: sdio: Fix several potential memory leaks in mmc_sdio_init_card()
2020-04-30 9:16 ` [PATCH 2/4] mmc: sdio: Fix several potential memory leaks " Ulf Hansson
@ 2020-05-01 2:55 ` Sasha Levin
0 siblings, 0 replies; 6+ messages in thread
From: Sasha Levin @ 2020-05-01 2:55 UTC (permalink / raw)
To: Sasha Levin, Ulf Hansson, linux-mmc, Ulf Hansson
Cc: Adrian Hunter, stable, stable
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/4] mmc: sdio: Re-use negotiated OCR mask when re-sending CMD8
2020-04-30 9:16 [PATCH 0/4] mmc: sdio: Fix various issues in mmc_sdio_init_card() Ulf Hansson
2020-04-30 9:16 ` [PATCH 1/4] mmc: sdio: Fix potential NULL pointer error " Ulf Hansson
2020-04-30 9:16 ` [PATCH 2/4] mmc: sdio: Fix several potential memory leaks " Ulf Hansson
@ 2020-04-30 9:16 ` Ulf Hansson
2020-04-30 9:16 ` [PATCH 4/4] mmc: sdio: Align the initialization commands in retry path for UHS-I Ulf Hansson
3 siblings, 0 replies; 6+ messages in thread
From: Ulf Hansson @ 2020-04-30 9:16 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson
Cc: Adrian Hunter, Douglas Anderson, Matthias Kaehlcke, Shawn Lin,
Yong Mao, Chaotian Jing
While initializing an SDIO card in mmc_sdio_init_card(), we may need to
retry the UHS-I specific initialization, in case the first attempt fails.
This leads to resending a CMD8, but also to restart from scratch with the
so called OCR mask negotiations. This is unnecessary as we already have a
negotiated OCR mask, so let's use that instead. In this way, the behaviour
also becomes more consistent with other similar paths.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/core/sdio.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 20eed28ea60d..853ac65f0485 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -543,12 +543,12 @@ static int mmc_sdio_init_uhs_card(struct mmc_card *card)
return err;
}
-static void mmc_sdio_resend_if_cond(struct mmc_host *host,
+static void mmc_sdio_resend_if_cond(struct mmc_host *host, u32 ocr,
struct mmc_card *card)
{
sdio_reset(host);
mmc_go_idle(host);
- mmc_send_if_cond(host, host->ocr_avail);
+ mmc_send_if_cond(host, ocr);
mmc_remove_card(card);
}
@@ -640,7 +640,7 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
if (rocr & ocr & R4_18V_PRESENT) {
err = mmc_set_uhs_voltage(host, ocr_card);
if (err == -EAGAIN) {
- mmc_sdio_resend_if_cond(host, card);
+ mmc_sdio_resend_if_cond(host, ocr_card, card);
retries--;
goto try_again;
} else if (err) {
@@ -712,7 +712,7 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
*/
err = sdio_read_cccr(card, ocr);
if (err) {
- mmc_sdio_resend_if_cond(host, card);
+ mmc_sdio_resend_if_cond(host, ocr_card, card);
if (ocr & R4_18V_PRESENT) {
/* Retry init sequence, but without R4_18V_PRESENT. */
retries = 0;
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/4] mmc: sdio: Align the initialization commands in retry path for UHS-I
2020-04-30 9:16 [PATCH 0/4] mmc: sdio: Fix various issues in mmc_sdio_init_card() Ulf Hansson
` (2 preceding siblings ...)
2020-04-30 9:16 ` [PATCH 3/4] mmc: sdio: Re-use negotiated OCR mask when re-sending CMD8 Ulf Hansson
@ 2020-04-30 9:16 ` Ulf Hansson
3 siblings, 0 replies; 6+ messages in thread
From: Ulf Hansson @ 2020-04-30 9:16 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson
Cc: Adrian Hunter, Douglas Anderson, Matthias Kaehlcke, Shawn Lin,
Yong Mao, Chaotian Jing
According to the comment in mmc_sdio_reinit_card(), some SDIO cards may
require a "[CMD5,5,3,7] init sequence", which isn't always obeyed in
mmc_sdio_init_card(). Especially, when we end up retrying the UHS-I
specific initialization, there is a missing CMD5.
Let's update the code to make the behaviour consistent and let's also take
the opportunity to clean up the code a bit, to avoid open coding.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/core/sdio.c | 53 ++++++++++++++++++++---------------------
1 file changed, 26 insertions(+), 27 deletions(-)
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 853ac65f0485..435de47a6ee0 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -543,13 +543,33 @@ static int mmc_sdio_init_uhs_card(struct mmc_card *card)
return err;
}
-static void mmc_sdio_resend_if_cond(struct mmc_host *host, u32 ocr,
- struct mmc_card *card)
+static int mmc_sdio_pre_init(struct mmc_host *host, u32 ocr,
+ struct mmc_card *card)
{
+ if (card)
+ mmc_remove_card(card);
+
+ /*
+ * Reset the card by performing the same steps that are taken by
+ * mmc_rescan_try_freq() and mmc_attach_sdio() during a "normal" probe.
+ *
+ * sdio_reset() is technically not needed. Having just powered up the
+ * hardware, it should already be in reset state. However, some
+ * platforms (such as SD8686 on OLPC) do not instantly cut power,
+ * meaning that a reset is required when restoring power soon after
+ * powering off. It is harmless in other cases.
+ *
+ * The CMD5 reset (mmc_send_io_op_cond()), according to the SDIO spec,
+ * is not necessary for non-removable cards. However, it is required
+ * for OLPC SD8686 (which expects a [CMD5,5,3,7] init sequence), and
+ * harmless in other situations.
+ *
+ */
+
sdio_reset(host);
mmc_go_idle(host);
mmc_send_if_cond(host, ocr);
- mmc_remove_card(card);
+ return mmc_send_io_op_cond(host, 0, NULL);
}
/*
@@ -640,7 +660,7 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
if (rocr & ocr & R4_18V_PRESENT) {
err = mmc_set_uhs_voltage(host, ocr_card);
if (err == -EAGAIN) {
- mmc_sdio_resend_if_cond(host, ocr_card, card);
+ mmc_sdio_pre_init(host, ocr_card, card);
retries--;
goto try_again;
} else if (err) {
@@ -712,7 +732,7 @@ static int mmc_sdio_init_card(struct mmc_host *host, u32 ocr,
*/
err = sdio_read_cccr(card, ocr);
if (err) {
- mmc_sdio_resend_if_cond(host, ocr_card, card);
+ mmc_sdio_pre_init(host, ocr_card, card);
if (ocr & R4_18V_PRESENT) {
/* Retry init sequence, but without R4_18V_PRESENT. */
retries = 0;
@@ -813,28 +833,7 @@ static int mmc_sdio_reinit_card(struct mmc_host *host)
{
int ret;
- /*
- * Reset the card by performing the same steps that are taken by
- * mmc_rescan_try_freq() and mmc_attach_sdio() during a "normal" probe.
- *
- * sdio_reset() is technically not needed. Having just powered up the
- * hardware, it should already be in reset state. However, some
- * platforms (such as SD8686 on OLPC) do not instantly cut power,
- * meaning that a reset is required when restoring power soon after
- * powering off. It is harmless in other cases.
- *
- * The CMD5 reset (mmc_send_io_op_cond()), according to the SDIO spec,
- * is not necessary for non-removable cards. However, it is required
- * for OLPC SD8686 (which expects a [CMD5,5,3,7] init sequence), and
- * harmless in other situations.
- *
- */
-
- sdio_reset(host);
- mmc_go_idle(host);
- mmc_send_if_cond(host, host->card->ocr);
-
- ret = mmc_send_io_op_cond(host, 0, NULL);
+ ret = mmc_sdio_pre_init(host, host->card->ocr, NULL);
if (ret)
return ret;
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread