From: Dan Carpenter <dan.carpenter@oracle.com>
To: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Cc: mustafa.ismail@intel.com, shiraz.saleem@intel.com, jgg@ziepe.ca,
linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org,
kernel-janitors@vger.kernel.org
Subject: Re: [PATCH] RDMA/irdma: Fix a potential memory allocation issue in 'irdma_prm_add_pble_mem()'
Date: Tue, 7 Dec 2021 16:14:28 +0300 [thread overview]
Message-ID: <20211207131428.GF1956@kadam> (raw)
In-Reply-To: <5e670b640508e14b1869c3e8e4fb970d78cbe997.1638692171.git.christophe.jaillet@wanadoo.fr>
On Sun, Dec 05, 2021 at 09:17:24AM +0100, Christophe JAILLET wrote:
> @@ -299,8 +298,7 @@ add_pble_prm(struct irdma_hmc_pble_rsrc *pble_rsrc)
> return 0;
>
> error:
> - if (chunk->bitmapbuf)
> - kfree(chunk->bitmapmem.va);
> + bitmap_free(chunk->bitmapbuf);
> kfree(chunk->chunkmem.va);
Thanks for removing the "chunk->bitmapbuf = chunk->bitmapmem.va;" stuff.
It was really confusing. The kfree(chunk->chunkmem.va) is equivalent to
kfree(chunk). A good rule of thumb is that when you have one error:
label to free everything then it's normally going to be buggy.
drivers/infiniband/hw/irdma/pble.c
281 pble_rsrc->next_fpm_addr += chunk->size;
282 ibdev_dbg(to_ibdev(dev),
283 "PBLE: next_fpm_addr = %llx chunk_size[%llu] = 0x%llx\n",
284 pble_rsrc->next_fpm_addr, chunk->size, chunk->size);
285 pble_rsrc->unallocated_pble -= (u32)(chunk->size >> 3);
286 list_add(&chunk->list, &pble_rsrc->pinfo.clist);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
"chunk" added to the "->pinfo.clist" list.
287 sd_reg_val = (sd_entry_type == IRDMA_SD_TYPE_PAGED) ?
288 sd_entry->u.pd_table.pd_page_addr.pa :
289 sd_entry->u.bp.addr.pa;
290
291 if (!sd_entry->valid) {
292 ret_code = irdma_hmc_sd_one(dev, hmc_info->hmc_fn_id, sd_reg_val,
293 idx->sd_idx, sd_entry->entry_type, true);
294 if (ret_code)
295 goto error;
^^^^^^^^^^^
296 }
297
298 sd_entry->valid = true;
299 return 0;
300
301 error:
302 if (chunk->bitmapbuf)
303 kfree(chunk->bitmapmem.va);
304 kfree(chunk->chunkmem.va);
^^^^^^^^^^^^^^^^^^^^^^^^^^
kfree(chunk) will lead to a use after free because it's still on the
list.
305
306 return ret_code;
307 }
regards,
dan carpenter
next prev parent reply other threads:[~2021-12-07 13:14 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-05 8:17 [PATCH] RDMA/irdma: Fix a potential memory allocation issue in 'irdma_prm_add_pble_mem()' Christophe JAILLET
2021-12-07 13:14 ` Dan Carpenter [this message]
2021-12-07 14:44 ` Saleem, Shiraz
2021-12-07 17:51 ` Jason Gunthorpe
2021-12-07 18:13 ` Saleem, Shiraz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211207131428.GF1956@kadam \
--to=dan.carpenter@oracle.com \
--cc=christophe.jaillet@wanadoo.fr \
--cc=jgg@ziepe.ca \
--cc=kernel-janitors@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=mustafa.ismail@intel.com \
--cc=shiraz.saleem@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).