kernel panic using CIFS share in smb2_push_mandatory_locks()

Bug #1795659 reported by Stefan Stranz on 2018-10-02
22
This bug affects 4 people
Affects Status Importance Assigned to Milestone
linux (Ubuntu)
High
Guilherme G. Piccoli
Xenial
Undecided
Guilherme G. Piccoli
Bionic
High
Guilherme G. Piccoli
Cosmic
High
Guilherme G. Piccoli
Disco
High
Guilherme G. Piccoli
linux-azure (Ubuntu)
Critical
Unassigned
Xenial
Critical
Unassigned
Bionic
Undecided
Unassigned
Cosmic
Undecided
Unassigned
Disco
Undecided
Unassigned

Bug Description

[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.

Stefan Stranz (sfs1988) wrote :
Joseph Salisbury (jsalisbury) wrote :

Did this issue start happening after an update/upgrade? Was there a prior kernel version where you were not having this particular problem?

Would it be possible for you to test the latest upstream kernel? Refer to https://wiki.ubuntu.com/KernelMainlineBuilds . Please test the latest v4.19 kernel[0].

If this bug is fixed in the mainline kernel, please add the following tag 'kernel-fixed-upstream'.

If the mainline kernel does not fix this bug, please add the tag: 'kernel-bug-exists-upstream'.

Once testing of the upstream kernel is complete, please mark this bug as "Confirmed".

Thanks in advance.

[0] http://kernel.ubuntu.com/~kernel-ppa/mainline/v4.19-rc6

affects: linux-hwe (Ubuntu) → linux (Ubuntu)
Changed in linux (Ubuntu):
importance: Undecided → High
status: New → Triaged
tags: added: kernel-da-key
Changed in linux (Ubuntu):
status: Triaged → Incomplete
Stefan Stranz (sfs1988) wrote :

I'm unable to fully test the upstream bugs as I can't install linux-headers-4.19.0-041900rc6-generic due to libssl1.1 not being available for 4.15. System does boot without the generic package on the new version though.

While i'm looking at upgrading the boxes to 18.04 from the 16.04 install, these are production servers and I can't just swap them over.

I can see the random reboots happening as far back as February, but I don't have a log from those so it could be a different issue then. I know both -34 and -36 kernel builds had the issue.

Launchpad Janitor (janitor) wrote :

[Expired for linux (Ubuntu) because there has been no activity for 60 days.]

Changed in linux (Ubuntu):
status: Incomplete → Expired
Guilherme G. Piccoli (gpiccoli) wrote :

Thanks for reporting that Stefan! There's a Debian bug report (although the kernel
version is different, seems the same issue): https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=830771 .

Also, recently I had a report of this issue happening in Azure instance, kernel version 4.15.0-1041-azure.

Cheers,

Guilherme

Changed in linux (Ubuntu):
assignee: nobody → Guilherme G. Piccoli (gpiccoli)
status: Expired → Confirmed
Changed in linux (Ubuntu Bionic):
status: New → Confirmed
importance: Undecided → High
assignee: nobody → Guilherme G. Piccoli (gpiccoli)
tags: added: sts
Guilherme G. Piccoli (gpiccoli) wrote :

Hi, have some news in this one. I've emailed Microsoft CIFS maintainers, they recommended the following patch as a potential fix:
b98749cac4a6 ("CIFS: keep FileInfo handle live during oplock break").

There's a PPA with a kernel built including this patch, for those interested in testing: https://launchpad.net/~dragan-s/+archive/ubuntu/lp1795659 .

Cheers,

Guilherme

Changed in linux (Ubuntu Cosmic):
status: New → Confirmed
Changed in linux (Ubuntu Disco):
status: New → Confirmed
Changed in linux (Ubuntu Cosmic):
importance: Undecided → High
Changed in linux (Ubuntu Disco):
importance: Undecided → High
Changed in linux (Ubuntu Cosmic):
assignee: nobody → Guilherme G. Piccoli (gpiccoli)
Changed in linux (Ubuntu Disco):
assignee: nobody → Guilherme G. Piccoli (gpiccoli)
Changed in linux (Ubuntu Disco):
status: Confirmed → Fix Released
Changed in linux (Ubuntu Cosmic):
status: Confirmed → Won't Fix
Changed in linux (Ubuntu Bionic):
status: Confirmed → In Progress
summary: - kernel panic using CIFS share smb2_push_mandatory_locks
+ kernel panic using CIFS share in smb2_push_mandatory_locks()
description: updated
description: updated
Guilherme G. Piccoli (gpiccoli) wrote :

SRU request sent to kernel-team mailing list: https://lists.ubuntu.com/archives/kernel-team/2019-July/102354.html

Cheers,

Guilherme

Changed in linux (Ubuntu Bionic):
status: In Progress → Fix Committed
Marcelo Cerri (mhcerri) on 2019-07-23
Changed in linux (Ubuntu Xenial):
status: New → Invalid
Changed in linux-azure (Ubuntu Bionic):
status: New → Invalid
Changed in linux-azure (Ubuntu Cosmic):
status: New → Invalid
Changed in linux-azure (Ubuntu Disco):
status: New → Invalid
Changed in linux-azure (Ubuntu):
importance: Undecided → Critical
status: New → Confirmed
Changed in linux-azure (Ubuntu Xenial):
importance: Undecided → Critical
Marcelo Cerri (mhcerri) on 2019-07-23
Changed in linux-azure (Ubuntu Xenial):
status: New → Fix Committed
Brad Figg (brad-figg) on 2019-07-24
tags: added: cscc

