[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v1 2/2] backup_ptes: fix leak on realloc failure
- To: Edwin Török <edvin.torok@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
- From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
- Date: Fri, 24 Feb 2023 15:00:55 +0000
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=FMszfW/goHMfjxmUPzM9M3YXrjXP3E0cFLVZ4cZIyMQ=; b=n1di9EUwGsX9/JIPt9fZOEel3fmTU89b7BGKTS4Dg+YSMIHq2KAiOrQfClkb56vNkDeiiJYhAf0W2mFEV4bD5HqhAO6eEQAyOrHlpDAnwjrP50BJksTIHqbG8Gl/G23eiLUWCIw8p/00Bps8lJ4Ve7jFxc68VyrL8peBmoQJ8kGElFxx/jkE2sa/B0FH3xYI/wyyXI+3qgV+CsDe+AZ9AftleNZDIkvLu0HpXacuFs3Q+jmc9ST8Drnw18Q7FUgsqC/xiZFPT9cUFDXO3ljGgam7G/omgLrO+ANaeJqzaZpURL09raTtPi7TdXq6HdY/10eJ+onqALaLkcfr+Xl0zg==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=bv/aHSRXCY3M1NRE1GVW5bUzLNV6H8TAtVN2y+NKhEQ+PAbHnMANaCvgySo5yEW7SUOuMRu0VrX/yQupSi0zTk7vlFPsDehobkgElq9FwvdJjOPrBBpFFjG2Kjg6y67eXdHZhhEVyXFwf8JV1YjaOkj0GDRMhLK5clmpdBLY2VJH1lp/nyEjKUAiXBAjIHu/yMcElHjGDY7Ad1kVpdWxXM6F/zj5npbldLgqaemnrxfnYiNXIgpNAgJrK8pUp9LAzycE7WhuY47SQ/aXfm1TUWHlPHyzzWi+jxIrJE0wZKX78TOWQTKnH0L69ERKcfQh2PePqxnER+gcnz03whg5Xw==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: Edwin Török <edwin.torok@xxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Anthony PERARD <anthony.perard@xxxxxxxxxx>, Juergen Gross <jgross@xxxxxxxx>
- Delivery-date: Fri, 24 Feb 2023 15:01:14 +0000
- Ironport-data: A9a23:RwPNwaOP3RxUnw3vrR2clsFynXyQoLVcMsEvi/4bfWQNrUoggmMCx mYXWz3Xb66OYWLweop3OdmwpBtS6JOEz4dkHAto+SlhQUwRpJueD7x1DKtS0wC6dZSfER09v 63yTvGacajYm1eF/k/F3oDJ9CU6jufQAOKnUoYoAwgpLSd8UiAtlBl/rOAwh49skLCRDhiE/ Nj/uKUzAnf8s9JPGj9Suv3rRC9H5qyo42tC5ABmPpingXeF/5UrJMNHTU2OByOQrrl8RoaSW +vFxbelyWLVlz9F5gSNy+uTnuUiG9Y+DCDW4pZkc/HKbitq/0Te5p0TJvsEAXq7vh3S9zxHJ HehgrTrIeshFvWkdO3wyHC0GQkmVUFN0OevzXRSLaV/ZqAJGpfh66wGMa04AWEX0vYrEEpE6 eA3ER8EMDean7iowZHjY+Y506zPLOGzVG8ekldJ6GiBSNMZG9XESaiM4sJE1jAtgMwIBezZe 8cSdTtoalLHfgFLPVAUTpk5mY9EhFGmK2Ee9A3T+PdxujCOpOBy+OGF3N79U9qGX8hK2G2fo XrL5T/RCRAGLt2PjzGC9xpAg8eewnmmAN9OS9VU8NZHsASTmXwcJiFNC3CKs6KLpUCaV+N2f hl8Fi0G6PJaGFaQZsnwWVi0rWCJujYYWsFMCKsq5QeV0K3W7g2FQG8eQVZpZdA5tclwWTsw0 VyhlNfuGDApu7qQIVqC8p+EoDX0PjIaRUcAajUDVhAt+MT4rcc4iRenczp4OKu8j9mwHC6qx TmP9XA6n+9K1ZNN0Lin91fahT7qvoLOUgM++gTQWCSi8x99Y4mmIYev7DA38Mp9EWpQdXHZ1 FBspiRUxLlm4U2l/MBVfNgwIQ==
- Ironport-hdrordr: A9a23:BgCm4a+1bNx4g7dJxfZuk+HFdr1zdoMgy1knxilNoENuH/Bwxv rFoB1E73TJYVYqN03IV+rwXpVoMkmskaKdhrNhQItKPTOWwldASbsP0WKM+UyCJ8STzJ8k6U 4kSdkENDSSNykFsS+Z2mmF+r8bqbHokZxAx92utkuFJTsaFJ2IhD0JbzpzfHcGIzWuSaBJdq Z1saF81kadkDksH4yG7j5vZZmwm/T70LbdJTIWDR8u7weDyRuu9b7BChCdmjsOTj9Vxr8m0G 7d1yj0/L+qvf2XwgLVkza71eUbpPLRjv94QOCcgMkcLTvhzi6ueYRaQrWH+Bwlve21714usd /U5zMtJd565X/9dny85THtxw7j+jAz7GKK8y7UvVLT5ejCAB4qActIgoxUNjPf9kobpdl5lI ZGxXiQuZZ7BQ7J2H2V3amDazha0m6P5VYym+8aiHJSFaMYdb9qtIQauGdYCo0JEi7W4J0uVM NuEMbfzvBLdk7yVQGTgkBfhPiXGlgjFBaPRUYP/uSTzjhthXh8i3AVwcQO901wg64Vet1h3a DpI65onLZBQos9dqRmHtoMRsOxFyjkXQ/MGHj6GyWmKIg3f1b277Ln6rQ84++nPLYSyoEppZ jHWFRE8UYvZkPVD9GU1pEjyGGNfIyEZ0Wu9ihi3ek9hlWlL4CbdRFrCWpe3fdIms9vQfEyAJ 2ISdVr6/yKFxqbJW8G5Xy5Z3BoEwhsbCQkgKdLZ7uwmLO6FmTLjJ2sTB+BHsulLR8UHkXCP1 AkYB/fYO1902HDYA6MvPGWYQKjRnDC
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 24/02/2023 1:36 pm, Edwin Török wrote:
> From: Edwin Török <edwin.torok@xxxxxxxxx>
>
> From `man 2 realloc`:
> `If realloc() fails, the original block is left untouched; it is not freed or
> moved.`
>
> Found using GCC -fanalyzer:
> ```
> | 184 | backup->entries = realloc(backup->entries,
> | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> | | | | |
> | | | | (91) when ‘realloc’ fails
> | | | (92) ‘old_ptes.entries’ leaks here; was
> allocated at (44)
> | | (90) ...to here
> ```
>
> Signed-off-by: Edwin Török <edwin.torok@xxxxxxxxx>
In terms of the fix, Acked-by: Andrew Cooper
<andrew.cooper3@xxxxxxxxxx>, but
> ---
> tools/libs/guest/xg_offline_page.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/tools/libs/guest/xg_offline_page.c
> b/tools/libs/guest/xg_offline_page.c
> index c594fdba41..a8bcea768b 100644
> --- a/tools/libs/guest/xg_offline_page.c
> +++ b/tools/libs/guest/xg_offline_page.c
> @@ -181,10 +181,13 @@ static int backup_ptes(xen_pfn_t table_mfn, int offset,
>
> if (backup->max == backup->cur)
> {
> - backup->entries = realloc(backup->entries,
> + void* orig = backup->entries;
void *orig, and a newline.
> + backup->entries = realloc(orig,
> backup->max * 2 * sizeof(struct
> pte_backup_entry));
> - if (backup->entries == NULL)
> + if (backup->entries == NULL) {
Newline.
Can be fixed on commit.
~Andrew
> + free(orig);
> return -1;
> + }
> else
> backup->max *= 2;
> }
|
