-
Notifications
You must be signed in to change notification settings - Fork 124
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
base: master
Are you sure you want to change the base?
Conversation
Builds on both Linux and OSX Then I built the new tcp tunnel on OSX with the new qemu and I couldn't SSH in on OSX. Brought the hfs back into linux and SSH tunnel fails, but fails to connect. It doesn't give the 255 error like before. I'll try again shortly but stalls without adding the new tcp-tunnel from xnu-qemu-arm64-tools.
|
In the tcp-tunnel’s Makefile, uncomment the part about No real easy way to have a ‘universal’ binary, EDEADLK is 11 on sys/errno.h on Darwin- replacing EAGAIN. If you’ve verified issues on macos with lldb, let me know. Nothing was changed for macos except making structs for sockaddr a alias they were originally were. |
Alright, will try now. If I use the non-patched tcp-tunnel, I can almost fully connect:
Trying your tips now My HFS is always dirty on Linux, I'll check it out, does it get dirty if I boot it on OSX first? |
FYI, trying your steps now
|
Okay I think it got dirty when I rsynced it from the KVM guest to host (compressed), about to test! |
@MCApollo amazing dude!
Will test the other way to make sure |
I get the same error, unless I tunnel in. I tried to tunnel in sshpass -p "alpine" ssh -N [email protected] -p 2222 5900:127.0.0.1:5900 Was trying to run VNC, signed screendumpd launch daemon with jtool and I can reach the VNC server and get a response via loop back tunneling except something about the VNC doesn't work. I cant tunnel out I also get error 35, I can only have one connection at a time. The VNC drops and the error is BigEndian = 0 or something. Can't you tunnel everything out on the one port, like a VPN? Whenever a connection drops, I can't use that host port again I opened Everything works and I can get thru ssh tunneled in, and then i get error 61, which I assume is Refused Connection in the console |
Hi @MCApollo First of all, thanks for the PR! It's great to see new features such as Linux support being added to the project! However, the extensive use of |
That’s a actually a better idea and was hoping someone would help with; Feel free to continue to use this PR, I know this code is god-awful and is a straight up bandaid until someone/I has the time. I can continue to work on this, been busy with some other things. There’s really two issues that need a checkbox
Some notes are:
|
hw/arm/guest-socket.c
Outdated
#ifdef __APPLE__ | ||
# define ADDR addr | ||
# undef NETWORK_HACK | ||
#else /* Linux, etc. */ | ||
# define ADDR darwin_addr | ||
# define NETWORK_HACK 1 | ||
#endif |
There was a problem hiding this comment.
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.
hw/arm/guest-socket.c
Outdated
#endif | ||
|
||
static int convert_error(int err) { | ||
#ifndef NETWORK_HACK |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#ifndef NETWORK_HACK | |
#ifdef __APPLE__ |
hw/arm/guest-socket.c
Outdated
# define NETWORK_HACK 1 | ||
#endif | ||
|
||
static int convert_error(int err) { |
There was a problem hiding this comment.
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
.
hw/arm/guest-socket.c
Outdated
#endif | ||
} | ||
|
||
static struct sockaddr_in convert_sockaddr_to(S_SOCKADDR_IN from) { |
There was a problem hiding this comment.
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:
- The
#ifndef
inside the function is redundant - The name of the function can be more explicit, such as
convert_sockaddr_darwin_to_gnu
- It should receive
darwin_sockaddr_in
explicitly (and theS_SOCKADDR_IN
define can be dropped)
static struct sockaddr_in convert_sockaddr_to(S_SOCKADDR_IN from) { | |
static struct sockaddr_in convert_sockaddr_darwin_to_gnu(darwin_sockaddr_in from) { |
hw/arm/guest-socket.c
Outdated
#ifndef NETWORK_HACK | ||
return from; | ||
#else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant...
hw/arm/guest-socket.c
Outdated
#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); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#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 |
hw/arm/guest-socket.c
Outdated
#ifdef NETWORK_HACK | ||
S_SOCKADDR_IN ADDR; | ||
#endif |
There was a problem hiding this comment.
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...
hw/arm/guest-socket.c
Outdated
cpu_memory_rw_debug(cpu, (target_ulong) g_addr, (uint8_t*) &ADDR, | ||
sizeof(ADDR), 0); | ||
|
||
#ifdef NETWORK_HACK | ||
addr = convert_sockaddr_to(ADDR); | ||
#endif |
There was a problem hiding this comment.
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
:
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 |
hw/arm/guest-socket.c
Outdated
#ifdef NETWORK_HACK | ||
ADDR = convert_sockaddr_from(addr); | ||
#endif | ||
cpu_memory_rw_debug(cpu, (target_ulong) g_addr, (uint8_t*) &ADDR, | ||
sizeof(ADDR), 1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#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 |
hw/arm/guest-socket.c
Outdated
} | ||
} | ||
|
||
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, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to bind
...
Sounds about right, thanks for being better than me at C- I’ll work on finishing so we can finally fix this. (TIL about compound literals for easier reading, 👍🏼) As far as accept goes, I’m not sure if Darwin acts differently vs Linux (see Correct me if I’m wrong, but my understanding is that this should set nonblock on the host side right? here: |
Darwin should be inheriting flags from the parent socket upon
I'd like to avoid conditional code as much as possible, which would leave the |
Yeah, the accept4 was a bandaid fix until I can confirm if it was a bug/“feature” since no documentation mentions it without digging through man pages. I think forcing non_blocking breaks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, looks good. We're close to the final revision. I still wish we could streamline the part where we copy the memory to/from the guest, but with pointers involved, I don't see a neat solution there.
hw/arm/guest-socket.c
Outdated
} else if ((guest_svcs_fds[retval] = | ||
#ifndef __APPLE__ | ||
accept4(guest_svcs_fds[sckt], | ||
(struct sockaddr *) &addr, | ||
&addrlen, SOCK_NONBLOCK)) < 0) | ||
#else | ||
accept(guest_svcs_fds[sckt], | ||
(struct sockaddr *) &addr, | ||
&addrlen)) < 0) { | ||
&addrlen)) < 0) | ||
#endif | ||
{ | ||
retval = -1; | ||
guest_svcs_errno = errno; | ||
guest_svcs_errno = darwin_error(errno); | ||
} else { | ||
#ifndef __APPLE__ | ||
struct darwin_sockaddr_in darwin_addr = convert_sockaddr_to_darwin(addr); | ||
cpu_memory_rw_debug(cpu, (target_ulong) g_addr, (uint8_t*) &darwin_addr, | ||
sizeof(darwin_addr), 1); | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use just accept
, and copy the flags in the #ifndef __APPLE__
section.
#define SOCKADDR sockaddr | ||
#define SOCKADDR_IN sockaddr_in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for these defines. You can use darwin_sockaddr
/darwin_sockaddr_in
directly. If you don't want to redefine the structs in Darwin, you can use #define darwin_sockaddr sockaddr
and #define darwin_sockaddr_in sockaddr_in
when __APPLE__
is defined.
That previous ADDR macro was there to avoid reductant/repeating cpu_rw code (if that’s a problem). |
|
@MCApollo any progress with this? |
bind is not working even on macOS after the latest changes, error 45, can't work on this no time, but just to let you know |
Run it under lldb for me, this PR shouldn’t affect any macos parts at all. |
i mean when kvm was implemented the tunneling stoped working so. |
@Maroc-OS are you running on ARM with KVM, or is there an issue when using regular QEMU emulation? Because the KVM changes should not affect the latter. |
@aronsky yep regular QEMU emulation. After updating to kvm commits it stopped working, and no ssh is there. here is a little script am using adapted from @MCApollo :
resulting crash of "MyUnixSocket" which is responsible of loading the (signed/unsigned both tested) tcp-tunnel binary, trying to launch it manually resulting "error 45" at binding |
Handles issue #22, open to changes.
EDIT