[PATCH 0/3] xfsprogs: random fixes

[PATCH 0/3] xfsprogs: random fixes

[Darrick J. Wong]

Hi all,

This series consists of fixes for crashes that I found while messing
around with libedit, new versions of ubuntu, and trying to fix all the
things that repair didn't catch but check did.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=random-fixes

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=random-fixes

fstests git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=random-fixes

[PATCH 1/3] libxcmd: don't crash if el_gets returns null

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

el_gets returns NULL if it fails to read any characters (due to EOF or
errors occurred).  strdup will crash if it is fed a NULL string, so
check the return value to avoid segfaulting.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 libxcmd/input.c |   20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)


diff --git a/libxcmd/input.c b/libxcmd/input.c
index 137856e3..a4548d7c 100644
--- a/libxcmd/input.c
+++ b/libxcmd/input.c
@@ -47,6 +47,7 @@ fetchline(void)
 	static EditLine	*el;
 	static History	*hist;
 	HistEvent	hevent;
+	const char	*cmd;
 	char		*line;
 	int		count;
 
@@ -59,13 +60,18 @@ fetchline(void)
 		el_set(el, EL_PROMPT, el_get_prompt);
 		el_set(el, EL_HIST, history, (const char *)hist);
 	}
-	line = strdup(el_gets(el, &count));
-	if (line) {
-		if (count > 0)
-			line[count-1] = '\0';
-		if (*line)
-			history(hist, &hevent, H_ENTER, line);
-	}
+	cmd = el_gets(el, &count);
+	if (!cmd)
+		return NULL;
+
+	line = strdup(cmd);
+	if (!line)
+		return NULL;
+
+	if (count > 0)
+		line[count-1] = '\0';
+	if (*line)
+		history(hist, &hevent, H_ENTER, line);
 	return line;
 }
 #else

Re: [PATCH 1/3] libxcmd: don't crash if el_gets returns null

[Christoph Hellwig]

On Sat, May 09, 2020 at 09:29:37AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> el_gets returns NULL if it fails to read any characters (due to EOF or
> errors occurred).  strdup will crash if it is fed a NULL string, so
> check the return value to avoid segfaulting.

Didn't I review this already?

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 2/3] find_api_violations: fix sed expression

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Apparently, the grep program in Ubuntu 20.04 is pickier about requiring
'(' to be escaped inside range expressions.  This causes a regression in
xfs/437, so fix it.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 tools/find-api-violations.sh |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)


