All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: sandeen@sandeen.net, djwong@kernel.org
Cc: Christoph Hellwig <hch@lst.de>,
	linux-xfs@vger.kernel.org, allison.henderson@oracle.com
Subject: [PATCH 13/17] mkfs: prevent corruption of passed-in suboption string values
Date: Wed, 19 Jan 2022 16:22:46 -0800	[thread overview]
Message-ID: <164263816636.863810.3932965298888705668.stgit@magnolia> (raw)
In-Reply-To: <164263809453.863810.8908193461297738491.stgit@magnolia>

From: Darrick J. Wong <djwong@kernel.org>

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>
---
 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


  parent reply	other threads:[~2022-01-20  0:22 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-01-20  0:21 [PATCHSET 00/17] xfsprogs: various 5.15 fixes Darrick J. Wong
2022-01-20  0:21 ` [PATCH 01/17] libxcmd: use emacs mode for command history editing Darrick J. Wong
2022-01-20  0:21 ` [PATCH 02/17] libxfs: shut down filesystem if we xfs_trans_cancel with deferred work items Darrick J. Wong
2022-02-04 21:36   ` Eric Sandeen
2022-02-04 21:47     ` Darrick J. Wong
2022-01-20  0:21 ` [PATCH 03/17] libxfs: don't leave dangling perag references from xfs_buf Darrick J. Wong
2022-02-04 22:05   ` Eric Sandeen
2022-01-20  0:21 ` [PATCH 04/17] libfrog: move the GETFSMAP definitions into libfrog Darrick J. Wong
2022-02-04 23:18   ` Eric Sandeen
2022-02-05  0:36     ` Darrick J. Wong
2022-02-07  1:05       ` Dave Chinner
2022-02-07 17:09         ` Darrick J. Wong
2022-02-07 21:32           ` Eric Sandeen
2022-02-10  3:33             ` Dave Chinner
2022-02-08 16:46   ` [PATCH v1.1 04/17] libfrog: always use the kernel GETFSMAP definitions Darrick J. Wong
2022-02-25 22:35     ` Eric Sandeen
2022-01-20  0:22 ` [PATCH 05/17] misc: add a crc32c self test to mkfs and repair Darrick J. Wong
2022-02-04 23:23   ` Eric Sandeen
2022-01-20  0:22 ` [PATCH 06/17] libxfs-apply: support filterdiff >= 0.4.2 only Darrick J. Wong
2022-01-20  0:22 ` [PATCH 07/17] xfs_db: fix nbits parameter in fa_ino[48] functions Darrick J. Wong
2022-02-25 21:45   ` Eric Sandeen
2022-01-20  0:22 ` [PATCH 08/17] xfs_repair: explicitly cast resource usage counts in do_warn Darrick J. Wong
2022-02-25 21:46   ` Eric Sandeen
2022-01-20  0:22 ` [PATCH 09/17] xfs_repair: explicitly cast directory inode numbers " Darrick J. Wong
2022-02-25 21:48   ` Eric Sandeen
2022-01-20  0:22 ` [PATCH 10/17] xfs_repair: fix indentation problems in upgrade_filesystem Darrick J. Wong
2022-02-25 21:53   ` Eric Sandeen
2022-01-20  0:22 ` [PATCH 11/17] xfs_repair: update secondary superblocks after changing features Darrick J. Wong
2022-02-25 21:57   ` Eric Sandeen
2022-01-20  0:22 ` [PATCH 12/17] xfs_scrub: report optional features in version string Darrick J. Wong
2022-01-20  1:16   ` Theodore Ts'o
2022-01-20  1:28     ` Darrick J. Wong
2022-01-20  1:32   ` [PATCH v2 " Darrick J. Wong
2022-02-25 22:14     ` Eric Sandeen
2022-02-26  0:04       ` Darrick J. Wong
2022-02-26  2:48         ` Darrick J. Wong
2022-02-26  2:53   ` [PATCH v3 " Darrick J. Wong
2022-02-28 21:38     ` Eric Sandeen
2022-01-20  0:22 ` Darrick J. Wong [this message]
2022-01-20  0:22 ` [PATCH 14/17] mkfs: add configuration files for the last few LTS kernels Darrick J. Wong
2022-01-20  0:22 ` [PATCH 15/17] mkfs: document sample configuration file location Darrick J. Wong
2022-01-20  0:23 ` [PATCH 16/17] mkfs: add a config file for x86_64 pmem filesystems Darrick J. Wong
2022-02-25 22:21   ` Eric Sandeen
2022-02-26  2:38     ` Darrick J. Wong
2022-02-26  2:52   ` [PATCH v2 " Darrick J. Wong
2022-02-28 21:37     ` Eric Sandeen
2022-01-20  0:23 ` [PATCH 17/17] mkfs: enable inobtcount and bigtime by default Darrick J. Wong
2022-02-25 22:22   ` Eric Sandeen
2022-01-28 22:44 ` [PATCH 18/17] xfs_scrub: fix reporting if we can't open raw block devices Darrick J. Wong
2022-01-31 12:28   ` Christoph Hellwig
2022-02-26  2:54 ` [PATCH 19/17] mkfs: increase default log size for new (aka bigtime) filesystems Darrick J. Wong
2022-02-26 21:37   ` Dave Chinner
2022-02-28 23:22     ` Darrick J. Wong
2022-03-01  0:42       ` Dave Chinner
2022-03-01  2:38         ` Darrick J. Wong
2022-03-01 15:55           ` Brian Foster
2022-03-01  3:10         ` Dave Chinner
2022-02-28 21:44   ` Eric Sandeen
2022-03-01  2:21     ` Darrick J. Wong
2022-03-01  2:44       ` Eric Sandeen

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=164263816636.863810.3932965298888705668.stgit@magnolia \
    --to=djwong@kernel.org \
    --cc=allison.henderson@oracle.com \
    --cc=hch@lst.de \
    --cc=linux-xfs@vger.kernel.org \
    --cc=sandeen@sandeen.net \
    /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.