• Daniel Borkmann's avatar
    net, scm: fix PaX detected msg_controllen overflow in scm_detach_fds · 6900317f
    Daniel Borkmann authored
    David and HacKurx reported a following/similar size overflow triggered
    in a grsecurity kernel, thanks to PaX's gcc size overflow plugin:
    
    (Already fixed in later grsecurity versions by Brad and PaX Team.)
    
    [ 1002.296137] PAX: size overflow detected in function scm_detach_fds net/core/scm.c:314
                   cicus.202_127 min, count: 4, decl: msg_controllen; num: 0; context: msghdr;
    [ 1002.296145] CPU: 0 PID: 3685 Comm: scm_rights_recv Not tainted 4.2.3-grsec+ #7
    [ 1002.296149] Hardware name: Apple Inc. MacBookAir5,1/Mac-66F35F19FE2A0D05, [...]
    [ 1002.296153]  ffffffff81c27366 0000000000000000 ffffffff81c27375 ffffc90007843aa8
    [ 1002.296162]  ffffffff818129ba 0000000000000000 ffffffff81c27366 ffffc90007843ad8
    [ 1002.296169]  ffffffff8121f838 fffffffffffffffc fffffffffffffffc ffffc90007843e60
    [ 1002.296176] Call Trace:
    [ 1002.296190]  [<ffffffff818129ba>] dump_stack+0x45/0x57
    [ 1002.296200]  [<ffffffff8121f838>] report_size_overflow+0x38/0x60
    [ 1002.296209]  [<ffffffff816a979e>] scm_detach_fds+0x2ce/0x300
    [ 1002.296220]  [<ffffffff81791899>] unix_stream_read_generic+0x609/0x930
    [ 1002.296228]  [<ffffffff81791c9f>] unix_stream_recvmsg+0x4f/0x60
    [ 1002.296236]  [<ffffffff8178dc00>] ? unix_set_peek_off+0x50/0x50
    [ 1002.296243]  [<ffffffff8168fac7>] sock_recvmsg+0x47/0x60
    [ 1002.296248]  [<ffffffff81691522>] ___sys_recvmsg+0xe2/0x1e0
    [ 1002.296257]  [<ffffffff81693496>] __sys_recvmsg+0x46/0x80
    [ 1002.296263]  [<ffffffff816934fc>] SyS_recvmsg+0x2c/0x40
    [ 1002.296271]  [<ffffffff8181a3ab>] entry_SYSCALL_64_fastpath+0x12/0x85
    
    Further investigation showed that this can happen when an *odd* number of
    fds are being passed over AF_UNIX sockets.
    
    In these cases CMSG_LEN(i * sizeof(int)) and CMSG_SPACE(i * sizeof(int)),
    where i is the number of successfully passed fds, differ by 4 bytes due
    to the extra CMSG_ALIGN() padding in CMSG_SPACE() to an 8 byte boundary
    on 64 bit. The padding is used to align subsequent cmsg headers in the
    control buffer.
    
    When the control buffer passed in from the receiver side *lacks* these 4
    bytes (e.g. due to buggy/wrong API usage), then msg->msg_controllen will
    overflow in scm_detach_fds():
    
      int cmlen = CMSG_LEN(i * sizeof(int));  <--- cmlen w/o tail-padding
      err = put_user(SOL_SOCKET, &cm->cmsg_level);
      if (!err)
        err = put_user(SCM_RIGHTS, &cm->cmsg_type);
      if (!err)
        err = put_user(cmlen, &cm->cmsg_len);
      if (!err) {
        cmlen = CMSG_SPACE(i * sizeof(int));  <--- cmlen w/ 4 byte extra tail-padding
        msg->msg_control += cmlen;
        msg->msg_controllen -= cmlen;         <--- iff no tail-padding space here ...
      }                                            ... wrap-around
    
    F.e. it will wrap to a length of 18446744073709551612 bytes in case the
    receiver passed in msg->msg_controllen of 20 bytes, and the sender
    properly transferred 1 fd to the receiver, so that its CMSG_LEN results
    in 20 bytes and CMSG_SPACE in 24 bytes.
    
    In case of MSG_CMSG_COMPAT (scm_detach_fds_compat()), I haven't seen an
    issue in my tests as alignment seems always on 4 byte boundary. Same
    should be in case of native 32 bit, where we end up with 4 byte boundaries
    as well.
    
    In practice, passing msg->msg_controllen of 20 to recvmsg() while receiving
    a single fd would mean that on successful return, msg->msg_controllen is
    being set by the kernel to 24 bytes instead, thus more than the input
    buffer advertised. It could f.e. become an issue if such application later
    on zeroes or copies the control buffer based on the returned msg->msg_controllen
    elsewhere.
    
    Maximum number of fds we can send is a hard upper limit SCM_MAX_FD (253).
    
    Going over the code, it seems like msg->msg_controllen is not being read
    after scm_detach_fds() in scm_recv() anymore by the kernel, good!
    
    Relevant recvmsg() handler are unix_dgram_recvmsg() (unix_seqpacket_recvmsg())
    and unix_stream_recvmsg(). Both return back to their recvmsg() caller,
    and ___sys_recvmsg() places the updated length, that is, new msg_control -
    old msg_control pointer into msg->msg_controllen (hence the 24 bytes seen
    in the example).
    
    Long time ago, Wei Yongjun fixed something related in commit 1ac70e7a
    ("[NET]: Fix function put_cmsg() which may cause usr application memory
    overflow").
    
    RFC3542, section 20.2. says:
    
      The fields shown as "XX" are possible padding, between the cmsghdr
      structure and the data, and between the data and the next cmsghdr
      structure, if required by the implementation. While sending an
      application may or may not include padding at the end of last
      ancillary data in msg_controllen and implementations must accept both
      as valid. On receiving a portable application must provide space for
      padding at the end of the last ancillary data as implementations may
      copy out the padding at the end of the control message buffer and
      include it in the received msg_controllen. When recvmsg() is called
      if msg_controllen is too small for all the ancillary data items
      including any trailing padding after the last item an implementation
      may set MSG_CTRUNC.
    
    Since we didn't place MSG_CTRUNC for already quite a long time, just do
    the same as in 1ac70e7a to avoid an overflow.
    
    Btw, even man-page author got this wrong :/ See db939c9b26e9 ("cmsg.3: Fix
    error in SCM_RIGHTS code sample"). Some people must have copied this (?),
    thus it got triggered in the wild (reported several times during boot by
    David and HacKurx).
    
    No Fixes tag this time as pre 2002 (that is, pre history tree).
    Reported-by: default avatarDavid Sterba <dave@jikos.cz>
    Reported-by: default avatarHacKurx <hackurx@gmail.com>
    Cc: PaX Team <pageexec@freemail.hu>
    Cc: Emese Revfy <re.emese@gmail.com>
    Cc: Brad Spengler <spender@grsecurity.net>
    Cc: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
    Cc: Eric Dumazet <edumazet@google.com>
    Reviewed-by: default avatarHannes Frederic Sowa <hannes@stressinduktion.org>
    Signed-off-by: default avatarDaniel Borkmann <daniel@iogearbox.net>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    6900317f
scm.c 7.67 KB