Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Linux support to qc_handle_{accept,bind,connect} functions. #23

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 122 additions & 25 deletions hw/arm/guest-socket.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,74 @@ static int32_t find_free_socket(void) {
return -1;
}


/* NETWORK_HACK:
See `guest-services/socket.h`:
Darwin still uses `sin_len` in sockaddr,
causing errors when copying memory from iOS.
*/
#ifdef __APPLE__
# define ADDR addr
# undef NETWORK_HACK
#else /* Linux, etc. */
# define ADDR darwin_addr
# define NETWORK_HACK 1
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no reason for the NETWORK_HACK and ADDR defines, we can condition the functions based on __APPLE__ only.


static int convert_error(int err) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest a function name that makes the platform clear, such as darwin_error.

#ifndef NETWORK_HACK
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#ifndef NETWORK_HACK
#ifdef __APPLE__

return err;
#else /* linux */
int result;

switch(err) {
case EAGAIN:
/* case EWOULDBLOCK: */
result = 35;
break;
case EDEADLK:
/* case EDEADLOCK: */
result = 11;
break;
/* case {...}: */
default:
result = err;
break;
}
return result;
#endif
}

static struct sockaddr_in convert_sockaddr_to(S_SOCKADDR_IN from) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notice that you only call this function when NETWORK_HACK is defined, which means the following:

  1. The #ifndef inside the function is redundant
  2. The name of the function can be more explicit, such as convert_sockaddr_darwin_to_gnu
  3. It should receive darwin_sockaddr_in explicitly (and the S_SOCKADDR_IN define can be dropped)
