[LibOS] Add dummy ancillary data support for sockets #1210

dimakuv · 2023-03-03T12:53:40Z

Description of the changes

Previously, Gramine returned -ENOSYS on recvmsg()/sendmsg() and similar syscalls if it detected the use of struct msghdr ancillary data (msg_control field). This PR is the first step in adding proper ancillary data support.

Currently no ancillary data type is truly supported:

on sendmsg():
- unknown/unsupported types force an error,
- SCM_RIGHTS/SCM_CREDENTIALS are ignored in TCP/UDP sockets;
on recvmsg(): msg_controllen is set to zero to indicate there is no ancillary data added to the received network packet.

How to test this PR?

I added a LibOS regression test + I disabled one misbehaving LTP test.

One can try this example: gramineproject/examples#57. Without this PR, it doesn't work properly (see test 2 in the README there).

This change is

dimakuv

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel)

a discussion (no related file):
This was found and debugged on the swtpm software, when I tried to enable it in Gramine.

Here are the relevant code snippets:

If you look closely at this code, you'll notice two things:

This recvmsg(...with ancillary data...) is called even on TCP/IP sockets, where it is simply ignored.
The ancillary data is used only in one command case (CMD_SET_DATAFD), and not used in all other cases (that make sense in Gramine-SGX).

You can even notice that CMD_SET_DATAFD is described as useful only for Unix Domain Sockets in the man page: https://github.com/stefanberger/swtpm/blob/e5fdd1c181d0414ad27a0f4f72c49020fbc1ac6e/man/man3/swtpm_ioctls.pod

So to workaround such flows, I think it is reasonable to merge this PR.

mkow

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv)

a discussion (no related file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

This was found and debugged on the swtpm software, when I tried to enable it in Gramine.

Here are the relevant code snippets:

https://github.com/stefanberger/swtpm/blob/185832c8d4be95fa9305279240ca33b7403cfc91/src/swtpm/ctrlchannel.c#L500-L506

https://github.com/stefanberger/swtpm/blob/185832c8d4be95fa9305279240ca33b7403cfc91/src/swtpm/ctrlchannel.c#L345

https://github.com/stefanberger/swtpm/blob/185832c8d4be95fa9305279240ca33b7403cfc91/src/swtpm/ctrlchannel.c#L810-L822

If you look closely at this code, you'll notice two things:

This recvmsg(...with ancillary data...) is called even on TCP/IP sockets, where it is simply ignored.

The ancillary data is used only in one command case (CMD_SET_DATAFD), and not used in all other cases (that make sense in Gramine-SGX).

You can even notice that CMD_SET_DATAFD is described as useful only for Unix Domain Sockets in the man page: https://github.com/stefanberger/swtpm/blob/e5fdd1c181d0414ad27a0f4f72c49020fbc1ac6e/man/man3/swtpm_ioctls.pod

So to workaround such flows, I think it is reasonable to merge this PR.

This doesn't sound correct, you are silently dropping the data which the user app sent? I don't like it, it will make the apps which actually use ancillary data super annoying to debug.

If swtpm is open source, isn't it easier to just fix it to not send the data, which it doesn't even use?

mkow

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv)

a discussion (no related file):

Previously, mkow (Michał Kowalczyk) wrote…

This doesn't sound correct, you are silently dropping the data which the user app sent? I don't like it, it will make the apps which actually use ancillary data super annoying to debug.

If swtpm is open source, isn't it easier to just fix it to not send the data, which it doesn't even use?

Ok, you have a warning, but still... I don't like making our emulation behave differently than Linux just to workaround one specific app.

dimakuv

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @mkow)

a discussion (no related file):

This doesn't sound correct, you are silently dropping the data which the user app sent? I don't like it, it will make the apps which actually use ancillary data super annoying to debug.

TCP/IP sockets cannot have ancillary data.

Well, but now I think -- why do we even need a warning message. It should be the same behavior as Linux, so doesn't require a warning.

dimakuv

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @mkow)

a discussion (no related file):

TCP/IP sockets cannot have ancillary data.

