diff mbox series

[2/4] xfs: namecheck attribute names before listing them

Message ID 157232183873.593721.440778415935090240.stgit@magnolia (mailing list archive)
State Accepted
Headers show
Series xfs: more metadata verifier tightening | expand

Commit Message

Darrick J. Wong Oct. 29, 2019, 4:03 a.m. UTC 
From: Darrick J. Wong <darrick.wong@oracle.com>

Actually call namecheck on attribute names before we hand them over to
userspace.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_attr_leaf.h |    4 +--
 fs/xfs/xfs_attr_list.c        |   60 +++++++++++++++++++++++++++--------------
 2 files changed, 41 insertions(+), 23 deletions(-)

Comments

Brian Foster Oct. 29, 2019, 10:03 a.m. UTC | #1 
On Mon, Oct 28, 2019 at 09:03:58PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Actually call namecheck on attribute names before we hand them over to
> userspace.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_attr_leaf.h |    4 +--
>  fs/xfs/xfs_attr_list.c        |   60 +++++++++++++++++++++++++++--------------
>  2 files changed, 41 insertions(+), 23 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.h b/fs/xfs/libxfs/xfs_attr_leaf.h
> index 7b74e18becff..bb0880057ee3 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.h
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.h
> @@ -67,8 +67,8 @@ int	xfs_attr3_leaf_add(struct xfs_buf *leaf_buffer,
>  				 struct xfs_da_args *args);
>  int	xfs_attr3_leaf_remove(struct xfs_buf *leaf_buffer,
>  				    struct xfs_da_args *args);
> -void	xfs_attr3_leaf_list_int(struct xfs_buf *bp,
> -				      struct xfs_attr_list_context *context);
> +int	xfs_attr3_leaf_list_int(struct xfs_buf *bp,
> +				struct xfs_attr_list_context *context);
>  
>  /*
>   * Routines used for shrinking the Btree.
> diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
> index 00758fdc2fec..c02f22d50e45 100644
> --- a/fs/xfs/xfs_attr_list.c
> +++ b/fs/xfs/xfs_attr_list.c
> @@ -49,14 +49,16 @@ xfs_attr_shortform_compare(const void *a, const void *b)
>   * we can begin returning them to the user.
>   */
>  static int
> -xfs_attr_shortform_list(xfs_attr_list_context_t *context)
> +xfs_attr_shortform_list(
> +	struct xfs_attr_list_context	*context)
>  {
> -	attrlist_cursor_kern_t *cursor;
> -	xfs_attr_sf_sort_t *sbuf, *sbp;
> -	xfs_attr_shortform_t *sf;
> -	xfs_attr_sf_entry_t *sfe;
> -	xfs_inode_t *dp;
> -	int sbsize, nsbuf, count, i;
> +	struct attrlist_cursor_kern	*cursor;
> +	struct xfs_attr_sf_sort		*sbuf, *sbp;
> +	struct xfs_attr_shortform	*sf;
> +	struct xfs_attr_sf_entry	*sfe;
> +	struct xfs_inode		*dp;
> +	int				sbsize, nsbuf, count, i;
> +	int				error = 0;
>  
>  	ASSERT(context != NULL);
>  	dp = context->dp;
> @@ -84,6 +86,11 @@ xfs_attr_shortform_list(xfs_attr_list_context_t *context)
>  	    (XFS_ISRESET_CURSOR(cursor) &&
>  	     (dp->i_afp->if_bytes + sf->hdr.count * 16) < context->bufsize)) {
>  		for (i = 0, sfe = &sf->list[0]; i < sf->hdr.count; i++) {
> +			if (!xfs_attr_namecheck(sfe->nameval, sfe->namelen)) {
> +				XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
> +						 context->dp->i_mount);
> +				return -EFSCORRUPTED;
> +			}
>  			context->put_listent(context,
>  					     sfe->flags,
>  					     sfe->nameval,
> @@ -161,10 +168,8 @@ xfs_attr_shortform_list(xfs_attr_list_context_t *context)
>  			break;
>  		}
>  	}
> -	if (i == nsbuf) {
> -		kmem_free(sbuf);
> -		return 0;
> -	}
> +	if (i == nsbuf)
> +		goto out;
>  
>  	/*
>  	 * Loop putting entries into the user buffer.
> @@ -174,6 +179,12 @@ xfs_attr_shortform_list(xfs_attr_list_context_t *context)
>  			cursor->hashval = sbp->hash;
>  			cursor->offset = 0;
>  		}
> +		if (!xfs_attr_namecheck(sbp->name, sbp->namelen)) {
> +			XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
> +					 context->dp->i_mount);
> +			error = -EFSCORRUPTED;
> +			goto out;
> +		}
>  		context->put_listent(context,
>  				     sbp->flags,
>  				     sbp->name,
> @@ -183,9 +194,9 @@ xfs_attr_shortform_list(xfs_attr_list_context_t *context)
>  			break;
>  		cursor->offset++;
>  	}
> -
> +out:
>  	kmem_free(sbuf);
> -	return 0;
> +	return error;
>  }
>  
>  /*
> @@ -284,7 +295,7 @@ xfs_attr_node_list(
>  	struct xfs_buf			*bp;
>  	struct xfs_inode		*dp = context->dp;
>  	struct xfs_mount		*mp = dp->i_mount;
> -	int				error;
> +	int				error = 0;
>  
>  	trace_xfs_attr_node_list(context);
>  
> @@ -358,7 +369,9 @@ xfs_attr_node_list(
>  	 */
>  	for (;;) {
>  		leaf = bp->b_addr;
> -		xfs_attr3_leaf_list_int(bp, context);
> +		error = xfs_attr3_leaf_list_int(bp, context);
> +		if (error)
> +			break;
>  		xfs_attr3_leaf_hdr_from_disk(mp->m_attr_geo, &leafhdr, leaf);
>  		if (context->seen_enough || leafhdr.forw == 0)
>  			break;
> @@ -369,13 +382,13 @@ xfs_attr_node_list(
>  			return error;
>  	}
>  	xfs_trans_brelse(context->tp, bp);
> -	return 0;
> +	return error;
>  }
>  
>  /*
>   * Copy out attribute list entries for attr_list(), for leaf attribute lists.
>   */
> -void
> +int
>  xfs_attr3_leaf_list_int(
>  	struct xfs_buf			*bp,
>  	struct xfs_attr_list_context	*context)
> @@ -417,7 +430,7 @@ xfs_attr3_leaf_list_int(
>  		}
>  		if (i == ichdr.count) {
>  			trace_xfs_attr_list_notfound(context);
> -			return;
> +			return 0;
>  		}
>  	} else {
>  		entry = &entries[0];
> @@ -457,6 +470,11 @@ xfs_attr3_leaf_list_int(
>  			valuelen = be32_to_cpu(name_rmt->valuelen);
>  		}
>  
> +		if (!xfs_attr_namecheck(name, namelen)) {
> +			XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
> +					 context->dp->i_mount);
> +			return -EFSCORRUPTED;
> +		}
>  		context->put_listent(context, entry->flags,
>  					      name, namelen, valuelen);
>  		if (context->seen_enough)
> @@ -464,7 +482,7 @@ xfs_attr3_leaf_list_int(
>  		cursor->offset++;
>  	}
>  	trace_xfs_attr_list_leaf_end(context);
> -	return;
> +	return 0;
>  }
>  
>  /*
> @@ -483,9 +501,9 @@ xfs_attr_leaf_list(xfs_attr_list_context_t *context)
>  	if (error)
>  		return error;
>  
> -	xfs_attr3_leaf_list_int(bp, context);
> +	error = xfs_attr3_leaf_list_int(bp, context);
>  	xfs_trans_brelse(context->tp, bp);
> -	return 0;
> +	return error;
>  }
>  
>  int
>
diff mbox series

Patch

diff --git a/fs/xfs/libxfs/xfs_attr_leaf.h b/fs/xfs/libxfs/xfs_attr_leaf.h
index 7b74e18becff..bb0880057ee3 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.h
+++ b/fs/xfs/libxfs/xfs_attr_leaf.h
@@ -67,8 +67,8 @@  int	xfs_attr3_leaf_add(struct xfs_buf *leaf_buffer,
 				 struct xfs_da_args *args);
 int	xfs_attr3_leaf_remove(struct xfs_buf *leaf_buffer,
 				    struct xfs_da_args *args);
-void	xfs_attr3_leaf_list_int(struct xfs_buf *bp,
-				      struct xfs_attr_list_context *context);
+int	xfs_attr3_leaf_list_int(struct xfs_buf *bp,
+				struct xfs_attr_list_context *context);
 
 /*
  * Routines used for shrinking the Btree.
diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
index 00758fdc2fec..c02f22d50e45 100644
--- a/fs/xfs/xfs_attr_list.c
+++ b/fs/xfs/xfs_attr_list.c
@@ -49,14 +49,16 @@  xfs_attr_shortform_compare(const void *a, const void *b)
  * we can begin returning them to the user.
  */
 static int
