Re: [PATCH AUTOSEL 5.5 13/28] staging: greybus: loopback_test: fix potential path truncations

[Greg Kroah-Hartman]

On Thu, Mar 26, 2020 at 07:23:42PM -0400, Sasha Levin wrote:
> From: Johan Hovold <johan@xxxxxxxxxx>
> 
> [ Upstream commit ae62cf5eb2792d9a818c2d93728ed92119357017 ]
> 
> Newer GCC warns about possible truncations of two generated path names as
> we're concatenating the configurable sysfs and debugfs path prefixes
> with a filename and placing the results in buffers of the same size as
> the maximum length of the prefixes.
> 
> 	snprintf(d->name, MAX_STR_LEN, "gb_loopback%u", dev_id);
> 
> 	snprintf(d->sysfs_entry, MAX_SYSFS_PATH, "%s%s/",
> 		 t->sysfs_prefix, d->name);
> 
> 	snprintf(d->debugfs_entry, MAX_SYSFS_PATH, "%sraw_latency_%s",
> 		 t->debugfs_prefix, d->name);
> 
> Fix this by separating the maximum path length from the maximum prefix
> length and reducing the latter enough to fit the generated strings.
> 
> Note that we also need to reduce the device-name buffer size as GCC
> isn't smart enough to figure out that we ever only used MAX_STR_LEN
> bytes of it.
> 
> Fixes: 6b0658f68786 ("greybus: tools: Add tools directory to greybus repo and add loopback")
> Signed-off-by: Johan Hovold <johan@xxxxxxxxxx>
> Link: https://lore.kernel.org/r/20200312110151.22028-4-johan@xxxxxxxxxx
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
> ---
>  drivers/staging/greybus/tools/loopback_test.c | 15 ++++++++-------
>  1 file changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/greybus/tools/loopback_test.c b/drivers/staging/greybus/tools/loopback_test.c
> index 5ce7d6fa086cc..3ee9109c38f60 100644
> --- a/drivers/staging/greybus/tools/loopback_test.c
> +++ b/drivers/staging/greybus/tools/loopback_test.c
> @@ -19,6 +19,7 @@
>  #include <signal.h>
>  
>  #define MAX_NUM_DEVICES 10
> +#define MAX_SYSFS_PREFIX 0x80
>  #define MAX_SYSFS_PATH	0x200
>  #define CSV_MAX_LINE	0x1000
>  #define SYSFS_MAX_INT	0x20
> @@ -67,7 +68,7 @@ struct loopback_results {
>  };
>  
>  struct loopback_device {
> -	char name[MAX_SYSFS_PATH];
> +	char name[MAX_STR_LEN];
>  	char sysfs_entry[MAX_SYSFS_PATH];
>  	char debugfs_entry[MAX_SYSFS_PATH];
>  	struct loopback_results results;
> @@ -93,8 +94,8 @@ struct loopback_test {
>  	int stop_all;
>  	int poll_count;
>  	char test_name[MAX_STR_LEN];
> -	char sysfs_prefix[MAX_SYSFS_PATH];
> -	char debugfs_prefix[MAX_SYSFS_PATH];
> +	char sysfs_prefix[MAX_SYSFS_PREFIX];
> +	char debugfs_prefix[MAX_SYSFS_PREFIX];
>  	struct timespec poll_timeout;
>  	struct loopback_device devices[MAX_NUM_DEVICES];
>  	struct loopback_results aggregate_results;
> @@ -907,10 +908,10 @@ int main(int argc, char *argv[])
>  			t.iteration_max = atoi(optarg);
>  			break;
>  		case 'S':
> -			snprintf(t.sysfs_prefix, MAX_SYSFS_PATH, "%s", optarg);
> +			snprintf(t.sysfs_prefix, MAX_SYSFS_PREFIX, "%s", optarg);
>  			break;
>  		case 'D':
> -			snprintf(t.debugfs_prefix, MAX_SYSFS_PATH, "%s", optarg);
> +			snprintf(t.debugfs_prefix, MAX_SYSFS_PREFIX, "%s", optarg);
>  			break;
>  		case 'm':
>  			t.mask = atol(optarg);
> @@ -961,10 +962,10 @@ int main(int argc, char *argv[])
>  	}
>  
>  	if (!strcmp(t.sysfs_prefix, ""))
> -		snprintf(t.sysfs_prefix, MAX_SYSFS_PATH, "%s", sysfs_prefix);
> +		snprintf(t.sysfs_prefix, MAX_SYSFS_PREFIX, "%s", sysfs_prefix);
>  
>  	if (!strcmp(t.debugfs_prefix, ""))
> -		snprintf(t.debugfs_prefix, MAX_SYSFS_PATH, "%s", debugfs_prefix);
> +		snprintf(t.debugfs_prefix, MAX_SYSFS_PREFIX, "%s", debugfs_prefix);
>  
>  	ret = find_loopback_devices(&t);
>  	if (ret)
> -- 
> 2.20.1

ALso already in all trees, please don't try to add it again.