osmocom
/
wireshark
11
0
Fork 1
Code Issues Pull Requests Projects Releases Wiki Activity

Add a capture file state for a pending read 

When not updating the packet list during a capture, the capture
file structure isn't set up, but there is a pending capture.

We currently treat that as "finished reading", but that means
that other code assumes that all the structures are set up and
can crash, and also don't prompt regarding unsaved packets when
trying to close Wireshark.

Add a state for FILE_READ_PENDING that sometimes should be treated
similar to FILE_CLOSED and sometimes should be treated similar to
FILE_READ_IN_PROGRESS.

This fixes a crash when enabling "update packet list during a capture"
while a capture is in progress, as well a crash when applying a filter
while a capture is in progress but real time packet list updates are
off.

Keep track of the number of packets that the capture child has reported
that haven't been read yet, so that the capture statistics stay accurate
even if the pref is toggled. Also run the main status bar statistics at
the end, so that if any packets are processed in cf_finish_tail() they
are reported.

This also restores status bar statistics for when update packet list
during a capture is off, which 461fb517d1
accidentally disabled.

Fix #4035
This commit is contained in:
John Thacker 2023-06-11 10:18:18 -04:00
parent c58705654d
commit 801554fb79
11 changed files with 81 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
capture/capture_session.h
View File

