Opened 9 years ago

Closed 8 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)

comment:1 Changed 9 years ago by beirdo

Status: newassigned

comment:2 Changed 8 years ago by beirdo

Owner: changed from danielk to beirdo

comment:3 Changed 8 years ago by Github

Milestone: unknown0.25
Resolution: fixed
Status: assignedclosed

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.

Branch: master Changeset: 83e3c5ebc475a28ece668111ec13dacf0d80fe1d

Note: See TracTickets for help on using tickets.