redis / hiredis

Minimalistic C client for Redis >= 1.2
BSD 3-Clause "New" or "Revised" License
6.15k stars 1.8k forks source link

Remove dangling pointer dereference in RedisQtAdapter destructor #1230

Open ochsnerd opened 9 months ago

ochsnerd commented 9 months ago

The destructor of RedisQtAdapter will be called AFTER the context has been freed. At that point, m_ctx is a dangling pointer and should not be dereferenced.

See the valgrind output of the example-qt.cpp:

==31513== 1 errors in context 1 of 1:
==31513== Invalid write of size 8
==31513==    at 0x10B885: RedisQtAdapter::~RedisQtAdapter() (qt.h:77)
==31513==    by 0x10B966: ExampleQt::~ExampleQt() (example-qt-fix.h:21)
==31513==    by 0x10BC70: main (example-qt-fix.cpp:47)
==31513==  Address 0x7a7b120 is 304 bytes inside a block of size 464 free'd
==31513==    at 0x484B27F: free (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==31513==    by 0x486260B: hi_free (in /home/david/hiredis_async/install/lib/libhiredis.so.1.2.1-dev)
==31513==    by 0x486442F: redisFree (in /home/david/hiredis_async/install/lib/libhiredis.so.1.2.1-dev)
==31513==    by 0x4860ABD: __redisAsyncFree (in /home/david/hiredis_async/install/lib/libhiredis.so.1.2.1-dev)
==31513==    by 0x4860B29: redisAsyncFree (in /home/david/hiredis_async/install/lib/libhiredis.so.1.2.1-dev)
==31513==    by 0x10B956: ExampleQt::~ExampleQt() (example-qt-fix.h:19)
==31513==    by 0x10BC70: main (example-qt-fix.cpp:47)
==31513==  Block was alloc'd at
==31513==    at 0x484DCD3: realloc (in /usr/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==31513==    by 0x485EDDD: hi_realloc (in /home/david/hiredis_async/install/lib/libhiredis.so.1.2.1-dev)
==31513==    by 0x485FDB7: redisAsyncInitialize (in /home/david/hiredis_async/install/lib/libhiredis.so.1.2.1-dev)
==31513==    by 0x486005E: redisAsyncConnectWithOptions (in /home/david/hiredis_async/install/lib/libhiredis.so.1.2.1-dev)
==31513==    by 0x486014C: redisAsyncConnect (in /home/david/hiredis_async/install/lib/libhiredis.so.1.2.1-dev)
==31513==    by 0x10BA67: ExampleQt::run() (example-qt-fix.cpp:22)
==31513==    by 0x10B4FC: ExampleQt::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) (moc_example-qt-fix.cpp:79)
==31513==    by 0x4CA3D40: QObject::event(QEvent*) (in /opt/Qt5.12.12/5.12.12/gcc_64/lib/libQt5Core.so.5.12.12)
==31513==    by 0x4C77312: QCoreApplication::notifyInternal2(QObject*, QEvent*) (in /opt/Qt5.12.12/5.12.12/gcc_64/lib/libQt5Core.so.5.12.12)
==31513==    by 0x4C79DC6: QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) (in /opt/Qt5.12.12/5.12.12/gcc_64/lib/libQt5Core.so.5.12.12)
==31513==    by 0x4CCF012: postEventSourceDispatch(_GSource*, int (*)(void*), void*) (in /opt/Qt5.12.12/5.12.12/gcc_64/lib/libQt5Core.so.5.12.12)
==31513==    by 0x5AEED3A: g_main_context_dispatch (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.7200.4)
==31513== 
==31513== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

Nulling redisAsyncContext.ev.data at that point makes no sense anyway, since the context has been freed already. So the proposed fix is to remove the offending code