Suggested change
static struct sockaddr_in convert_sockaddr_to(S_SOCKADDR_IN from) {
static struct sockaddr_in convert_sockaddr_darwin_to_gnu(darwin_sockaddr_in from) {

#ifndef NETWORK_HACK
return from;
#else
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant...

struct sockaddr_in to;
to.sin_family = from.sin_family;
to.sin_port = from.sin_port;
to.sin_addr = from.sin_addr;
memset(&to.sin_zero, 0, sizeof(to.sin_zero));

return to;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can use modern features (compound literals and struct initialization) to make code more compact, something like:

Suggested change
struct sockaddr_in to;
to.sin_family = from.sin_family;
to.sin_port = from.sin_port;
to.sin_addr = from.sin_addr;
memset(&to.sin_zero, 0, sizeof(to.sin_zero));
return to;
return (sockaddr_in) {
from.sin_family,
from.sin_port,
from.sin_addr,
{ [0 ... sizeof(from.sin_zero) - 1] = 0 } };

#endif
}

static S_SOCKADDR_IN convert_sockaddr_from(struct sockaddr_in from) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above...

Suggested change
static S_SOCKADDR_IN convert_sockaddr_from(struct sockaddr_in from) {
static struct darwin_sockaddr_in convert_sockaddr_gnu_to_darwin(sockaddr_in from) {

#ifndef NETWORK_HACK
return from;
#else
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant...

S_SOCKADDR_IN to;
to.sin_family = from.sin_family;
to.sin_port = from.sin_port;
to.sin_addr = from.sin_addr;
to.sin_len = sizeof(to);
memset(&to.sin_zero, 0, sizeof(to.sin_zero));

return to;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above...

Suggested change
S_SOCKADDR_IN to;
to.sin_family = from.sin_family;
to.sin_port = from.sin_port;
to.sin_addr = from.sin_addr;
to.sin_len = sizeof(to);
memset(&to.sin_zero, 0, sizeof(to.sin_zero));
return to;
return (darwin_sockaddr_in) {
sizeof(struct darwin_sockaddr_in),
from.sin_family,
from.sin_port,
from.sin_addr,
{ [0 ... sizeof(from.sin_zero) - 1] = 0 } };

I know I'm using sizeof(from.sin_zero) instead of sizeof(to.sin_zero), but luckily, those sizes are the same.

#endif
}


int32_t qc_handle_socket(CPUState *cpu, int32_t domain, int32_t type,
int32_t protocol)
{
Expand All @@ -49,44 +117,59 @@ int32_t qc_handle_socket(CPUState *cpu, int32_t domain, int32_t type,
guest_svcs_errno = ENOTSOCK;
} else if ((guest_svcs_fds[retval] = socket(domain, type, protocol)) < 0) {
retval = -1;
guest_svcs_errno = errno;
guest_svcs_errno = convert_error(errno);
}

return retval;
}

int32_t qc_handle_accept(CPUState *cpu, int32_t sckt, struct sockaddr *g_addr,
socklen_t *g_addrlen)
int32_t qc_handle_accept(CPUState *cpu, int32_t sckt, S_SOCKADDR *g_addr,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

S_SOCKADDR can always be darwin_sockaddr

SOCKLEN_T *g_addrlen)
{
struct sockaddr_in addr;
socklen_t addrlen;
SOCKLEN_T addrlen;

addrlen = sizeof(addr);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
SOCKLEN_T addrlen;
addrlen = sizeof(addr);
socklen_t addrlen = sizeof(addr);

VERIFY_FD(sckt);

int retval = find_free_socket();

// TODO: timeout
if (retval < 0) {
guest_svcs_errno = ENOTSOCK;
} else if ((guest_svcs_fds[retval] = accept(guest_svcs_fds[sckt],
} else if ((guest_svcs_fds[retval] =
#ifdef NETWORK_HACK
accept4(guest_svcs_fds[sckt],
(struct sockaddr *) &addr,
&addrlen, SOCK_NONBLOCK)) < 0)
aronsky marked this conversation as resolved.
Show resolved Hide resolved
#else
accept(guest_svcs_fds[sckt],
(struct sockaddr *) &addr,
&addrlen)) < 0) {
&addrlen)) < 0)
#endif
{
retval = -1;
guest_svcs_errno = errno;
guest_svcs_errno = convert_error(errno);
} else {
cpu_memory_rw_debug(cpu, (target_ulong) g_addr, (uint8_t*) &addr,
sizeof(addr), 1);
#ifdef NETWORK_HACK
S_SOCKADDR_IN ADDR = convert_sockaddr_from(addr);
#endif
cpu_memory_rw_debug(cpu, (target_ulong) g_addr, (uint8_t*) &ADDR,
sizeof(ADDR), 1);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#ifdef NETWORK_HACK
S_SOCKADDR_IN ADDR = convert_sockaddr_from(addr);
#endif
cpu_memory_rw_debug(cpu, (target_ulong) g_addr, (uint8_t*) &ADDR,
sizeof(ADDR), 1);
#ifndef __APPLE__
darwin_sockaddr_in darwin_addr = convert_sockaddr_gnu_to_darwin(addr);
cpu_memory_rw_debug(cpu, (target_ulong) g_addr, (uint8_t*) &darwin_addr,
sizeof(darwin_addr), 1);
#else
cpu_memory_rw_debug(cpu, (target_ulong) g_addr, (uint8_t*) &addr,
sizeof(addr), 1);
#endif

cpu_memory_rw_debug(cpu, (target_ulong) g_addrlen,
(uint8_t*) &addrlen, sizeof(addrlen), 1);
}

return retval;
}

