Re: Mtd alignment bug affecting 1.4.X including current git

[Kelly Anderson <kelly@silka.with-linux.com>]

On 04/01/11 02:16, Artem Bityutskiy wrote:
> On Mon, 2011-03-28 at 07:48 -0600, Kelly Anderson wrote:
>> If you create a patch I'll test it for you.
> Kelly, would you please test the following patch:

Seems to work fine.

> From: Artem Bityutskiy<Artem.Bityutskiy@nokia.com>
> Subject: [PATCH] libmtd: fix OOB read and write interface
>
> When reading and writing OOB we specify the address as absolute
> offset from the beginning of the MTD device. This offset is
> basically an absolute page offset plus the OOB offset. And it does
> not have to be aligned to the min. I/O unit size (NAND page size).
>
> So fix the 'do_oob_op()' function and remove incorrect checking
> that the offset is page-aligned. This check leads to the following
> errors:
>
> libmtd: error!: unaligned address 2, mtd0 page size is 2048
>
> But obviously, the intent was to write to offset 2 of the OOB area
> of the very first NAND page.
>
> Instead of that incorrect check, we should check that the OOB offset
> we write to is within the OOB size and the length is withing the OOB
> size. This patch adds such check.
>
> Reported-by: Kelly Anderson<kelly@silka.with-linux.com>
> Signed-off-by: Artem Bityutskiy<Artem.Bityutskiy@nokia.com>
> ---
>   lib/libmtd.c |   12 ++++++++----
>   1 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/lib/libmtd.c b/lib/libmtd.c
> index e0c0934..e313fc3 100644
> --- a/lib/libmtd.c
> +++ b/lib/libmtd.c
> @@ -1083,6 +1083,7 @@ int do_oob_op(libmtd_t desc, const struct mtd_dev_info *mtd, int fd,
>   	struct mtd_oob_buf64 oob64;
>   	struct mtd_oob_buf oob;
>   	unsigned long long max_offs;
> +	unsigned int oob_offs;
>   	const char *cmd64_str, *cmd_str;
>   	struct libmtd *lib = (struct libmtd *)desc;
>
> @@ -1102,10 +1103,13 @@ int do_oob_op(libmtd_t desc, const struct mtd_dev_info *mtd, int fd,
>   		errno = EINVAL;
>   		return -1;
>   	}
> -	if (start % mtd->min_io_size) {
> -		errmsg("unaligned address %llu, mtd%d page size is %d",
> -		       (unsigned long long)start, mtd->mtd_num,
> -		       mtd->min_io_size);
> +
> +	oob_offs = start&  (mtd->min_io_size - 1);
> +	if (oob_offs + length>  mtd->oob_size) {
> +		errmsg("Cannot write %llu OOB bytes to address %llu "
> +		       "(OOB offset %u) - mtd%d OOB size is only %d bytes",
> +		       (unsigned long long)length, (unsigned long long)start,
> +		       oob_offs, mtd->mtd_num,  mtd->oob_size);
>   		errno = EINVAL;
>   		return -1;
>   	}