diff --git a/tools/find-api-violations.sh b/tools/find-api-violations.sh
index b175ca10..c25fccca 100755
--- a/tools/find-api-violations.sh
+++ b/tools/find-api-violations.sh
@@ -18,8 +18,14 @@ check_if_api_calls() {
 	while read f; do grep "^$f(" libxfs/*.c; done | sed -e 's/^.*:xfs_/xfs_/g' -e 's/.$//g'
 }
 
+# Generate a grep search expression for troublesome API call sites.
+# " foo(", ",foo(", "-foo(", and "(foo(" are examples.
+grep_pattern() {
+	sed -e 's/^/[[:space:],-\\(]/g' -e 's/$/(/g'
+}
+
 find_libxfs_violations() {
-	grep -r -n -f <(find_possible_api_calls | check_if_api_calls | sed -e 's/^/[[:space:],-(]/g' -e 's/$/(/g' ) $tool_dirs
+	grep -r -n -f <(find_possible_api_calls | check_if_api_calls | grep_pattern) $tool_dirs
 }
 
 # libxfs calls without negated error codes
@@ -33,7 +39,7 @@ find_possible_libxfs_api_calls() {
 }
 
 find_libxfs_api_violations() {
-	grep -r -n -f <(find_possible_libxfs_api_calls | sed -e 's/^/[[:space:],-(]/g' -e 's/$/(/g') $tool_dirs
+	grep -r -n -f <(find_possible_libxfs_api_calls | grep_pattern) $tool_dirs
 }
 
 (find_libxfs_violations ; find_errcode_violations ; find_libxfs_api_violations) | sort -g -t ':' -k 2 | sort -g -t ':' -k 1 | uniq

Re: [PATCH 2/3] find_api_violations: fix sed expression

[Christoph Hellwig]

On Sat, May 09, 2020 at 09:29:43AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Apparently, the grep program in Ubuntu 20.04 is pickier about requiring
> '(' to be escaped inside range expressions.  This causes a regression in
> xfs/437, so fix it.

Mentioning the actual sed version would be a lot more helpful..

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/3] find_api_violations: fix sed expression

[Darrick J. Wong]

On Sat, May 09, 2020 at 09:36:44AM -0700, Christoph Hellwig wrote:
> On Sat, May 09, 2020 at 09:29:43AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Apparently, the grep program in Ubuntu 20.04 is pickier about requiring
> > '(' to be escaped inside range expressions.  This causes a regression in
> > xfs/437, so fix it.
> 
> Mentioning the actual sed version would be a lot more helpful..

GNU grep 3.4.

--D

> Otherwise looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/3] find_api_violations: fix sed expression

[Christoph Hellwig]

On Sat, May 09, 2020 at 09:38:21AM -0700, Darrick J. Wong wrote:
> On Sat, May 09, 2020 at 09:36:44AM -0700, Christoph Hellwig wrote:
> > On Sat, May 09, 2020 at 09:29:43AM -0700, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <darrick.wong@oracle.com>
> > > 
> > > Apparently, the grep program in Ubuntu 20.04 is pickier about requiring
> > > '(' to be escaped inside range expressions.  This causes a regression in
> > > xfs/437, so fix it.
> > 
> > Mentioning the actual sed version would be a lot more helpful..
> 
> GNU grep 3.4.

That should go into the changelog for commit instead of the distro
version.

Re: [PATCH 2/3] find_api_violations: fix sed expression

[Eric Sandeen]

On 5/9/20 11:44 AM, Christoph Hellwig wrote:
> On Sat, May 09, 2020 at 09:38:21AM -0700, Darrick J. Wong wrote:
>> On Sat, May 09, 2020 at 09:36:44AM -0700, Christoph Hellwig wrote:
>>> On Sat, May 09, 2020 at 09:29:43AM -0700, Darrick J. Wong wrote:
>>>> From: Darrick J. Wong <darrick.wong@oracle.com>
>>>>
>>>> Apparently, the grep program in Ubuntu 20.04 is pickier about requiring
>>>> '(' to be escaped inside range expressions.  This causes a regression in
>>>> xfs/437, so fix it.
>>>
>>> Mentioning the actual sed version would be a lot more helpful..
>>
>> GNU grep 3.4.
> 
> That should go into the changelog for commit instead of the distro
> version.
> 

I've fixed it on merge

[PATCH 3/3] xfs_db: bounds-check access to the dbmap array

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Try to check the array boundaries of the dbmap array so that we don't
just segfault.

Found by fuzzing xfs/358 with recs[1].blockcount = ones.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 db/check.c |   43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)


diff --git a/db/check.c b/db/check.c
index c9bafa8e..6f047877 100644
--- a/db/check.c
+++ b/db/check.c
@@ -1294,6 +1294,18 @@ check_blist(
 	return 0;
 }
 
+static inline bool
+dbmap_boundscheck(
+	xfs_agnumber_t	agno,
+	xfs_agblock_t	agbno)
+{
+	if (agno >= mp->m_sb.sb_agcount)
+		return false;
+	if (agbno >= mp->m_sb.sb_agblocks)
+		return false;
+	return true;
+}
+
 static void
 check_dbmap(
 	xfs_agnumber_t	agno,
@@ -1307,6 +1319,12 @@ check_dbmap(
 	dbm_t		d;
 
 	for (i = 0, p = &dbmap[agno][agbno]; i < len; i++, p++) {
+		if (!dbmap_boundscheck(agno, agbno + i)) {
+			dbprintf(_("block %u/%u beyond end of expected area\n"),
+				agno, agbno + i);
+			error++;
+			break;
+		}
 		d = (dbm_t)*p;
 		if (ignore_reflink && (d == DBM_UNKNOWN || d == DBM_DATA ||
 				       d == DBM_RLDATA))
@@ -1468,6 +1486,13 @@ check_range(
 	return 1;
 }
 
+static inline bool
+rdbmap_boundscheck(
+	xfs_rfsblock_t	bno)
+{
+	return bno < mp->m_sb.sb_agblocks;
+}
+
 static void
 check_rdbmap(
 	xfs_rfsblock_t	bno,
@@ -1478,6 +1503,12 @@ check_rdbmap(
 	char		*p;
 
 	for (i = 0, p = &dbmap[mp->m_sb.sb_agcount][bno]; i < len; i++, p++) {
+		if (!rdbmap_boundscheck(bno + i)) {
+			dbprintf(_("rtblock %llu beyond end of expected area\n"),
+				bno + i);
+			error++;
+			break;
+		}
 		if ((dbm_t)*p != type) {
 			if (!sflag || CHECK_BLIST(bno + i))
 				dbprintf(_("rtblock %llu expected type %s got "
@@ -1599,6 +1630,12 @@ check_set_dbmap(
 	check_dbmap(agno, agbno, len, type1, is_reflink(type2));
 	mayprint = verbose | blist_size;
 	for (i = 0, p = &dbmap[agno][agbno]; i < len; i++, p++) {
+		if (!dbmap_boundscheck(agno, agbno + i)) {
+			dbprintf(_("block %u/%u beyond end of expected area\n"),
+				agno, agbno + i);
+			error++;
+			break;
+		}
 		if (*p == DBM_RLDATA && type2 == DBM_DATA)
 			;	/* do nothing */
 		else if (*p == DBM_DATA && type2 == DBM_DATA)
@@ -1627,6 +1664,12 @@ check_set_rdbmap(
 	check_rdbmap(bno, len, type1);
 	mayprint = verbose | blist_size;
 	for (i = 0, p = &dbmap[mp->m_sb.sb_agcount][bno]; i < len; i++, p++) {
+		if (!rdbmap_boundscheck(bno + i)) {
+			dbprintf(_("rtblock %llu beyond end of expected area\n"),
+				bno + i);
+			error++;
+			break;
+		}
 		*p = (char)type2;
 		if (mayprint && (verbose || CHECK_BLIST(bno + i)))
 			dbprintf(_("setting rtblock %llu to %s\n"),

Re: [PATCH 3/3] xfs_db: bounds-check access to the dbmap array

[Christoph Hellwig]

Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>