linux-kernel - Re: [PATCH] RDMA/irdma: Fix a potential memory allocation issue in 'irdma_prm_add_pble

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20211207131428.GF1956@kadam>
Date:   Tue, 7 Dec 2021 16:14:28 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc:     mustafa.ismail@...el.com, shiraz.saleem@...el.com, jgg@...pe.ca,
        linux-rdma@...r.kernel.org, linux-kernel@...r.kernel.org,
        kernel-janitors@...r.kernel.org
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.

   305  
   306          return ret_code;
   307  }

regards,
dan carpenter