[Hyper-V] x86,pageattr: prevent overflow in slow_virt_to_phys() for X86_PAE

Bug #1549601 reported by Joshua R. Poulson
16
This bug affects 2 people
Affects Status Importance Assigned to Milestone
linux (Ubuntu)
Fix Released
Medium
Joseph Salisbury
Trusty
Fix Released
Medium
Joseph Salisbury
Wily
Fix Released
Medium
Joseph Salisbury
Xenial
Fix Released
Medium
Joseph Salisbury

Bug Description

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=d1cd1210834649ce1ca6bafe5ac25d2f40331343

x86, pageattr: Prevent overflow in slow_virt_to_phys() for X86_PAE
pte_pfn() returns a PFN of long (32 bits in 32-PAE), so "long <<
PAGE_SHIFT" will overflow for PFNs above 4GB.

Due to this issue, some Linux 32-PAE distros, running as guests on Hyper-V,
with 5GB memory assigned, can't load the netvsc driver successfully and
hence the synthetic network device can't work (we can use the kernel parameter
mem=3000M to work around the issue).

Cast pte_pfn() to phys_addr_t before shifting.

Revision history for this message
Brad Figg (brad-figg) wrote : Missing required logs.

This bug is missing log files that will aid in diagnosing the problem. From a terminal window please run:

apport-collect 1549601

and then change the status of the bug to 'Confirmed'.

If, due to the nature of the issue you have encountered, you are unable to run this command, please add a comment stating that fact and change the bug status to 'Confirmed'.

This change has been made by an automated script, maintained by the Ubuntu Kernel Team.

Changed in linux (Ubuntu):
status: New → Incomplete
Revision history for this message
Dexuan Cui (decui) wrote :

It turns out the issue also exists in the latest mainline kernel!

The fix "x86, pageattr: Prevent overflow in slow_virt_to_phys() for X86_PAE" is there, but a later patch "x86/mm: Fix slow_virt_to_phys() to handle large PAT bit"
 (https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=34437e67a6727885bdf6cbfd8441b1ac43a1ee65)
actually removed the fix unintentionally, so we have the regression...

I have made a new fix and post it to LKML just now (<email address hidden> was Cc-ed):
 http://marc.info/?l=linux-kernel&m=145638841908383&w=2

Chris Valean (cvalean)
Changed in linux (Ubuntu):
status: Incomplete → Confirmed
Revision history for this message
Joshua R. Poulson (jrp) wrote :

Good catch Chris!

Revision history for this message
Joshua R. Poulson (jrp) wrote :

Thanks for your analysis Dexuan, I had not had a change to look at the mainline differences yet.

tags: added: kernel-da-key kernel-hyper-v
Changed in linux (Ubuntu):
importance: Undecided → Medium
status: Confirmed → Triaged
Revision history for this message
Joshua R. Poulson (jrp) wrote :
Download full text (3.6 KiB)

I'm expecting an upstream fix for this problem soon, based on this discussion (lkml):

From Dexuan Cui:
"d1cd12108346: x86, pageattr: Prevent overflow in slow_virt_to_phys() for X86_PAE"
was unintentionally removed by the recent
"34437e67a672: x86/mm: Fix slow_virt_to_phys() to handle large PAT bit".

And, the variable 'phys_addr' was defined as "unsigned long" by mistake -- it should
be "phys_addr_t".

As a result, Hyper-V network driver in 32-PAE Linux guest can't work again.