@ -100,6 +100,7 @@ struct _capture_session {
#endif
gboolean session_will_restart; /**< Set when session will restart */
guint32 count; /**< Total number of frames captured */
guint32 count_pending; /**< Number of frames captured but not yet read */
capture_options *capture_opts; /**< options for this capture */
capture_file *cf; /**< handle to cfile */
wtap_rec rec; /**< record we're reading packet metadata into */

1
capture/capture_sync.c
View File

@ -136,6 +136,7 @@ capture_session_init(capture_session *cap_session, capture_file *cf,
cap_session->group = getgid();
#endif
cap_session->count = 0;
cap_session->count_pending = 0;
cap_session->session_will_restart = FALSE;
cap_session->new_file = new_file;

1
cfile.h
View File

@ -26,6 +26,7 @@ extern "C" {
/* Current state of file. */
typedef enum {
FILE_CLOSED, /* No file open */
FILE_READ_PENDING, /* A file to read, but haven't opened it yet */
FILE_READ_IN_PROGRESS, /* Reading a file we've opened */
FILE_READ_ABORTED, /* Read aborted by user */
FILE_READ_DONE /* Read completed */

10
file.c
View File

@ -347,7 +347,7 @@ void
cf_close(capture_file *cf)
{
cf->stop_flag = FALSE;
if (cf->state == FILE_CLOSED)
if (cf->state == FILE_CLOSED || cf->state == FILE_READ_PENDING)
return; /* Nothing to do */
/* Die if we're in the middle of reading a file. */
@ -908,7 +908,9 @@ cf_continue_tail(capture_file *cf, volatile int to_read, wtap_rec *rec,
void
cf_fake_continue_tail(capture_file *cf)
{
cf->state = FILE_READ_DONE;
if (cf->state == FILE_CLOSED) {
cf->state = FILE_READ_PENDING;
}
}
cf_read_status_t
@ -1680,6 +1682,10 @@ rescan_packets(capture_file *cf, const char *action, const char *action_item, gb
guint32 frames_count;
gboolean queued_rescan_type = RESCAN_NONE;
if (cf->state == FILE_CLOSED || cf->state == FILE_READ_PENDING) {
return;
}
/* Rescan in progress, clear pending actions. */
cf->redissection_queued = RESCAN_NONE;
ws_assert(!cf->read_lock);

41
ui/capture.c
View File

@ -406,17 +406,15 @@ capture_input_new_file(capture_session *cap_session, gchar *new_file)
if(capture_opts->save_file != NULL) {
/* we start a new capture file, close the old one (if we had one before). */
/* (we can only have an open capture file in real_time_mode!) */
if( ((capture_file *) cap_session->cf)->state != FILE_CLOSED) {
if(capture_opts->real_time_mode) {
cap_session->session_will_restart = TRUE;
capture_callback_invoke(capture_cb_capture_update_finished, cap_session);
cf_finish_tail((capture_file *)cap_session->cf,
&cap_session->rec, &cap_session->buf, &err,
&cap_session->frame_dup_cache, cap_session->frame_cksum);
cf_close((capture_file *)cap_session->cf);
} else {
capture_callback_invoke(capture_cb_capture_fixed_finished, cap_session);
}
if (((capture_file*)cap_session->cf)->state == FILE_READ_PENDING) {
capture_callback_invoke(capture_cb_capture_fixed_finished, cap_session);
} else if (((capture_file*)cap_session->cf)->state != FILE_CLOSED) {
cap_session->session_will_restart = TRUE;
capture_callback_invoke(capture_cb_capture_update_finished, cap_session);
cf_finish_tail((capture_file *)cap_session->cf,
&cap_session->rec, &cap_session->buf, &err,
&cap_session->frame_dup_cache, cap_session->frame_cksum);
cf_close((capture_file *)cap_session->cf);
}
g_free(capture_opts->save_file);
is_tempfile = FALSE;
@ -537,7 +535,23 @@ capture_input_new_packets(capture_session *cap_session, int to_read)
ws_assert(capture_opts->save_file);
if(capture_opts->real_time_mode) {
if (((capture_file*)cap_session->cf)->state == FILE_READ_PENDING) {
/* Attempt to open the capture file and set up to read from it. */
switch (cf_open((capture_file*)cap_session->cf, capture_opts->save_file, WTAP_TYPE_AUTO, cf_is_tempfile((capture_file*)cap_session->cf), &err)) {
case CF_OK:
break;
case CF_ERROR:
/* Don't unlink (delete) the save file - leave it around,
for debugging purposes. */
g_free(capture_opts->save_file);
capture_opts->save_file = NULL;
capture_kill_child(cap_session);
}
capture_callback_invoke(capture_cb_capture_update_started, cap_session);
}
/* Read from the capture file the number of records the child told us it added. */
to_read += cap_session->count_pending;
cap_session->count_pending = 0;
switch (cf_continue_tail((capture_file *)cap_session->cf, to_read,
&cap_session->rec, &cap_session->buf, &err,
&cap_session->frame_dup_cache, cap_session->frame_cksum)) {
@ -560,6 +574,7 @@ capture_input_new_packets(capture_session *cap_session, int to_read)
}
} else {
cf_fake_continue_tail((capture_file *)cap_session->cf);
cap_session->count_pending += to_read;
capture_callback_invoke(capture_cb_capture_fixed_continue, cap_session);
}
@ -719,7 +734,7 @@ capture_input_closed(capture_session *cap_session, gchar *msg)
/* We started a capture; process what's left of the capture file if
we were in "update list of packets in real time" mode, or process
all of it if we weren't. */
if(capture_opts->real_time_mode) {
if(((capture_file*)cap_session->cf)->state == FILE_READ_IN_PROGRESS) {
cf_read_status_t status;
/* Read what remains of the capture file. */
@ -780,7 +795,7 @@ capture_input_closed(capture_session *cap_session, gchar *msg)
exit_application(0);
break;
}
} else {
} else if (((capture_file*)cap_session->cf)->state == FILE_READ_PENDING) {
/* first of all, we are not doing a capture any more */
capture_callback_invoke(capture_cb_capture_fixed_finished, cap_session);

12
ui/logray/logray_main_window.cpp
View File

@ -1791,11 +1791,14 @@ bool LograyMainWindow::testCaptureFileClose(QString before_what, FileCloseContex
}
#ifdef HAVE_LIBPCAP
if (capture_file_.capFile()->state == FILE_READ_IN_PROGRESS) {
if (capture_file_.capFile()->state == FILE_READ_IN_PROGRESS ||
capture_file_.capFile()->state == FILE_READ_PENDING) {
/*
* This (FILE_READ_IN_PROGRESS) is true if we're reading a capture file
* FILE_READ_IN_PROGRESS is true if we're reading a capture file
* *or* if we're doing a live capture. From the capture file itself we
* cannot differentiate the cases, so check the current capture session.
* FILE_READ_PENDING is only used for a live capture, but it doesn't
* hurt to check it here.
*/
capture_in_progress = captureSession()->state != CAPTURE_STOPPED;
}
@ -1988,7 +1991,8 @@ bool LograyMainWindow::testCaptureFileClose(QString before_what, FileCloseContex
void LograyMainWindow::captureStop() {
stopCapture();
while (capture_file_.capFile() && capture_file_.capFile()->state == FILE_READ_IN_PROGRESS) {
while (capture_file_.capFile() && (capture_file_.capFile()->state == FILE_READ_IN_PROGRESS ||
capture_file_.capFile()->state == FILE_READ_PENDING)) {
WiresharkApplication::processEvents();
}
}
@ -2406,7 +2410,7 @@ void LograyMainWindow::setMenusForCaptureFile(bool force_disable)
bool can_save = false;
bool can_save_as = false;
if (force_disable || capture_file_.capFile() == NULL || capture_file_.capFile()->state == FILE_READ_IN_PROGRESS) {
if (force_disable || capture_file_.capFile() == NULL || capture_file_.capFile()->state == FILE_READ_IN_PROGRESS || capture_file_.capFile()->state == FILE_READ_PENDING) {
/* We have no capture file or we're currently reading a file */
enable = false;
} else {

2
ui/qt/capture_file.cpp
View File

@ -97,7 +97,7 @@ CaptureFile::~CaptureFile()
bool CaptureFile::isValid() const
{
if (cap_file_ && cap_file_->state != FILE_CLOSED) { // XXX FILE_READ_IN_PROGRESS as well?
if (cap_file_ && cap_file_->state != FILE_CLOSED && cap_file_->state != FILE_READ_PENDING) { // XXX FILE_READ_IN_PROGRESS as well?
return true;
}
return false;

37
ui/qt/main_status_bar.cpp
View File

@ -385,48 +385,43 @@ void MainStatusBar::showCaptureStatistics()
#ifdef HAVE_LIBPCAP
if (cap_file_) {
/* Do we have any packets? */
if (cs_fixed_ && cs_count_ > 0) {
if (prefs.gui_show_selected_packet && rows.count() == 1) {
packets_str.append(QString(tr("Selected Packet: %1 %2 "))
.arg(rows.at(0))
.arg(UTF8_MIDDLE_DOT));
}
packets_str.append(QString(tr("Packets: %1"))
.arg(cs_count_));
} else if (cs_count_ > 0) {
if (!cs_fixed_) {
cs_count_ = cap_file_->count;
}
if (cs_count_ > 0) {
if (prefs.gui_show_selected_packet && rows.count() == 1) {
packets_str.append(QString(tr("Selected Packet: %1 %2 "))
.arg(rows.at(0))
.arg(UTF8_MIDDLE_DOT));
}
packets_str.append(QString(tr("Packets: %1 %4 Displayed: %2 (%3%)"))
.arg(cap_file_->count)
.arg(cs_count_)
.arg(cap_file_->displayed_count)
.arg((100.0*cap_file_->displayed_count)/cap_file_->count, 0, 'f', 1)
.arg((100.0*cap_file_->displayed_count)/cs_count_, 0, 'f', 1)
.arg(UTF8_MIDDLE_DOT));
if (rows.count() > 1) {
packets_str.append(QString(tr(" %1 Selected: %2 (%3%)"))
.arg(UTF8_MIDDLE_DOT)
.arg(rows.count())
.arg((100.0*rows.count())/cap_file_->count, 0, 'f', 1));
.arg((100.0*rows.count())/cs_count_, 0, 'f', 1));
}
if (cap_file_->marked_count > 0) {
packets_str.append(QString(tr(" %1 Marked: %2 (%3%)"))
.arg(UTF8_MIDDLE_DOT)
.arg(cap_file_->marked_count)
.arg((100.0*cap_file_->marked_count)/cap_file_->count, 0, 'f', 1));
.arg((100.0*cap_file_->marked_count)/cs_count_, 0, 'f', 1));
}
if (cap_file_->drops_known) {
packets_str.append(QString(tr(" %1 Dropped: %2 (%3%)"))
.arg(UTF8_MIDDLE_DOT)
.arg(cap_file_->drops)
.arg((100.0*cap_file_->drops)/cap_file_->count, 0, 'f', 1));
.arg((100.0*cap_file_->drops)/cs_count_, 0, 'f', 1));
}
if (cap_file_->ignored_count > 0) {
packets_str.append(QString(tr(" %1 Ignored: %2 (%3%)"))
.arg(UTF8_MIDDLE_DOT)
.arg(cap_file_->ignored_count)
.arg((100.0*cap_file_->ignored_count)/cap_file_->count, 0, 'f', 1));
.arg((100.0*cap_file_->ignored_count)/cs_count_, 0, 'f', 1));
}
if (cap_file_->packet_comment_count > 0) {
packets_str.append(QString(tr(" %1 Comments: %2"))
@ -443,6 +438,15 @@ void MainStatusBar::showCaptureStatistics()
.arg(computed_elapsed%1000, 3, 10, QLatin1Char('0')));
}
}
} else if (cs_fixed_ && cs_count_ > 0) {
/* There shouldn't be any rows without a cap_file_ but this is benign */
if (prefs.gui_show_selected_packet && rows.count() == 1) {
packets_str.append(QString(tr("Selected Packet: %1 %2 "))
.arg(rows.at(0))
.arg(UTF8_MIDDLE_DOT));
}
packets_str.append(QString(tr("Packets: %1"))
.arg(cs_count_));
}
#endif // HAVE_LIBPCAP
@ -650,6 +654,9 @@ void MainStatusBar::captureEventHandler(CaptureEvent ev)
case CaptureEvent::Continued:
updateCaptureStatistics(ev.capSession());
break;
case CaptureEvent::Finished:
updateCaptureStatistics(ev.capSession());
break;
default:
break;
}

2
ui/qt/packet_list.cpp
View File

@ -2119,7 +2119,7 @@ void PacketList::rowsInserted(const QModelIndex &parent, int start, int end)
void PacketList::resizeAllColumns(bool onlyTimeFormatted)
{
if (!cap_file_ || cap_file_->state == FILE_CLOSED)
if (!cap_file_ || cap_file_->state == FILE_CLOSED || cap_file_->state == FILE_READ_PENDING)
return;
for (int col = 0; col < cap_file_->cinfo.num_cols; col++) {

4
ui/qt/time_shift_dialog.cpp
View File

@ -225,11 +225,11 @@ void TimeShiftDialog::applyTimeShift()
{
const gchar *err_str = NULL;
if (!cap_file_ || cap_file_->state == FILE_CLOSED) return;
if (!cap_file_ || cap_file_->state == FILE_CLOSED || cap_file_->state == FILE_READ_PENDING) return;
syntax_err_.clear();
if (cap_file_->state == FILE_READ_IN_PROGRESS) {
syntax_err_ = tr("Time shifting is not available capturing packets.");
syntax_err_ = tr("Time shifting is not available while capturing packets.");
} else if (ts_ui_->shiftAllButton->isChecked()) {
err_str = time_shift_all(cap_file_,
ts_ui_->shiftAllTimeLineEdit->text().toUtf8().constData());

12
ui/qt/wireshark_main_window.cpp
View File

@ -1832,11 +1832,14 @@ bool WiresharkMainWindow::testCaptureFileClose(QString before_what, FileCloseCon
}
#ifdef HAVE_LIBPCAP
if (capture_file_.capFile()->state == FILE_READ_IN_PROGRESS) {
if (capture_file_.capFile()->state == FILE_READ_IN_PROGRESS ||
capture_file_.capFile()->state == FILE_READ_PENDING) {
/*
* This (FILE_READ_IN_PROGRESS) is true if we're reading a capture file
* FILE_READ_IN_PROGRESS is true if we're reading a capture file
* *or* if we're doing a live capture. From the capture file itself we
* cannot differentiate the cases, so check the current capture session.
* FILE_READ_PENDING is only used for a live capture, but it doesn't
* hurt to check it here.
*/
capture_in_progress = captureSession()->state != CAPTURE_STOPPED;
}
@ -2029,7 +2032,8 @@ bool WiresharkMainWindow::testCaptureFileClose(QString before_what, FileCloseCon
void WiresharkMainWindow::captureStop() {
stopCapture();
while (capture_file_.capFile() && capture_file_.capFile()->state == FILE_READ_IN_PROGRESS) {
while (capture_file_.capFile() && (capture_file_.capFile()->state == FILE_READ_IN_PROGRESS ||
capture_file_.capFile()->state == FILE_READ_PENDING)) {
WiresharkApplication::processEvents();
}
}
@ -2496,7 +2500,7 @@ void WiresharkMainWindow::setMenusForCaptureFile(bool force_disable)
bool can_save = false;
bool can_save_as = false;
if (force_disable || capture_file_.capFile() == NULL || capture_file_.capFile()->state == FILE_READ_IN_PROGRESS) {
if (force_disable || capture_file_.capFile() == NULL || capture_file_.capFile()->state == FILE_READ_IN_PROGRESS || capture_file_.capFile()->state == FILE_READ_PENDING) {
/* We have no capture file or we're currently reading a file */
enable = false;
} else {