diff mbox series

[40/36] xfs_io: fix label parsing and validation

Message ID 20190320193608.GD1183@magnolia (mailing list archive)
State Accepted, archived
Headers show
Series xfsprogs-5.0: fix various problems | expand

Commit Message

Darrick J. Wong March 20, 2019, 7:36 p.m. UTC
From: Darrick J. Wong <darrick.wong@oracle.com>

When we're trying to set a new label, check the length to make sure we
won't overflow the label size, and size label[] so that we can use
strncpy without static checker complaints.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 io/label.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Eric Sandeen April 4, 2019, 9:51 p.m. UTC | #1
On 3/20/19 2:36 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> When we're trying to set a new label, check the length to make sure we
> won't overflow the label size, and size label[] so that we can use
> strncpy without static checker complaints.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

Oh there it is :)  Yeah, this is better than what i had, which
just truncated a too-long label IIRC.

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

> ---
>  io/label.c |   10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/io/label.c b/io/label.c
> index 602ece89..72e07964 100644
> --- a/io/label.c
> +++ b/io/label.c
> @@ -40,7 +40,7 @@ label_f(
>  {
>  	int		c;
>  	int		error;
> -	char		label[FSLABEL_MAX];
> +	char		label[FSLABEL_MAX + 1];
>  
>  	if (argc == 1) {
>  		memset(label, 0, sizeof(label));
> @@ -54,7 +54,13 @@ label_f(
>  			label[0] = '\0';
>  			break;
>  		case 's':
> -			strncpy(label, optarg, sizeof(label));
> +			if (strlen(optarg) > FSLABEL_MAX) {
> +				errno = EINVAL;
> +				error = 1;
> +				goto out;
> +			}
> +			strncpy(label, optarg, sizeof(label) - 1);
> +			label[sizeof(label) - 1] = 0;

Hm, so this can send up to FSLABEL_MAX chars to the kernel w/o a null
termination (the null term is at FSLABEL_MAX+1 right?)

But, I guess that's for the kernel to guard against if it accepts a
label that long, if it even cares.

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

>  			break;
>  		default:
>  			return command_usage(&label_cmd);
>
diff mbox series

Patch

diff --git a/io/label.c b/io/label.c
index 602ece89..72e07964 100644
--- a/io/label.c
+++ b/io/label.c
@@ -40,7 +40,7 @@  label_f(
 {
 	int		c;
 	int		error;
-	char		label[FSLABEL_MAX];
+	char		label[FSLABEL_MAX + 1];
 
 	if (argc == 1) {
 		memset(label, 0, sizeof(label));
@@ -54,7 +54,13 @@  label_f(
 			label[0] = '\0';
 			break;
 		case 's':
-			strncpy(label, optarg, sizeof(label));
+			if (strlen(optarg) > FSLABEL_MAX) {
+				errno = EINVAL;
+				error = 1;
+				goto out;
+			}
+			strncpy(label, optarg, sizeof(label) - 1);
+			label[sizeof(label) - 1] = 0;
 			break;
 		default:
 			return command_usage(&label_cmd);