* [PATCH v2 2/2] mmc: cqhci: Fix a tiny potential memory leak on error condition
2019-02-25 19:22 [PATCH v2 1/2] mmc: cqhci: fix space allocated for transfer descriptor Alamy Liu
@ 2019-02-25 19:22 ` Alamy Liu
2019-02-27 13:44 ` Adrian Hunter
2019-02-27 14:11 ` Ulf Hansson
2019-02-27 13:34 ` [PATCH v2 1/2] mmc: cqhci: fix space allocated for transfer descriptor Adrian Hunter
2019-02-27 14:11 ` Ulf Hansson
2 siblings, 2 replies; 6+ messages in thread
From: Alamy Liu @ 2019-02-25 19:22 UTC (permalink / raw)
To: Adrian Hunter, Asutosh Das, Ritesh Harjani, Ulf Hansson
Cc: linux-mmc, linux-kernel, stable
Free up the allocated memory in the case of error return
--- code study ---
The value of mmc_host->cqe_enabled stays 'false'.
Thus, cqhci_disable (mmc_cqe_ops->cqe_disable) won't be called to
free the memory.
Also, cqhci_disable() seems to be designed to disable and free
all resources, not suitable to handle this corner case.
Fixes: a4080225f51d ("mmc: cqhci: support for command queue enabled host")
Signed-off-by: Alamy Liu <alamy.liu@gmail.com>
---
drivers/mmc/host/cqhci.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/cqhci.c b/drivers/mmc/host/cqhci.c
index 974997b6cb..795893ba47 100644
--- a/drivers/mmc/host/cqhci.c
+++ b/drivers/mmc/host/cqhci.c
@@ -218,12 +218,21 @@ static int cqhci_host_alloc_tdl(struct cqhci_host *cq_host)
cq_host->desc_size,
&cq_host->desc_dma_base,
GFP_KERNEL);
+ if (!cq_host->desc_base)
+ return -ENOMEM;
+
cq_host->trans_desc_base = dmam_alloc_coherent(mmc_dev(cq_host->mmc),
cq_host->data_size,
&cq_host->trans_desc_dma_base,
GFP_KERNEL);
- if (!cq_host->desc_base || !cq_host->trans_desc_base)
+ if (!cq_host->trans_desc_base) {
+ dmam_free_coherent(mmc_dev(cq_host->mmc), cq_host->desc_size,
+ cq_host->desc_base,
+ cq_host->desc_dma_base);
+ cq_host->desc_base = NULL;
+ cq_host->desc_dma_base = 0;
return -ENOMEM;
+ }
pr_debug("%s: cqhci: desc-base: 0x%p trans-base: 0x%p\n desc_dma 0x%llx trans_dma: 0x%llx\n",
mmc_hostname(cq_host->mmc), cq_host->desc_base, cq_host->trans_desc_base,
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 2/2] mmc: cqhci: Fix a tiny potential memory leak on error condition
2019-02-25 19:22 ` [PATCH v2 2/2] mmc: cqhci: Fix a tiny potential memory leak on error condition Alamy Liu
@ 2019-02-27 13:44 ` Adrian Hunter
2019-02-27 14:11 ` Ulf Hansson
1 sibling, 0 replies; 6+ messages in thread
From: Adrian Hunter @ 2019-02-27 13:44 UTC (permalink / raw)
To: Alamy Liu, Asutosh Das, Ritesh Harjani, Ulf Hansson
Cc: linux-mmc, linux-kernel, stable
On 25/02/19 9:22 PM, Alamy Liu wrote:
> Free up the allocated memory in the case of error return
>
> --- code study ---
> The value of mmc_host->cqe_enabled stays 'false'.
> Thus, cqhci_disable (mmc_cqe_ops->cqe_disable) won't be called to
> free the memory.
> Also, cqhci_disable() seems to be designed to disable and free
> all resources, not suitable to handle this corner case.
>
> Fixes: a4080225f51d ("mmc: cqhci: support for command queue enabled host")
> Signed-off-by: Alamy Liu <alamy.liu@gmail.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/mmc/host/cqhci.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/cqhci.c b/drivers/mmc/host/cqhci.c
> index 974997b6cb..795893ba47 100644
> --- a/drivers/mmc/host/cqhci.c
> +++ b/drivers/mmc/host/cqhci.c
> @@ -218,12 +218,21 @@ static int cqhci_host_alloc_tdl(struct cqhci_host *cq_host)
> cq_host->desc_size,
> &cq_host->desc_dma_base,
> GFP_KERNEL);
> + if (!cq_host->desc_base)
> + return -ENOMEM;
> +
> cq_host->trans_desc_base = dmam_alloc_coherent(mmc_dev(cq_host->mmc),
> cq_host->data_size,
> &cq_host->trans_desc_dma_base,
> GFP_KERNEL);
> - if (!cq_host->desc_base || !cq_host->trans_desc_base)
> + if (!cq_host->trans_desc_base) {
> + dmam_free_coherent(mmc_dev(cq_host->mmc), cq_host->desc_size,
> + cq_host->desc_base,
> + cq_host->desc_dma_base);
> + cq_host->desc_base = NULL;
> + cq_host->desc_dma_base = 0;
> return -ENOMEM;
> + }
>
> pr_debug("%s: cqhci: desc-base: 0x%p trans-base: 0x%p\n desc_dma 0x%llx trans_dma: 0x%llx\n",
> mmc_hostname(cq_host->mmc), cq_host->desc_base, cq_host->trans_desc_base,
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 2/2] mmc: cqhci: Fix a tiny potential memory leak on error condition
2019-02-25 19:22 ` [PATCH v2 2/2] mmc: cqhci: Fix a tiny potential memory leak on error condition Alamy Liu
2019-02-27 13:44 ` Adrian Hunter
@ 2019-02-27 14:11 ` Ulf Hansson
1 sibling, 0 replies; 6+ messages in thread
From: Ulf Hansson @ 2019-02-27 14:11 UTC (permalink / raw)
To: Alamy Liu
Cc: Adrian Hunter, Asutosh Das, Ritesh Harjani,
linux-mmc@vger.kernel.org, Linux Kernel Mailing List, # 4.0+
On Mon, 25 Feb 2019 at 20:22, Alamy Liu <alamy.liu@gmail.com> wrote:
>
> Free up the allocated memory in the case of error return
>
> --- code study ---
Using three dashes makes "git am" believe this hole text snippet is a
comment and drops it from the changelog. I fixed it up this time, no
worries.
> The value of mmc_host->cqe_enabled stays 'false'.
> Thus, cqhci_disable (mmc_cqe_ops->cqe_disable) won't be called to
> free the memory.
> Also, cqhci_disable() seems to be designed to disable and free
> all resources, not suitable to handle this corner case.
>
> Fixes: a4080225f51d ("mmc: cqhci: support for command queue enabled host")
> Signed-off-by: Alamy Liu <alamy.liu@gmail.com>
Applied for fixes and added a stable tag, thanks!
Kind regards
Uffe
> ---
> drivers/mmc/host/cqhci.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/cqhci.c b/drivers/mmc/host/cqhci.c
> index 974997b6cb..795893ba47 100644
> --- a/drivers/mmc/host/cqhci.c
> +++ b/drivers/mmc/host/cqhci.c
> @@ -218,12 +218,21 @@ static int cqhci_host_alloc_tdl(struct cqhci_host *cq_host)
> cq_host->desc_size,
> &cq_host->desc_dma_base,
> GFP_KERNEL);
> + if (!cq_host->desc_base)
> + return -ENOMEM;
> +
> cq_host->trans_desc_base = dmam_alloc_coherent(mmc_dev(cq_host->mmc),
> cq_host->data_size,
> &cq_host->trans_desc_dma_base,
> GFP_KERNEL);
> - if (!cq_host->desc_base || !cq_host->trans_desc_base)
> + if (!cq_host->trans_desc_base) {
> + dmam_free_coherent(mmc_dev(cq_host->mmc), cq_host->desc_size,
> + cq_host->desc_base,
> + cq_host->desc_dma_base);
> + cq_host->desc_base = NULL;
> + cq_host->desc_dma_base = 0;
> return -ENOMEM;
> + }
>
> pr_debug("%s: cqhci: desc-base: 0x%p trans-base: 0x%p\n desc_dma 0x%llx trans_dma: 0x%llx\n",
> mmc_hostname(cq_host->mmc), cq_host->desc_base, cq_host->trans_desc_base,
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] mmc: cqhci: fix space allocated for transfer descriptor
2019-02-25 19:22 [PATCH v2 1/2] mmc: cqhci: fix space allocated for transfer descriptor Alamy Liu
2019-02-25 19:22 ` [PATCH v2 2/2] mmc: cqhci: Fix a tiny potential memory leak on error condition Alamy Liu
@ 2019-02-27 13:34 ` Adrian Hunter
2019-02-27 14:11 ` Ulf Hansson
2 siblings, 0 replies; 6+ messages in thread
From: Adrian Hunter @ 2019-02-27 13:34 UTC (permalink / raw)
To: Alamy Liu, Asutosh Das, Ritesh Harjani, Ulf Hansson
Cc: linux-mmc, linux-kernel, stable
On 25/02/19 9:22 PM, Alamy Liu wrote:
> There is not enough space being allocated when DCMD is disabled.
>
> CQE_DCMD is not necessary to be enabled when CQE is enabled.
> (Software could halt CQE to send command)
>
> In the case that CQE_DCMD is not enabled, it still needs to allocate
> space for data transfer. For instance:
> CQE_DCMD is enabled: 31 slots space (one slot used by DCMD)
> CQE_DCMD is disabled: 32 slots space
>
> Fixes: a4080225f51d ("mmc: cqhci: support for command queue enabled host")
> Signed-off-by: Alamy Liu <alamy.liu@gmail.com>
Acked-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> drivers/mmc/host/cqhci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/cqhci.c b/drivers/mmc/host/cqhci.c
> index 4cc7863c13..974997b6cb 100644
> --- a/drivers/mmc/host/cqhci.c
> +++ b/drivers/mmc/host/cqhci.c
> @@ -202,7 +202,7 @@ static int cqhci_host_alloc_tdl(struct cqhci_host *cq_host)
> cq_host->desc_size = cq_host->slot_sz * cq_host->num_slots;
>
> cq_host->data_size = cq_host->trans_desc_len * cq_host->mmc->max_segs *
> - (cq_host->num_slots - 1);
> + cq_host->mmc->cqe_qdepth;
>
> pr_debug("%s: cqhci: desc_size: %zu data_sz: %zu slot-sz: %d\n",
> mmc_hostname(cq_host->mmc), cq_host->desc_size, cq_host->data_size,
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 1/2] mmc: cqhci: fix space allocated for transfer descriptor
2019-02-25 19:22 [PATCH v2 1/2] mmc: cqhci: fix space allocated for transfer descriptor Alamy Liu
2019-02-25 19:22 ` [PATCH v2 2/2] mmc: cqhci: Fix a tiny potential memory leak on error condition Alamy Liu
2019-02-27 13:34 ` [PATCH v2 1/2] mmc: cqhci: fix space allocated for transfer descriptor Adrian Hunter
@ 2019-02-27 14:11 ` Ulf Hansson
2 siblings, 0 replies; 6+ messages in thread
From: Ulf Hansson @ 2019-02-27 14:11 UTC (permalink / raw)
To: Alamy Liu
Cc: Adrian Hunter, Asutosh Das, Ritesh Harjani,
linux-mmc@vger.kernel.org, Linux Kernel Mailing List, # 4.0+
On Mon, 25 Feb 2019 at 20:22, Alamy Liu <alamy.liu@gmail.com> wrote:
>
> There is not enough space being allocated when DCMD is disabled.
>
> CQE_DCMD is not necessary to be enabled when CQE is enabled.
> (Software could halt CQE to send command)
>
> In the case that CQE_DCMD is not enabled, it still needs to allocate
> space for data transfer. For instance:
> CQE_DCMD is enabled: 31 slots space (one slot used by DCMD)
> CQE_DCMD is disabled: 32 slots space
>
> Fixes: a4080225f51d ("mmc: cqhci: support for command queue enabled host")
> Signed-off-by: Alamy Liu <alamy.liu@gmail.com>
Applied for fixes and added a stable tag, thanks!
Kind regards
Uffe
> ---
> drivers/mmc/host/cqhci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/cqhci.c b/drivers/mmc/host/cqhci.c
> index 4cc7863c13..974997b6cb 100644
> --- a/drivers/mmc/host/cqhci.c
> +++ b/drivers/mmc/host/cqhci.c
> @@ -202,7 +202,7 @@ static int cqhci_host_alloc_tdl(struct cqhci_host *cq_host)
> cq_host->desc_size = cq_host->slot_sz * cq_host->num_slots;
>
> cq_host->data_size = cq_host->trans_desc_len * cq_host->mmc->max_segs *
> - (cq_host->num_slots - 1);
> + cq_host->mmc->cqe_qdepth;
>
> pr_debug("%s: cqhci: desc_size: %zu data_sz: %zu slot-sz: %d\n",
> mmc_hostname(cq_host->mmc), cq_host->desc_size, cq_host->data_size,
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread