Em Wed, 08 Jan 2014 14:33:30 +0100
Andrzej Hajda <a.hajda@xxxxxxxxxxx> escreveu:
> On 01/08/2014 01:21 PM, Mauro Carvalho Chehab wrote:
> > Em Wed, 8 Jan 2014 11:37:37 +0300
> > Dan Carpenter <dan.carpenter@xxxxxxxxxx> escreveu:
> >
> >> The other thing that concerned me with this was the sparse warning:
> >>
> >> drivers/media/i2c/s5k5baf.c:481:26: error: bad constant expression
> > Hmm...
> > static void s5k5baf_write_arr_seq(struct s5k5baf *state, u16 addr,
> > u16 count, const u16 *seq)
> > {
> > struct i2c_client *c = v4l2_get_subdevdata(&state->sd);
> > __be16 buf[count + 1];
> > int ret, n;
> >
> > Yeah, allocating data like that at stack is not nice.
> >
> > I would simply replace the static allocation here by a dynamic one.
> Sequences are very short (usually few words) and their length is known
> in compile time.
> The only exception are sequences provided by firmware file and for them
> I can add check in s5k5baf_write_nseq to make it safe.
>
> Replacing it with dynamic allocation seems to me unnecessary in this
> particular case, it would result in memory allocation/free for every
> single access to
> the device. What do you think?
Well, if you know in advance what's the maximum size, just replace it by
something like:
__be16 buf[64];
and check if count + 1 is less or equal to sizeof(buf).
As the kernel stack is really small, we should avoid allocating large
data there (1KB is large, on that sense).
Also, it would be possible o use i2cdev to inject very large payloads
to be sent to a random I2C device. That could cause a machine OOPS
or to use it to inject a security breach code.
So, we should really avoid using dynamic static allocation, specially
on I2C handlers.
Regards,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
