Project

General

Profile

Bug #12977

smb3 server encryption leak in smb2_send_reply

Added by Spencer Berger 17 days ago. Updated 4 days ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
-
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

When smb encryption is enabled (encrypt=required), copying a large file from a share caused the transfer and machine to eventually hang. I broke into mdb and did a $<systemdump


> ::findleaks
findleaks: thread fffffe0bdd329be0's stack swapped out; false positives possible
findleaks: thread fffffe0bdcc8d800's stack swapped out; false positives possible
findleaks: thread fffffe0becf3a080's stack swapped out; false positives possible
findleaks: thread fffffe0bd5de1800's stack swapped out; false positives possible
findleaks: thread fffffe0bebd6b800's stack swapped out; false positives possible
findleaks: thread fffffe0bd525f800's stack swapped out; false positives possible
findleaks: thread fffffe0bed7a9400's stack swapped out; false positives possible
findleaks: thread fffffe0bdb7b6840's stack swapped out; false positives possible
findleaks: thread fffffe0bdcbe4460's stack swapped out; false positives possible
findleaks: thread fffffe0bdd179020's stack swapped out; false positives possible
findleaks: thread fffffe0bed37e3e0's stack swapped out; false positives possible
CACHE             LEAKED           BUFCTL CALLER
fffffe0bb842a008       1 fffffe0bcfcd82d8 AcpiOsAllocate+0x1c
fffffe0bb843b008       1 fffffe0beede51b0 allocb+0x50
fffffe0bb843a688       1 fffffe0becadf9b0 dblk_constructor+0x46
fffffe0bb842c348       5 fffffe0bf5568080 smb2_send_reply+0x7d
fffffe0bb842d348       3 fffffe0bee3216f0 smb2_send_reply+0x7d
fffffe0bb842c688       4 fffffe0c01a6d108 smb2_send_reply+0x7d
fffffe0bb842a9c8      11 fffffe0bf65ab670 smb2_send_reply+0x7d
fffffe0bb842d9c8       1 fffffe0bf5528c40 smb2_send_reply+0x7d
fffffe0bb842d008       9 fffffe0c02029dd8 smb2_send_reply+0x7d
fffffe0bb842d688       2 fffffe0bfcfd7b68 smb2_send_reply+0x7d
fffffe0bb842a9c8       1 fffffe0bf3934de0 smb2_send_reply+0x7d
fffffe0bb842d008       3 fffffe0c02029eb0 smb2_send_reply+0x7d
fffffe0bb842f348       3 fffffe0bf39530f0 smb2_send_reply+0x7d
fffffe0bb842c9c8       4 fffffe0bdc0f6970 smb2_send_reply+0x7d
fffffe0bb842c008       8 fffffe0bececd1f8 smb2_send_reply+0x7d
fffffe0bb842a9c8       4 fffffe0bfa018e00 smb2_send_reply+0x7d
fffffe0bb8434688       1 fffffe0c025e0618 smb2_send_reply+0x7d
fffffe0bb8437688   79821 fffffe0c05fffec8 smb2_send_reply+0x7d
fffffe0bb842c348       7 fffffe0c0201fd88 smb2_send_reply+0x7d
fffffe0bb8435008       1 fffffe0c0252e7c0 smb2_send_reply+0x7d
fffffe0bf78f3348       2 fffffe0c0c8a36c8 smb_mbuf_alloc+0x16
fffffe0bb842d348       1 fffffe0bdb110990 vn_make_ops+0x42
fffffe0bb842d348       1 fffffe0bdd67a978 vn_make_ops+0x42
------------------------------------------------------------------------
           Total   79895 buffers, 5885097440 bytes
> fffffe0c05fffec8$<bufctl_audit
            ADDR          BUFADDR        TIMESTAMP           THREAD
                            CACHE          LASTLOG         CONTENTS
fffffe0c05fffec8 fffffe0c06140000       2ffcd56c76 fffffe0010dccc20
                 fffffe0bb8437688 fffffe0bb9f05800                0
                 kmem_slab_alloc_impl+0x198
                 kmem_slab_alloc+0x62
                 kmem_cache_alloc+0x15b
                 kmem_alloc+0x4b
                 smb2_send_reply+0x7d
                 smb2sr_work+0x47f
                 smb2_tq_work+0x73
                 taskq_d_thread+0xbc
                 thread_start+0xb

The function (https://github.com/illumos/illumos-gate/blob/6937e379563aa5fe8a003acdcd316b89044dd8dd/usr/src/uts/common/fs/smbsrv/smb2_dispatch.c#L1470) initially didn't look like it leaked tmpbuf because smb_session_send(smb_session_t *session, uint8_t nbt_type, mbuf_chain_t *mbc) unconditionally frees mbc. But this entails calling mbc->chain->m_ext.ext_ref, and MBC_ATTACH_BUF(&enc_reply, tmpbuf, buflen) sets this to mclrefnoop, which does what its name says.

I tested a quick patch (attached) and copied the file successfully. Not sure if this is the nicest way to patch, you could use smb_mbuf_kmem_ref or something.


Files

History

#1

Updated by Gordon Ross 17 days ago

  • Status changed from New to In Progress
  • Assignee set to Matt Barden
#2

Updated by Gordon Ross 4 days ago

Spencer's patch looks OK to me. We have work underway (for large SMB messages) that will change how this works, so I wouldn't worry too much right now about trying to get fancy using an mbuf cleanup function.

#3

Updated by Electric Monk 4 days ago

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

git commit 2c834067bf731e663c6269489bf5dc0a9b4c6299

commit  2c834067bf731e663c6269489bf5dc0a9b4c6299
Author: Spencer Berger <bergerspencer@gmail.com>
Date:   2020-08-05T15:32:49.000Z

    12977 smb3 server encryption leak in smb2_send_reply
    Reviewed by: Gordon Ross <gordon.w.ross@gmail.com>
    Reviewed by: Dan McDonald <danmcd@joyent.com>
    Approved by: Robert Mustacchi <rm@fingolfin.org>

Also available in: Atom PDF