Re: [PATCH] mtd-utils: lib: mtd_read: Take the buffer offset into account when reading

[Brian Norris <computersforpeace@gmail.com>]

Hi Marcus,

On Tue, Oct 06, 2015 at 02:13:23PM +0200, Marcus Prebble wrote:
> Hi mtd-list,
> 
> Assuming the read() call does not return zero and the result is less
> than len, the current implementation will overwrite the data already
> read in buf which doesn't seem correct.

In addition, this means we might not actually fill up the entire buffer,
since 2 or more short read()'s might get us to exit the loop with a
cumulative value in 'rd', but only a partially-filled buffer. That could
cause a user to try and handle garbage/uninitialized data.

> Suggested patch attached (git format-patch)
> 
> -Marcus

> From 5dccbbd87604665e896472d52f3351c14c24d2b3 Mon Sep 17 00:00:00 2001
> From: Marcus Prebble <prebble@axis.com>
> Date: Mon, 5 Oct 2015 17:32:54 +0200
> Subject: [PATCH] mtd_read: Take the buffer offset into account when reading
> 
> Subsequent calls to read() within the loop will now no longer
> overwrite the existing contents of buf.
> ---
>  lib/libmtd.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/libmtd.c b/lib/libmtd.c
> index 60b4782..bf6d71f 100644
> --- a/lib/libmtd.c
> +++ b/lib/libmtd.c
> @@ -1072,10 +1072,10 @@ int mtd_read(const struct mtd_dev_info *mtd, int fd, int eb, int offs,
>  				  mtd->mtd_num, seek);
>  
>  	while (rd < len) {
> -		ret = read(fd, buf, len);
> +		ret = read(fd, buf + rd, len - rd);
>  		if (ret < 0)
>  			return sys_errmsg("cannot read %d bytes from mtd%d (eraseblock %d, offset %d)",
> -					  len, mtd->mtd_num, eb, offs);
> +					  len - rd, mtd->mtd_num, eb, offs + rd);
>  		rd += ret;
>  	}
>  

Patch looks OK. Did you test it? Have you seen MTD drivers that will
return short reads?

Brian