Project

General

Profile

Actions
Copy link

Bug #12824

closed

recvmsg(): adjust final cmsg->cmsg_len upon MSG_CTRUNC

Added by Andy Fiddaman almost 3 years ago. Updated almost 3 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Andy Fiddaman
Category:
kernel
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:
718
External Bug:

Description

When an application calls revcmsg(3SOCKET) to retrieve some ancillary data, but provides a buffer that is too small to hold the entire result, the returned message is truncated and the MSG_CTRUNC flag is set in the reply. However, the header in the truncated message still reflects the size of the original (non-truncated) data; this is different behaviour to other Operating Systems (Linux in particular).

Most of the time this is not a problem as on receipt of a truncated message an application will just discard it. If, however, the message is a list of file descriptors (an SCM_RIGHTS message) then, in order to avoid leaking file descriptors and a potential DoS, the application must take control of the file descriptors which did fit in the available buffer space - usually to close them.

The illumos behaviour seems to be allowed by POSIX (which is pretty vague in the whole area of truncation) but is not what is generally expected by software. A couple of examples where a buffer overrun would occur since the application relies on the cmsg_len value in the header:

We should change the illumos behaviour here for least surprise and better compatibility with third party software.

There has some discussion on the developers' email list:

https://illumos.topicbox.com/groups/developer/T1ecc601b7df24f6b/questions-about-recvmsg-and-msgctrunc-behaviour

and I opened a bug against dbus when I first encountered this:

https://gitlab.freedesktop.org/dbus/dbus/-/issues/304

There's an amount of incorrect conjecture in there, but it does reinforce the rationale for this change.

Actions
Copy link
 #1

Updated by Andy Fiddaman almost 3 years ago

  • Subject changed from recvmsg(): adjust final cmsg->cmsg_size upon MSG_CTRUNC to recvmsg(): adjust final cmsg->cmsg_len upon MSG_CTRUNC
Actions
Copy link
 #2

Updated by Andy Fiddaman almost 3 years ago

I submitted a merge request to the upstream dbus project at

https://gitlab.freedesktop.org/dbus/dbus/-/merge_requests/165

This adds a workaround for this behaviour for illumos kernels without this fix.

Actions
Copy link
 #3

Updated by Andy Fiddaman almost 3 years ago

Testing notes:

The new tests pass, both 32- and 64-bit variants, and the unpatched dbus test suite also passes with this change:

bloody2% /opt/os-tests/tests/sockfs/rights.32
     + : PASS send 1, recv 1
     + : PASS send 10, recv 10
     + : PASS send 2, recv 1
     + : PASS send 2, recv 1, buffer 5
     + : PASS send 2, recv 1, buffer 6
     + : PASS send 2, recv 1, buffer 7
     + : PASS send 2, recv 0, hdronly
     + : PASS send 2, recv 0, hdr - 1
     + : PASS send 2, recv 0, hdr - 5
     + : PASS send 1, recv 1, pre 8
     + : PASS send 1, recv 1, pre 7
     + : PASS send 1, recv 1, pre 6
     + : PASS send 1, recv 1, pre 5
     + : PASS send 2, recv 1, pre 8
     + : PASS send 2, recv 1, pre 7
     + : PASS send 2, recv 1, pre 6
     + : PASS send 2, recv 1, pre 5
     + : PASS send 2, recv 1, pre 4
     + : PASS send 2, recv 1, pre 3
     + : PASS send 2, recv 1, pre 2
     + : PASS send 2, recv 1, pre 1
     + : PASS send 2, recv 1, pre 8, buffer 5
     + : PASS send 2, recv 1, pre 8, buffer 6
     + : PASS send 2, recv 1, pre 8, buffer 7
     + : PASS send 10, recv 1, pre 8


bloody2% /opt/os-tests/tests/sockfs/rights.64
     + : PASS send 1, recv 1
     + : PASS send 10, recv 10
     + : PASS send 2, recv 1
     + : PASS send 2, recv 1, buffer 5
     + : PASS send 2, recv 1, buffer 6
     + : PASS send 2, recv 1, buffer 7
     + : PASS send 2, recv 0, hdronly
     + : PASS send 2, recv 0, hdr - 1
     + : PASS send 2, recv 0, hdr - 5
     + : PASS send 1, recv 1, pre 8
     + : PASS send 1, recv 1, pre 7
     + : PASS send 1, recv 1, pre 6
     + : PASS send 1, recv 1, pre 5
     + : PASS send 2, recv 1, pre 8
     + : PASS send 2, recv 1, pre 7
     + : PASS send 2, recv 1, pre 6
     + : PASS send 2, recv 1, pre 5
     + : PASS send 2, recv 1, pre 4
     + : PASS send 2, recv 1, pre 3
     + : PASS send 2, recv 1, pre 2
     + : PASS send 2, recv 1, pre 1
     + : PASS send 2, recv 1, pre 8, buffer 5
     + : PASS send 2, recv 1, pre 8, buffer 6
     + : PASS send 2, recv 1, pre 8, buffer 7
     + : PASS send 10, recv 1, pre 8
Actions
Copy link
 #4

Updated by Electric Monk almost 3 years ago

  • Status changed from In Progress to Closed
  • % Done changed from 0 to 100

git commit d865fc92e4b640c73c2957a20b3d82622c741be5

commit  d865fc92e4b640c73c2957a20b3d82622c741be5
Author: Andy Fiddaman <omnios@citrus-it.co.uk>
Date:   2020-06-19T17:46:18.000Z

    12824 recvmsg(): adjust final cmsg->cmsg_len upon MSG_CTRUNC
    Reviewed by: Robert Mustacchi <rm@fingolfin.org>
    Reviewed by: Igor Kozhukhov <igor@dilos.org>
    Approved by: Dan McDonald <danmcd@joyent.com>

Actions
Copy link

Also available in: Atom PDF