[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#933013: linux-source: kernel panic using CIFS share in smb2_push_mandatory_locks()



Package: linux-source
Severity: normal

[Impact]

* We got reports of a kernel crash in cifs module with the following
* signature:

BUG: unable to handle kernel NULL pointer dereference at
0000000000000038
IP: smb2_push_mandatory_locks+0x10e/0x3b0 [cifs]
PGD 0 P4D 0
RIP: 0010:smb2_push_mandatory_locks+0x10e/0x3b0 [cifs]
Call Trace:
    cifs_oplock_break+0x12f/0x3d0 [cifs]
    process_one_work+0x14d/0x410
    worker_thread+0x4b/0x460
    kthread+0x105/0x140
    [...]

    * Low-level analysis (decodecode script output and the objdump of
    * the function) revealed that we are crashing in a NULL ptr
    * dereference when trying to access "cfile->tlink"; below, a snippet
    * of the objdump at function smb2_push_mandatory_locks():
   
    [...]
    mov 0x10(%r14),%r15 # %r15 = cifsFileInfo *cfile
    mov 0x18(%r14),%rbx # %rbx = cifsLockInfo *li = (fdlocks->locks)
    lea 0x18(%r14),%r12
    mov 0x90(%r15),%rax # %rax = struct tcon_link *tlink (cfile->tlink)
    cmp %r12,%rbx
    mov 0x38(%rax),%rax # <--- TRAP [trying to get cifs_tcon *tl_tcon]
    [...]

    * After discussing the issue with CIFS maintainers (Steve French and
    * Pavel Shilovsky) they suggested commit b98749cac4a6 ("CIFS: keep
    * FileInfo handle live during oplock break")
    * [http://git.kernel.org/linus/b98749cac4a6] as a fix for multiple
    * reports of this kind of crash.
    * The fix was sent to stable kernels and is present in Ubuntu
    * kernels 5.0 and newer. We are requesting the SRU for this patch
    * here in order to fix the crashes, after reports of successful
    * testing with the patch (see below section) and since the patch is
    * restricted to the cifs module scope and accepted on linux stable.
    * Alternatively the issue is known to be avoided when oplocks are
    * disabled using "cifs.enable_oplocks=N" module parameter.
    
    [Test case]

    * Unfortunately we cannot reproduce the issue. The patch proposed
    * here was
    validated by us with xfstests (instructions followed from
    https://wiki.samba.org/index.php/Xfstesting-cifs) and fio. Also, we
    have a user report of test validation using LISA
    (https://github.com/LIS/LISAv2).
    * Using xfstest with the exclusions proposed in the link above we
    * managed to get the same results as a non-patched kernel, i.e., the
    * same tests failed in both kernels, we didn't get worse results
    * with the patch. Fio also didn't show noticeable performance
    * regression with the patch.

    [Regression potential]

    * The patch was validated by the cifs filesystem maintainers (in
    * fact they suggested its inclusion in Ubuntu) and by the
    * aforementioned tests; also, the scope is restricted to cifs only
    * so the likelihood of regressions is considered low.
    * Due to the nature of the code modification (add a new reference of
    * a file handler and manipulate it in different places), I consider
    * that if we have a regression it'll manifest as deadlock/blocked
    * tasks, not something more serious like crashes or data corruption.


-- System Information:
Debian Release: buster/sid
  APT prefers bionic-updates
  APT policy: (500, 'bionic-updates'), (500, 'bionic-security'), (500, 'bionic'), (100, 'bionic-backports')
Architecture: amd64 (x86_64)

Kernel: Linux 4.4.0-17763-Microsoft
Locale: LANG=C.UTF-8, LC_CTYPE=C.UTF-8 (charmap=UTF-8), LANGUAGE=C.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: unable to detect

Versions of packages linux-source depends on:
pn  linux-source-4.15.0  <none>

linux-source recommends no packages.

linux-source suggests no packages.


Reply to: