Wireshark-commits: [Wireshark-commits] master 9118d95: Qt/PacketList: read packet record in private
From: Wireshark code review <code-review-do-not-reply@xxxxxxxxxxxxx>
Date: Sun, 30 Sep 2018 16:30:13 +0000
URL: https://code.wireshark.org/review/gitweb?p=wireshark.git;a=commit;h=9118d959a4f6231a7e928161620e579467b610a9 Submitter: "Guy Harris <guy@xxxxxxxxxxxx>" Changed: branch: master Repository: wireshark Commits: 9118d95 by Peter Wu (peter@xxxxxxxxxxxxx): Qt/PacketList: read packet record in private buffer To prevent potential interference with other users of the capture file, read data in a private buffer instead of reusing the one from capFile. An accidental (?) change in commit v2.9.0rc0-2001-g123bcb0362 resulted in "cf_read_record" reallocating the capture_file->buf buffer. That issue combined with the current behavior would result in a crash when ignoring a packet followed by two times opening a context menu: ==32187==ERROR: AddressSanitizer: heap-use-after-free on address 0x7fda91642800 at pc 0x55a98f3faaa7 bp 0x7fffa2807860 sp 0x7fffa2807858 READ of size 1 at 0x7fda91642800 thread T0 #0 0x55a98f3faaa6 in QByteArray::operator[](int) const /usr/include/qt/QtCore/qbytearray.h:476:47 #1 0x55a9901006eb in ByteViewText::drawLine(QPainter*, int, int) ui/qt/widgets/byte_view_text.cpp:370:35 #2 0x55a9900fd109 in ByteViewText::paintEvent(QPaintEvent*) ui/qt/widgets/byte_view_text.cpp:217:9 ... #50 0x55a98e9fd32a in PacketList::contextMenuEvent(QContextMenuEvent*) ui/qt/packet_list.cpp:614:15 ... 0x7fda91642800 is located 0 bytes inside of 3038371-byte region [0x7fda91642800,0x7fda919284a3) freed by thread T0 here: #0 0x55a98e65fd99 in __interceptor_realloc (run/wireshark+0x1019d99) #1 0x7fdac6e1bb88 in g_realloc /build/src/glib/glib/gmem.c:164 #2 0x7fdaac12c908 in wtap_read_packet_bytes wiretap/wtap.c:1368:2 #3 0x7fdaabf01e5a in libpcap_read_packet wiretap/libpcap.c:789:7 #4 0x7fdaabef887d in libpcap_seek_read wiretap/libpcap.c:690:7 #5 0x7fdaac12d5f5 in wtap_seek_read wiretap/wtap.c:1431:7 #6 0x55a98e6c8611 in cf_read_record_r file.c:1566:8 #7 0x55a98e6c88c5 in cf_read_record file.c:1576:10 #8 0x55a98ea0b725 in PacketList::getFilterFromRowAndColumn() ui/qt/packet_list.cpp:1041:14 #9 0x55a98e94e4a1 in MainWindow::setMenusForSelectedPacket() ui/qt/main_window_slots.cpp:1175:39 previously allocated by thread T0 here: #0 0x55a98e65fd99 in __interceptor_realloc (run/wireshark+0x1019d99) #1 0x7fdac6e1bb88 in g_realloc /build/src/glib/glib/gmem.c:164 #2 0x7fdaac12c908 in wtap_read_packet_bytes wiretap/wtap.c:1368:2 #3 0x7fdaabf01e5a in libpcap_read_packet wiretap/libpcap.c:789:7 #4 0x7fdaabef887d in libpcap_seek_read wiretap/libpcap.c:690:7 #5 0x7fdaac12d5f5 in wtap_seek_read wiretap/wtap.c:1431:7 #6 0x55a98e6c8611 in cf_read_record_r file.c:1566:8 #7 0x55a98e6c88c5 in cf_read_record file.c:1576:10 #8 0x55a98e6e0bde in cf_select_packet file.c:3777:8 #9 0x55a98e9ea2ff in PacketList::selectionChanged(QItemSelection const&, QItemSelection const&) ui/qt/packet_list.cpp:420:9 This should be fixed now by I4f1264a406a28c79491dcd77c552193bf3cdf62d, but let's avoid the shared buffer. It's not exactly a hot code path anyway. Change-Id: I548d7293a822601f4eb882672477540f066a066b Reviewed-on: https://code.wireshark.org/review/29921 Petri-Dish: Peter Wu <peter@xxxxxxxxxxxxx> Tested-by: Petri Dish Buildbot Reviewed-by: Guy Harris <guy@xxxxxxxxxxxx> Actions performed: from 5a401cc [Automatic update for 2018-09-30] add 9118d95 Qt/PacketList: read packet record in private buffer Summary of changes: ui/qt/packet_list.cpp | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)
- Prev by Date: [Wireshark-commits] master-2.4 bd7695c: [Automatic update for 2018-09-30]
- Next by Date: [Wireshark-commits] master 6ca646e: Qt/FrameInformation: read packet record in private buffer
- Previous by thread: [Wireshark-commits] master-2.4 bd7695c: [Automatic update for 2018-09-30]
- Next by thread: [Wireshark-commits] master 6ca646e: Qt/FrameInformation: read packet record in private buffer
- Index(es):