-xfs_attr_shortform_list(xfs_attr_list_context_t *context)
+xfs_attr_shortform_list(
+	struct xfs_attr_list_context	*context)
 {
-	attrlist_cursor_kern_t *cursor;
-	xfs_attr_sf_sort_t *sbuf, *sbp;
-	xfs_attr_shortform_t *sf;
-	xfs_attr_sf_entry_t *sfe;
-	xfs_inode_t *dp;
-	int sbsize, nsbuf, count, i;
+	struct attrlist_cursor_kern	*cursor;
+	struct xfs_attr_sf_sort		*sbuf, *sbp;
+	struct xfs_attr_shortform	*sf;
+	struct xfs_attr_sf_entry	*sfe;
+	struct xfs_inode		*dp;
+	int				sbsize, nsbuf, count, i;
+	int				error = 0;
 
 	ASSERT(context != NULL);
 	dp = context->dp;
@@ -84,6 +86,11 @@  xfs_attr_shortform_list(xfs_attr_list_context_t *context)
 	    (XFS_ISRESET_CURSOR(cursor) &&
 	     (dp->i_afp->if_bytes + sf->hdr.count * 16) < context->bufsize)) {
 		for (i = 0, sfe = &sf->list[0]; i < sf->hdr.count; i++) {
+			if (!xfs_attr_namecheck(sfe->nameval, sfe->namelen)) {
+				XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
+						 context->dp->i_mount);
+				return -EFSCORRUPTED;
+			}
 			context->put_listent(context,
 					     sfe->flags,
 					     sfe->nameval,
@@ -161,10 +168,8 @@  xfs_attr_shortform_list(xfs_attr_list_context_t *context)
 			break;
 		}
 	}
