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

net_sendto() fails if passed sizeof(struct sockaddr_in) #189

Open
mardy opened this issue Feb 9, 2025 · 2 comments
Open

net_sendto() fails if passed sizeof(struct sockaddr_in) #189

mardy opened this issue Feb 9, 2025 · 2 comments

Comments

@mardy
Copy link
Contributor

mardy commented Feb 9, 2025

Bug Report

What's the issue you encountered?

Calling net_sendto() on a UDP socket fails if the last parameter is set to sizeof(struct sockaddr_in), as is usually done in POSIX.

How can the issue be reproduced?

  1. Checkout the udptest example from wii-examples
  2. Apply the following patch:
--- a/devices/network/udptest/source/udptest.c
+++ b/devices/network/udptest/source/udptest.c
@@ -117,6 +117,7 @@ static void handle_client(s32 sock)
            client.sin_port, len, buffer);
 
     len_out = snprintf(buffer_out, sizeof(buffer_out), "Echo: %.*s\n", len, buffer);
+    sock_len = sizeof(client);
     net_sendto(sock, buffer_out, len_out, 0, (struct sockaddr *)&client, sock_len);
     if (strncmp(buffer, "exit", 4) == 0) exit(0);
 }
  1. Run the program, and test it using nc as instructed. The net_sendto() method will fail, and no data will be sent back to the nc client.

Environment?

  • What host OS are you using?
    • Linux
  • Official release or unofficial/self-compiled build:
    • N/A

Additional context?

The example normally works because the sock_len variable retains the value it previously got from net_recvfrom(), which is 8. However, libogc network.h header defined the sockaddr_in struct as follows:

struct sockaddr_in {
  u8 sin_len;
  u8 sin_family; 
  u16 sin_port;
  struct in_addr sin_addr;
  s8 sin_zero[8];
};          

The sin_zero member causes the struct size to grow to 16 bytes, which is the usual size in Linux and Windows, but causes problems to the network stack on the Wii. This padding member should probably be removed.

Note also that in libogc code net_bind(), net_accept(), net_connect() net_getsockname() all use the magic number "8" instead of a proper sizeof(struct sockaddr_in).

I'll prepare a PR for this, but unfortunately I won't be able to test the lwip implementation, since I don't have a modded GameCube, so I'll be able to test the Wii only. But as far as I can see from the lwip code, it should be robust against struct size changes.

@DacoTaco
Copy link
Member

DacoTaco commented Feb 9, 2025

i can test both wii and gamecube when the change comes in, no worries @mardy :)

also, how does dolphin handle the struct? does it reply/act like its 8 or 16 bytes?

also, how does that struct look in unix/macos and android? :/

@mardy
Copy link
Contributor Author

mardy commented Feb 9, 2025

i can test both wii and gamecube when the change comes in, no worries @mardy :)

Excellent! And BTW, I noticed that lwip has its own defininition of the structures in gc/lwip/sockets.h, which I will not change. So, the GC should not be affected at all.

also, how does dolphin handle the struct? does it reply/act like its 8 or 16 bytes?

In Dolphin everything works, because it just forwards the data to the host, which is perfectly happy with the size being 16, so there it works (I actually wonder if I should file a bug there, too). Now my changes are ready (PR soon!) and I've tested them on Dolphin as well, and they work fine.

also, how does that struct look in unix/macos and android? :/

In Linux it's 16 bytes, so I think that Android will be the same.

mardy added a commit to mardy/libogc that referenced this issue Feb 9, 2025
Having this structure being 16 bytes long causes net_sendto not to work;
setting it to 8 bytes means that we can get rid of a few magic numbers
in libogc's code (which actually show that the structure is meant to be
8 bytes only).

Changes have been tested on both a real Wii and on Dolphin, using the
sockettest and udptest from wii-examples.

Fixes: devkitPro#189
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants