| Summary: | NULL pointer dereference in KPtyDevicePrivate::doWait | ||
|---|---|---|---|
| Product: | [Unmaintained] kdelibs | Reporter: | Christopher Yeleighton <giecrilj> |
| Component: | kdecore | Assignee: | kdelibs bugs <kdelibs-bugs> |
| Status: | RESOLVED WORKSFORME | ||
| Severity: | crash | CC: | adaptee, cpigat242, mpyne, ossi |
| Priority: | NOR | ||
| Version: | 4.10.5 | ||
| Target Milestone: | --- | ||
| Platform: | Compiled Sources | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
| Attachments: | check readNotifier | ||
Created attachment 81919 [details]
check readNotifier
Most of KPtyDevice seems to pay no mind to whether readNotifier is valid, as the object is created as part of opening the PTY. The only exception seems to be if there is already a master file descriptor setup. So I think the proposed fix is inappropriate, as presumably many more null checks would be needed. I believe the actual bug is in the testcase itself. KPtyProcess::start() is really KProcess::start(), which is documented as starting the process, waiting for it to complete, and returning the exit code. When utmp support is enabled, the transition from a running process to a finished process would cause the PTY to be logged out, which invalidates all socket notifiers (including readNotifier). In other words the process was already allowed to run to completion and so we don't bother with its PTY anymore; the underlying KPtyDevice is essentially in an invalid state. I believe the test case should use p.execute() instead of p.start() with the rest of the testcase being more-or-less satisfactory the way it is. I've CC'ed the KPty dev to double-check my logic though. the test log indicates quite clearly that the root cause is a failure to open a pty. the test should do QVERIFY(p.pty()->isOpen()); right after instantiating the KPtyProcess. actual user code of KPtyProcess (konsole in particular) should be checked whether it does this check, too. (In reply to comment #3) > the test log indicates quite clearly that the root cause is a failure to > open a pty. > the test should do QVERIFY(p.pty()->isOpen()); right after instantiating the > KPtyProcess. > actual user code of KPtyProcess (konsole in particular) should be checked > whether it does this check, too. Library code should not rely on the customer observing complicated conditions and transitions or otherwise we will crash. I understand that is sometimes necessary for efficiency reasons (e.g. invalid iterators) but I think this is not the case this time. it's unreasonable to harden the code against each possible abuse. latest when memory corruption comes into play, there is no way to get it right. the correct reaction would be refusing to start the process, a condition which is more likely to be tested by the user. (In reply to comment #5) > it's unreasonable to harden the code against each possible abuse. latest > when memory corruption comes into play, there is no way to get it right. > the correct reaction would be refusing to start the process, a condition > which is more likely to be tested by the user. A NULL pointer is not a manifestation of corrupted memory; just the opposite, it is commonly used to indicate no value. Thank you for the crash report. As it has been a while since this was reported, can you please test and confirm if this issue is still occurring or if this bug report can be marked as resolved. I have set the bug status to "needsinfo" pending your response, please change back to "reported" or "resolved/worksforme" when you respond, thank you. Dear Bug Submitter, This bug has been in NEEDSINFO status with no change for at least 15 days. Please provide the requested information as soon as possible and set the bug status as REPORTED. Due to regular bug tracker maintenance, if the bug is still in NEEDSINFO status with no change in 30 days the bug will be closed as RESOLVED > WORKSFORME due to lack of needed information. For more information about our bug triaging procedures please read the wiki located here: https://community.kde.org/Guidelines_and_HOWTOs/Bug_triaging If you have already provided the requested information, please mark the bug as REPORTED so that the KDE team knows that the bug is ready to be confirmed. Thank you for helping us make KDE software even better for everyone! This bug has been in NEEDSINFO status with no change for at least 30 days. The bug is now closed as RESOLVED > WORKSFORME due to lack of needed information. For more information about our bug triaging procedures please read the wiki located here: https://community.kde.org/Guidelines_and_HOWTOs/Bug_triaging Thank you for helping us make KDE software even better for everyone! |
The expression (readNotifier->isEnabled()) at kpty/kptydevice.cpp:414 dereferences a NULL pointer (readNotifier). Reproducible: Always Steps to Reproduce: 1. /home/abuild/rpmbuild/BUILD/kdelibs-4.10.5/build/kpty/tests/kptyprocesstest Actual Results: 1. qttest(8225)/kdecore (KPty/K3Process) KPty::open: Can't open a pseudo teletype ********* Start testing of KPtyProcessTest ********* Config: Using QTest library 4.8.5, Qt 4.8.5 PASS : KPtyProcessTest::initTestCase() QDEBUG : KPtyProcessTest::test_pty_basic() qttest(8225)/kdecore (KPty/K3Process) KPty::open: Can't open a pseudo teletype QFATAL : KPtyProcessTest::test_pty_basic() Received signal 11 FAIL! : KPtyProcessTest::test_pty_basic() Received a fatal error. Loc: [Unknown file(0)] Totals: 1 passed, 1 failed, 0 skipped ********* Finished testing of KPtyProcessTest ********* Expected Results: 1. Let the test fail without crashing. #0 0x00007ffff69783d5 in __GI_raise (sig=sig@entry=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:56 #1 0x00007ffff6979858 in __GI_abort () at abort.c:90 #2 0x00007ffff706dc54 in qt_message_output (msgType=msgType@entry=QtFatalMsg, buf=<optimized out>) at global/qglobal.cpp:2323 #3 0x00007ffff706de08 in qt_message(QtMsgType, const char *, typedef __va_list_tag __va_list_tag *) (msgType=msgType@entry=QtFatalMsg, msg= 0x7ffff79bb1e0 "Received signal %d", ap=ap@entry=0x7fffffffcb18) at global/qglobal.cpp:2369 #4 0x00007ffff706df94 in qFatal (msg=<optimized out>) at global/qglobal.cpp:2552 #5 0x00007ffff79a90e4 in QTest::FatalSignalHandler::signal (signum=<optimized out>) at qtestcase.cpp:1729 #6 <signal handler called> #7 0x00007ffff7bd6e84 in KPtyDevicePrivate::doWait (this=0x651c40, msecs=<optimized out>, reading=true) at /home/abuild/rpmbuild/BUILD/kdelibs-4.10.5/build/kpty/kptydevice.moc:108 #8 0x00000000004036ed in KPtyProcessTest::test_pty_basic ( this=<optimized out>) at /home/abuild/rpmbuild/BUILD/kdelibs-4.10.5/kpty/tests/kptyprocesstest.cpp:134 #9 0x00007ffff717be57 in QMetaMethod::invoke (this=this@entry=0x7fffffffd4e0, object=object@entry=0x7fffffffdf00, connectionType=connectionType@entry= Qt::DirectConnection, returnValue=..., val0=..., val1=..., val2=..., val3= ..., val4=..., val5=..., val6=..., val7=..., val8=..., val9=...) at kernel/qmetaobject.cpp:1664 #10 0x00007ffff717e22c in QMetaObject::invokeMethod (obj=0x7fffffffdf00, member=<optimized out>, type=Qt::DirectConnection, ret=..., val0=..., val1= ..., val2=..., val3=..., val4=..., val5=..., val6=..., val7=..., val8=..., val9=...) at kernel/qmetaobject.cpp:1179 #11 0x00007ffff79ac2e2 in invokeMethod (val9=..., val8=..., val7=..., val6= ..., val5=..., val4=..., val3=..., val2=..., val1=..., val0=..., type= Qt::DirectConnection, member=0x65f560 "test_pty_basic", obj=<optimized out>) at ../../src/corelib/kernel/qobjectdefs.h:418 #12 qInvokeTestMethodDataEntry (slot=0x65f560 "test_pty_basic") at qtestcase.cpp:1423 #13 QTest::qInvokeTestMethod (slotName= 0x4051f1 <qt_meta_stringdata_KPtyProcessTest+17> "test_pty_basic()", data=data@entry=0x0) at qtestcase.cpp:1531 ---Type <return> to continue, or q <return> to quit---f 7 #14 0x00007ffff79ad177 in qInvokeTestMethods (testObject=0x7fffffffdf00) at qtestcase.cpp:1696 #15 QTest::qExec (testObject=0x7fffffffdf00, argc=<optimized out>, argv=<optimized out>) at qtestcase.cpp:1919 #16 0x0000000000402e3a in main (argc=1, argv=0x7fffffffe068) at /home/abuild/rpmbuild/BUILD/kdelibs-4.10.5/kpty/tests/kptyprocesstest.cpp:220