Bug #13501


smbutil view crashes in ndr_clnt_get_frags()

Added by Matt Barden 12 months ago. Updated about 1 month ago.

cifs - CIFS server and client
Start date:
Due date:
% Done:


Estimated time:
Gerrit CR:


In testing #13169 (and re-tested against illumos-gate without that change), I discovered that, when the NDR client attempts to reassemble fragmented responses, it cores due to tripping the stack smash detection in ndr_clnt_get_frags():

Loading modules: [ ]
> $c`syscall+0x13(fef0c3c4, 1c, 58dcef98, 0, 8047a28, fedee000)
0xfee8d48f(8047ab4)`__stack_chk_fail_local+0x17(fedd6ab1, 8047910, 10000, 806c498, 8047a5c, 200f000)`ndr_clnt_get_frags+0x79(806c428, 8047ab0)`ndr_clnt_call+0x2dd(806c454, f, 8047c7c)`ndr_rpc_call+0x38(8047ce4, f, 8047c7c)
srvsvc_net_share_enum+0x98(8047ce4, 8069668, 1, 8047ce0, fedca000, 806c428)
share_enum_rpc+0xa4(8069c50, 8069668, 3, fedca000, 0, 8067420)
cmd_view+0x191(2, 8047e04, 8047d98, 80533c1)
main+0xc1(8047d9c, fef6d628, 8047dd8, 8053017, 3, 8047e00)
_start_crt+0x96(3, 8047e00, fefd06ee, 0, 0, 0)
_start+0x1a(3, 8047ed0, 8047ed8, 8047edd, 0, 8047ef5)

That function has a local variable, ndr_common_header_t hdr, which gets passed to ndr_clnt_get_frag, which passes it to ndr_decode_frag_hdr. That function then copies 24 bytes of data (the size of an NDR response header) from the message directly into the structure. However, ndr_common_header_t is only 16-bytes long, and so it writes 8 bytes too many to the stack.

After the recent userland stack protector changes, this causes the process to crash in the stack check function. Prior to this change, however, this seemingly has no effect; the extra 8 bytes are written to space on the stack used to align the stack to 16 bytes.

Only the parts of the header in the common header get used for fragment reassembly (in both the client and server), and so the solution is to only copy the common part of the header.

Issue can be reproduced by using 'smbutil view' against a server with a very large number of shares.

Actions #1

Updated by Electric Monk 12 months ago

  • Gerrit CR set to 1216
Actions #2

Updated by Gordon Ross 11 months ago

  • Category changed from lib - userland libraries to cifs - CIFS server and client
Actions #3

Updated by Matt Barden 11 months ago

  • Description updated (diff)
Actions #4

Updated by Matt Barden 11 months ago

Testing was done as follows:

1. Configure a server with a very large number of shares (the number of which can be reduced through use of share comments) - enough that the next step spans multiple RPC messages when viewed in wireshark.
2. smbutil view //user@server

If the user is a domain user, and the server is localhost (aka run smbutil from the configured server), this also tests that NTLM pass-thru authentication continues to work.

Actions #5

Updated by Electric Monk 11 months ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit 4d00d81bf82141f996e032f9c53e0e996a5f7204

commit  4d00d81bf82141f996e032f9c53e0e996a5f7204
Author: Matt Barden <>
Date:   2021-02-27T16:04:49.000Z

    13501 smbutil view crashes in ndr_clnt_get_frags()
    Reviewed by: Evan Layton <>
    Reviewed by: Gordon Ross <>
    Approved by: Robert Mustacchi <>

Actions #6

Updated by Gordon Ross about 1 month ago

  • Description updated (diff)

Also available in: Atom PDF