[PATCH 0/3] xfsprogs: random fixes

[PATCH 0/3] xfsprogs: random fixes

[Darrick J. Wong]

Hi all,

This is a rollup of various minor fixes for 5.7.

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] xfs_db: 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>
---
 db/input.c |   23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)


diff --git a/db/input.c b/db/input.c
index 553025bc..448e84b0 100644
--- a/db/input.c
+++ b/db/input.c
@@ -230,14 +230,21 @@ fetchline(void)
 	}
 
 	if (inputstacksize == 1) {
-		line = xstrdup(el_gets(el, &count));
-		if (line) {
-			if (count > 0)
-				line[count-1] = '\0';
-			if (*line) {
-				history(hist, &hevent, H_ENTER, line);
-				logprintf("%s", line);
-			}
+		const char	*cmd;
+
+		cmd = el_gets(el, &count);
+		if (!cmd)
+			return NULL;
+
+		line = xstrdup(cmd);
+		if (!line)
+			return NULL;
+
+		if (count > 0)
+			line[count-1] = '\0';
+		if (*line) {
+			history(hist, &hevent, H_ENTER, line);
+			logprintf("%s", line);
 		}
 	} else {
 		line = fetchline_internal();

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

[Eric Sandeen]

On 5/18/20 7:52 PM, 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.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  db/input.c |   23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
> 
> 
> diff --git a/db/input.c b/db/input.c
> index 553025bc..448e84b0 100644
> --- a/db/input.c
> +++ b/db/input.c
> @@ -230,14 +230,21 @@ fetchline(void)
>  	}
>  
>  	if (inputstacksize == 1) {
> -		line = xstrdup(el_gets(el, &count));
> -		if (line) {
> -			if (count > 0)
> -				line[count-1] = '\0';
> -			if (*line) {
> -				history(hist, &hevent, H_ENTER, line);
> -				logprintf("%s", line);
> -			}
> +		const char	*cmd;
> +
> +		cmd = el_gets(el, &count);
> +		if (!cmd)
> +			return NULL;
> +
> +		line = xstrdup(cmd);
> +		if (!line)
> +			return NULL;
> +
> +		if (count > 0)
> +			line[count-1] = '\0';
> +		if (*line) {
> +			history(hist, &hevent, H_ENTER, line);
> +			logprintf("%s", line);
>  		}
>  	} else {
>  		line = fetchline_internal();
> 

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

[Christoph Hellwig]

On Mon, May 18, 2020 at 05:52:17PM -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.

Looks good:

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

And I'm pretty sure I've reviewed this before..

[PATCH 2/3] xfs_db: fix rdbmap_boundscheck

[Darrick J. Wong]

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

This predicate should check the a rt block number against number of
rtblocks, not the number of AG blocks.  Ooops.

Fixes: 7161cd21b3ed ("xfs_db: bounds-check access to the dbmap array")
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 db/check.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/db/check.c b/db/check.c
index c6fce605..12c03b6d 100644
--- a/db/check.c
+++ b/db/check.c
@@ -1490,7 +1490,7 @@ static inline bool
 rdbmap_boundscheck(
 	xfs_rfsblock_t	bno)
 {
-	return bno < mp->m_sb.sb_agblocks;
+	return bno < mp->m_sb.sb_rblocks;
 }
 
 static void

Re: [PATCH 2/3] xfs_db: fix rdbmap_boundscheck

[Eric Sandeen]

On 5/18/20 7:52 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> This predicate should check the a rt block number against number of
> rtblocks, not the number of AG blocks.  Ooops.
> 
> Fixes: 7161cd21b3ed ("xfs_db: bounds-check access to the dbmap array")
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

welp, sorry I missed this on regression testing;.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  db/check.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> 
> diff --git a/db/check.c b/db/check.c
> index c6fce605..12c03b6d 100644
> --- a/db/check.c
> +++ b/db/check.c
> @@ -1490,7 +1490,7 @@ static inline bool
>  rdbmap_boundscheck(
>  	xfs_rfsblock_t	bno)
>  {
> -	return bno < mp->m_sb.sb_agblocks;
> +	return bno < mp->m_sb.sb_rblocks;
>  }
>  
>  static void
> 

Re: [PATCH 2/3] xfs_db: fix rdbmap_boundscheck

[Christoph Hellwig]

On Mon, May 18, 2020 at 05:52:23PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> This predicate should check the a rt block number against number of
> rtblocks, not the number of AG blocks.  Ooops.
> 
> Fixes: 7161cd21b3ed ("xfs_db: bounds-check access to the dbmap array")
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Looks good,

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

[PATCH 3/3] debian: replace libreadline with libedit

[Darrick J. Wong]

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

Now that upstream has dropped libreadline support entirely, switch the
debian package over to libedit.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 debian/control |    2 +-
 debian/rules   |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)


diff --git a/debian/control b/debian/control
index 0b3205f5..ddd17850 100644
--- a/debian/control
+++ b/debian/control
@@ -3,7 +3,7 @@ Section: admin
 Priority: optional
 Maintainer: XFS Development Team <linux-xfs@vger.kernel.org>
 Uploaders: Nathan Scott <nathans@debian.org>, Anibal Monsalve Salazar <anibal@debian.org>
-Build-Depends: uuid-dev, dh-autoreconf, debhelper (>= 5), gettext, libtool, libreadline-gplv2-dev, libblkid-dev (>= 2.17), linux-libc-dev, libdevmapper-dev, libattr1-dev, libicu-dev, dh-python, pkg-config
+Build-Depends: uuid-dev, dh-autoreconf, debhelper (>= 5), gettext, libtool, libedit-dev, libblkid-dev (>= 2.17), linux-libc-dev, libdevmapper-dev, libattr1-dev, libicu-dev, dh-python, pkg-config
 Standards-Version: 4.0.0
 Homepage: https://xfs.wiki.kernel.org/
 
diff --git a/debian/rules b/debian/rules
index e8509fb3..7304222c 100755
--- a/debian/rules
+++ b/debian/rules
@@ -25,7 +25,7 @@ stdenv = @GZIP=-q; export GZIP;
 
 options = export DEBUG=-DNDEBUG DISTRIBUTION=debian \
 	  INSTALL_USER=root INSTALL_GROUP=root \
-	  LOCAL_CONFIGURE_OPTIONS="--enable-readline=yes --enable-blkid=yes --disable-ubsan --disable-addrsan --disable-threadsan --enable-lto" ;
+	  LOCAL_CONFIGURE_OPTIONS="--enable-editline=yes --enable-blkid=yes --disable-ubsan --disable-addrsan --disable-threadsan --enable-lto" ;
 diopts  = $(options) \
 	  export OPTIMIZER=-Os LOCAL_CONFIGURE_OPTIONS="--enable-gettext=no --disable-ubsan --disable-addrsan --disable-threadsan --enable-lto" ;
 checkdir = test -f debian/rules

Re: [PATCH 3/3] debian: replace libreadline with libedit

[Eric Sandeen]

On 5/18/20 7:52 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Now that upstream has dropped libreadline support entirely, switch the
> debian package over to libedit.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

I don't really speak Debian, but this seems sane.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  debian/control |    2 +-
>  debian/rules   |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/debian/control b/debian/control
> index 0b3205f5..ddd17850 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -3,7 +3,7 @@ Section: admin
>  Priority: optional
>  Maintainer: XFS Development Team <linux-xfs@vger.kernel.org>
>  Uploaders: Nathan Scott <nathans@debian.org>, Anibal Monsalve Salazar <anibal@debian.org>
> -Build-Depends: uuid-dev, dh-autoreconf, debhelper (>= 5), gettext, libtool, libreadline-gplv2-dev, libblkid-dev (>= 2.17), linux-libc-dev, libdevmapper-dev, libattr1-dev, libicu-dev, dh-python, pkg-config
> +Build-Depends: uuid-dev, dh-autoreconf, debhelper (>= 5), gettext, libtool, libedit-dev, libblkid-dev (>= 2.17), linux-libc-dev, libdevmapper-dev, libattr1-dev, libicu-dev, dh-python, pkg-config
>  Standards-Version: 4.0.0
>  Homepage: https://xfs.wiki.kernel.org/
>  
> diff --git a/debian/rules b/debian/rules
> index e8509fb3..7304222c 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -25,7 +25,7 @@ stdenv = @GZIP=-q; export GZIP;
>  
>  options = export DEBUG=-DNDEBUG DISTRIBUTION=debian \
>  	  INSTALL_USER=root INSTALL_GROUP=root \
> -	  LOCAL_CONFIGURE_OPTIONS="--enable-readline=yes --enable-blkid=yes --disable-ubsan --disable-addrsan --disable-threadsan --enable-lto" ;
> +	  LOCAL_CONFIGURE_OPTIONS="--enable-editline=yes --enable-blkid=yes --disable-ubsan --disable-addrsan --disable-threadsan --enable-lto" ;
>  diopts  = $(options) \
>  	  export OPTIMIZER=-Os LOCAL_CONFIGURE_OPTIONS="--enable-gettext=no --disable-ubsan --disable-addrsan --disable-threadsan --enable-lto" ;
>  checkdir = test -f debian/rules
> 

Re: [PATCH 3/3] debian: replace libreadline with libedit

[Christoph Hellwig]

On Mon, May 18, 2020 at 05:52:30PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Now that upstream has dropped libreadline support entirely, switch the
> debian package over to libedit.

Looks good:

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

Do we need to resync with the real Debian packaging?  That has switched
over quite a while ago.

Re: [PATCH 3/3] debian: replace libreadline with libedit

[Darrick J. Wong]

On Wed, May 20, 2020 at 10:37:13AM -0700, Christoph Hellwig wrote:
> On Mon, May 18, 2020 at 05:52:30PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Now that upstream has dropped libreadline support entirely, switch the
> > debian package over to libedit.
> 
> Looks good:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> Do we need to resync with the real Debian packaging?  That has switched
> over quite a while ago.

It did?  It looks to me like debian uses the upstream packaging, even
though they occasionally curse us for doing so.

Now, Ubuntu seems to be doing some sort of valueadd thing around blkid
and "broken configure" but AFAICT they're not sending whatever they
fixed back to us.  Party like it's 2005 again.

--D