[adding linux-mmc ML and kernel@xxxxxxxxxxxxxx to Cc:]
Hello Rob,
On Wed, Aug 08, 2018 at 04:34:49PM -0400, Rob Mosher wrote:
> I see you guys committed recently to mmc-utils, so sending this over.
>
> Cheers
>
> lsmmc.c:356:3: error: 'strncpy' accessing 4096 bytes at offsets 0 and 1
> overlaps 4095 bytes at offset 1 [-Werror=restrict]
> strncpy(&line[0], &line[1], sizeof(line));
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Right, strncpy(3) specifies that the strings must not overlap.
> $ git show
> commit 24a86c433e7e9cfd37f3d4688871036b5afea08b
> Author: Rob Mosher <nyt-kernel@xxxxxxxxxxxxxxxxxxx>
> Date: Wed Aug 8 15:55:40 2018 -0400
>
> Fix strncpy overlap
>
> This fixes an overlap condition when strncpy is called
>
> Signed-off-by: Rob Mosher <nyt-kernel@xxxxxxxxxxxxxxxxxxx>
>
> diff --git a/lsmmc.c b/lsmmc.c
> index c4faa00..893f8e1 100644
> --- a/lsmmc.c
> +++ b/lsmmc.c
> @@ -318,6 +318,7 @@ char *read_file(char *name)
> {
> char *preparsed;
> char line[4096];
> + int i, space_count = 0;
> FILE *f;
>
> f = fopen(name, "r");
> @@ -352,8 +353,13 @@ char *read_file(char *name)
> while (isspace(line[strlen(line) - 1]))
> line[strlen(line) - 1] = '\0';
>
> - while (isspace(line[0]))
> - strncpy(&line[0], &line[1], sizeof(line));
> + while (isspace(line[space_count]))
> + space_count++;
> +
> + if (space_count > 0) {
> + for(i=0;line[i] != '\0';i++)
> + line[i] = line[i+space_count];
> + }
Your patch does two things at once:
- Instead of moving the string by 1 for each space, count the spaces
and move in one go
- open code strncpy in a variant that works.
Both should be noted in the commit log.
Also I wonder what happens if line only contains spaces. Then
while (isspace(line[strlen(line) - 1]))
line[strlen(line) - 1] = '\0';
accesses line[-1] in the end.
Note however that moving the whitespace stripped string in the line
buffer is not needed at all given that the function in question just
returns
strdup(line)
.
So it should be good enough to do something like:
char *linestart = line;
size_t len;
/* find the first non-space char */
while (*linestart && isspace(*linestart))
++linestart;
/* right-strip the remaining string */
len = strnlen(linestart, sizeof(line) - (linestart - line) - 1);
while (len && isspace(linestart[len - 1]))
--len;
linestart[len] = '\0';
return strdup(linestart);
This is untested however and I didn't verify if the 2nd parameter to
strnlen is right, maybe the -1 can be dropped?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
