** Description changed:

- Ubuntu 18.04.2 LTS
- Linux SRV013 4.15.0-47-generic #50-Ubuntu SMP Wed Mar 13 10:44:52 UTC 2019 
x86_64 x86_64 x86_64 GNU/Linux
+ [Impact]
  
- DELL R740, 2 CPU (40 Cores, 80 Threads), 384 GiB RAM
+ * We got reports of a kernel crash in cifs module with the following
+ signature:
  
- top - 12:39:53 up  3:41,  4 users,  load average: 66.19, 64.06, 76.90
- Tasks: 1076 total,   1 running, 675 sleeping,  12 stopped,   1 zombie
- %Cpu(s): 28.2 us,  0.3 sy,  0.0 ni, 71.5 id,  0.0 wa,  0.0 hi,  0.1 si,  0.0 
st
- KiB Mem : 39483801+total, 24077185+free, 57428284 used, 96637872 buff/cache
- KiB Swap:   999420 total,   999420 free,        0 used. 33477683+avail Mem
+ detected buffer overflow in strcat
+ kernel BUG at <...>/lib/string.c:1052!
+ invalid opcode: 0000 [#1] SMP PTI
+ RIP: 0010:fortify_panic+0x13/0x1f
+ Call Trace:
+  smb21_set_oplock_level+0xde/0x190 [cifs]
+  smb3_set_oplock_level+0x22/0x90 [cifs]
+  smb2_set_fid+0x76/0xb0 [cifs]
+  cifs_new_fileinfo+0x268/0x3c0 [cifs]
+  ? smb2_get_lease_key+0x40/0x40 [cifs]
+  ? cifs_new_fileinfo+0x268/0x3c0 [cifs]
+  cifs_open+0x57c/0x8d0 [cifs]
+  do_dentry_open+0x1fe/0x320
+ [...]
  
+ * By analyzing the code of smb21_set_oplock_level(), we've noticed the
+ only way fortify function strcat() would get overflow was if the value
+ of cinode->oplock got corrupted in a another thread leading to a buffer
+ write bigger then buffer size. In this function, the 'message' buffer
+ writes are governed by cinode->oplock, so only a different thread
+ cleaning the oplock value would lead to 'message' overflow.
  
- We've seen the following bug many times since we introduced new machines
- running Ubuntu 18. Wasn't an issue older machines running Ubuntu 16.
- Three different machines are affected, so it's rather not a hardware
- issue.
+ * By the same time we worked this analysis, a fix was proposed upstream
+ for this issue  in the form of commit 6a54b2e002c9 ("cifs: fix strcat
+ buffer overflow and reduce raciness in smb21_set_oplock_level()"), by
+ the same reporter of this LP. The fix is simple and directly addresses
+ this problem, so we hereby request its SRU into Bionic kernel - it's
+ already present in Ubuntu kernel version 5.0 and newer, as well as linux
+ stable branches.
  
+ [Test case]
  
- | detected buffer overflow in strcat
- | ------------[ cut here ]------------
- | kernel BUG at /build/linux-6ZmFRN/linux-4.15.0/lib/string.c:1052!
- | invalid opcode: 0000 [#1] SMP PTI
- | Modules linked in: [...]
- | Hardware name: Dell Inc. PowerEdge R740/0923K0, BIOS 1.6.11 11/20/2018
- | RIP: 0010:fortify_panic+0x13/0x22
- |  [...]
- | Call Trace:
- |  smb21_set_oplock_level+0x147/0x1a0 [cifs]
- |  smb3_set_oplock_level+0x22/0x90 [cifs]
- |  smb2_set_fid+0x76/0xb0 [cifs]
- |  cifs_new_fileinfo+0x259/0x390 [cifs]
- |  ? smb2_get_lease_key+0x40/0x40 [cifs]
- |  ? cifs_new_fileinfo+0x259/0x390 [cifs]
- |  cifs_open+0x3db/0x8d0 [cifs]
- |  [...]
+ * 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.
  
- (Full dmesg output attached)
+ * 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.
  
- After hitting this bug there are many cifs related dmesg entries,
- processes lock up and eventually the systems freezes.
+ [Regression potential]
  
- 
- The share is mounted using:
- //server/share  /mnt/server/ cifs 
defaults,auto,iocharset=utf8,noperm,file_mode=0777,dir_mode=0777,credentials=/root/passwords/share,domain=myDomain,uid=myUser,gid=10513,mfsymlinks
- 
- Currently we're testing the cifs mount options "cache=none" as the bug
- seems to be oplock related.
+ * The patch was validated by the cifs filesystem maintainers and by the
+ aforementioned tests; also, the scope is restricted to cifs only so the
+ likelihood of regressions is considered low. The commit introduces no
+ functional changes and the only affected path was just refactored in a
+ way to prevent overflow and reduce race potential.

-- 
You received this bug notification because you are a member of Kernel
Packages, which is subscribed to linux in Ubuntu.
https://bugs.launchpad.net/bugs/1824981

Title:
  cifs set_oplock buffer overflow in strcat

Status in linux package in Ubuntu:
  Fix Released
Status in linux source package in Bionic:
  In Progress
Status in linux source package in Cosmic:
  Won't Fix
Status in linux source package in Disco:
  In Progress
Status in linux source package in Eoan:
  Fix Released

Bug description:
  [Impact]

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

  detected buffer overflow in strcat
  kernel BUG at <...>/lib/string.c:1052!
  invalid opcode: 0000 [#1] SMP PTI
  RIP: 0010:fortify_panic+0x13/0x1f
  Call Trace:
   smb21_set_oplock_level+0xde/0x190 [cifs]
   smb3_set_oplock_level+0x22/0x90 [cifs]
   smb2_set_fid+0x76/0xb0 [cifs]
   cifs_new_fileinfo+0x268/0x3c0 [cifs]
   ? smb2_get_lease_key+0x40/0x40 [cifs]
   ? cifs_new_fileinfo+0x268/0x3c0 [cifs]
   cifs_open+0x57c/0x8d0 [cifs]
   do_dentry_open+0x1fe/0x320
  [...]

  * By analyzing the code of smb21_set_oplock_level(), we've noticed the
  only way fortify function strcat() would get overflow was if the value
  of cinode->oplock got corrupted in a another thread leading to a
  buffer write bigger then buffer size. In this function, the 'message'
  buffer writes are governed by cinode->oplock, so only a different
  thread cleaning the oplock value would lead to 'message' overflow.

  * By the same time we worked this analysis, a fix was proposed
  upstream for this issue  in the form of commit 6a54b2e002c9 ("cifs:
  fix strcat buffer overflow and reduce raciness in
  smb21_set_oplock_level()"), by the same reporter of this LP. The fix
  is simple and directly addresses this problem, so we hereby request
  its SRU into Bionic kernel - it's already present in linux stable
  branches.

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

  * 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 and by
  the aforementioned tests; also, the scope is restricted to cifs only
  so the likelihood of regressions is considered low. The commit
  introduces no functional changes and the only affected path was just
  refactored in a way to prevent overflow and reduce race potential.

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1824981/+subscriptions

-- 
Mailing list: https://launchpad.net/~kernel-packages
Post to     : kernel-packages@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kernel-packages
More help   : https://help.launchpad.net/ListHelp

Reply via email to