Commit Graph

34 Commits

Author SHA1 Message Date
Gerald Combs 750ffac7b6 Win32: Fix a return value check.
CreateNamedPipe returns INVALID_HANDLE_VALUE on failure.

Change-Id: I79ad5144e084520db8197b69c4ad34431d183009
Reviewed-on: https://code.wireshark.org/review/34704
Reviewed-by: Gerald Combs <gerald@wireshark.org>
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot
Reviewed-by: Guy Harris <guy@alum.mit.edu>
2019-10-03 20:56:07 +00:00
Tomasz Moń 1f2d36aa0b wsutil: Respect working directory on Windows
Pass working directory parameter to win32_create_process() in
ws_pipe_spawn_sync().

Change-Id: I0abbc4fbf733138b20c2a34845b147530417c91e
Reviewed-on: https://code.wireshark.org/review/34533
Petri-Dish: Michael Mann <mmann78@netscape.net>
Tested-by: Petri Dish Buildbot
Reviewed-by: Michael Mann <mmann78@netscape.net>
2019-09-15 13:37:13 +00:00
Guy Harris 20800366dd HTTPS (almost) everywhere.
Change all wireshark.org URLs to use https.

Fix some broken links while we're at it.

Change-Id: I161bf8eeca43b8027605acea666032da86f5ea1c
Reviewed-on: https://code.wireshark.org/review/34089
Reviewed-by: Guy Harris <guy@alum.mit.edu>
2019-07-26 18:44:40 +00:00
Tomasz Moń b075b8c7f7 wsutil: Fix 30 seconds freeze if process fails to start
In ws_pipe_wait_for_pipe() on Windows, the WaitForMultipleObjects() waits
on the pending pipe connection events and process handle. If the process
handle is signalled, then it means that the process did exit without
connecting to the pipes.

The WaitForMultipleObjects() was not waiting on the process handle and thus
if the process did fail without connecting to pipes the Wireshark gui was
frozen for 30 seconds.

This change fixes the freeze by increasing the number of handles, so
WaitForMultipleObjects() is aware of the process handle.

Change-Id: Id13824a60baf4be7795cbe1d5ed1c7932edbff45
Reviewed-on: https://code.wireshark.org/review/33028
Reviewed-by: Gerald Combs <gerald@wireshark.org>
2019-04-30 15:51:11 +00:00
Peter Wu dd1245f5be ws_pipe: fix memory leaks in spawn arguments handling
On Windows, ws_pipe_spawn_sync always leaks 'winargs', and leaks 'argv'
on some error paths. Fix these and refactor the common argument parsing
functionality to reduce duplication of functionality.

Change-Id: I8fa5ca45aec20b53f6fa243b0dd07241a345f7ab
Reviewed-on: https://code.wireshark.org/review/32932
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Tomasz Moń <desowin@gmail.com>
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
2019-04-23 23:18:29 +00:00
Peter Wu 51ac1047a7 ws_pipe: fix return value of ws_pipe_spawn_async on error path
The function returns a GPid, not a gboolean. Callers (mmdbresolv and
extcap) only assume WS_INVALID_PID to be invalid (as documented).

Change-Id: I40b491272a451f569864fa3259009d6d3fcce772
Fixes: v2.5.1rc0-413-g1a0987904f ("Generalize our process spawning code.")
Reviewed-on: https://code.wireshark.org/review/32933
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Tomasz Moń <desowin@gmail.com>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2019-04-22 14:57:28 +00:00
Tomasz Moń a051d5d869 wsutil: Refactor WIN32 ws_pipe_wait_for_pipe()
The ws_pipe_wait_for_pipe() implementation had multiple issues:
  * Use auto-reset events with ConnectNamedPipe (should be manual-reset)
  * Leaking event handles
  * Not checking return value from CreateEvent()
  * Waiting on closed handles

This change fixes all the above mentioned issues.

Bug: 15696
Change-Id: Ia0c389a902655f85eccb0c59288b4a7d49da48c9
Reviewed-on: https://code.wireshark.org/review/32896
Petri-Dish: Guy Harris <guy@alum.mit.edu>
Tested-by: Petri Dish Buildbot
Reviewed-by: Tomasz Moń <desowin@gmail.com>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2019-04-21 07:31:34 +00:00
Tomasz Moń 1998de886c wsutil: Warn on empty arguments
Callers should not include empty strings in arguments list. Log warning
message instead of silently dropping remaining arguments.

Change-Id: Ia68c7b90cec860e032f81a4008aa005b07ebcfd5
Ping-Bug: 15586
Reviewed-on: https://code.wireshark.org/review/32849
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2019-04-18 06:00:15 +00:00
Tomasz Moń 9d7d4c015c wsutil: Read stderr and stdout data asynchronously
This significantly reduces the initialization time when system is using
small pipe buffers. No time is lost on periodic process status checks as
WaitForMultipleObjects() returns when the stdout/stderr data was read
and/or when process finishes.