Well, that's not exactly true. Here are the ancillary data types that could be on the socket during sendmsg():

https://elixir.bootlin.com/linux/latest/source/net/core/sock.c#L2757
(so, SO_MARK, SO_TIMESTAMPING_OLD and SCM_TXTIME are applicable to TCP, it seems, but they are all clearly unsupported by Gramine so could be ignored)

Here are the ancillary data types that could be on the socket during recvmsg():

https://elixir.bootlin.com/linux/latest/source/net/ipv4/tcp.c#L2684
(so, SO_TIMESTAMPNS_NEW, TCP_CM_INQ are applicable to TCP, but they are unsupported by Gramine so could be ignored)
(btw, found an interesting example of this: https://github.com/torvalds/linux/blob/master/tools/testing/selftests/net/tcp_inq.c)

TLDR: Yeah, looks like Linux added support for ancillary data for TCP/IP sockets in recent years, though Gramine has no mechanics to support these data. So what about this:

I'll create another PR that has more specific handling of ancillary data,
sendmsg will fail on any ancillary data
recmsg will not add any ancillary data

This will be kinda similar to the current PR, but more explicit. @mkow ?

dimakuv

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @mkow)

a discussion (no related file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

TCP/IP sockets cannot have ancillary data.

Well, that's not exactly true. Here are the ancillary data types that could be on the socket during sendmsg():

https://elixir.bootlin.com/linux/latest/source/net/core/sock.c#L2757

(so, SO_MARK, SO_TIMESTAMPING_OLD and SCM_TXTIME are applicable to TCP, it seems, but they are all clearly unsupported by Gramine so could be ignored)

Here are the ancillary data types that could be on the socket during recvmsg():

https://elixir.bootlin.com/linux/latest/source/net/ipv4/tcp.c#L2684

(so, SO_TIMESTAMPNS_NEW, TCP_CM_INQ are applicable to TCP, but they are unsupported by Gramine so could be ignored)

(btw, found an interesting example of this: https://github.com/torvalds/linux/blob/master/tools/testing/selftests/net/tcp_inq.c)

TLDR: Yeah, looks like Linux added support for ancillary data for TCP/IP sockets in recent years, though Gramine has no mechanics to support these data. So what about this:

I'll create another PR that has more specific handling of ancillary data,

sendmsg will fail on any ancillary data

recmsg will not add any ancillary data

This will be kinda similar to the current PR, but more explicit. @mkow ?

I experimented with TCP and different ancillary data types:

`sendmsg()`

If msg_control is set to an empty buffer, then sendmsg() returns EINVAL, triggering this case: https://elixir.bootlin.com/linux/v6.2/source/net/core/sock.c#L2794
If msg_control is set to SCM_RIGHTS, then sendmsg() sends the packet and ignores ancillary data, triggering this case: https://elixir.bootlin.com/linux/v6.2/source/net/core/sock.c#L2792
If msg_control is set to some non-socket level (i.e. cmsg_level = IPPROTO_IP), then ignores this ancillary data; this case: https://elixir.bootlin.com/linux/v6.2/source/net/core/sock.c#L2810
If msg_control is set to socket-specific SO_TIMESTAMPING, then sendmsg() uses this ancillary data, triggering this case: https://elixir.bootlin.com/linux/v6.2/source/net/core/sock.c#L2780

`recvmsg()`

If sendmsg() sent no ancillary data, recvmsg() gets nothing (msg_controllen = 0)
If sendmsg() sent SCM_RIGHTS, recvmsg() gets nothing (msg_controllen = 0)
If sendmsg() sent IPPROTO_IP, recvmsg() gets nothing (msg_controllen = 0)
If client socket has setsockopt(SOL_SOCKET, SO_TIMESTAMPING...) with SOF_TIMESTAMPING_RX_SOFTWARE | SOF_TIMESTAMPING_SOFTWARE, recvmsg() gets SO_TIMESTAMPING_OLD ancillary data (with msg_controllen = 24)

Conclusion

TCP/IP sockets in Linux:

can send and receive ancillary data (I experimented with SO_TIMESTAMPING)
sendmsg() ignores UDS-specific ancillary data or non-socket-level data and fails with EINVAL on unrecognized data
recvmsg() returns msg_controllen = 0 if no ancillary data was attached to this packet

Thus, I should implement the "ignore all ancillary data" in the similar manner in this PR.

dimakuv

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @mkow)

a discussion (no related file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

I experimented with TCP and different ancillary data types:

sendmsg()

If msg_control is set to an empty buffer, then sendmsg() returns EINVAL, triggering this case: https://elixir.bootlin.com/linux/v6.2/source/net/core/sock.c#L2794

If msg_control is set to SCM_RIGHTS, then sendmsg() sends the packet and ignores ancillary data, triggering this case: https://elixir.bootlin.com/linux/v6.2/source/net/core/sock.c#L2792

If msg_control is set to some non-socket level (i.e. cmsg_level = IPPROTO_IP), then ignores this ancillary data; this case: https://elixir.bootlin.com/linux/v6.2/source/net/core/sock.c#L2810

If msg_control is set to socket-specific SO_TIMESTAMPING, then sendmsg() uses this ancillary data, triggering this case: https://elixir.bootlin.com/linux/v6.2/source/net/core/sock.c#L2780

recvmsg()

If sendmsg() sent no ancillary data, recvmsg() gets nothing (msg_controllen = 0)

If sendmsg() sent SCM_RIGHTS, recvmsg() gets nothing (msg_controllen = 0)

If sendmsg() sent IPPROTO_IP, recvmsg() gets nothing (msg_controllen = 0)

If client socket has setsockopt(SOL_SOCKET, SO_TIMESTAMPING...) with SOF_TIMESTAMPING_RX_SOFTWARE | SOF_TIMESTAMPING_SOFTWARE, recvmsg() gets SO_TIMESTAMPING_OLD ancillary data (with msg_controllen = 24)

Conclusion

TCP/IP sockets in Linux:

can send and receive ancillary data (I experimented with SO_TIMESTAMPING)

sendmsg() ignores UDS-specific ancillary data or non-socket-level data and fails with EINVAL on unrecognized data

recvmsg() returns msg_controllen = 0 if no ancillary data was attached to this packet

Thus, I should implement the "ignore all ancillary data" in the similar manner in this PR.

Btw, a good resource for this timestamping functionality: https://www.kernel.org/doc/Documentation/networking/timestamping.txt

dimakuv

Reviewable status: 0 of 11 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @mkow)

a discussion (no related file):
FYI: This PR is completely rewritten now (as of revision 3). It adds proper support (though everything is still resulting in error or is ignored).

I went deep into the Linux sources, and imitated what Linux does. Here are the relevant sources:

stefanberger

Reviewed 11 of 11 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @mkow)

