> Subject: Re: [PATCH] RDMA/irdma: Fix a potential memory allocation issue in
> 'irdma_prm_add_pble_mem()'
>
> 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.
>
Ugh! Yes, this is a bug. I will send a separate fix out shortly for this one.
Shiraz
