From: Glenn Washburn <development@efficientek.com>
To: Daniel Kiper <dkiper@net-space.pl>, grub-devel@gnu.org
Cc: Glenn Washburn <development@efficientek.com>
Subject: [PATCH] cryptodisk: Fix Coverity use after free bug
Date: Sat, 1 Jan 2022 15:48:25 -0600 [thread overview]
Message-ID: <20220101214825.1662409-1-development@efficientek.com> (raw)
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 <grub/util/misc.h>
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 <development@efficientek.com>
---
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
next reply other threads:[~2022-01-01 21:48 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-01 21:48 Glenn Washburn [this message]
2022-01-13 18:44 ` [PATCH] cryptodisk: Fix Coverity use after free bug Daniel Kiper
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=20220101214825.1662409-1-development@efficientek.com \
--to=development@efficientek.com \
--cc=dkiper@net-space.pl \
--cc=grub-devel@gnu.org \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.