This bug is awaiting verification that the kernel in -proposed solves the problem. Please test the kernel and update this bug with the results. If the problem is solved, change the tag 'verification-needed-bionic' to 'verification-done-bionic'. If the problem still exists, change the tag 'verification-needed-bionic' to 'verification-failed-bionic'.

If verification is not done by 5 working days from today, this fix will be dropped from the source code, and this bug will be closed.

See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you!

tags: added: verification-needed-bionic
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package linux-azure - 4.15.0-1052.57

---------------
linux-azure (4.15.0-1052.57) xenial; urgency=medium

  * xenial/linux-azure: 4.15.0-1052.57 -proposed tracker (LP: #1837632)

  * kernel panic using CIFS share in smb2_push_mandatory_locks() (LP: #1795659)
    - CIFS: keep FileInfo handle live during oplock break

 -- Marcelo Henrique Cerri <email address hidden> Tue, 23 Jul 2019 13:19:53 -0300

Changed in linux-azure (Ubuntu Xenial):
status: Fix Committed → Fix Released
Changed in linux-azure (Ubuntu):
status: Confirmed → Fix Released
Changed in linux (Ubuntu Xenial):
assignee: nobody → Guilherme G. Piccoli (gpiccoli)

I've validated the -proposed kernel for Bionic (4.15.0-56) using the xfstests suite mentioned in the description. Interestingly, the patch seems to fix the test generic/504, which failed both in smb2 and smb3 only in kernel 4.15.0-55. Other than that, the same amount of tests failed in both cases, and no significant performance impact was noticed.

Cheers,

Guilherme

tags: added: verification-done-bionic
removed: verification-needed-bionic

This bug is awaiting verification that the kernel in -proposed solves the problem. Please test the kernel and update this bug with the results. If the problem is solved, change the tag 'verification-needed-xenial' to 'verification-done-xenial'. If the problem still exists, change the tag 'verification-needed-xenial' to 'verification-failed-xenial'.

If verification is not done by 5 working days from today, this fix will be dropped from the source code, and this bug will be closed.

See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you!

tags: added: verification-needed-xenial
Launchpad Janitor (janitor) wrote :
Download full text (171.3 KiB)

This bug was fixed in the package linux - 4.15.0-58.64

---------------
linux (4.15.0-58.64) bionic; urgency=medium

  * unable to handle kernel NULL pointer dereference at 000000000000002c (IP:
    iget5_locked+0x9e/0x1f0) (LP: #1838982)
    - Revert "ovl: set I_CREATING on inode being created"
    - Revert "new primitive: discard_new_inode()"

linux (4.15.0-57.63) bionic; urgency=medium

  * CVE-2019-1125
    - x86/cpufeatures: Carve out CQM features retrieval
    - x86/cpufeatures: Combine word 11 and 12 into a new scattered features word
    - x86/speculation: Prepare entry code for Spectre v1 swapgs mitigations
    - x86/speculation: Enable Spectre v1 swapgs mitigations
    - x86/entry/64: Use JMP instead of JMPQ
    - x86/speculation/swapgs: Exclude ATOMs from speculation through SWAPGS

  * Packaging resync (LP: #1786013)
    - update dkms package versions

linux (4.15.0-56.62) bionic; urgency=medium

  * bionic/linux: 4.15.0-56.62 -proposed tracker (LP: #1837626)

  * Packaging resync (LP: #1786013)
    - [Packaging] resync git-ubuntu-log
    - [Packaging] update helper scripts

  * CVE-2019-2101
    - media: uvcvideo: Fix 'type' check leading to overflow

  * hibmc-drm Causes Unreadable Display for Huawei amd64 Servers (LP: #1762940)
    - [Config] Set CONFIG_DRM_HISI_HIBMC to arm64 only
    - SAUCE: Make CONFIG_DRM_HISI_HIBMC depend on ARM64

  * Bionic: support for Solarflare X2542 network adapter (sfc driver)
    (LP: #1836635)
    - sfc: make mem_bar a function rather than a constant
    - sfc: support VI strides other than 8k
    - sfc: add Medford2 (SFC9250) PCI Device IDs
    - sfc: improve PTP error reporting
    - sfc: update EF10 register definitions
    - sfc: populate the timer reload field
    - sfc: update MCDI protocol headers
    - sfc: support variable number of MAC stats
    - sfc: expose FEC stats on Medford2
    - sfc: expose CTPIO stats on NICs that support them
    - sfc: basic MCDI mapping of 25/50/100G link speeds
    - sfc: support the ethtool ksettings API properly so that 25/50/100G works
    - sfc: add bits for 25/50/100G supported/advertised speeds
    - sfc: remove tx and MCDI handling from NAPI budget consideration
    - sfc: handle TX timestamps in the normal data path
    - sfc: add function to determine which TX timestamping method to use
    - sfc: use main datapath for HW timestamps if available
    - sfc: only enable TX timestamping if the adapter is licensed for it
    - sfc: MAC TX timestamp handling on the 8000 series
    - sfc: on 8000 series use TX queues for TX timestamps
    - sfc: only advertise TX timestamping if we have the license for it
    - sfc: simplify RX datapath timestamping
    - sfc: support separate PTP and general timestamping
    - sfc: support second + quarter ns time format for receive datapath
    - sfc: support Medford2 frequency adjustment format
    - sfc: add suffix to large constant in ptp
    - sfc: mark some unexported symbols as static
    - sfc: update MCDI protocol headers
    - sfc: support FEC configuration through ethtool
    - sfc: remove ctpio_dmabuf_start from stats
    - sfc: stop the TX queue before pushing new buffers

  * [18.04 FEAT] zKVM: Add hardwar...

Changed in linux (Ubuntu Bionic):
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.