[mythtv] lirc_client.c sending empty CODE cmd to LIRC -> segfault

[Michael T. Dean]

On 02/09/2010 10:39 PM, Michael T. Dean wrote:
>  On 02/09/2010 05:42 PM, David Kubicek wrote:
> > On 02/09/2010 10:09 PM, Michael T. Dean wrote:
> >> On 02/09/2010 03:42 PM, David Kubicek wrote:
> >>> I was just playing with the optional lircrcd LIRC daemon and
> >>> it kept segfaulting on me. I found the bug in the daemon
> >>> (latest CVS version), but the reason for this is MythTV sending
> >>> an empty CODE command. Until the bug is fixed in either of
> >>> these two applications, it'll break advanced LIRC
> >>> configurations...
> >>>
> >>> I haven't looked into MythTV, so I have no patch for you, but
> >>> I'll be sending a patch to LIRC people. If you actually want
> >>> the patch and it would be integrated soon (e.i. not in 6
> >>> months:)) Well, the command packet is actually from lircd, but
> >>> it really originates in MythTV.
> >>>
> >>> LOG:
> >>>
> >>> lircrcd[3673]: received command: "CODE 01007f0000000000 1e Down
> >>> imon_ultrabay_pad"
> >>>
> >> Are you sure it's not: http://svn.mythtv.org/trac/ticket/7653 ?
> >> Can you test that patch and provide feedback on it?
> >>
> >> Also, can you get a fix into lircrcd to accept line feeds
> >> properly (you mentioned you're already planning to send a patch
> >> to the LIRC devs for something else)?
> >>
> > That's right, the patch is finished and I'm going to notify LIRC
> > people tomorrow. I know about #7653 (found it via google), first I
> > made the change (added '\n'), but problem persisted.
> >
> > LIRC CVS checkout and gdb revealed a NULL dereference bug. The
> > CODE handler didn't have a NULL check after strtok() (message 'CODE
> > ' triggers it). The remaining command handlers are OK. So yes,
> > this is new and in both applications. In MythTV I can trigger 'CODE
> > ' easily by push-repeat of directional movement, like scrolling
> > down the list. Fast repeat rate possibly required. Sometimes it
> > happens instantly, sometimes I have to jiggle a few other buttons
> > around and keep scrolling up and down for a while.
> >
> > As for #7652, not including '\n' after CODE cmd is a protocol
> > violation to which lircrcd apparently react by exit. Either they
> > consider "lost sync" critical or it's just not handled very
> > gracefully. So there is no segfault in that case, just controlled
> > shutdown. It's still present in CVS and #7652 really should be
> > fixed. After all, it's just adding a '\n' at the end of one string
> > in libmythui/lirc_client.c...
>
>  Ah, I see...  I misread #7652--thought it was saying there was a
>  DOS/*nix linefeed difference and it didn't like it, so I thought
>  maybe you could fix it to accept either.  But, since we're sending
>  neither, that's not the issue.  :)
>
>  Thanks for the info.  Maybe I can look at getting #7652 in tomorrow.

So the only place we're dealing with LIRC's CODE is in 
libs/libmythui/lirc_client.{h,c} , which comes from LIRC's 
tools/lirc_client.{h,c}.  I can see that LIRC's "official" lirc_client.c 
doesn't yet include a line feed character at the end of the CODE line 
(see 
http://lirc.cvs.sourceforge.net/viewvc/lirc/lirc/tools/lirc_client.c?revision=5.29.2.1&view=markup 
on line 1672 ).

It seems Daniel K modified our version of lirc_client.{h,c} for thread 
safety before importing it so it has quite a few differences to the 
upstream version, but I'd still like to see the linefeed fix sent 
upstream.  Also, is your patch for LIRC for the tools/lirc_client.{h,c} 
code?  If so, once it's in the upstream copy, we could easily pull an 
update from them.

Attached is a (completely untested) patch that upgrades our version of 
lirc_client to be in line with the version in LIRC 0.8.6 (and a patch 
showing the differences in LIRC's unmodified lirc_client tool).  Feel 
free to test out the patch, but it likely won't help with the issue 
you're seeing.  I probably won't look at updating lirc_client until 
after 0.23 is released so the changes can get some testing before being 
backported to stable.  If you can push some changes (including the line 
feed change) into the upstream copy, it will make it easier to get those 
changes into our copy.  :)

Thanks,
Mike

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: mythtv-upgrade_lirc_client_to_0_8_6.patch
URL: <http://mythtv.org/pipermail/mythtv-dev/attachments/20100213/8e30a558/attachment.txt>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: lirc_client-upgrade_from_0_8_4a_to_0_8_6.patch
URL: <http://mythtv.org/pipermail/mythtv-dev/attachments/20100213/8e30a558/attachment.asc>