[PATCH v5 0/2] MR1015: wpcap: Convert Unix sockaddr to win32 before duplication
Interpreting the fist two bytes of the `sockaddr` struct as `sa_family` produces incorrect behaviour on macOS and (presumbly) BSD. This change uses both Unix and Windows socket headers to convert the libpcap provided sockaddr before duplicating it. This fixes sockaddr being always NULL on macOS/BSD and ipv6 addresses not working in general since `AF_INET6` differs on Windows(23), macOS(30) and Linux(10) For reference https://www.ibm.com/docs/en/i/7.3?topic=family-af-inet-address and this is from the <sys/socket.h> on macOS ``` /* * [XSI] Structure used by kernel to store most addresses. */ struct sockaddr { __uint8_t sa_len; /* total length */ sa_family_t sa_family; /* [XSI] address family */ char sa_data[14]; /* [XSI] addr value (actually larger) */ }; ``` -- v5: wpcap: Convert unix sockaddr to win32 sockaddr https://gitlab.winehq.org/wine/wine/-/merge_requests/1015
From: Marc-Aurel Zent <marc_aurel(a)me.com> --- dlls/wpcap/unixlib.h | 14 ++++++-------- dlls/wpcap/wpcap.c | 13 +++++++------ 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/dlls/wpcap/unixlib.h b/dlls/wpcap/unixlib.h index 8cd64ef0210..9ebcf225c5d 100644 --- a/dlls/wpcap/unixlib.h +++ b/dlls/wpcap/unixlib.h @@ -17,18 +17,16 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA */ -struct sockaddr_hdr -{ - unsigned short sa_family; -}; +/* Oblique struct that either holds a Unix or Win32 sockaddr depending on context */ +struct pcap_sockaddr; struct pcap_address { struct pcap_address *next; - struct sockaddr_hdr *addr; - struct sockaddr_hdr *netmask; - struct sockaddr_hdr *broadaddr; - struct sockaddr_hdr *dstaddr; + struct pcap_sockaddr *addr; + struct pcap_sockaddr *netmask; + struct pcap_sockaddr *broadaddr; + struct pcap_sockaddr *dstaddr; }; struct pcap_interface diff --git a/dlls/wpcap/wpcap.c b/dlls/wpcap/wpcap.c index df35954bfa7..0e6c55d4904 100644 --- a/dlls/wpcap/wpcap.c +++ b/dlls/wpcap/wpcap.c @@ -238,11 +238,12 @@ static char *build_win32_description( const struct pcap_interface *unix_dev ) return ret; } -static struct sockaddr_hdr *dup_sockaddr( const struct sockaddr_hdr *addr ) +static struct pcap_sockaddr *dup_sockaddr( const struct pcap_sockaddr *addr ) { - struct sockaddr_hdr *ret; + struct pcap_sockaddr *ret; + short sa_family = *((short *)addr); - switch (addr->sa_family) + switch (sa_family) { case AF_INET: { @@ -251,7 +252,7 @@ static struct sockaddr_hdr *dup_sockaddr( const struct sockaddr_hdr *addr ) dst->sin_family = src->sin_family; dst->sin_port = src->sin_port; dst->sin_addr = src->sin_addr; - ret = (struct sockaddr_hdr *)dst; + ret = (struct pcap_sockaddr *)dst; break; } case AF_INET6: @@ -263,11 +264,11 @@ static struct sockaddr_hdr *dup_sockaddr( const struct sockaddr_hdr *addr ) dst->sin6_flowinfo = src->sin6_flowinfo; dst->sin6_addr = src->sin6_addr; dst->sin6_scope_id = src->sin6_scope_id; - ret = (struct sockaddr_hdr *)dst; + ret = (struct pcap_sockaddr *)dst; break; } default: - FIXME( "address family %u not supported\n", addr->sa_family ); + FIXME( "address family %d not supported\n", sa_family ); return NULL; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1015
From: Marc-Aurel Zent <marc_aurel(a)me.com> ...in unixlib before duplicationg it on the PE side. --- dlls/wpcap/unixlib.c | 40 ++++++++++++++++++++++++++++++++++++++++ dlls/wpcap/unixlib.h | 1 + dlls/wpcap/wpcap.c | 14 +++++++------- 3 files changed, 48 insertions(+), 7 deletions(-) diff --git a/dlls/wpcap/unixlib.c b/dlls/wpcap/unixlib.c index a44627a7754..ae850efb677 100644 --- a/dlls/wpcap/unixlib.c +++ b/dlls/wpcap/unixlib.c @@ -34,6 +34,12 @@ #include "windef.h" #include "winbase.h" #include "winternl.h" +#define USE_WS_PREFIX +/* the following 2 libpcap defines interfere with winsock2.h */ +#undef SOCKET +#undef INVALID_SOCKET +#include "winsock2.h" +#include "ws2ipdef.h" #include "wine/unixlib.h" #include "wine/debug.h" @@ -42,6 +48,39 @@ WINE_DEFAULT_DEBUG_CHANNEL(wpcap); WINE_DECLARE_DEBUG_CHANNEL(winediag); +static NTSTATUS convert_sockaddr( void *args ) +{ + struct sockaddr_storage *addr = args; + struct sockaddr_storage unix_addr = *addr; + + switch (unix_addr.ss_family) + { + case AF_INET: + { + struct sockaddr_in *src = (struct sockaddr_in *)&unix_addr; + SOCKADDR_IN *dst = (SOCKADDR_IN *)addr; + dst->sin_family = WS_AF_INET; + dst->sin_port = (USHORT)src->sin_port; + memcpy(&dst->sin_addr, &src->sin_addr, sizeof(IN_ADDR)); + return STATUS_SUCCESS; + } + case AF_INET6: + { + struct sockaddr_in6 *src = (struct sockaddr_in6 *)&unix_addr; + SOCKADDR_IN6 *dst = (SOCKADDR_IN6 *)addr; + dst->sin6_family = WS_AF_INET6; + dst->sin6_port = (USHORT)src->sin6_port; + dst->sin6_flowinfo = (ULONG)src->sin6_flowinfo; + memcpy(&dst->sin6_addr, &src->sin6_addr, sizeof(IN6_ADDR)); + dst->sin6_scope_id = (ULONG)src->sin6_scope_id; + return STATUS_SUCCESS; + } + default: + FIXME( "unix address family %u not supported\n", unix_addr.ss_family ); + return STATUS_NOT_IMPLEMENTED; + } +} + static NTSTATUS wrap_activate( void *args ) { struct pcap *pcap = args; @@ -372,6 +411,7 @@ static NTSTATUS wrap_tstamp_type_val_to_name( void *args ) const unixlib_entry_t __wine_unix_call_funcs[] = { + convert_sockaddr, wrap_activate, wrap_breakloop, wrap_can_set_rfmon, diff --git a/dlls/wpcap/unixlib.h b/dlls/wpcap/unixlib.h index 9ebcf225c5d..e953bcb6911 100644 --- a/dlls/wpcap/unixlib.h +++ b/dlls/wpcap/unixlib.h @@ -263,6 +263,7 @@ struct tstamp_type_val_to_name_params enum pcap_funcs { + unix_convert_sockaddr, unix_activate, unix_breakloop, unix_can_set_rfmon, diff --git a/dlls/wpcap/wpcap.c b/dlls/wpcap/wpcap.c index 0e6c55d4904..5fe27984ff4 100644 --- a/dlls/wpcap/wpcap.c +++ b/dlls/wpcap/wpcap.c @@ -238,12 +238,15 @@ static char *build_win32_description( const struct pcap_interface *unix_dev ) return ret; } -static struct pcap_sockaddr *dup_sockaddr( const struct pcap_sockaddr *addr ) +static struct pcap_sockaddr *dup_sockaddr( struct pcap_sockaddr *addr ) { - struct pcap_sockaddr *ret; - short sa_family = *((short *)addr); + struct pcap_sockaddr *ret = NULL; + unsigned short ss_family; - switch (sa_family) + if (PCAP_CALL( convert_sockaddr, addr )) return NULL; + ss_family = ((struct sockaddr_storage *)addr)->ss_family; + + switch (ss_family) { case AF_INET: { @@ -267,9 +270,6 @@ static struct pcap_sockaddr *dup_sockaddr( const struct pcap_sockaddr *addr ) ret = (struct pcap_sockaddr *)dst; break; } - default: - FIXME( "address family %d not supported\n", sa_family ); - return NULL; } return ret; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1015
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=125281 Your paranoid android. === debian11 (build log) === Task: Could not create the win32 wineprefix: Failed to disable the crash dialogs: Task: WineTest did not produce the win32 report
You shouldn't need another Unix call. The idea would be to pass a win32 adapter -> unix device mapping to findalldevs(). You'd call it first to get the needed size for the pcap_interface array, then allocate memory and then call it again to retrieve the array. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1015#note_11847
participants (4)
-
Hans Leidekker (＠hans) -
Marc-Aurel Zent
-
Marc-Aurel Zent (＠marzent)
-
Marvin