diff mbox series

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

Message ID 158984953767.623441.453227281468842512.stgit@magnolia (mailing list archive)
State Accepted
Headers show
Series xfsprogs: random fixes | expand

Commit Message

Darrick J. Wong May 19, 2020, 12:52 a.m. UTC 
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(-)

Comments

Eric Sandeen May 19, 2020, 1:55 a.m. UTC | #1 
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();
>
Christoph Hellwig May 20, 2020, 5:36 p.m. UTC | #2 
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..
diff mbox series

Patch

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();