Opened 13 years ago
Closed 13 years ago
#9246 closed Bug Report (fixed)
lirc handling code is branching based on uninitialized data
Reported by: | beirdo | Owned by: | beirdo |
---|---|---|---|
Priority: | minor | Milestone: | 0.25 |
Component: | MythTV - General | Version: | Master Head |
Severity: | low | Keywords: | |
Cc: | Ticket locked: | no |
Description
==00:00:02:57.622 28658== Conditional jump or move depends on uninitialised value(s) ==00:00:02:57.622 28658== at 0x7A1E414: LIRC::GetCodes() (lirc.cpp:526) ==00:00:02:57.622 28658== by 0x7A1D81F: LIRC::run() (lirc.cpp:466) ==00:00:02:57.622 28658== by 0xEDF8774: ??? (in /usr/lib/libQtCore.so.4.6.2) ==00:00:02:57.622 28658== by 0xBC0C9C9: start_thread (pthread_create.c:300) ==00:00:02:57.622 28658== by 0xFA9670C: clone (clone.S:112) ==00:00:02:57.622 28658== Uninitialised value was created by a heap allocation ==00:00:02:57.622 28658== at 0x4C275A2: realloc (vg_replace_malloc.c:525) ==00:00:02:57.622 28658== by 0xEDFB599: QByteArray::realloc(int) (in /usr/lib/libQtCore.so.4.6.2) ==00:00:02:57.622 28658== by 0xEDFB978: QByteArray::resize(int) (in /usr/lib/libQtCore.so.4.6.2) ==00:00:02:57.622 28658== by 0x7A1E3F9: LIRC::GetCodes() (lirc.cpp:525) ==00:00:02:57.622 28658== by 0x7A1D81F: LIRC::run() (lirc.cpp:466) ==00:00:02:57.622 28658== by 0xEDF8774: ??? (in /usr/lib/libQtCore.so.4.6.2) ==00:00:02:57.622 28658== by 0xBC0C9C9: start_thread (pthread_create.c:300) ==00:00:02:57.622 28658== by 0xFA9670C: clone (clone.S:112)
The code in question is:
buf.resize(buf_offset); ret = buf.split('\n'); buf.resize(tmpc); if (buf.endsWith('\n'))
Line 526 is the "endsWith". The issue here is that when you increase the size of a QByteArray with resize(), the new capacity is composed of uninitialized bytes, not zeroed out.
I was considering fixing this, but I'm not 100% sure what is intended here, so I thought I'd punt it back to the author.
Change History (3)
Note: See
TracTickets for help on using
tickets.
Rework LIRC::GetCodes?() to simplify buffer usage
Fixes #9246
This started as a fix for a valgrind-found branch based on uninitialized data. As I was in there trying to make it not be branching based on the last character after expanding the buffer, I simplified the algorithm as well.
Now, we resize the buffer to hold 128 new bytes just before doing the read, and always read up to 128 bytes. Instead of resizing after, I changed it to a truncate, and use takeLast() rather than back() and pop_back() on the returned list.