stefanberger

Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @mkow)

dimakuv

Reviewable status: 10 of 12 files reviewed, 1 unresolved discussion, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @mkow and @stefanberger)

a discussion (no related file):
FYI: This PR is ready for review. Because the PR was reworked completely for revision 3, please review from scratch (ignore revisions 1 and 2).

dimakuv

Reviewable status: 10 of 12 files reviewed, 2 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @mkow and @stefanberger)

libos/test/ltp/ltp.cfg line 1666 at r4 (raw file):

# subtest 8 requires SCM_RIGHTS support (to send an FD from sender to receiver) and doesn't check
# for errors (Gramine fails with -ENOSYS), leading to a hang on the receiver side because it tries
# to receive something that was never sent.

Btw, I created an issue in the LTP github repo: linux-test-project/ltp#1020

Not sure though this is a "real enough" issue, and that LTP folks will fix it

mkow

Reviewed 10 of 11 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 9 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv)

a discussion (no related file):
Heh, I see a lot happened in the meantime :)

Regarding my original comment: The message I got from this PR originally was that this is a hack to make swtpm work and it's not how Linux works, thus my original blocking comment.

I experimented with TCP and different ancillary data types:
[...]

Jeez, what an ugly interface...

-- commits line 12 at r4:

Suggestion:

    - SCM_RIGHTS/SCM_CREDENTIALS are ignored in TCP/UDP sockets (same as on Linux);

