[PATCH v2 0/2] MR1015: Draft: wpcap: use UNIX 98/BSD 4.4 sockaddr header
Interpreting the fist two bytes of the `sockaddr` struct as `sa_family` produces incorrect behaviour on macOS and (presumbly) BSD. This change is however still backwards compatible with Linux since `sa_family` is of effective type `uint8_t` and `sa_len` is currently disregarded. 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) */ }; ``` -- v2: wpcap: Convert unix sockaddr to win32 sockaddr wpcap: Use oblique struct for pcap 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, 49 insertions(+), 6 deletions(-) diff --git a/dlls/wpcap/unixlib.c b/dlls/wpcap/unixlib.c index a44627a7754..d96d8a016ba 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 *addr = args; + struct sockaddr unix_addr = *addr; + + switch (unix_addr.sa_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.sa_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..c5a81c0dbb1 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: { @@ -268,8 +271,7 @@ static struct pcap_sockaddr *dup_sockaddr( const struct pcap_sockaddr *addr ) break; } default: - FIXME( "address family %d not supported\n", sa_family ); - return NULL; + /* unreachable after a successful convert_sockaddr */ } 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=125267 Your paranoid android. === debian11 (build log) === ../wine/dlls/wpcap/wpcap.c:273:5: error: label at end of compound statement Task: The win32 Wine build failed === debian11 (build log) === ../wine/dlls/wpcap/wpcap.c:273:5: error: label at end of compound statement Task: The wow64 Wine build failed
participants (3)
-
Marc-Aurel Zent -
Marc-Aurel Zent (＠marzent)
-
Marvin