Re: [PATCH] regmap: debugfs: check count when read regmap file

From: Mark Brown
Date: Mon Mar 09 2020 - 08:05:47 EST


On Sun, Mar 08, 2020 at 09:10:58PM +0800, peng.fan@xxxxxxx wrote:
> From: Peng Fan <peng.fan@xxxxxxx>

> @@ -283,9 +283,10 @@ static ssize_t regmap_map_read_file(struct file *file, char __user *user_buf,
> size_t count, loff_t *ppos)
> {
> struct regmap *map = file->private_data;
> + size_t num = count > map->max_register ? map->max_register : count;

I can see that it might be useful to limit the read size (though our
error checking is doing the right thing here, it's just that kmalloc()
is very verbose) but this doesn't seem like a good limit, especially for
smaller register maps. Since it's limiting reads to the number of
registers it's going to result in it being impossible to dump the full
register map in a single read. This is fine from a filesystem API point
of view, reads can always return less data than was asked for, but it's
annoying from the point of view of anyone hacking together something
like a little program to monitor a specific register during testing or
whatever. If the register map is small enough you won't even be able to
read a single register in a read which is going to be annoying. Having
either a lower bound or a more generous upper bound would be better.

Please also write normal conditional statements, the ternery operator
isn't great for legibility.

Attachment: signature.asc
Description: PGP signature