Fixes: "commmit 34437e67a672: x86/mm: Fix slow_virt_to_phys() to handle large PAT bit"
Signed-off-by: Dexuan Cui <email address hidden>
Cc: Toshi Kani <email address hidden>
Cc: Andrew Morton <email address hidden>
Cc: Thomas Gleixner <email address hidden>
Cc: K. Y. Srinivasan <email address hidden>
Cc: Haiyang Zhang <email address hidden>
Cc: <email address hidden>
Cc: <email address hidden>
Cc: <email address hidden>
Cc: <email address hidden>
Cc: <email address hidden>
Cc: <email address hidden>
---
 arch/x86/mm/pageattr.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index 2440814..9cf96d8 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -419,24 +419,30 @@ pmd_t *lookup_pmd_address(unsigned long address)
 phys_addr_t slow_virt_to_phys(void *__virt_addr)
 {
        unsigned long virt_addr = (unsigned long)__virt_addr;
- unsigned long phys_addr, offset;
+ phys_addr_t phys_addr;
+ unsigned long offset;
        enum pg_level level;
        pte_t *pte;

        pte = lookup_address(virt_addr, &level);
        BUG_ON(!pte);

+ /*
+ * pXX_pfn() returns unsigned long, which must be cast to phys_addr_t
+ * before being left-shifted PAGE_SHIFT bits -- this trick is to
+ * make 32-PAE kernel work correctly.
+ */
        switch (level) {
        case PG_LEVEL_1G:
- phys_addr = pud_pfn(*(pud_t *)pte) << PAGE_SHIFT;
+ phys_addr = (phys_addr_t)pud_pfn(*(pud_t *)pte) << PAGE_SHIFT;
                offset = virt_addr & ~PUD_PAGE_MASK;
                break;
        case PG_LEVEL_2M:
- phys_addr = pmd_pfn(*(pmd_t *)pte) << PAGE_SHIFT;
+ phys_addr = (phys_addr_t)pmd_pfn(*(pmd_t *)pte) << PAGE_SHIFT;
                offset = virt_addr & ~PMD_PAGE_MASK;
                break;
        default:
- phys_addr = pte_pfn(*pte) << PAGE_SHIFT;
+ phys_addr = (phys_addr_t)pte_pfn(*pte) << PAGE_SHIFT;
                offset = virt_addr & ~PAGE_MASK;
        }

From Toshi Kani:
On Thu, 2016-02-25 at 01:58 -0800, Dexuan Cui wrote:
> "d1cd12108346: x86, pageattr: Prevent overflow in slow_virt_to_phys() for
> X86_PAE"
> was unintentionally removed by the recent
> "34437e67a672: x86/mm: Fix slow_virt_to_phys() to handle large PAT bit".
>
> And, the variable 'phys_addr' was defined as "unsigned long" by mistake
> -- it should
> be "phys_addr_t".
>
> As a result, Hyper-V network driver in 32-PAE Linux guest can't work
> again.
>
> Fixes: "commmit 34437e67a672: x86/mm: Fix slow_virt_to_phys() to handle
> large PAT bit"
> Signed-off-by: Dexuan Cui <email address hidden>
> Cc: Toshi Kan...

Read more...

Revision history for this message
Joshua R. Poulson (jrp) wrote :
Changed in linux (Ubuntu Wily):
status: New → Triaged
Changed in linux (Ubuntu Trusty):
status: New → Triaged
Changed in linux (Ubuntu Wily):
importance: Undecided → Medium
Changed in linux (Ubuntu Trusty):
importance: Undecided → Medium
Changed in linux (Ubuntu Xenial):
assignee: nobody → Joseph Salisbury (jsalisbury)
Changed in linux (Ubuntu Wily):
assignee: nobody → Joseph Salisbury (jsalisbury)
Changed in linux (Ubuntu Trusty):
assignee: nobody → Joseph Salisbury (jsalisbury)
Changed in linux (Ubuntu Xenial):
status: Triaged → In Progress
Changed in linux (Ubuntu Trusty):
status: Triaged → Fix Committed
status: Fix Committed → Triaged
Changed in linux (Ubuntu Wily):
status: Triaged → In Progress
Changed in linux (Ubuntu Xenial):
status: In Progress → Fix Released
Changed in linux (Ubuntu Wily):
status: In Progress → Fix Released
status: Fix Released → In Progress
Changed in linux (Ubuntu Xenial):
status: Fix Released → In Progress
Revision history for this message
Joseph Salisbury (jsalisbury) wrote :

I built a Xenial test kernel with commit bf70e551. The test kernel can be downloaded from:

http://kernel.ubuntu.com/~jsalisbury/lp1549601/xenial/

Can you test this kernel and see if it resolves this bug?

I'm also building test kernels for Wily and Trusty. The have slightly different backports and have some prereqs. I'll post a link to them shortly.

Changed in linux (Ubuntu Trusty):
status: Triaged → In Progress
Revision history for this message
Joseph Salisbury (jsalisbury) wrote :

I also built a Wily and Trusty test kernel with commit bf70e551. Those kernels can be downloaded from:

Wily: http://kernel.ubuntu.com/~jsalisbury/lp1549601/wily

Trusty: http://kernel.ubuntu.com/~jsalisbury/lp1549601/trusty

Can you also test this kernels? If we can receive positive test feedback by March 11th, we can proceed to submit the patches for kernel SRU review in an attempt to make the upcoming kernel SRU cycle starting on March 11th. Assuming the patches are accepted for SRU and land in the upcoming SRU cycle, the fix should be available from the -updates pocket by April 2nd.

Revision history for this message
Adrian Suhov (asuhov) wrote :

Hi!

I tested all three kernels on both WS2012R2 and WS2008R2 and I got good results. Netvsc driver loaded successfully everytime with different memory settings. You can merge the patches.

Revision history for this message
Chris Valean (cvalean) wrote :

Hi Joe,

Adrian is from my team and his test results are valid. Can you please confirm that this patch will be included in the March 11th SRUs?
Thank you!

Revision history for this message
Joseph Salisbury (jsalisbury) wrote :

I submitted the SRU request, which is prior to the March 11th cutoff, so this fix should be in the next SRU cycle.

Brad Figg (brad-figg)
Changed in linux (Ubuntu Trusty):
status: In Progress → Fix Committed
Changed in linux (Ubuntu Wily):
status: In Progress → Fix Committed
Tim Gardner (timg-tpi)
Changed in linux (Ubuntu Xenial):
status: In Progress → Fix Committed
Revision history for this message
Kamal Mostafa (kamalmostafa) wrote :

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-trusty' to 'verification-done-trusty'.

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-trusty
tags: added: verification-needed-wily
Revision history for this message
Kamal Mostafa (kamalmostafa) wrote :

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-wily' to 'verification-done-wily'.

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!

Revision history for this message
Chris Valean (cvalean) wrote :

Verified this and confirmed as good with the following:

- Trusty - proposed kernel 3.13.0.85.91
- Wily - proposed kernel 4.2.0.35.38

tags: added: verification-done-trusty verification-done-wily
removed: verification-needed-trusty verification-needed-wily
Revision history for this message
Launchpad Janitor (janitor) wrote :
Download full text (11.0 KiB)

This bug was fixed in the package linux - 3.13.0-85.129

---------------
linux (3.13.0-85.129) trusty; urgency=low

  [ Brad Figg ]

  * Release Tracking Bug
    - LP: #1558727

  [ Upstream Kernel Changes ]

  * Revert "Revert "af_unix: Revert 'lock_interruptible' in stream receive
    code""

linux (3.13.0-84.128) trusty; urgency=low

  [ Brad Figg ]

  * Release Tracking Bug
    - LP: #1557596

  [ Upstream Kernel Changes ]

  * Revert "af_unix: Revert 'lock_interruptible' in stream receive code"
    - LP: #1540731
  * seccomp: cap SECCOMP_RET_ERRNO data to MAX_ERRNO
    - LP: #1496073
  * net/mlx4_en: Remove dependency between timestamping capability and
    service_task
    - LP: #1537859
  * net/mlx4_en: Fix HW timestamp init issue upon system startup
    - LP: #1537859
  * x86/mm: Fix slow_virt_to_phys() for X86_PAE again
    - LP: #1549601
  * iw_cxgb3: Fix incorrectly returning error on success
    - LP: #1557191
  * EVM: Use crypto_memneq() for digest comparisons
    - LP: #1557191
  * x86/entry/compat: Add missing CLAC to entry_INT80_32
    - LP: #1557191
  * iio: dac: mcp4725: set iio name property in sysfs
    - LP: #1557191
  * iommu/vt-d: Fix 64-bit accesses to 32-bit DMAR_GSTS_REG
    - LP: #1557191
  * PCI/AER: Flush workqueue on device remove to avoid use-after-free
    - LP: #1557191
  * libata: disable forced PORTS_IMPL for >= AHCI 1.3
    - LP: #1557191
  * mac80211: start_next_roc only if scan was actually running
    - LP: #1557191
  * mac80211: Requeue work after scan complete for all VIF types.
    - LP: #1557191
  * rfkill: fix rfkill_fop_read wait_event usage
    - LP: #1557191
  * crypto: shash - Fix has_key setting
    - LP: #1557191
  * drm/i915/dp: fall back to 18 bpp when sink capability is unknown
    - LP: #1557191
  * target: Fix WRITE_SAME/DISCARD conversion to linux 512b sectors
    - LP: #1557191
  * crypto: algif_hash - wait for crypto_ahash_init() to complete
    - LP: #1557191
  * iio: inkern: fix a NULL dereference on error
    - LP: #1557191
  * intel_scu_ipcutil: underflow in scu_reg_access()
    - LP: #1557191
  * ALSA: seq: Fix race at closing in virmidi driver
    - LP: #1557191
  * ALSA: rawmidi: Remove kernel WARNING for NULL user-space buffer check
    - LP: #1557191
  * ALSA: pcm: Fix potential deadlock in OSS emulation
    - LP: #1557191
  * ALSA: seq: Fix yet another races among ALSA timer accesses
    - LP: #1557191
  * ALSA: timer: Fix link corruption due to double start or stop
    - LP: #1557191
  * libata: fix sff host state machine locking while polling
    - LP: #1557191
  * cputime: Prevent 32bit overflow in time[val|spec]_to_cputime()
    - LP: #1557191
  * ASoC: dpcm: fix the BE state on hw_free
    - LP: #1557191
  * module: wrapper for symbol name.
    - LP: #1557191
  * ALSA: hda - Add fixup for Mac Mini 7,1 model
    - LP: #1557191
  * ALSA: Move EXPORT_SYMBOL() in appropriate places
    - LP: #1557191
  * ALSA: rawmidi: Make snd_rawmidi_transmit() race-free
    - LP: #1557191
  * ALSA: rawmidi: Fix race at copying & updating the position
    - LP: #1557191
  * ALSA: seq: Fix lockdep warnings due to double mutex locks
    - LP: #1557191
  * drivers/scsi/sg.c: mark VMA as VM_IO...

Changed in linux (Ubuntu Trusty):
status: Fix Committed → Fix Released
Revision history for this message
Launchpad Janitor (janitor) wrote :
Download full text (20.9 KiB)

This bug was fixed in the package linux - 4.2.0-35.40

---------------
linux (4.2.0-35.40) wily; urgency=low

  [ Brad Figg ]

  * Release Tracking Bug
    - LP: #1557706

  [ Upstream Kernel Changes ]

  * Revert "workqueue: make sure delayed work run in local cpu"
    - LP: #1556269
  * Revert "ALSA: hda - Fix noise on Gigabyte Z170X mobo"
    - LP: #1556269
  * KVM: VMX: Fix host initiated access to guest MSR_TSC_AUX
    - LP: #1552592
  * locking/qspinlock: Move __ARCH_SPIN_LOCK_UNLOCKED to qspinlock_types.h
    - LP: #1545330
  * [media] usbvision fix overflow of interfaces array
    - LP: #1556269
  * [media] usbvision: fix crash on detecting device with invalid
    configuration
    - LP: #1556269
  * ASN.1: Fix non-match detection failure on data overrun
    - LP: #1556269
  * iw_cxgb3: Fix incorrectly returning error on success
    - LP: #1556269
  * EVM: Use crypto_memneq() for digest comparisons
    - LP: #1556269
  * vmstat: explicitly schedule per-cpu work on the CPU we need it to run
    on
    - LP: #1556269
  * x86/entry/compat: Add missing CLAC to entry_INT80_32
    - LP: #1556269
  * iio-light: Use a signed return type for ltr501_match_samp_freq()
    - LP: #1556269
  * iio: add IIO_TRIGGER dependency to STK8BA50
    - LP: #1556269
  * iio: add HAS_IOMEM dependency to VF610_ADC
    - LP: #1556269
  * iio: dac: mcp4725: set iio name property in sysfs
    - LP: #1556269
  * iommu/vt-d: Fix 64-bit accesses to 32-bit DMAR_GSTS_REG
    - LP: #1556269
  * iio: light: acpi-als: Report data as processed
    - LP: #1556269
  * iio:adc:ti_am335x_adc Fix buffered mode by identifying as software
    buffer.
    - LP: #1556269
  * ASoC: rt5645: fix the shift bit of IN1 boost
    - LP: #1556269
  * ARCv2: STAR 9000950267: Handle return from intr to Delay Slot #2
    - LP: #1556269
  * cgroup: make sure a parent css isn't offlined before its children
    - LP: #1556269
  * ARM: OMAP2+: Fix wait_dll_lock_timed for rodata
    - LP: #1556269
  * ARM: OMAP2+: Fix l2dis_3630 for rodata
    - LP: #1556269
  * ARM: OMAP2+: Fix save_secure_ram_context for rodata
    - LP: #1556269
  * ARM: OMAP2+: Fix l2_inv_api_params for rodata
    - LP: #1556269
  * ARM: OMAP2+: Fix ppa_zero_params and ppa_por_params for rodata
    - LP: #1556269
  * rtlwifi: rtl8821ae: Fix 5G failure when EEPROM is incorrectly encoded
    - LP: #1556269
  * PCI/AER: Flush workqueue on device remove to avoid use-after-free
    - LP: #1556269
  * ARM: dts: Fix wl12xx missing clocks that cause hangs
    - LP: #1556269
  * libata: disable forced PORTS_IMPL for >= AHCI 1.3
    - LP: #1556269
  * mac80211: Requeue work after scan complete for all VIF types.
    - LP: #1556269
  * rfkill: fix rfkill_fop_read wait_event usage
    - LP: #1556269
  * ARM: dts: at91: sama5d4: fix instance id of DBGU
    - LP: #1556269
  * ARM: dts: at91: sama5d4ek: add phy address and IRQ for macb0
    - LP: #1556269
  * ARM: dts: at91: sama5d4 xplained: fix phy0 IRQ type
    - LP: #1556269
  * crypto: shash - Fix has_key setting
    - LP: #1556269
  * Input: vmmouse - fix absolute device registration
    - LP: #1556269
  * spi: atmel: fix gpio chip-select in case of non-DT platform
    - LP: #1556269
  ...

Changed in linux (Ubuntu Wily):
status: Fix Committed → Fix Released
Revision history for this message
Chris Valean (cvalean) wrote :

Will these patches be released in final 16.04?

I see that commit bf70e5513dfea29c3682e7eb3dbb45f0723bac09 is present in the xenial kernel source.

Revision history for this message
Tim Gardner (timg-tpi) wrote :

Not sure why this didn't close automatically. As far as I can tell Xenial commit e728c9a3629770bc73bc2f884feeaf901801c28f ('x86/mm: Fix slow_virt_to_phys() for X86_PAE again') was released in Ubuntu--4.4.0-13.28

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

Other bug subscribers

Remote bug watches

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