Project

General

Profile

Actions

Bug #13506

closed

smbfs panic on failed open for append

Added by Gordon Ross 10 months ago. Updated 10 months ago.

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

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

In an smbfs mount, attempting to open for append on a file for which you lack sufficient access results in a crash dump.

How to reproduce:

On server side:

touch x444 x644
chmod 444 x444
chmod 644 x644

On the client side (in the smbfs mount)

$ echo foo >> x644
$ echo foo >> x444
(no response, client has panic'ed)

That 2nd open for append should have returned:

-bash: x444: Permission denied

The crash dump looks like this:

root@oi-dev:# mdb -k 0
Loading modules: [ unix genunix specfs dtrace mac cpu.generic uppc apix scsi_vhci zfs mpt sd ip hook neti sockfs arp usba uhci mm fctl stmf stmf_sbd lofs idm random cpc crypto fcip ufs nsmb ptm nfs sppp ipc smbfs ]
> ::status
debugging crash dump vmcore.0 (64-bit) from oi-dev
operating system: 5.11 illumos-7bbcfb4168 (i86pc)
build version: heads/master-0-g7bbcfb4168-dirty

image uuid: e7a797f8-bcc1-cb90-ee28-e17bb8c46a2e
panic message: BAD TRAP: type=e (#pf Page fault) rp=ffffff00084b5640 addr=18 occurred in module "nsmb" due to a NULL pointer dereference
dump content: kernel pages only
> $C
ffffff00084b57b0 smb_rwuio+0x26(0, 1, ffffff00084b5810, ffffff00084b57f8, 3c)
ffffff00084b5890 smbfs_bio+0x264(ffffff02658ac500, 1, ffffff024cbc0680)
ffffff00084b5900 smbfs_rdwrlbn+0x104(ffffff025c358d80, ffffff0004d9d5d8, 0, 1000, 100, ffffff024cbc0680)
ffffff00084b59c0 smbfs_putapage+0x1cd(ffffff025c358d80, ffffff0004d9d5d8, 0, 0, 0, ffffff024cbc0680)
ffffff00084b5a70 pvn_vplist_dirty+0x2fd(ffffff025c358d80, 0, fffffffff8013e80, 0, ffffff024cbc0680)
ffffff00084b5b20 smbfs_putpage+0x2be(ffffff025c358d80, 0, 0, 0, ffffff024cbc0680, 0)
ffffff00084b5bb0 smbfs_close+0x169(ffffff025c358d80, 210a, 1, 6, ffffff024cbc0680, 0)
ffffff00084b5c30 fop_close+0x60(ffffff025c358d80, 210a, 1, 6, ffffff024cbc0680, 0)
ffffff00084b5c70 closef+0x5e(ffffff0267a394b8)
ffffff00084b5ce0 closeandsetf+0x1d9(1, ffffff0267a39378)
ffffff00084b5f00 fcntl+0x734(a, 9, 1)
ffffff00084b5f10 sys_syscall+0x177()

Actions #1

Updated by Gordon Ross 10 months ago

Core dump analysis:

root@oi-dev:# mdb -k 0
Loading modules: [ unix genunix specfs dtrace mac cpu.generic uppc apix scsi_vhci zfs mpt sd ip hook neti sockfs arp usba uhci mm fctl stmf stmf_sbd lofs idm random cpc crypto fcip ufs nsmb ptm nfs sppp ipc smbfs ]
> ::status
debugging crash dump vmcore.0 (64-bit) from oi-dev
operating system: 5.11 illumos-7bbcfb4168 (i86pc)
build version: heads/master-0-g7bbcfb4168-dirty

image uuid: e7a797f8-bcc1-cb90-ee28-e17bb8c46a2e
panic message: BAD TRAP: type=e (#pf Page fault) rp=ffffff00084b5640 addr=18 occurred in module "nsmb" due to a NULL pointer dereference
dump content: kernel pages only
> $C
ffffff00084b57b0 smb_rwuio+0x26(0, 1, ffffff00084b5810, ffffff00084b57f8, 3c)
ffffff00084b5890 smbfs_bio+0x264(ffffff02658ac500, 1, ffffff024cbc0680)
ffffff00084b5900 smbfs_rdwrlbn+0x104(ffffff025c358d80, ffffff0004d9d5d8, 0, 1000, 100, ffffff024cbc0680)
ffffff00084b59c0 smbfs_putapage+0x1cd(ffffff025c358d80, ffffff0004d9d5d8, 0, 0, 0, ffffff024cbc0680)
ffffff00084b5a70 pvn_vplist_dirty+0x2fd(ffffff025c358d80, 0, fffffffff8013e80, 0, ffffff024cbc0680)
ffffff00084b5b20 smbfs_putpage+0x2be(ffffff025c358d80, 0, 0, 0, ffffff024cbc0680, 0)
ffffff00084b5bb0 smbfs_close+0x169(ffffff025c358d80, 210a, 1, 6, ffffff024cbc0680, 0)
ffffff00084b5c30 fop_close+0x60(ffffff025c358d80, 210a, 1, 6, ffffff024cbc0680, 0)
ffffff00084b5c70 closef+0x5e(ffffff0267a394b8)
ffffff00084b5ce0 closeandsetf+0x1d9(1, ffffff0267a39378)
ffffff00084b5f00 fcntl+0x734(a, 9, 1)
ffffff00084b5f10 sys_syscall+0x177()
> ffffff02658ac500 ::print "struct buf" 
{
    b_flags = 0x84111
    b_forw = 0
    b_back = 0
    av_forw = 0
    av_back = 0
    b_dev = 0
    b_bcount = 0x1000
    b_un = {
        b_addr = 0xfffffe00a1cf7000
        b_fs = 0xfffffe00a1cf7000
        b_cg = 0xfffffe00a1cf7000
        b_dino = 0xfffffe00a1cf7000
        b_daddr = 0xfffffe00a1cf7000
    }
    _b_blkno = {
        _f = 0
        _p = {
            _l = 0
            _u = 0
        }
    }
    b_obs1 = '\0'
    b_resid = 0                       
    b_start = 0
    b_proc = 0
    b_pages = 0xffffff0004d9d5d8
    b_obs2 = 0
    b_bufsize = 0x1000
    b_iodone = 0
    b_vp = 0xffffff025c358d80     <<<  follow this next
    b_chain = 0
    b_obs3 = 0
    b_error = 0
    b_private = 0
    b_edev = 0
    b_sem = {
        _opaque = [ 0, 0 ]
    }
    b_io = {
        _opaque = [ 0, 0 ]
    }
    b_list = 0
    b_shadow = 0
    b_dip = 0
    b_file = 0xffffff025c358d80
    b_offset = 0                      
}
> 0xffffff025c358d80 ::print vnode_t
{
    v_lock = {
        _opaque = [ 0 ]
    }
    v_flag = 0x400
    v_count = 0x2
    v_data = 0xffffff0268e62a88     <<<  follow this next
    v_vfsp = 0xffffff0266d4ae00
    v_stream = 0
    v_type = 1 (VREG)
    v_rdev = 0xffffffffffffffff
    v_vfsmountedhere = 0
    v_op = 0xffffff025eeac300
    v_pages = 0xffffff025bc5eda0
    v_filocks = 0
    v_shrlocks = 0
    v_nbllock = {
        _opaque = [ 0 ]
    }
    v_cv = {
        _opaque = 0
    }
    v_locality = 0
    v_femhead = 0                     
    v_path = 0xffffff025b9db070 "/g/Temp/x444" 
    v_path_stamp = 0x1bfdfc9123
    v_rdcnt = 0
    v_wrcnt = 0x1
    v_mmap_read = 0
    v_mmap_write = 0
    v_mpssdata = 0
    v_fopdata = 0
    v_vsd_lock = {
        _opaque = [ 0 ]
    }
    v_vsd = 0
    v_xattrdir = 0
    v_count_dnlc = 0
}
> 0xffffff0268e62a88 ::print smbnode_t
{
    r__hdr = {
        hdr_avl_node = {
            avl_child = [ 0, 0 ]
            avl_pcb = 0xffffff0268f47d45
        }
        hdr_n_rpath = 0xffffff025b9db080 "\Temp\x444" 
        hdr_n_rplen = 0xa
    }
    n_mount = 0xffffff025ad25c40
    r_vnode = 0xffffff025c358d80
    r_freef = 0
    r_freeb = 0
    r_rwlock = {
        count = 0
        waiters = 0
        owner = 0
        lock = {
            _opaque = [ 0 ]
        }
        cv = {
            _opaque = 0
        }
    }                                 
    r_lkserlock = {
        count = 0x1
        waiters = 0
        owner = 0
        lock = {
            _opaque = [ 0 ]
        }
        cv = {
            _opaque = 0
        }
    }
    r_statelock = {
        _opaque = [ 0 ]
    }
    n_dirrefs = 0
    n_dirseq = 0
    n_dirofs = 0
    n_fidrefs = 0x1
    n_fid = 0                  <<< and here's our problem...
    n_ovtype = 1 (VREG)
    r_cred = 0xffffff024cbc0680
    r_nextr = 0
    r_mapcnt = 0                      
    r_inmap = 0
    r_count = 0
    r_awcount = 0
    r_gcount = 0
    r_flags = 0x100
    n_flag = 0
    r_error = 0
    r_cv = {
        _opaque = 0
    }
    [...]
}
> 

When we're in the paging support functions, we should have already ensured that we have an open file.
After adding some sanity checks about where (smbnode_t *)->n_fid should be filled in, it turns out that this is a fairly simple failiure to propagate an error return at the end of smbfs_smb_open.

Actions #2

Updated by Electric Monk 10 months ago

  • Gerrit CR set to 1221
Actions #3

Updated by Gordon Ross 10 months ago

Someone asked about the history. This has been there since:
#9735 Need to provide SMB 2.1 Client

Actions #4

Updated by Gordon Ross 10 months ago

  • Status changed from In Progress to Pending RTI
Actions #5

Updated by Gordon Ross 10 months ago

Ah yes, testing (as in the description)
Before the fix:

$ mount -F smbfs //gwr@vmhost/gwr /g
$ echo foo >> /g/x444
(panic)

After the fix:
$ mount -F smbfs //gwr@vmhost/gwr /g
$ echo foo >> /g/x444
-bash: /g/x444: Permission denied
$

Actions #6

Updated by Electric Monk 10 months ago

  • Status changed from Pending RTI to Closed
  • % Done changed from 0 to 100

git commit 168091e5da87ff8dbec35e48d0cebe8b5221365d

commit  168091e5da87ff8dbec35e48d0cebe8b5221365d
Author: Gordon Ross <gordon.ross@tintri.com>
Date:   2021-02-11T20:29:40.000Z

    13506 smbfs panic on failed open for append
    Reviewed by: Dan McDonald <danmcd@joyent.com>
    Reviewed by: Yuri Pankov <ypankov@tintri.com>
    Reviewed by: Evan Layton <elayton@tintri.com>
    Approved by: Robert Mustacchi <rm@fingolfin.org>

Actions

Also available in: Atom PDF