Project

General

Profile

Bug #8935

SMB ioctl fixes incomplete

Added by Dan McDonald almost 3 years ago. Updated almost 3 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
cifs - CIFS server and client
Start date:
2017-12-22
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage
Gerrit CR:

Description

The recent fixes to the SMB ioctl interface seem to have resolved the issue of the copyin() that is directly part of smb_drv_ioctl().

However, this is not the only length field prefixing variable-length data in the interface. For example, smb_svcenum_t includes a member se_buflen that contains the length of the buffer for enumeration data. It also includes se_nlimit and friends which are used to control how much to write out. These are eventually used in smb_user_enum_private(), at the bottom of a long stack of calls.

smb_svcenum_t is not the only one of the union members that's variable-length, either! And any one of them that's unchecked will lead us to write over the end of the buffer. To truly fix this bug, all of these need to be carefully inspected and length checking against the original allocation length needs to be added at each stage. We cannot trust counts from userland like se_nlimit unless they are checked!

#1

Updated by Electric Monk almost 3 years ago

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

git commit 1d443a93389b4537c420db30646aa0fc4c56273c

commit  1d443a93389b4537c420db30646aa0fc4c56273c
Author: Dan McDonald <danmcd@joyent.com>
Date:   2017-12-22T21:21:49.000Z

    8935 SMB ioctl fixes incomplete
    Reviewed by: Alex Wilson <alex.wilson@joyent.com>
    Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
    Reviewed by: Rui Loura <rui.loura@joyent.com>
    Reviewed by: Garrett D'Amore <garrett@damore.org>
    Reviewed by: Dominik Hassler <hasslerd@gmx.li>
    Approved by: Garrett D'Amore <garrett@damore.org>

Also available in: Atom PDF