s390/mm: fix write access check in gup_huge_pmd()

Bug #1730596 reported by bugproxy on 2017-11-07
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Ubuntu on IBM z Systems
Critical
Canonical Kernel Team
linux (Ubuntu)
Critical
Joseph Salisbury
Xenial
Critical
Joseph Salisbury
Zesty
Critical
Joseph Salisbury
Artful
Critical
Joseph Salisbury
Bionic
Critical
Joseph Salisbury

Bug Description

== SRU Justification ==
The check for the _SEGMENT_ENTRY_PROTECT bit in gup_huge_pmd() is the
wrong way around. It must not be set for write==1, and not be checked for
write==0. Fix this similar to how it was fixed for ptes long time ago in
commit 25591b0 ("[S390] fix get_user_pages_fast").

One impact of this bug would be unnecessarily using the gup slow path for
write==0 on r/w mappings. A potentially more severe impact would be that
gup_huge_pmd() will succeed for write==1 on r/o mappings.

This bug is fixed by mainline commit ba385c0594, which is in mainline as of v4.14-rc2. It was also cc'd to upstream stable. It has already been accepted in upstream v4.13.y, so Artful and Bionic have the fix via the 4.13.5 stable updates.

== Fix ==
commit ba385c0594e723d41790ecfb12c610e6f90c7785
Author: Gerald Schaefer <email address hidden>
Date: Mon Sep 18 16:51:51 2017 +0200

    s390/mm: fix write access check in gup_huge_pmd()

== Regression Potential ==
This patch is specific to s390. It has also been accepted by upstream stable, so additional upstream review has been done.

Addl information

Problem: The check for the _SEGMENT_ENTRY_PROTECT bit in
              gup_huge_pmd() is the wrong way around. It must not be set
              for write==1, and not be checked for write==0. Allowing
              write==1 with protection bit set, instead of breaking out
              to the slow path, will result in a missing faultin_page()
              to clear the protection bit (for valid writable mappings),
              and the async I/O write operation will fail to write to
              such a mapping.
Solution: Fix it by correctly checking the protection bit like it is
              also done in gup_pte_range() and gup_huge_pud().
Reproduction: Async I/O workload on buffers that are mapped as transparent
              hugepages.
Upstream-ID: ba385c0594e723d41790ecfb12c610e6f90c7785

bugproxy (bugproxy) on 2017-11-07
tags: added: architecture-s39064 bugnameltc-161009 severity-high targetmilestone-inin1604
Changed in ubuntu:
assignee: nobody → Skipper Bug Screeners (skipper-screen-team)
affects: ubuntu → linux (Ubuntu)
Changed in ubuntu-z-systems:
importance: Undecided → High
assignee: nobody → Canonical Kernel Team (canonical-kernel-team)
tags: added: kernel-da-key
Frank Heimes (frank-heimes) wrote :

raising the importance to critical (after discussing with IBM)

Changed in ubuntu-z-systems:
importance: High → Critical
Changed in linux (Ubuntu):
importance: Undecided → Critical
status: New → Triaged
tags: added: kernel-key
removed: kernel-da-key
Changed in ubuntu-z-systems:
status: New → Triaged
Joseph Salisbury (jsalisbury) wrote :

I built Xenial, Zesty and Artful test kernels with commit ba385c0594e723d. The test kernels can be downloaded from:

http://kernel.ubuntu.com/~jsalisbury/lp1730596/

Can you test these kernels to see if they resolve this bug? If they do, I'll submit an SRU request.

Changed in linux (Ubuntu Xenial):
status: New → In Progress
Changed in linux (Ubuntu Zesty):
status: New → In Progress
Changed in linux (Ubuntu Artful):
status: New → In Progress
Changed in linux (Ubuntu Bionic):
status: Triaged → In Progress
Changed in linux (Ubuntu Xenial):
importance: Undecided → Critical
Changed in linux (Ubuntu Zesty):
importance: Undecided → Critical
Changed in linux (Ubuntu Artful):
importance: Undecided → Critical
Changed in linux (Ubuntu Bionic):
assignee: Skipper Bug Screeners (skipper-screen-team) → Joseph Salisbury (jsalisbury)
Changed in linux (Ubuntu Artful):
assignee: nobody → Joseph Salisbury (jsalisbury)
Changed in linux (Ubuntu Zesty):
assignee: nobody → Joseph Salisbury (jsalisbury)
Changed in linux (Ubuntu Xenial):
assignee: nobody → Joseph Salisbury (jsalisbury)
Joseph Salisbury (jsalisbury) wrote :

This commit is already in Artful and Bionic master-next, so testing of Xenial and Zesty is the most important.

Changed in linux (Ubuntu Bionic):
status: In Progress → Fix Committed
Changed in linux (Ubuntu Artful):
status: In Progress → Fix Committed
Changed in ubuntu-z-systems:
status: Triaged → In Progress
description: updated
Changed in linux (Ubuntu Zesty):
status: In Progress → Fix Committed

------- Comment From <email address hidden> 2017-11-10 05:00 EDT-------
As mentioned within a previous comment, this is a preventiv fix, which should be applied to Ubuntu 16.04 and newer. Fix is already upstream with kernel 4.14 rc2 and should be applied to the distros in the field..

------- Comment From <email address hidden> 2017-11-10 05:06 EDT-------
Addl information:"in reply to comment #2 yes, tested that the patch fixes the issue"