Bug: 14657
Change-Id: I61fabf986577db7102a3136df83d2d92c7156727
Reviewed-on: https://code.wireshark.org/review/32773
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Tomasz Moń <desowin@gmail.com>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2019-04-15 04:09:19 +00:00
Tomasz Moń b95933a031 wsutil: Fix pipe handle leaks on Windows
If the PATHEXT environment variable contains .py and extcap is enabled,
then Wireshark did leak 2 pipes for every .py file present in the extcap
directory (regardless if there was .bat wrapper for it or not).

Bug: 15689
Change-Id: Iae402c0075ee8155a7205a59711bbae734da7e9e
Reviewed-on: https://code.wireshark.org/review/32812
Tested-by: Petri Dish Buildbot
Reviewed-by: Guy Harris <guy@alum.mit.edu>
2019-04-10 19:51:40 +00:00
Peter Wu 0da9763d11 Win32: change DLL search path to accomodate extcap and Npcap
Extcap executables require libwsutil.dll from the program directory.
These were loaded by setting the PATH environment variable, but this
is not thread-safe (and caused sporadic tests failures as a result).
Use SetDllDirectory instead, this also prevents loading DLL files
from arbitrary directories in PATH.

To make this work, the search logic for Npcap has to be modified to
avoid relying on SetDllDirectory. This implies that Npcap cannot be
used on Windows 7 anymore until KB2533623 (July 2011) is applied.

Change-Id: I3fc42ff76e75ae162b6dd31103451fb8f71c09e6
Reviewed-on: https://code.wireshark.org/review/30804
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2018-11-29 05:21:16 +00:00
Peter Wu 5e304f7718 ws_pipe_spawn_*: fix deadlock in g_spawn on Linux with threads
The deadlock can be observed with a slow malloc implementation, e.g.

    ASAN_OPTIONS=fast_unwind_on_malloc=0 tshark --version

(This calls extcap_run_all which uses threads and ws_pipe_spawn_sync.)

Change-Id: Iff329c465c53ed177980368cd645f59222f88dd3
Reviewed-on: https://code.wireshark.org/review/30777
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2018-11-25 07:11:02 +00:00
Peter Wu 85dc646327 ws_pipe_spawn_sync: mark arguments as const where possible
This avoids an unnecessary explicit cast. For clarity, rename the
working directory argument to match g_spawn_sync.

Change-Id: Idf7072cd590e686294d953f77da2a52c861a89c0
Reviewed-on: https://code.wireshark.org/review/30763
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2018-11-22 20:49:44 +00:00
Peter Wu a4f366c698 ws_pipe_spawn_sync: debug log elapsed time
Tested with `G_MESSAGES_DEBUG=all tshark --version`.

Change-Id: Iae525b9de197f012c21693a91155f931d4dcc1f7
Ping-Bug: 15295
Reviewed-on: https://code.wireshark.org/review/30759
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2018-11-22 12:10:24 +00:00
Jaap Keuter 83c17c4d17 Allow trace of sync spawns
When Wireshark uses a synchronous spawn (e.g., to launch an extcap)
it would be nice to be able to see what command line is constructed
to launch the process, and to see what comes back. The output will
go to the g_log.

Change-Id: Iec6baeebc026cd80398084c9644fc916ab068e2f
Reviewed-on: https://code.wireshark.org/review/30475
Petri-Dish: Dario Lombardo <lomato@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
2018-11-20 14:53:45 +00:00
Pascal Quantin bbaf495162 ws_pipe_close(): also close the handle after terminating process
Change-Id: Ie19eba9706e2af1a58a6946e8af68ab90c0c8a7d
Reviewed-on: https://code.wireshark.org/review/30456
Reviewed-by: Pascal Quantin <pascal.quantin@gmail.com>
2018-11-01 09:58:26 +00:00
Pascal Quantin bcaf997f8a wsutil: introduce ws_pipe_close() helper and use it to terminate mmdbresolve
We were not calling TerminateProcess() to stop mmdbresolve.Exe process on
Windows.

Bug: 15248
Change-Id: Ic90cf438a8003a6fefb023b7056984681ce09b46
Reviewed-on: https://code.wireshark.org/review/30449
Petri-Dish: Pascal Quantin <pascal.quantin@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2018-11-01 06:26:13 +00:00
Guy Harris 942d9e62e6 Note that you have to be careful reading messages from a pipe.
Change-Id: I354e0633b8bd1da3d95fcb770fb9d5a0e4ee5880
Reviewed-on: https://code.wireshark.org/review/30212
Reviewed-by: Guy Harris <guy@alum.mit.edu>
2018-10-14 19:47:37 +00:00
Stig Bjørlykke f86d6203a9 extcap: Read stderr on extcap error without an infinite loop
Check if data is available on stderr before doing a blocking read() to
avoid an infinite read loop when having less data than STDERR_BUFFER_SIZE.

