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>

diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 3a41e17f855508a72e2fa5c7ae270db49f8f07ea..fcad6b5527a53788889f437c4dafc4727c009629 100644 (file)
--- 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