Project

General

Profile

Bug #11458

SMB ioctl issues still incomplete

Added by Jason King 3 months ago. Updated 3 months ago.

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

100%

Estimated time:
Difficulty:
Medium
Tags:

Description

While #8935 fixed a most of the SMB ioctl issues, it still allowed an SMB ioctl request size to change during an ioctl. We should explicitly disallow this.

History

#1

Updated by Joshua M. Clulow 3 months ago

Testing Notes (from Jason)

The testing was done with the attached patch applied to SmartOS (and running the resulting image) — ignoring the attached patch, the code in smb_init.c is identical between illumos-joyent and illumos-gate.

To test, I first ran the following D script:

dtrace -n 'fbt::smb_drv_ioctl:return { trace(arg1) }'

I then restarted the smb/server instance to verify smb_drv_ioctl was being called:

dtrace: description 'fbt::smb_drv_ioctl:return ' matched 1 probe
CPU     ID                    FUNCTION:NAME
  1   3612             smb_drv_ioctl:return                 0
  9   3612             smb_drv_ioctl:return                 0
  9   3612             smb_drv_ioctl:return                 0
  9   3612             smb_drv_ioctl:return                 0
 10   3612             smb_drv_ioctl:return                 0
 10   3612             smb_drv_ioctl:return                 0
 10   3612             smb_drv_ioctl:return                 0
 11   3612             smb_drv_ioctl:return                 0

I then ran the following script to force a change in the length field in request's smb_ioc_header_t->len field:

#!/usr/sbin/dtrace -s

#pragma D option destructive

fbt::smb_drv_ioctl:entry
{
    self->trace = 1;
}

fbt::smb_drv_ioctl:return
/self->trace/
{
    self->trace = 0;
    self->ok = 0;
    trace(arg1);
}

fbt::smb_crc_gen:entry
/self->trace/
{
    printf("hi");
    self->ok = 1
}

fbt::ddi_copyin:entry
/self->trace && self->ok/
{
    this->ioc = (smb_ioc_header_t *)copyin(arg0, sizeof (smb_ioc_header_t));
    this->ioc->len++;
    copyout(this->ioc, arg0, sizeof (smb_ioc_header_t));
}

And disabled, then re-enabled the smb/server instance and observed the results:

dtrace: allowing destructive actions
CPU     ID                    FUNCTION:NAME
  9   5653                smb_crc_gen:entry hi
  9   3612             smb_drv_ioctl:return                22

As EINVAL is 22, this returned as expected.

#2

Updated by Electric Monk 3 months ago

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

git commit e83c55625a2d838a6e4fe2413b8eb464ec8af2a3

commit  e83c55625a2d838a6e4fe2413b8eb464ec8af2a3
Author: Jason King <jason.king@joyent.com>
Date:   2019-07-11T23:25:59.000Z

    11458 SMB ioctl issues still incomplete
    Reviewed by: Dan McDonald <danmcd@joyent.com>
    Approved by: Richard Lowe <richlowe@richlowe.net>

Also available in: Atom PDF