search

haskell / network

Low-level networking interface
http://hackage.haskell.org/package/network
Other
325 stars 187 forks source link

Fix bitsize of cmsgHdrLen and msgCtrlLen on Posix #535

Closed zeldin closed 2 years ago

zeldin commented 2 years ago

These fields are size_t, so using CInt on LP64 only accesses 32 bits of the total 64. This is especially noticable on big endian, where the lower 32 bits of the number are written to the upper 32 bits of the field.

kazu-yamamoto commented 2 years ago

Good catch.

I think that int comes from macOS which is my main machine. Its manual says that the type is int for this field. And when I apply your patch to master, the test gets stacked.

We need to find a good way both for Linux and macOS.

kazu-yamamoto commented 2 years ago

Never mind. I was looking at master. (Old text: We should modify cbits/cmsg.c, too.)

kazu-yamamoto commented 2 years ago

I made a mistake. On macOS, CMSG_LEN is defined:

#define CMSG_LEN(l)             (__DARWIN_ALIGN32(sizeof(struct cmsghdr)) + (l))

So, its return type is size_t.

I'm puzzled why tests fail with this PR.

zeldin commented 2 years ago

Looking at <sys/socket.h> on Monterey, the sizes of the fields are indeed different. cmsg_len and msg_controllen are socklen_t, which is 32 bits, while msg_iovlen is int (meaning that it was mismatched already before)... Now I kind of wonder what POSIX.1-2008 actually say about these fields. 😟

kazu-yamamoto commented 2 years ago

The network stack of macOS is a bit strange. Could you put #ifdef to take macOS as a special case?

zeldin commented 2 years ago

Should be possible I guess. Although it's plausible that it's actually Linux that is the special case. macOS is ostensibly based on BSD, which is the source of the sendmsg API. Anyway, I think I need to set up ghc on my mac so that I can run the tests there as well.

zeldin commented 2 years ago

Yes, POSIX.1-2008 supports the macOS definition of the structs, so I guess Linux unilaterally changed the API to fix security issues. Thus, Linux is the special case.

zeldin commented 2 years ago

Actually, rather than hardcoding special cases, I think the best solution would be to just use #type. I'll see if I can cook up something that works.

zeldin commented 2 years ago

There, now the tests pass both on macOS (arm64) and Linux (ppc64) for me. The second force-push was just updating the commit message to be more accurate. Unfortunately it was not possible to use #type to match the type of a field member (presumable because this would require use of typeof(), which is C2x), so I still had to use #ifdef where Linux used a different type name from what POSIX specifies.

kazu-yamamoto commented 2 years ago

Merged. Thanks you for your awesome contribution!