Append data instead of overwrite when doing multiple read() to fetch
available data.

This is a regression from g6a949ed155.

Bug: 15205
Change-Id: I84b232aeafb6123f77f3f5d48bbe89326fe7eb0f
Reviewed-on: https://code.wireshark.org/review/30209
Petri-Dish: Stig Bjørlykke <stig@bjorlykke.org>
Tested-by: Petri Dish Buildbot
Reviewed-by: Roland Knall <rknall@gmail.com>
2018-10-14 19:05:27 +00:00
Tomasz Moń 9ecc7906ef Do not pass pipe handle to WaitForMultipleObjects
The Remarks section in WaitForMultipleObjects describes what kind of
handles the function can wait for. Pipe handles are not listed there.

The problem was introduced in c18459e66e

While it might be possible to setup overlapped reads on the pipe handles
and then wait on overlapped events, it would result in quite complex
code. As a tradeoff, simply keep peeking at the pipes every 100 ms.

Change-Id: I6ba4f4bf4c1d2af856027cca36ffd6d4f7f49f36
Bug: 14657
Reviewed-on: https://code.wireshark.org/review/29163
Petri-Dish: Roland Knall <rknall@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Roland Knall <rknall@gmail.com>
2018-08-16 20:25:35 +00:00
Tomasz Moń c18459e66e Fix extcap initialization deadlock
On Windows the code calling extcap worked as follows:
1. Create stdout and stderr pipes with default buffer size
2. Execute extcap redirecting output to the pipes
3. Wait for extcap process to exit
4. Read the data from stdout pipe

This resulted in deadlock when the extcap wrote more data than the pipe
could buffer. This was especially seen with USBPcap as it is quite
normal to have plenty of USB devices connected.

Fix the issue by contantly reading the stdout data and storing it in
GString. To prevent similar deadlock on the stderr, the stderr data is
being constantly monitored as well (and discarded).

Change-Id: I0f93e6d79617cef0e828aef2b96fad2757227923
Bug: 14657
Reviewed-on: https://code.wireshark.org/review/29159
Petri-Dish: Pascal Quantin <pascal.quantin@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Roland Knall <rknall@gmail.com>
2018-08-16 17:30:22 +00:00
Dario Lombardo 63025ae2e1 ws_pipe: don't check for negative number using DWORD.
DWORD on windows is unsigned, then there is no point in checking
for negative values.

Change-Id: I0b03fb19ebdff86e610cd4571fc30c49b7bd1284
Reviewed-on: https://code.wireshark.org/review/27766
Petri-Dish: Dario Lombardo <lomato@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2018-05-24 13:57:27 +00:00
Dario Lombardo 83f6dd4ef9 ws_pipe: fix return code on windows.
Change-Id: I30d24a5cad1991c2ce6a0f2a63fbe715b92943ec
Reviewed-on: https://code.wireshark.org/review/27745
Reviewed-by: Kenneth Soerensen <knnthsrnsn@gmail.com>
Petri-Dish: Dario Lombardo <lomato@gmail.com>
Tested-by: Dario Lombardo <lomato@gmail.com>
Reviewed-by: Roland Knall <rknall@gmail.com>
2018-05-23 16:04:11 +00:00
Dario Lombardo 68ec514b5f wsutil: null-terminate string in ws_read_string_from_pipe (CID: 1364684).
Change-Id: I713e7466843e5ccaa7252744c57c7ac4c7020809
Reviewed-on: https://code.wireshark.org/review/27422
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Dario Lombardo <lomato@gmail.com>
2018-05-10 15:06:45 +00:00
Gerald Combs ffe2f138ab ws_pipe: Zero an array.
Make sure we zero-initialize pipeinsts, otherwise ConnectNamedPipe will
have indeterminate behavior according to the MSDN documentation for the
OVERLAPPED structure.

Change-Id: I38d9680cf01b0a8f9e566a85a7a330f6c0aa9a48
Ping-Bug: 14532
Reviewed-on: https://code.wireshark.org/review/26784
Reviewed-by: Gerald Combs <gerald@wireshark.org>
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2018-04-08 22:31:56 +00:00
Gerald Combs a2f9267615 Windows: Always assign newly-created processes to our job.
Move ws_pipe_kill_child_on_exit to win32-utils. Add win32_create_process,
which calls CreateProcess + AssignProcessToJobObject. Use
win32_create_process instead of CreateProcess everywhere.

Bug: 1419
Change-Id: I7a1f17dddf6a73f6973d54621f271b69311400d1
Reviewed-on: https://code.wireshark.org/review/26448
Reviewed-by: Gerald Combs <gerald@wireshark.org>
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2018-03-13 17:18:30 +00:00
Gerald Combs dcc3875847 Add ws_pipe_kill_child_on_exit.
Add ws_pipe_kill_child_on_exit, which associates a child process handle
with a job object that has the JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE
flag set.

Call it when we create a process in ws_pipe_spawn_sync and
ws_pipe_spawn_async. Note that we might want to use it elsewhere.

Change-Id: Ia0f6863ea4df0ab8623bb923a49da7776d83bd33
Reviewed-on: https://code.wireshark.org/review/26398
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot
Reviewed-by: Gerald Combs <gerald@wireshark.org>
2018-03-09 22:46:05 +00:00
Gerald Combs 80d652f06e More spawned process handling updates.
Document ws_pipe.h. Define invalid PIDs in one place.

Extcap didn't use stdin before 1a0987904f. Make sure we close it.

Change-Id: I7a69cd9b5137ae82435e64628a22e4d812d58f89
Reviewed-on: https://code.wireshark.org/review/26226
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot
Reviewed-by: Gerald Combs <gerald@wireshark.org>
2018-03-02 18:07:58 +00:00
Gerald Combs 1a0987904f Generalize our process spawning code.
Move the contents of extcap_spawn to ws_pipe. Rename various extcap_*
prefixes to ws_pipe_*. Open stdin when we spawn processes.

Change-Id: I9286295443ee955bb6328b0ed6f945ee0bb2a798
Reviewed-on: https://code.wireshark.org/review/26216
Reviewed-by: Gerald Combs <gerald@wireshark.org>
Petri-Dish: Gerald Combs <gerald@wireshark.org>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2018-03-02 05:22:20 +00:00
Dan Robertson 9ff64c8d7c musl: compile with musl libc
Ensure that wsutil/ws_pipe.c includes <sys/select.h> as as both
the timeval struct and the select function are used.

Change-Id: Idbd9e9a5b9cbee9977a423c32e55be81bb6425c3
Reviewed-on: https://code.wireshark.org/review/25616
Petri-Dish: Jaap Keuter <jaap.keuter@xs4all.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Guy Harris <guy@alum.mit.edu>
2018-02-10 17:40:50 +00:00
Dario Lombardo 8cd389e161 replace SPDX identifier GPL-2.0+ with GPL-2.0-or-later.
The first is deprecated, as per https://spdx.org/licenses/.

Change-Id: I8e21e1d32d09b8b94b93a2dc9fbdde5ffeba6bed
Reviewed-on: https://code.wireshark.org/review/25661
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Petri-Dish: Dario Lombardo <lomato@gmail.com>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
2018-02-08 14:57:36 +00:00
Uli Heilmeier 5dc7ce6ccc ws_pipe: fix format error for size_t buffer_size
Fix error: format '%ld' expects argument of type 'long int', but argument 4 has type 'size_t'

Change-Id: I86ec4076bb7e8c11d5cf82187a46a528bf43c514
Reviewed-on: https://code.wireshark.org/review/25109
Petri-Dish: Dario Lombardo <lomato@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Dario Lombardo <lomato@gmail.com>
2018-01-02 14:47:33 +00:00
Dario Lombardo 098fbc70ee ws_pipe: provide a null terminated string anyway (CID: 1364684).
Change-Id: I4143b499986cbd31db524db73dde72ddbe26077d
Reviewed-on: https://code.wireshark.org/review/25072
Petri-Dish: Dario Lombardo <lomato@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Michael Mann <mmann78@netscape.net>
2018-01-02 03:38:16 +00:00
Guy Harris 6a949ed155 Put special pipe-handling code into libwsutil.
Ask, in a comment, why we're doing PeekNamedPipe() when we're trying
to read everyting in the pipe, up to the EOF, into a string.

On UN*X, do the same "read up to an EOF and then NUL-terminate the
result" stuff that we did on Windows; nothing guarantees that, on all
UN*Xes, in all circumstances, until the end of time, world without end,
amen, we can do one read and get the entire string.

Change-Id: I578802b23fec1051139eaefd9a09fe2a6de06a11
Reviewed-on: https://code.wireshark.org/review/24959
Petri-Dish: Guy Harris <guy@alum.mit.edu>
Tested-by: Petri Dish Buildbot
Reviewed-by: Guy Harris <guy@alum.mit.edu>
2017-12-23 20:43:32 +00:00