Signed-off-by: Jun Yang <jun.yang@suse.com> (cherry picked from commit d086f91960b934aa5acce9643ee81d1539b7e2e4)
71 lines
2.2 KiB
Diff
71 lines
2.2 KiB
Diff
From 99c7877759c0d0e7cd1b386c717e25dbc6f5ce61 Mon Sep 17 00:00:00 2001
|
|
From: "Darrick J. Wong" <djwong@kernel.org>
|
|
Date: Fri, 25 Feb 2022 17:42:16 -0500
|
|
Subject: [PATCH] mkfs: prevent corruption of passed-in suboption string values
|
|
|
|
Eric and I were trying to play with mkfs.configuration files, when I
|
|
spotted this (with the libini package from Ubuntu 20.04):
|
|
|
|
# cat << EOF > /tmp/r
|
|
[data]
|
|
su=2097152
|
|
sw=1
|
|
EOF
|
|
# mkfs.xfs -f -c options=/tmp/r /dev/sda
|
|
Parameters parsed from config file /tmp/r successfully
|
|
-d su option requires a value
|
|
|
|
It turns out that libini's parser uses stack variables(!) to store the
|
|
value of a key=value pair that it parses, and passes this stack array to
|
|
the parse_cfgopt function. If the particular option calls getstr(),
|
|
then we save the value of that pointer (not its contents) to the
|
|
cli_params. Being a stack array, the contents will be overwritten by
|
|
other function calls, which means that our value of '2097152' has been
|
|
destroyed by the time we actually call getnum when we're validating the
|
|
new fs config.
|
|
|
|
We never noticed this until now because the only other caller was
|
|
getsubopt on the argv array, which gets chopped up but left intact in
|
|
memory. The solution is to make a private copy of those strings if we
|
|
ever save them for later. For now we'll be lazy and let the memory
|
|
leak, since mkfs is not a long-running process.
|
|
|
|
Fixes: 33c62516 ("mkfs: add initial ini format config file parsing support")
|
|
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
|
|
Reviewed-by: Christoph Hellwig <hch@lst.de>
|
|
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
|
|
---
|
|
mkfs/xfs_mkfs.c | 11 ++++++++++-
|
|
1 file changed, 10 insertions(+), 1 deletion(-)
|
|
|
|
diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
|
|
index 3a41e17f..fcad6b55 100644
|
|
--- a/mkfs/xfs_mkfs.c
|
|
+++ b/mkfs/xfs_mkfs.c
|
|
@@ -1438,12 +1438,21 @@ getstr(
|
|
struct opt_params *opts,
|
|
int index)
|
|
{
|
|
+ char *ret;
|
|
+
|
|
check_opt(opts, index, true);
|
|
|
|
/* empty strings for string options are not valid */
|
|
if (!str || *str == '\0')
|
|
reqval(opts->name, opts->subopts, index);
|
|
- return (char *)str;
|
|
+
|
|
+ ret = strdup(str);
|
|
+ if (!ret) {
|
|
+ fprintf(stderr, _("Out of memory while saving suboptions.\n"));
|
|
+ exit(1);
|
|
+ }
|
|
+
|
|
+ return ret;
|
|
}
|
|
|
|
static int
|
|
--
|
|
2.35.3
|
|
|