libos/src/fs/socket/fs.c line 39 at r4 (raw file):

    };
    unsigned int flags = 0;
    return do_recvmsg(handle, &iov, /*iov_len=*/1, /*msg_control=*/NULL, /*msg_controllen=*/NULL,

Could we rename it to msg_controllen_ptr or something in this style? It's quite confusing to see things like msg_controllen = 0 almost next to msg_controllen = NULL when reading the socket code, because it looks like a bug. This is especially visible in this flie.
And maybe the same with addrlen?

Code quote:

/*msg_controllen=*/NULL

libos/src/net/ip.c line 682 at r4 (raw file):

        if (cmsg->cmsg_level != SOL_SOCKET) {
            /*
             * Currently don't support:

(it's in imperative mood otherwise)

Suggestion:

We currently don't support:

libos/src/net/ip.c line 693 at r4 (raw file):

        switch (cmsg->cmsg_type) {
            /* currently don't support below SOL_SOCKET types */

ditto

libos/src/net/ip.c line 778 at r4 (raw file):

    if (msg_control && msg_controllen) {
        /*
         * Currently don't support:

ditto

libos/src/net/unix.c line 435 at r4 (raw file):

        rest_msg_controllen -= CMSG_ALIGN(cmsg->cmsg_len);
        cmsg = (struct cmsghdr*)((char*)cmsg + CMSG_ALIGN(cmsg->cmsg_len));

Purely theoretical (I think) trivia: This cast to struct cmsghdr* is an UB in C, because it's possible that the resulting pointer won't point to neither a struct cmsghdr object nor right behind any such object if the msg_control array is not padded (I'm not sure if it has to be, probably not).

You could fix it by checking if the buffer we got is big enough to reach the (char*)cmsg + CMSG_ALIGN(cmsg->cmsg_len) address and executing this line only if it is, but probably not worth the effort.

I think C has this rule because lines like this one would be problematic if msg_control was allocated near the end of address space, because the addition could overflow the pointer then.

libos/test/regression/tcp_ancillary.c line 21 at r4 (raw file):

#define PORT   11110

#define MSG_SPACE CMSG_SPACE(sizeof(int)) + CMSG_SPACE(sizeof(struct ucred))

Suggestion:

#define MSG_SPACE (CMSG_SPACE(sizeof(int)) + CMSG_SPACE(sizeof(struct ucred)))

libos/test/regression/tcp_ancillary.c line 42 at r4 (raw file):

    char c = 0;
    ssize_t x = CHECK(write(pipefd, &c, sizeof(c)));
    if (x != 1) {

Suggestion:

if (x != sizeof(c))

libos/test/regression/tcp_ancillary.c line 117 at r4 (raw file):

        .sin_port = htons(PORT),
        .sin_addr = {
            /* TODO: remove this once Ubuntu 18.04 is deprecated. */

What exactly? The whole assignment of .sin_addr? Why? Could you explain this a bit more in the comment?

kailun-qin

Reviewable status: all files reviewed, 10 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @dimakuv)

libos/src/net/ip.c line 677 at r4 (raw file):

    size_t rest_msg_controllen = msg_controllen;
    while (cmsg && rest_msg_controllen >= sizeof(struct cmsghdr)) {
        if (cmsg->cmsg_len < sizeof(struct cmsghdr) || cmsg->cmsg_len > rest_msg_controllen)

Do we need to check against CMSG_ALIGN(cmsg->cmsg_len)?

Code quote:

cmsg->cmsg_len

dimakuv

Reviewable status: 6 of 12 files reviewed, 10 unresolved discussions, not enough approvals from maintainers (2 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @kailun-qin, @mkow, and @stefanberger)

-- commits line 12 at r4:
Yep, I'll add during final rebase

libos/src/fs/socket/fs.c line 39 at r4 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

Could we rename it to msg_controllen_ptr or something in this style? It's quite confusing to see things like msg_controllen = 0 almost next to msg_controllen = NULL when reading the socket code, because it looks like a bug. This is especially visible in this flie.
And maybe the same with addrlen?

Done.

libos/src/net/ip.c line 677 at r4 (raw file):

Previously, kailun-qin (Kailun Qin) wrote…

Do we need to check against CMSG_ALIGN(cmsg->cmsg_len)?

Done. You're right, otherwise we could have a broken corner case: cmsg->cmsg_len == rest_msg_controllen but CMSG_ALIGN(cmsg->cmsg_len) > rest_msg_controllen (e.g. if cmsg->cmsg_len == 7).

In that case, rest_msg_controllen -= CMSG_ALIGN(cmsg->cmsg_len); could result in integer underflow. Fixed now (by changing only the second part of this IF).

libos/src/net/ip.c line 682 at r4 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

(it's in imperative mood otherwise)

Done.

libos/src/net/ip.c line 693 at r4 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

ditto

Done.

libos/src/net/ip.c line 778 at r4 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

ditto

Done.

libos/src/net/unix.c line 435 at r4 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

Purely theoretical (I think) trivia: This cast to struct cmsghdr* is an UB in C, because it's possible that the resulting pointer won't point to neither a struct cmsghdr object nor right behind any such object if the msg_control array is not padded (I'm not sure if it has to be, probably not).

You could fix it by checking if the buffer we got is big enough to reach the (char*)cmsg + CMSG_ALIGN(cmsg->cmsg_len) address and executing this line only if it is, but probably not worth the effort.

I think C has this rule because lines like this one would be problematic if msg_control was allocated near the end of address space, because the addition could overflow the pointer then.

I agree with your assessment. I also stopped for a bit because of this CMSG_ALIGN() that could "shift" the cmsg pointer past the msg_control object boundary, which is a UB.

But @kailun-qin noticed above that there is still potential for integer overflow, see that comment. I fixed it now.

libos/test/regression/tcp_ancillary.c line 21 at r4 (raw file):

#define PORT   11110

#define MSG_SPACE CMSG_SPACE(sizeof(int)) + CMSG_SPACE(sizeof(struct ucred))

Done.

libos/test/regression/tcp_ancillary.c line 42 at r4 (raw file):

    char c = 0;
    ssize_t x = CHECK(write(pipefd, &c, sizeof(c)));
    if (x != 1) {

Done. Wow, that's some pedantry :)

libos/test/regression/tcp_ancillary.c line 117 at r4 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

What exactly? The whole assignment of .sin_addr? Why? Could you explain this a bit more in the comment?

Yes, remove this whole assignment of .sin_addr (because it would be implicitly set to zero anyway).

There is some weird bug in Ubuntu 18.04, and nobody wanted to care about this corner case. See the discussion starting here: #684 (review)

(Note that I based this new test on the already-existing tcp_msg_peek.c test, where the same TODO can be seen.)

If you want to, I can change the text of the TODO, but I'm not sure to what.

mkow

Reviewed 6 of 6 files at r5, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @dimakuv and @kailun-qin)

libos/src/net/unix.c line 435 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

I agree with your assessment. I also stopped for a bit because of this CMSG_ALIGN() that could "shift" the cmsg pointer past the msg_control object boundary, which is a UB.

But @kailun-qin noticed above that there is still potential for integer overflow, see that comment. I fixed it now.

Yup, seems to be the same issue, but I didn't notice that it actually leads to underflowing rest_msg_controllen. So, turns out it wasn't purely theoretical after all, although because of a different part of the code :)

libos/test/regression/tcp_ancillary.c line 42 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Done. Wow, that's some pedantry :)

I disagree ;) This is how I check such places for correctness when reviewing: I read such constructions and check if there's a comparison between the return value and the exact expression which was used as a size for I/O. And here I had to stop and think because it was different ;)

libos/test/regression/tcp_ancillary.c line 117 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Yes, remove this whole assignment of .sin_addr (because it would be implicitly set to zero anyway).

There is some weird bug in Ubuntu 18.04, and nobody wanted to care about this corner case. See the discussion starting here: #684 (review)

(Note that I based this new test on the already-existing tcp_msg_peek.c test, where the same TODO can be seen.)

If you want to, I can change the text of the TODO, but I'm not sure to what.

Oh, so it's about GCC, not Ubuntu. Actually, we already started dropping 18.04, maybe just remove this line now?

dimakuv

Reviewable status: all files reviewed, 4 unresolved discussions, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel), "fixup! " found in commit messages' one-liners (waiting on @dimakuv, @kailun-qin, and @mkow)

libos/test/regression/tcp_ancillary.c line 117 at r4 (raw file):

Previously, mkow (Michał Kowalczyk) wrote…

Oh, so it's about GCC, not Ubuntu. Actually, we already started dropping 18.04, maybe just remove this line now?

What do you mean by started dropping 18.04? It's in the CI (e.g. Jenkins-18.04), so the current PR will fail if I remove this line.

mkow

Reviewable status: all files reviewed, 2 unresolved discussions, "fixup! " found in commit messages' one-liners (waiting on @dimakuv and @kailun-qin)

libos/test/regression/tcp_ancillary.c line 117 at r4 (raw file):

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

What do you mean by started dropping 18.04? It's in the CI (e.g. Jenkins-18.04), so the current PR will fail if I remove this line.

We're after v1.4, which was the last version which supports it and I think we already started removing some compat. But yeah, we need to disable the CI pipelines first.

kailun-qin

Reviewable status: all files reviewed, 1 unresolved discussion, "fixup! " found in commit messages' one-liners (waiting on @dimakuv)

Previously, Gramine returned `-ENOSYS` on `recvmsg()`/`sendmsg()` and similar syscalls if it detected the use of `struct msghdr` ancillary data (`msg_control` field). This commit is the first step in adding proper ancillary data support. Currently no ancillary data type is truly supported: - on `sendmsg()`: - unknown/unsupported types force an error, - SCM_RIGHTS/SCM_CREDENTIALS are ignored in TCP/UDP sockets (same as on Linux); - on `recvmsg()`: `msg_controllen` is set to zero to indicate there is no ancillary data added to the received network packet. Signed-off-by: Dmitrii Kuvaiskii <[email protected]>

dimakuv

Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: Intel) (waiting on @mkow)

-- commits line 12 at r4:

Previously, dimakuv (Dmitrii Kuvaiskii) wrote…

Yep, I'll add during final rebase

Done

mkow

Reviewed all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved

dimakuv commented Mar 3, 2023

View reviewed changes

dimakuv mentioned this pull request Mar 3, 2023

CMD_SET_DATAFD uses ancillary data (struct msghdr::msg_control) even on TCP/IP sockets stefanberger/swtpm#792

Closed

dimakuv force-pushed the dimakuv/ignore-ancillary-data-on-recvmsg branch from 6027536 to 6413ddb Compare March 3, 2023 14:09

dimakuv mentioned this pull request Mar 3, 2023

Add swtpm example gramineproject/examples#57

Open

dimakuv force-pushed the dimakuv/ignore-ancillary-data-on-recvmsg branch from 6413ddb to 553b05f Compare March 3, 2023 14:21

mkow requested changes Mar 3, 2023

View reviewed changes

mkow reviewed Mar 3, 2023

View reviewed changes

dimakuv commented Mar 3, 2023

View reviewed changes

dimakuv commented Mar 6, 2023

View reviewed changes

dimakuv force-pushed the dimakuv/ignore-ancillary-data-on-recvmsg branch from 553b05f to 7ec752d Compare March 6, 2023 15:47

dimakuv commented Mar 6, 2023

View reviewed changes

stefanberger reviewed Mar 6, 2023

View reviewed changes

dimakuv force-pushed the dimakuv/ignore-ancillary-data-on-recvmsg branch 2 times, most recently from 05829fd to 0b161f1 Compare March 6, 2023 17:20

dimakuv mentioned this pull request Mar 6, 2023

recvmsg01.c doesn't have error checking in many places linux-test-project/ltp#1020

Closed

dimakuv force-pushed the dimakuv/ignore-ancillary-data-on-recvmsg branch from 0b161f1 to 749005e Compare March 6, 2023 18:07

dimakuv changed the title ~~[LibOS] Ignore ancillary data on recvmsg()~~ [LibOS] Add dummy ancillary data support for sockets Mar 7, 2023

dimakuv commented Mar 7, 2023

View reviewed changes

mkow requested changes Mar 8, 2023

View reviewed changes

kailun-qin reviewed Mar 8, 2023

View reviewed changes

dimakuv commented Mar 8, 2023

View reviewed changes

mkow requested changes Mar 8, 2023

View reviewed changes

dimakuv commented Mar 8, 2023

View reviewed changes

mkow previously approved these changes Mar 8, 2023

View reviewed changes

kailun-qin reviewed Mar 9, 2023

View reviewed changes

dimakuv dismissed mkow’s stale review via 299dde8 March 9, 2023 12:55

dimakuv force-pushed the dimakuv/ignore-ancillary-data-on-recvmsg branch from 896e1a6 to 299dde8 Compare March 9, 2023 12:55

dimakuv commented Mar 9, 2023

View reviewed changes

mkow approved these changes Mar 9, 2023

View reviewed changes

dimakuv merged commit 299dde8 into master Mar 10, 2023

dimakuv deleted the dimakuv/ignore-ancillary-data-on-recvmsg branch March 10, 2023 09:15

dimakuv mentioned this pull request Apr 19, 2023

Add new changes to "Gramine features" before v1.5 release #1295

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LibOS] Add dummy ancillary data support for sockets #1210

[LibOS] Add dummy ancillary data support for sockets #1210

dimakuv commented Mar 3, 2023 •

edited

Loading

dimakuv left a comment

mkow left a comment

mkow left a comment

dimakuv left a comment

dimakuv left a comment

dimakuv left a comment

dimakuv left a comment

dimakuv left a comment

stefanberger left a comment

stefanberger left a comment

dimakuv left a comment

dimakuv left a comment

mkow left a comment

kailun-qin left a comment

dimakuv left a comment

mkow left a comment

dimakuv left a comment

mkow left a comment

kailun-qin left a comment

dimakuv left a comment

mkow left a comment

[LibOS] Add dummy ancillary data support for sockets #1210

[LibOS] Add dummy ancillary data support for sockets #1210

Conversation

dimakuv commented Mar 3, 2023 • edited Loading

Description of the changes

How to test this PR?

dimakuv left a comment

Choose a reason for hiding this comment

mkow left a comment

Choose a reason for hiding this comment

mkow left a comment

Choose a reason for hiding this comment

dimakuv left a comment

Choose a reason for hiding this comment

dimakuv left a comment

Choose a reason for hiding this comment

dimakuv left a comment

Choose a reason for hiding this comment

sendmsg()

recvmsg()

Conclusion

dimakuv left a comment

Choose a reason for hiding this comment

sendmsg()

recvmsg()

Conclusion

dimakuv left a comment

Choose a reason for hiding this comment

stefanberger left a comment

Choose a reason for hiding this comment

stefanberger left a comment

Choose a reason for hiding this comment

dimakuv left a comment

Choose a reason for hiding this comment

dimakuv left a comment

Choose a reason for hiding this comment

mkow left a comment

Choose a reason for hiding this comment

kailun-qin left a comment

Choose a reason for hiding this comment

dimakuv left a comment

Choose a reason for hiding this comment

mkow left a comment

Choose a reason for hiding this comment

dimakuv left a comment

Choose a reason for hiding this comment

mkow left a comment

Choose a reason for hiding this comment

kailun-qin left a comment

Choose a reason for hiding this comment

dimakuv left a comment

Choose a reason for hiding this comment

mkow left a comment

Choose a reason for hiding this comment

dimakuv commented Mar 3, 2023 •

edited

Loading

`sendmsg()`

`recvmsg()`

`sendmsg()`

`recvmsg()`