Bug in lirc_serial (and possibly other drivers)

[Matt G. <th...@xa...>]

Hi there,

First of all, apologies if I'm way off the mark here: I'm fairly new to
lirc and I may well be wrong.  I've been working on a driver for lirc
for the EP93xx series processors' IO ports, and after a fair amount of
head-scratching I found a bug in my code which is also in the serial
driver (as that's what I used as a basis for my driver).

Bit of a complex one this, so bear with me, I'll try and explain as best
I can (and assuming I have the right end of the stick).  I believe this
bug only appears because of the way the ARM compiler works, but it's a
valid bug nonetheless.

In init_module of lirc_serial.c, the plugin registers with lirc_dev via
lirc_register_plugin().  At this point it passes a pointer to its
'plugin' structure, which contains a pointer to its read buffer 'rbuf'. 
At this point, the rbuf is NOT initialised (or rather, it is initialised
to all zeros by the linker.)

The code for lirc_register_plugin inside lirc_dev.c then takes a copy of
one of the fields in the read structure:
    ir->chunk_size = ir->buf->chunk_size;

NB this is the value ZERO.  This is the only point where ir->chunk_size
is initialised, so it will remain zero.  The only place this value is
used is in irctl_read:
{
    struct irctl *ir = &irctls[MINOR(file->f_dentry->d_inode->i_rdev)];
    unsigned char buf[ir->chunk_size];

...uh-oh - we've made a zero-length array...on the ARM this crashes the
kernel out (as you'd expect!)  On x86 I suspect alignment issues (or
blind fluke) might just let this work out, but it's still wrong!

As the ir structure has a buf pointer, it could just use the
ir->buf->chunk_size.  This is what I've modified the code to do, and it
seems to have fixed the issue.  So firstly: Why is there an
ir->chunk_size and also an ir->buf->chunk_size ?  Is there some subtlety
I'm missing (perhaps relating to user read functions ir->p.fops->read)? 
If there is a reason for it, then the rbuf needs be initialised before
lirc_register_plugin() takes a copy of the chunk size.

Apologies if this is the wrong list for such things.  Don't hesitate to
ask me for clarification if I haven't explained myself too well, I'm not
the world's greatest at saying what I mean :)

While I'm here, thank you for such a great project; despite this setback
I have my tiny little embedded system reading remote controls within
only a day - fantastic.

Thanks in advance, hope this issue can be fixed,

Regards,

Matt
-==-
//