From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1n3mF5-0003Ek-3O for mharc-grub-devel@gnu.org; Sat, 01 Jan 2022 16:48:47 -0500 Received: from eggs.gnu.org ([209.51.188.92]:56330) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1n3mF3-0003EY-DT for grub-devel@gnu.org; Sat, 01 Jan 2022 16:48:45 -0500 Received: from [2607:f8b0:4864:20::f2c] (port=45710 helo=mail-qv1-xf2c.google.com) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1n3mF1-0001LD-6P for grub-devel@gnu.org; Sat, 01 Jan 2022 16:48:44 -0500 Received: by mail-qv1-xf2c.google.com with SMTP id a9so27594340qvd.12 for ; Sat, 01 Jan 2022 13:48:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=efficientek-com.20210112.gappssmtp.com; s=20210112; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=EdUk9Z45pyGNnINAVwWzAenGC/c5hRNNf0efle6bNV4=; b=AxwXqLjImo1MuvDJtK1XGbsy7ALSNvM3rY5nkORj6u9eFSJbMLBWse1YL0J7E+qlQy gRn/6cZ0FC/BnYgP//ptvDHd4qTOPHNLxdw9HcCrUCKAHeGdOZzXxfn5sxXYsvEMnvU2 jgrXHSnzBEVL7gq/w/fCoDbErULBfYo5ZGXln889/Kt+yQ0jOB04ZlKUElyxwGMV2lf5 /0fvH2kc4jPAQu0WT4mdWTPNuSCyxGd3F73tvCuGhm3xtbxXCG0qNRMIJ2ylX2xpDGfX lQdiOKU5u7RUq+JsFNi7wjo08XOuBz2KYN/mDgSBP5KsaVGVL9zK9LwWZaQEq5brb6H0 fRKA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=EdUk9Z45pyGNnINAVwWzAenGC/c5hRNNf0efle6bNV4=; b=NtwHj3M7Ocb6W1jSwZ3mxVwI84wUlhSYA32sOroDngUCzWDqoYkr7NieLwTiYDK3Tv 3zZAme4bPAFlpieghYSuW7WFuVUjrBwXBZSNGW4s1s2BR6MyqG4qHenZiIVDnKNMavDq /wgiU6rzlOjw4Q8aUIK3mUKylyCe/uaiTRMZmRHoZPF0pVB7rBMlmhFvkMwd+kNMG+D4 4CBLnRdkoYT8qIAmdsGXJmWRWv46OZRbT5m4IbZsEV5tT4ZOHyOjC+LKXhP9wJTurrKm Y0UjfEKNDBhfKQmvksJf/2ttu1T2C5CMsOTr1WqzdWVeFAyK30JjXRrX9XasZYvH3EeK eKlw== X-Gm-Message-State: AOAM531xIgmgcj800XffCQpcaLp7TAVSBTOeANZHdL3G8JFNgcDkRDVB JRJJDYyd8T33R5nhkQ0CtxIdnAyNVGCLLw== X-Google-Smtp-Source: ABdhPJzQ1L4c6yUqYchYgZqGeJIuweW+RmbmjVC8e62QQDqQ/EPlQ5k4sqwuxDOFaAlqt5Qb4gDCUw== X-Received: by 2002:ad4:5c4c:: with SMTP id a12mr36848025qva.30.1641073721852; Sat, 01 Jan 2022 13:48:41 -0800 (PST) Received: from localhost.localdomain ([37.218.244.251]) by smtp.gmail.com with ESMTPSA id w8sm25946994qtc.36.2022.01.01.13.48.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 01 Jan 2022 13:48:41 -0800 (PST) From: Glenn Washburn To: Daniel Kiper , grub-devel@gnu.org Cc: Glenn Washburn Subject: [PATCH] cryptodisk: Fix Coverity use after free bug Date: Sat, 1 Jan 2022 15:48:25 -0600 Message-Id: <20220101214825.1662409-1-development@efficientek.com> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Host-Lookup-Failed: Reverse DNS lookup failed for 2607:f8b0:4864:20::f2c (failed) Received-SPF: pass client-ip=2607:f8b0:4864:20::f2c; envelope-from=development@efficientek.com; helo=mail-qv1-xf2c.google.com X-Spam_score_int: 8 X-Spam_score: 0.8 X-Spam_bar: / X-Spam_report: (0.8 / 5.0 requ) DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RDNS_NONE=0.793, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 01 Jan 2022 21:48:45 -0000 The Coverity output is: *** CID 366905: Memory - illegal accesses (USE_AFTER_FREE) /grub-core/disk/cryptodisk.c: 1064 in grub_cryptodisk_scan_device_real() 1058 cleanup: 1059 if (askpass) 1060 { 1061 cargs->key_len = 0; 1062 grub_free (cargs->key_data); 1063 } >>> CID 366905: Memory - illegal accesses (USE_AFTER_FREE) >>> Using freed pointer "dev". 1064 return dev; 1065 } 1066 1067 #ifdef GRUB_UTIL 1068 #include 1069 grub_err_t Here the 'dev' variable can point to a freed cryptodisk device if the function grub_cryptodisk_insert() fails. This can happen only on a OOM condition, but when this happens grub_cryptodisk_insert() calls grub_free on the passed device. Since grub_cryptodisk_scan_device_real() assumes that grub_cryptodisk_insert() is always successful, it will return the device, though the device was freed. Change grub_cryptodisk_insert() to not free the passed device on failure. Then on grub_cryptodisk_insert() failure, free the device pointer. This is done by going to the label 'error', which will call cryptodisk_close() to free the device and set the device pointer to NULL, so that a pointer to freed memory is not returned. Signed-off-by: Glenn Washburn --- Having reviewed the Coverity error, I believe this is the fix needed to resolve the use after free reported by Coverity. However, I do not currently have Coverity setup, so I don't have a way to test if this is both necessary and sufficient to resolve the Coverity error. Regardess, I do believe that is does fix a real use after free bug. Glenn --- grub-core/disk/cryptodisk.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c index 497097394..e7c4795fd 100644 --- a/grub-core/disk/cryptodisk.c +++ b/grub-core/disk/cryptodisk.c @@ -889,10 +889,7 @@ grub_cryptodisk_insert (grub_cryptodisk_t newdev, const char *name, { newdev->source = grub_strdup (name); if (!newdev->source) - { - grub_free (newdev); - return grub_errno; - } + return grub_errno; newdev->id = last_cryptodisk_id++; newdev->source_id = source->id; @@ -1044,7 +1041,9 @@ grub_cryptodisk_scan_device_real (const char *name, if (ret != GRUB_ERR_NONE) goto error; - grub_cryptodisk_insert (dev, name, source); + ret = grub_cryptodisk_insert (dev, name, source); + if (ret != GRUB_ERR_NONE) + goto error; goto cleanup; } -- 2.27.0