Changed in linux (Ubuntu Xenial):
status: In Progress → Fix Committed
Changed in ubuntu-z-systems:
status: In Progress → Fix Committed

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-zesty' to 'verification-done-zesty'. If the problem still exists, change the tag 'verification-needed-zesty' to 'verification-failed-zesty'.

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-zesty
bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2017-11-14 03:24 EDT-------
Already verified for Zesty - see also comment #4

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

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
bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2017-11-14 07:18 EDT-------
Already verified for Xenial - see also comment #4

tags: added: verification-done-xenial
removed: verification-needed-xenial
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package linux - 4.10.0-40.44

---------------
linux (4.10.0-40.44) zesty; urgency=low

  * linux: 4.10.0-40.44 -proposed tracker (LP: #1731269)

  * s390/mm: fix write access check in gup_huge_pmd() (LP: #1730596)
    - s390/mm: fix write access check in gup_huge_pmd()

 -- Kleber Sacilotto de Souza <email address hidden> Thu, 09 Nov 2017 15:24:07 +0100

Changed in linux (Ubuntu Zesty):
status: Fix Committed → Fix Released
Launchpad Janitor (janitor) wrote :
Download full text (14.3 KiB)

This bug was fixed in the package linux - 4.4.0-101.124

---------------
linux (4.4.0-101.124) xenial; urgency=low

  * linux: 4.4.0-101.124 -proposed tracker (LP: #1731264)

  * s390/mm: fix write access check in gup_huge_pmd() (LP: #1730596)
    - s390/mm: fix write access check in gup_huge_pmd()

linux (4.4.0-100.123) xenial; urgency=low

  * linux: 4.4.0-100.123 -proposed tracker (LP: #1729273)

  * Xenial update to 4.4.95 stable release (LP: #1729107)
    - USB: devio: Revert "USB: devio: Don't corrupt user memory"
    - USB: core: fix out-of-bounds access bug in usb_get_bos_descriptor()
    - USB: serial: metro-usb: add MS7820 device id
    - usb: cdc_acm: Add quirk for Elatec TWN3
    - usb: quirks: add quirk for WORLDE MINI MIDI keyboard
    - usb: hub: Allow reset retry for USB2 devices on connect bounce
    - ALSA: usb-audio: Add native DSD support for Pro-Ject Pre Box S2 Digital
    - can: gs_usb: fix busy loop if no more TX context is available
    - usb: musb: sunxi: Explicitly release USB PHY on exit
    - usb: musb: Check for host-mode using is_host_active() on reset interrupt
    - can: esd_usb2: Fix can_dlc value for received RTR, frames
    - drm/nouveau/bsp/g92: disable by default
    - drm/nouveau/mmu: flush tlbs before deleting page tables
    - ALSA: seq: Enable 'use' locking in all configurations
    - ALSA: hda: Remove superfluous '-' added by printk conversion
    - i2c: ismt: Separate I2C block read from SMBus block read
    - brcmsmac: make some local variables 'static const' to reduce stack size
    - bus: mbus: fix window size calculation for 4GB windows
    - clockevents/drivers/cs5535: Improve resilience to spurious interrupts
    - rtlwifi: rtl8821ae: Fix connection lost problem
    - KEYS: encrypted: fix dereference of NULL user_key_payload
    - lib/digsig: fix dereference of NULL user_key_payload
    - KEYS: don't let add_key() update an uninstantiated key
    - pkcs7: Prevent NULL pointer dereference, since sinfo is not always set.
    - parisc: Avoid trashing sr2 and sr3 in LWS code
    - parisc: Fix double-word compare and exchange in LWS code on 32-bit kernels
    - sched/autogroup: Fix autogroup_move_group() to never skip sched_move_task()
    - f2fs crypto: replace some BUG_ON()'s with error checks
    - f2fs crypto: add missing locking for keyring_key access
    - fscrypt: fix dereference of NULL user_key_payload
    - KEYS: Fix race between updating and finding a negative key
    - fscrypto: require write access to mount to set encryption policy
    - FS-Cache: fix dereference of NULL user_key_payload
    - Linux 4.4.95

  * Xenial update to 4.4.94 stable release (LP: #1729105)
    - percpu: make this_cpu_generic_read() atomic w.r.t. interrupts
    - drm/dp/mst: save vcpi with payloads
    - MIPS: Fix minimum alignment requirement of IRQ stack
    - sctp: potential read out of bounds in sctp_ulpevent_type_enabled()
    - bpf/verifier: reject BPF_ALU64|BPF_END
    - udpv6: Fix the checksum computation when HW checksum does not apply
    - ip6_gre: skb_push ipv6hdr before packing the header in ip6gre_header
    - net: emac: Fix napi poll list corruption
    - packet: hold bind lock when rebinding to fa...

Changed in linux (Ubuntu Xenial):
status: Fix Committed → Fix Released
Changed in ubuntu-z-systems:
status: Fix Committed → Fix Released
tags: added: kernel-da-key
removed: kernel-key
Dimitri John Ledkov (xnox) wrote :

Is this fix released in artful & bionic too?

Changed in linux (Ubuntu Artful):
status: Fix Committed → Fix Released
Changed in linux (Ubuntu Bionic):
status: Fix Committed → Fix Released
bugproxy (bugproxy) wrote :

------- Comment From <email address hidden> 2018-01-12 05:26 EDT-------
IBM bugzilla status-> closed; now Fix Released within Artful/Bionic

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers