Re: [PATCH 3/4] mtd: Introduce mtd_block_isreserved()

[Brian Norris <computersforpeace@gmail.com>]

On Thu, May 15, 2014 at 05:15:18PM -0300, Ezequiel Garcia wrote:
> On 14 May 04:57 PM, Brian Norris wrote:
> > All? Like what? mtd_read_oob()? mtd_get_fact_prot_info()? These return
> > -EOPNOTSUPP, which is an informative error code. But that's different
> > than returning 0 for mtd_block_isbad() or mtd_block_isreserved(), even
> > if the block doesn't exist.
> 
> Ah... I misunderstood your comment. The point is that you don't want to
> return 0, when the parameters may be are wrong.

Right. I think it is wrong to return '0' for the out-of-bounds case.

> So, I'll fix this patch by changing both, mtd_block_isbad and
> mtd_block_isreserved, just as you asked in the first place.

Sounds good.

> > (Also, is it just me, or is mtd_writev() missing bounds checking?)
> 
> Hm... so it seems. I guess the check should be something like:
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index d201fee..21ce0f4 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -1066,11 +1066,22 @@ static int default_mtd_writev(struct mtd_info *mtd, const struct kvec *vecs,
>  int mtd_writev(struct mtd_info *mtd, const struct kvec *vecs,
>  	       unsigned long count, loff_t to, size_t *retlen)
>  {
> +	int i;
> +
>  	*retlen = 0;
>  	if (!(mtd->flags & MTD_WRITEABLE))
>  		return -EROFS;
>  	if (!mtd->_writev)
>  		return default_mtd_writev(mtd, vecs, count, to, retlen);
> +
> +	if (to < 0 || to > mtd->size)
> +		return -EINVAL;
> +	for (i = 0; i < count; i++) {
> +		if (!vecs[i].iov_len)
> +			continue;
> +		if (vecs[i].iov_len > mtd->size - to)

This isn't quite the right check. It's OK for the first iteration, but
it's not a good check for vecs[i].iov_len where i > 0. Maybe just check
the sum of all the lengths against `mtd->size - to'.

> +			return -EINVAL;
> +	}
>  	return mtd->_writev(mtd, vecs, count, to, retlen);
>  }
>  EXPORT_SYMBOL_GPL(mtd_writev);
> 
> Right?

Brian