int32_t qc_handle_bind(CPUState *cpu, int32_t sckt, struct sockaddr *g_addr,
socklen_t addrlen)
int32_t qc_handle_bind(CPUState *cpu, int32_t sckt, S_SOCKADDR *g_addr,
SOCKLEN_T addrlen)
{
struct sockaddr_in addr;
#ifdef NETWORK_HACK
S_SOCKADDR_IN ADDR;
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will necessarily be darwin_sockaddr_in darwin_addr, if __APPLE__ is defined...


VERIFY_FD(sckt);

Expand All @@ -95,25 +178,34 @@ int32_t qc_handle_bind(CPUState *cpu, int32_t sckt, struct sockaddr *g_addr,
if (addrlen > sizeof(addr)) {
guest_svcs_errno = ENOMEM;
} else {
cpu_memory_rw_debug(cpu, (target_ulong) g_addr, (uint8_t*) &addr,
sizeof(addr), 0);
cpu_memory_rw_debug(cpu, (target_ulong) g_addr, (uint8_t*) &ADDR,
sizeof(ADDR), 0);

#ifdef NETWORK_HACK
addr = convert_sockaddr_to(ADDR);
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the change in accept:

Suggested change
cpu_memory_rw_debug(cpu, (target_ulong) g_addr, (uint8_t*) &ADDR,
sizeof(ADDR), 0);
#ifdef NETWORK_HACK
addr = convert_sockaddr_to(ADDR);
#endif
#ifndef __APPLE__
cpu_memory_rw_debug(cpu, (target_ulong) g_addr, (uint8_t*) &darwin_addr,
sizeof(darwin_addr), 0);
addr = convert_sockaddr_to(darwin_addr);
#else
cpu_memory_rw_debug(cpu, (target_ulong) g_addr, (uint8_t*) &addr,
sizeof(addr), 0);
#endif

if ((retval = bind(guest_svcs_fds[sckt], (struct sockaddr *) &addr,
addrlen)) < 0) {
guest_svcs_errno = errno;
guest_svcs_errno = convert_error(errno);
} else {
cpu_memory_rw_debug(cpu, (target_ulong) g_addr, (uint8_t*) &addr,
sizeof(addr), 1);
#ifdef NETWORK_HACK
ADDR = convert_sockaddr_from(addr);
#endif
cpu_memory_rw_debug(cpu, (target_ulong) g_addr, (uint8_t*) &ADDR,
sizeof(ADDR), 1);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
#ifdef NETWORK_HACK
ADDR = convert_sockaddr_from(addr);
#endif
cpu_memory_rw_debug(cpu, (target_ulong) g_addr, (uint8_t*) &ADDR,
sizeof(ADDR), 1);
#ifndef __APPLE__
darwin_addr = convert_sockaddr_from(addr);
cpu_memory_rw_debug(cpu, (target_ulong) g_addr, (uint8_t*) &addr,
sizeof(addr), 1);
#else
cpu_memory_rw_debug(cpu, (target_ulong) g_addr, (uint8_t*) &addr,
sizeof(addr), 1);
#endif

}
}

return retval;
}

int32_t qc_handle_connect(CPUState *cpu, int32_t sckt, struct sockaddr *g_addr,
socklen_t addrlen)
int32_t qc_handle_connect(CPUState *cpu, int32_t sckt, S_SOCKADDR *g_addr,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to bind...

SOCKLEN_T addrlen)
{
struct sockaddr_in addr;
#ifdef NETWORK_HACK
S_SOCKADDR_IN ADDR;
#endif

VERIFY_FD(sckt);

Expand All @@ -122,13 +214,18 @@ int32_t qc_handle_connect(CPUState *cpu, int32_t sckt, struct sockaddr *g_addr,
if (addrlen > sizeof(addr)) {
guest_svcs_errno = ENOMEM;
} else {
cpu_memory_rw_debug(cpu, (target_ulong) g_addr, (uint8_t*) &addr,
sizeof(addr), 0);

cpu_memory_rw_debug(cpu, (target_ulong) g_addr, (uint8_t*) &ADDR,
sizeof(ADDR), 0);
#ifdef NETWORK_HACK
addr = convert_sockaddr_to(ADDR);
#endif
if ((retval = connect(guest_svcs_fds[sckt], (struct sockaddr *) &addr,
addrlen)) < 0) {
guest_svcs_errno = errno;
guest_svcs_errno = convert_error(errno);
} else {
#ifdef NETWORK_HACK
ADDR = convert_sockaddr_from(addr);
#endif
cpu_memory_rw_debug(cpu, (target_ulong) g_addr, (uint8_t*) &addr,
sizeof(addr), 1);
}
Expand All @@ -144,7 +241,7 @@ int32_t qc_handle_listen(CPUState *cpu, int32_t sckt, int32_t backlog)
int retval = 0;

if ((retval = listen(guest_svcs_fds[sckt], backlog)) < 0) {
guest_svcs_errno = errno;
guest_svcs_errno = convert_error(errno);
}

return retval;
Expand All @@ -162,7 +259,7 @@ int32_t qc_handle_recv(CPUState *cpu, int32_t sckt, void *g_buffer,
if (length > MAX_BUF_SIZE) {
guest_svcs_errno = ENOMEM;
} else if ((retval = recv(guest_svcs_fds[sckt], buffer, length, flags)) <= 0) {
guest_svcs_errno = errno;
guest_svcs_errno = convert_error(errno);
} else {
cpu_memory_rw_debug(cpu, (target_ulong) g_buffer, buffer, retval, 1);
}
Expand All @@ -184,7 +281,7 @@ int32_t qc_handle_send(CPUState *cpu, int32_t sckt, void *g_buffer,
cpu_memory_rw_debug(cpu, (target_ulong) g_buffer, buffer, length, 0);

if ((retval = send(guest_svcs_fds[sckt], buffer, length, flags)) < 0) {
guest_svcs_errno = errno;
guest_svcs_errno = convert_error(errno);
}
}

Expand Down
53 changes: 43 additions & 10 deletions include/hw/arm/guest-services/socket.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,39 @@
#include "sys/socket.h"
#endif

#ifdef __APPLE__
#define S_SOCKADDR struct sockaddr
#define S_SOCKADDR_IN struct sockaddr_in
#define SOCKLEN_T socklen_t
#else /* __linux__ */
/* XXX: A lot of this is defined by the standard.
But doesn't hurt to ensure Darwin/BSD lengths.
MCApollo marked this conversation as resolved.
Show resolved Hide resolved

- in_addr_t should be uint32 on all systems:
typedef unsigned int in_addr_t;
struct in_addr {
in_addr_t s_addr;
};
*/
struct darwin_sockaddr {
unsigned char sa_len;
unsigned char sa_family;
char sa_data[14];
};
struct darwin_sockaddr_in {
unsigned char sin_len;
unsigned char sin_family;
unsigned short sin_port;
struct in_addr sin_addr;
char sin_zero[8];
};
typedef unsigned int darwin_socklen_t;

#define S_SOCKADDR struct darwin_sockaddr
#define S_SOCKADDR_IN struct darwin_sockaddr_in
#define SOCKLEN_T darwin_socklen_t
#endif

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wredundant-decls"
extern int32_t guest_svcs_errno;
Expand All @@ -47,14 +80,14 @@ typedef struct __attribute__((packed)) {

typedef struct __attribute__((packed)) {
int32_t socket;
struct sockaddr *addr;
socklen_t *addrlen;
S_SOCKADDR *addr;
SOCKLEN_T *addrlen;
} qc_accept_args_t;

typedef struct __attribute__((packed)) {
int32_t socket;
struct sockaddr *addr;
socklen_t addrlen;
S_SOCKADDR *addr;
SOCKLEN_T addrlen;
} qc_bind_args_t, qc_connect_args_t;

typedef struct __attribute__((packed)) {
Expand All @@ -72,12 +105,12 @@ typedef struct __attribute__((packed)) {
#ifndef OUT_OF_TREE_BUILD
int32_t qc_handle_socket(CPUState *cpu, int32_t domain, int32_t type,
int32_t protocol);
int32_t qc_handle_accept(CPUState *cpu, int32_t sckt, struct sockaddr *addr,
socklen_t *addrlen);
int32_t qc_handle_bind(CPUState *cpu, int32_t sckt, struct sockaddr *addr,
socklen_t addrlen);
int32_t qc_handle_connect(CPUState *cpu, int32_t sckt, struct sockaddr *addr,
socklen_t addrlen);
int32_t qc_handle_accept(CPUState *cpu, int32_t sckt, S_SOCKADDR *addr,
SOCKLEN_T *addrlen);
int32_t qc_handle_bind(CPUState *cpu, int32_t sckt, S_SOCKADDR *addr,
SOCKLEN_T addrlen);
int32_t qc_handle_connect(CPUState *cpu, int32_t sckt, S_SOCKADDR *addr,
SOCKLEN_T addrlen);
int32_t qc_handle_listen(CPUState *cpu, int32_t sckt, int32_t backlog);
int32_t qc_handle_recv(CPUState *cpu, int32_t sckt, void *buffer,
size_t length, int32_t flags);
Expand Down