-	if (i == nsbuf) {
-		kmem_free(sbuf);
-		return 0;
-	}
+	if (i == nsbuf)
+		goto out;
 
 	/*
 	 * Loop putting entries into the user buffer.
@@ -174,6 +179,12 @@  xfs_attr_shortform_list(xfs_attr_list_context_t *context)
 			cursor->hashval = sbp->hash;
 			cursor->offset = 0;
 		}
+		if (!xfs_attr_namecheck(sbp->name, sbp->namelen)) {
+			XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
+					 context->dp->i_mount);
+			error = -EFSCORRUPTED;
+			goto out;
+		}
 		context->put_listent(context,
 				     sbp->flags,
 				     sbp->name,
@@ -183,9 +194,9 @@  xfs_attr_shortform_list(xfs_attr_list_context_t *context)
 			break;
 		cursor->offset++;
 	}
-
+out:
 	kmem_free(sbuf);
-	return 0;
+	return error;
 }
 
 /*
@@ -284,7 +295,7 @@  xfs_attr_node_list(
 	struct xfs_buf			*bp;
 	struct xfs_inode		*dp = context->dp;
 	struct xfs_mount		*mp = dp->i_mount;
-	int				error;
+	int				error = 0;
 
 	trace_xfs_attr_node_list(context);
 
@@ -358,7 +369,9 @@  xfs_attr_node_list(
 	 */
 	for (;;) {
 		leaf = bp->b_addr;
-		xfs_attr3_leaf_list_int(bp, context);
+		error = xfs_attr3_leaf_list_int(bp, context);
+		if (error)
+			break;
 		xfs_attr3_leaf_hdr_from_disk(mp->m_attr_geo, &leafhdr, leaf);
 		if (context->seen_enough || leafhdr.forw == 0)
 			break;
@@ -369,13 +382,13 @@  xfs_attr_node_list(
 			return error;
 	}
 	xfs_trans_brelse(context->tp, bp);
-	return 0;
+	return error;
 }
 
 /*
  * Copy out attribute list entries for attr_list(), for leaf attribute lists.
  */
-void
+int
 xfs_attr3_leaf_list_int(
 	struct xfs_buf			*bp,
 	struct xfs_attr_list_context	*context)
@@ -417,7 +430,7 @@  xfs_attr3_leaf_list_int(
 		}
 		if (i == ichdr.count) {
 			trace_xfs_attr_list_notfound(context);
-			return;
+			return 0;
 		}
 	} else {
 		entry = &entries[0];
@@ -457,6 +470,11 @@  xfs_attr3_leaf_list_int(
 			valuelen = be32_to_cpu(name_rmt->valuelen);
 		}
 
+		if (!xfs_attr_namecheck(name, namelen)) {
+			XFS_ERROR_REPORT(__func__, XFS_ERRLEVEL_LOW,
+					 context->dp->i_mount);
+			return -EFSCORRUPTED;
+		}
 		context->put_listent(context, entry->flags,
 					      name, namelen, valuelen);
 		if (context->seen_enough)
@@ -464,7 +482,7 @@  xfs_attr3_leaf_list_int(
 		cursor->offset++;
 	}
 	trace_xfs_attr_list_leaf_end(context);
-	return;
+	return 0;
 }
 
 /*
@@ -483,9 +501,9 @@  xfs_attr_leaf_list(xfs_attr_list_context_t *context)
 	if (error)
 		return error;
 
-	xfs_attr3_leaf_list_int(bp, context);
+	error = xfs_attr3_leaf_list_int(bp, context);
 	xfs_trans_brelse(context->tp, bp);
-	return 0;
+	return error;
 }
 
 int