ACK: [PATCH] lib: fwts_alloc: re-work the memory parsing again

[ivanhu]

On 06/12/2017 05:34 PM, Colin King wrote:
> From: Colin Ian King <colin.king at canonical.com>
>
> Unfortunately the previous fix of mine causes tainted variable warnings
> on CoverityScan static analysis.  I'm putting the original code back and
> adding a file position check to fix the bug that the previous patch fixed.
> The issue is that fscanf on the last line of a /proc/self/maps may cause
> an infinite loop if it does not parse with the expected input and we don't
> hit the EOF.  So instead, check that the parsing isn't stuck on file position
> and if it has not moved forward then we break out of the loop.
>
> Fixes: 56ca0bef8cea79 ("lib: fwts_alloc: only parse mem info once we have a valid line read in")
> Signed-off-by: Colin Ian King <colin.king at canonical.com>
> ---
>   src/lib/src/fwts_alloc.c | 19 +++++++++++++++----
>   1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/src/lib/src/fwts_alloc.c b/src/lib/src/fwts_alloc.c
> index e65b181e..4f136777 100644
> --- a/src/lib/src/fwts_alloc.c
> +++ b/src/lib/src/fwts_alloc.c
> @@ -234,12 +234,12 @@ static void *fwts_low_mmap(const size_t requested_size)
>   {
>   	FILE *fp;
>   	char pathname[1024];
> -	char buffer[4096];
>   	void *addr_start;
>   	void *addr_end;
>   	void *last_addr_end = NULL;
>   	void *first_addr_start = NULL;
>   	void *ret = MAP_FAILED;
> +	long pos, prev_pos = 0;
>   
>   	if (requested_size == 0)	/* Illegal */
>   		return MAP_FAILED;
> @@ -252,9 +252,20 @@ static void *fwts_low_mmap(const size_t requested_size)
>   	if (!fp)
>   		return fwts_low_mmap_walkdown(requested_size);
>   
> -	while (fgets(buffer, sizeof(buffer) - 1, fp)) {
> -		if (sscanf(buffer, "%p-%p %*s %*x %*s %*u %1023s\n",
> -		    &addr_start, &addr_end, pathname) != 3)
> +	while (!feof(fp)) {
> +		int n;
> +
> +		n = fscanf(fp, "%p-%p %*s %*x %*s %*u %1023s\n",
> +		    &addr_start, &addr_end, pathname);
> +
> +		/*  At same point of previous scanf, then force a loop exit */
> +		pos = ftell(fp);
> +		if (pos == prev_pos)
> +			break;
> +		prev_pos = pos;
> +
> +		/*  We need exactly 3 parsed items of data */
> +		if (n != 3)
>   			continue;
>   		/*
>   		 *  Sanity check data

Acked-by: Ivan Hu <ivan.hu at canonical.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/fwts-devel/attachments/20170613/bf9646cc/attachment.html>