Messages in this thread | | | From | David Howells <> | Subject | Re: Is ->sendmsg() allowed to change the msghdr struct it is given? | Date | Tue, 27 Jun 2023 14:09:21 +0100 |
| |
Paolo Abeni <pabeni@redhat.com> wrote:
> udp_sendmsg() can set the MSG_TRUNC bit in msg->msg_flags, so I guess > that kind of actions are sort of allowed. Still, AFAICS, the kernel > based msghdr is not copied back to the user-space, so such change > should be almost a no-op in practice. > > @David: which would be the end goal for such action?
Various places in the kernel use sock_sendmsg() - and I've added a bunch more with the MSG_SPLICE_PAGES patches. For some of the things I've added, there's a loop which used to call ->sendpage() and now calls sock_sendmsg(). In most of those places, msghdr will get reset each time round the loop - but not in all cases.
Of particular immediate interest is net/ceph/messenger_v2.c. If you go to:
https://lore.kernel.org/r/3111635.1687813501@warthog.procyon.org.uk/
and look at the resultant code:
static int do_sendmsg(struct socket *sock, struct iov_iter *it) { struct msghdr msg = { .msg_flags = CEPH_MSG_FLAGS }; int ret;
msg.msg_iter = *it; while (iov_iter_count(it)) { ret = sock_sendmsg(sock, &msg); if (ret <= 0) { if (ret == -EAGAIN) ret = 0; return ret; }
iov_iter_advance(it, ret); }
WARN_ON(msg_data_left(&msg)); return 1; }
for example. It could/would malfunction if sendmsg() is allowed to modify msghdr - or if it doesn't update msg_iter. Likewise:
static int do_try_sendpage(struct socket *sock, struct iov_iter *it) { struct msghdr msg = { .msg_flags = CEPH_MSG_FLAGS }; struct bio_vec bv; int ret;
if (WARN_ON(!iov_iter_is_bvec(it))) return -EINVAL;
while (iov_iter_count(it)) { /* iov_iter_iovec() for ITER_BVEC */ bvec_set_page(&bv, it->bvec->bv_page, min(iov_iter_count(it), it->bvec->bv_len - it->iov_offset), it->bvec->bv_offset + it->iov_offset);
/* * MSG_SPLICE_PAGES cannot properly handle pages with * page_count == 0, we need to fall back to sendmsg if * that's the case. * * Same goes for slab pages: skb_can_coalesce() allows * coalescing neighboring slab objects into a single frag * which triggers one of hardened usercopy checks. */ if (sendpage_ok(bv.bv_page)) msg.msg_flags |= MSG_SPLICE_PAGES; else msg.msg_flags &= ~MSG_SPLICE_PAGES;
iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, &bv, 1, bv.bv_len); ret = sock_sendmsg(sock, &msg); if (ret <= 0) { if (ret == -EAGAIN) ret = 0; return ret; }
iov_iter_advance(it, ret); }
return 1; }
could be similarly affected if ->sendmsg() mucks about with msg_flags.
David
| |