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

bubblewrap misuses CMSG_DATA() macro #637

Closed
mcatanzaro opened this issue Jun 20, 2024 · 1 comment · Fixed by #662
Closed

bubblewrap misuses CMSG_DATA() macro #637

mcatanzaro opened this issue Jun 20, 2024 · 1 comment · Fixed by #662

Comments

@mcatanzaro
Copy link

mcatanzaro commented Jun 20, 2024

cmsg(3) says:

       CMSG_DATA()
              returns  a pointer to the data portion of a cmsghdr.  The pointer returned cannot be assumed to be suitably aligned for accessing arbitrary
              payload data types.  Applications should not cast it to a pointer type matching the payload, but should instead use memcpy(3) to copy  data
              to or from a suitably declared object.

send_pid_on_socket() and read_pid_from_socket() in bubblewrap's utils.c both cast the return value exactly like the documentation says not to do.

(This should be a very easy newcomers issue.)

@mcatanzaro
Copy link
Author

Separately, it looks like the msg_control and msg_controllen fields are also supposed to be aligned. cmsg(3) shows this example:

           union {         /* Ancillary data buffer, wrapped in a union
                              in order to ensure it is suitably aligned */
               char buf[CMSG_SPACE(sizeof(myfds))];
               struct cmsghdr align;
           } u;

           msg.msg_iov = &io;
           msg.msg_iovlen = 1;
           msg.msg_control = u.buf;
           msg.msg_controllen = sizeof(u.buf);

smcv added a commit to smcv/bubblewrap that referenced this issue Oct 16, 2024
As documented in cmsg(3), the alignment of control messages is not
guaranteed, so for portability to architectures with strong alignment
requirements we should memcpy to and from a suitably aligned instance
of the desired data structure on the stack.

Helps: containers#637
Signed-off-by: Simon McVittie <[email protected]>
smcv added a commit to smcv/bubblewrap that referenced this issue Oct 16, 2024
A char array on the stack is not guaranteed to have any particular
alignment.

Resolves: containers#637
Signed-off-by: Simon McVittie <[email protected]>
smcv added a commit to smcv/bubblewrap that referenced this issue Oct 17, 2024
As documented in cmsg(3), the alignment of control messages is not
guaranteed, so for portability to architectures with strong alignment
requirements we should memcpy to and from a suitably aligned instance
of the desired data structure on the stack.

Helps: containers#637
Signed-off-by: Simon McVittie <[email protected]>
smcv added a commit to smcv/bubblewrap that referenced this issue Oct 17, 2024
A char array on the stack is not guaranteed to have any particular
alignment.

Resolves: containers#637
Signed-off-by: Simon McVittie <[email protected]>
smcv added a commit to smcv/bubblewrap that referenced this issue Oct 17, 2024
As documented in cmsg(3), the alignment of control messages is not
guaranteed, so for portability to architectures with strong alignment
requirements we should memcpy to and from a suitably aligned instance
of the desired data structure on the stack.

Helps: containers#637
Signed-off-by: Simon McVittie <[email protected]>
smcv added a commit to smcv/bubblewrap that referenced this issue Oct 17, 2024
A char array on the stack is not guaranteed to have any particular
alignment.

Resolves: containers#637
Signed-off-by: Simon McVittie <[email protected]>
@smcv smcv closed this as completed in #662 Oct 18, 2024
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

Successfully merging a pull request may close this issue.

1 participant