| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAPDyKFqRaiE5=9FCs-vj9y62QQormLuAvCfPvxhEL3zbsaBQ7A@mail.gmail.com>
Date: Tue, 25 Aug 2015 14:05:32 +0200
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Haibo Chen <haibo.chen@...escale.com>
Cc: labbott@...hat.com, Adrian Hunter <adrian.hunter@...el.com>,
Dong Aisheng <aisheng.dong@...escale.com>,
Jiri Slaby <jslaby@...e.cz>,
linux-mmc <linux-mmc@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mmc: sdhci: fix dma memory leak in sdhci_pre_req()
On 25 August 2015 at 04:02, Haibo Chen <haibo.chen@...escale.com> wrote:
> Currently one mrq->data maybe execute dma_map_sg() twice
> when mmc subsystem prepare over one new request, and the
> following log show up:
> sdhci[sdhci_pre_dma_transfer] invalid cookie: 24, next-cookie 25
>
> In this condition, mrq->date map a dma-memory(1) in sdhci_pre_req
> for the first time, and map another dma-memory(2) in sdhci_prepare_data
> for the second time. But driver only unmap the dma-memory(2), and
> dma-memory(1) never unmapped, which cause the dma memory leak issue.
>
> This patch use another method to map the dma memory for the mrq->data
> which can fix this dma memory leak issue.
>
> Fixes: commit 348487cb28e66b0 ("mmc: sdhci: use pipeline mmc requests to improve performance")
I removed "commit" and used 12 digits of the commit hash.
> Cc: stable@...r.kernel.org # 4.0+
As you already have the fixed commit hash, I removed the stable tag.
> Reported-and-tested-by: Jiri Slaby <jslaby@...e.cz>
> Signed-off-by: Haibo Chen <haibo.chen@...escale.com>
Thanks, applied for next!
Kind regards
Uffe
> ---
> drivers/mmc/host/sdhci.c | 67 ++++++++++++++++++------------------------------
> drivers/mmc/host/sdhci.h | 8 +++---
> 2 files changed, 29 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> index c83d110..8d2864b 100644
> --- a/drivers/mmc/host/sdhci.c
> +++ b/drivers/mmc/host/sdhci.c
> @@ -54,8 +54,7 @@ static void sdhci_finish_command(struct sdhci_host *);
> static int sdhci_execute_tuning(struct mmc_host *mmc, u32 opcode);
> static void sdhci_enable_preset_value(struct sdhci_host *host, bool enable);
> static int sdhci_pre_dma_transfer(struct sdhci_host *host,
> - struct mmc_data *data,
> - struct sdhci_host_next *next);
> + struct mmc_data *data);
> static int sdhci_do_get_cd(struct sdhci_host *host);
>
> #ifdef CONFIG_PM
> @@ -495,7 +494,7 @@ static int sdhci_adma_table_pre(struct sdhci_host *host,
> goto fail;
> BUG_ON(host->align_addr & host->align_mask);
>
> - host->sg_count = sdhci_pre_dma_transfer(host, data, NULL);
> + host->sg_count = sdhci_pre_dma_transfer(host, data);
> if (host->sg_count < 0)
> goto unmap_align;
>
> @@ -634,9 +633,11 @@ static void sdhci_adma_table_post(struct sdhci_host *host,
> }
> }
>
> - if (!data->host_cookie)
> + if (data->host_cookie == COOKIE_MAPPED) {
> dma_unmap_sg(mmc_dev(host->mmc), data->sg,
> data->sg_len, direction);
> + data->host_cookie = COOKIE_UNMAPPED;
> + }
> }
>
> static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
> @@ -832,7 +833,7 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
> } else {
> int sg_cnt;
>
> - sg_cnt = sdhci_pre_dma_transfer(host, data, NULL);
> + sg_cnt = sdhci_pre_dma_transfer(host, data);
> if (sg_cnt <= 0) {
> /*
> * This only happens when someone fed
> @@ -948,11 +949,13 @@ static void sdhci_finish_data(struct sdhci_host *host)
> if (host->flags & SDHCI_USE_ADMA)
> sdhci_adma_table_post(host, data);
> else {
> - if (!data->host_cookie)
> + if (data->host_cookie == COOKIE_MAPPED) {
> dma_unmap_sg(mmc_dev(host->mmc),
> data->sg, data->sg_len,
> (data->flags & MMC_DATA_READ) ?
> DMA_FROM_DEVICE : DMA_TO_DEVICE);
> + data->host_cookie = COOKIE_UNMAPPED;
> + }
> }
> }
>
> @@ -2105,49 +2108,36 @@ static void sdhci_post_req(struct mmc_host *mmc, struct mmc_request *mrq,
> struct mmc_data *data = mrq->data;
>
> if (host->flags & SDHCI_REQ_USE_DMA) {
> - if (data->host_cookie)
> + if (data->host_cookie == COOKIE_GIVEN ||
> + data->host_cookie == COOKIE_MAPPED)
> dma_unmap_sg(mmc_dev(host->mmc), data->sg, data->sg_len,
> data->flags & MMC_DATA_WRITE ?
> DMA_TO_DEVICE : DMA_FROM_DEVICE);
> - mrq->data->host_cookie = 0;
> + data->host_cookie = COOKIE_UNMAPPED;
> }
> }
>
> static int sdhci_pre_dma_transfer(struct sdhci_host *host,
> - struct mmc_data *data,
> - struct sdhci_host_next *next)
> + struct mmc_data *data)
> {
> int sg_count;
>
> - if (!next && data->host_cookie &&
> - data->host_cookie != host->next_data.cookie) {
> - pr_debug(DRIVER_NAME "[%s] invalid cookie: %d, next-cookie %d\n",
> - __func__, data->host_cookie, host->next_data.cookie);
> - data->host_cookie = 0;
> + if (data->host_cookie == COOKIE_MAPPED) {
> + data->host_cookie = COOKIE_GIVEN;
> + return data->sg_count;
> }
>
> - /* Check if next job is already prepared */
> - if (next ||
> - (!next && data->host_cookie != host->next_data.cookie)) {
> - sg_count = dma_map_sg(mmc_dev(host->mmc), data->sg,
> - data->sg_len,
> - data->flags & MMC_DATA_WRITE ?
> - DMA_TO_DEVICE : DMA_FROM_DEVICE);
> -
> - } else {
> - sg_count = host->next_data.sg_count;
> - host->next_data.sg_count = 0;
> - }
> + WARN_ON(data->host_cookie == COOKIE_GIVEN);
>
> + sg_count = dma_map_sg(mmc_dev(host->mmc), data->sg, data->sg_len,
> + data->flags & MMC_DATA_WRITE ?
> + DMA_TO_DEVICE : DMA_FROM_DEVICE);
>
> if (sg_count == 0)
> - return -EINVAL;
> + return -ENOSPC;
>
> - if (next) {
> - next->sg_count = sg_count;
> - data->host_cookie = ++next->cookie < 0 ? 1 : next->cookie;
> - } else
> - host->sg_count = sg_count;
> + data->sg_count = sg_count;
> + data->host_cookie = COOKIE_MAPPED;
>
> return sg_count;
> }
> @@ -2157,16 +2147,10 @@ static void sdhci_pre_req(struct mmc_host *mmc, struct mmc_request *mrq,
> {
> struct sdhci_host *host = mmc_priv(mmc);
>
> - if (mrq->data->host_cookie) {
> - mrq->data->host_cookie = 0;
> - return;
> - }
> + mrq->data->host_cookie = COOKIE_UNMAPPED;
>
> if (host->flags & SDHCI_REQ_USE_DMA)
> - if (sdhci_pre_dma_transfer(host,
> - mrq->data,
> - &host->next_data) < 0)
> - mrq->data->host_cookie = 0;
> + sdhci_pre_dma_transfer(host, mrq->data);
> }
>
> static void sdhci_card_event(struct mmc_host *mmc)
> @@ -3038,7 +3022,6 @@ int sdhci_add_host(struct sdhci_host *host)
> host->max_clk = host->ops->get_max_clock(host);
> }
>
> - host->next_data.cookie = 1;
> /*
> * In case of Host Controller v3.00, find out whether clock
> * multiplier is supported.
> diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
> index 67046ca..7c02ff4 100644
> --- a/drivers/mmc/host/sdhci.h
> +++ b/drivers/mmc/host/sdhci.h
> @@ -309,9 +309,10 @@ struct sdhci_adma2_64_desc {
> */
> #define SDHCI_MAX_SEGS 128
>
> -struct sdhci_host_next {
> - unsigned int sg_count;
> - s32 cookie;
> +enum sdhci_cookie {
> + COOKIE_UNMAPPED,
> + COOKIE_MAPPED,
> + COOKIE_GIVEN,
> };
>
> struct sdhci_host {
> @@ -505,7 +506,6 @@ struct sdhci_host {
> unsigned int tuning_mode; /* Re-tuning mode supported by host */
> #define SDHCI_TUNING_MODE_1 0
>
> - struct sdhci_host_next next_data;
> unsigned long private[0] ____cacheline_aligned;
> };
>
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists