Wrong memory allocation (or deallocation) in WWAN driver cdc-wdm.c

Bug #1074157 reported by Kai Petzke
12
This bug affects 1 person
Affects Status Importance Assigned to Milestone
linux (Ubuntu)
Fix Released
Medium
Unassigned
Oneiric
Fix Released
Undecided
Ming Lei
Precise
Fix Released
Undecided
Ming Lei
Quantal
Fix Released
Undecided
Unassigned
Raring
Fix Released
Medium
Unassigned

Bug Description

SRU Justification

Impact: dma badness with cdc-wdm, regression introduced by commit
cafbe85 upstream.

Fix: Adjust usb_free_coherent calls.

Test case: it is reported the problem happens on suspend/resume with
cdc-wdm, bad dma messages reported on dmesg

--------------------------------------------------------------------------------------

When going into suspend-to-RAM or suspend-to-disk, I regularly get error messages like this one in the system log and on the console:
    [167511.661550] ehci_hcd 0000:00:1d.0: dma_pool_free buffer-128, ffff880036442000/36442000 (bad dma)

There is no crash, just this error message.

I did some debugging of this thing myself by diving into the kernel sources, and ended up with drivers/usb/class/cdc-wdm.c as the likely source of the problem: Allocation and deallocation of DMA buffer space differ on size. Here is the allocation:

        desc->inbuf = usb_alloc_coherent(interface_to_usbdev(intf),
                                         desc->wMaxCommand,
                                         GFP_KERNEL,
                                         &desc->response->transfer_dma);

and here is the corresponding deallocation (is in the code twize):
        usb_free_coherent(interface_to_usbdev(desc->intf),
                          desc->bMaxPacketSize0,
                        desc->inbuf,
                        desc->response->transfer_dma);

        usb_free_coherent(interface_to_usbdev(desc->intf),
                          desc->bMaxPacketSize0,
                          desc->inbuf,
                          desc->response->transfer_dma);

Note, that desc->wMaxCommand is used as the size for allocation, and desc->bMaxPacketSize0 is used as the size for deallocation/free. This should both be the same number. To find out, whether this should read desc->bMaxPacketSize0 in all three places, or desc->wMaxCommand in all three places is beyond my scope, though I'd assume, it is desc->bMaxPacketSize0.

The device in question is reported by "lsusb" as:
  Bus 002 Device 024: ID 0bdb:1911 Ericsson Business Mobile Networks BV

ProblemType: Bug
DistroRelease: Ubuntu 12.04
Package: linux-image-3.2.0-32-generic 3.2.0-32.51
ProcVersionSignature: Ubuntu 3.2.0-32.51-generic 3.2.30
Uname: Linux 3.2.0-32-generic x86_64
AlsaVersion: Advanced Linux Sound Architecture Driver Version 1.0.24.
ApportVersion: 2.0.1-0ubuntu14
Architecture: amd64
ArecordDevices:
 **** List of CAPTURE Hardware Devices ****
 card 0: PCH [HDA Intel PCH], device 0: CONEXANT Analog [CONEXANT Analog]
   Subdevices: 1/1
   Subdevice #0: subdevice #0
AudioDevicesInUse:
 USER PID ACCESS COMMAND
 /dev/snd/controlC0: kai 1795 F.... pulseaudio
Card0.Amixer.info:
 Card hw:0 'PCH'/'HDA Intel PCH at 0xd1600000 irq 43'
   Mixer name : 'Intel CougarPoint HDMI'
   Components : 'HDA:14f1506e,17aa21ed,00100002 HDA:80862805,80860101,00100000'
   Controls : 14
   Simple ctrls : 6
Card29.Amixer.info:
 Card hw:29 'ThinkPadEC'/'ThinkPad Console Audio Control at EC reg 0x30, fw unknown'
   Mixer name : 'ThinkPad EC (unknown)'
   Components : ''
   Controls : 1
   Simple ctrls : 1
Card29.Amixer.values:
 Simple mixer control 'Console',0
   Capabilities: pswitch pswitch-joined penum
   Playback channels: Mono
   Mono: Playback [on]
Date: Thu Nov 1 22:20:00 2012
HibernationDevice: RESUME=UUID=81618807-56b1-407c-97f3-937b647ae1d3
InstallationMedia: Kubuntu 12.04 LTS "Precise Pangolin" - Release amd64 (20120423)
MachineType: LENOVO 3045A64
ProcEnviron:
 LANGUAGE=
 TERM=xterm
 PATH=(custom, user)
 LANG=de_DE.UTF-8
 SHELL=/bin/bash
ProcFB: 0 inteldrmfb
ProcKernelCmdLine: BOOT_IMAGE=/boot/vmlinuz-3.2.0-32-generic root=UUID=ee9e4d1f-4cd6-40b1-8e46-2ccb3bdca2b6 ro quiet splash vt.handoff=7
RelatedPackageVersions:
 linux-restricted-modules-3.2.0-32-generic N/A
 linux-backports-modules-3.2.0-32-generic N/A
 linux-firmware 1.79.1
SourcePackage: linux
StagingDrivers: rts_pstor mei
UpgradeStatus: No upgrade log present (probably fresh install)
dmi.bios.date: 01/11/2012
dmi.bios.vendor: LENOVO
dmi.bios.version: 8QET54WW (1.15 )
dmi.board.asset.tag: Not Available
dmi.board.name: 3045A64
dmi.board.vendor: LENOVO
dmi.board.version: Not Available
dmi.chassis.asset.tag: No Asset Information
dmi.chassis.type: 10
dmi.chassis.vendor: LENOVO
dmi.chassis.version: Not Available
dmi.modalias: dmi:bvnLENOVO:bvr8QET54WW(1.15):bd01/11/2012:svnLENOVO:pn3045A64:pvrThinkPadX121e:rvnLENOVO:rn3045A64:rvrNotAvailable:cvnLENOVO:ct10:cvrNotAvailable:
dmi.product.name: 3045A64
dmi.product.version: ThinkPad X121e
dmi.sys.vendor: LENOVO

Revision history for this message
Kai Petzke (petzke) wrote :
Brad Figg (brad-figg)
Changed in linux (Ubuntu):
status: New → Confirmed
tags: added: precise
tags: added: patch
Revision history for this message
Joseph Salisbury (jsalisbury) wrote :

Would it be possible for you to test the latest upstream kernel? Refer to https://wiki.ubuntu.com/KernelMainlineBuilds . Please test the latest v3.7 kernel[0] (Not a kernel in the daily directory) and install both the linux-image and linux-image-extra .deb packages.

Once you've tested the upstream kernel, please remove the 'needs-upstream-testing' tag. Please only remove that one tag and leave the other tags. This can be done by clicking on the yellow pencil icon next to the tag located at the bottom of the bug description and deleting the 'needs-upstream-testing' text.

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

If you are unable to test the mainline kernel, for example it will not boot, please add the tag: 'kernel-unable-to-test-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/v3.7-rc4-raring/

tags: added: kernel-da-key needs-upstream-testing
Changed in linux (Ubuntu):
importance: Undecided → Medium
status: Confirmed → Incomplete
Revision history for this message
Kai Petzke (petzke) wrote :

Thank you very much for your response!

I have tested with the following kernel:
Linux ibmkai 3.7.0-030700rc4-generic #201211041435 SMP Sun Nov 4 19:35:50 UTC 2012 x86_64 x86_64 x86_64 GNU/Linux

The result is, that the error message in the log file disappeared. The 3.7 kernel has other problems, so that I won't keep it for more extensive testing. For example, with the 3.7 kernel, the system will go into hibernation at most once after each reboot.

tags: added: kernel-fixed-upstream
removed: needs-upstream-testing
Changed in linux (Ubuntu):
status: Incomplete → Confirmed
Revision history for this message
Joseph Salisbury (jsalisbury) wrote :

Thanks for testing. That would indicate this bug is fixed upstream. Can you also test the latest 3.2 stable kernel to see if the fix made its way there:

http://kernel.ubuntu.com/~kernel-ppa/mainline/v3.2.33-precise/

Revision history for this message
Herton R. Krzesinski (herton) wrote :

I was looking at this, and the problem I believe should remain with latest 3.2 stable.

Kai Petzke, the change you propose actually reverts this commit on the linux kernel:
commit cafbe85fb0d00d32988905c4978df433ca9b6512
Author: Bjørn Mork <email address hidden>
USB: cdc-wdm: better allocate a buffer that is at least as big as we tell the USB core

The commit actually missed the fact that it should change the usb_free_coherent calls as well, so the right fix should be that you should modify all usb_free_coherent that frees desc->inbuf to use desc->wMaxCommand instead.

3.7 upstream kernel should not be affected, because the driver was after that changed to not use anymore usb_*_coherent functions. Can you try contact upstream about this problem (the author and linux-usb list I think should be appropriate)?

description: updated
Tim Gardner (timg-tpi)
Changed in linux (Ubuntu Oneiric):
assignee: nobody → Herton R. Krzesinski (herton)
status: New → Fix Committed
assignee: Herton R. Krzesinski (herton) → Ming Lei (tom-leiming)
Changed in linux (Ubuntu Precise):
assignee: nobody → Ming Lei (tom-leiming)
status: New → Fix Committed
Changed in linux (Ubuntu Quantal):
status: New → Fix Released
Changed in linux (Ubuntu Raring):
status: Confirmed → Fix Released
tags: added: blocks-hwcert-enablement
Revision history for this message
Luis Henriques (henrix) wrote :

This bug is awaiting verification that the kernel for Precise in -proposed solves the problem (3.2.0-36.56). Please test the kernel and update this bug with the results. If the problem is solved, change the tag 'verification-needed-precise' to 'verification-done-precise'.

If verification is not done by one week 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-precise
Revision history for this message
Luis Henriques (henrix) wrote :

This bug is awaiting verification that the kernel for Oneiric in -proposed solves the problem (3.0.0-30.47). Please test the kernel and update this bug with the results. If the problem is solved, change the tag 'verification-needed-oneiric' to 'verification-done-oneiric'.

If verification is not done by one week 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-oneiric
Revision history for this message
Luis Henriques (henrix) wrote :

I'm tagging this as verified as the fix has made its way into stable, and should be included in Precise in the next stable update (its already queued in master-next branch).

tags: added: verification-done-oneiric verification-done-precise
removed: verification-needed-oneiric verification-needed-precise
Revision history for this message
Adam Conrad (adconrad) wrote : Update Released

The verification of this Stable Release Update has completed successfully and the package has now been released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regresssions.

Revision history for this message
Launchpad Janitor (janitor) wrote :
Download full text (5.9 KiB)

This bug was fixed in the package linux - 3.0.0-30.47

---------------
linux (3.0.0-30.47) oneiric-proposed; urgency=low

  [Brad Figg]

  * Release Tracking Bug
    - LP: #1095352

  [ Colin Ian King ]

  * SAUCE: samsung-laptop: disable in UEFI mode
    - LP: #1040557

  [ Herton Ronaldo Krzesinski ]

  * SAUCE: usb: cdc-wdm: fix regression on buffer deallocation
    - LP: #1074157
  * [Config] updateconfigs for 3.0.57 stable update

  [ Kees Cook ]

  * SAUCE: exec: do not leave bprm->interp on stack
    - LP: #1068888
    - CVE-2012-4530

  [ Upstream Kernel Changes ]

  * Revert "sched, autogroup: Stop going ahead if autogroup is disabled"
    - LP: #1088641
  * ALSA: pcmcia - Use pcmcia_request_irq()
    - LP: #1087264
  * drivers/block/DAC960: fix DAC960_V2_IOCTL_Opcode_T -Wenum-compare
    warning
    - LP: #1087264
  * drivers/block/DAC960: fix -Wuninitialized warning
    - LP: #1087264
  * riva/fbdev: fix several -Wuninitialized
    - LP: #1087264
  * ifenslave: Fix unused variable warnings.
    - LP: #1087264
  * x86-32: Fix invalid stack address while in softirq
    - LP: #1087264
  * x86, microcode, AMD: Add support for family 16h processors
    - LP: #1087264
  * rtlwifi: rtl8192cu: Add new USB ID
    - LP: #1087264
  * mwifiex: report error to MMC core if we cannot suspend
    - LP: #1087264
  * SCSI: isci: copy fis 0x34 response into proper buffer
    - LP: #1087264
  * ALSA: ua101, usx2y: fix broken MIDI output
    - LP: #1087264
  * PARISC: fix virtual aliasing issue in get_shared_area()
    - LP: #1087264
  * PARISC: fix user-triggerable panic on parisc
    - LP: #1087264
  * mtd: slram: invalid checking of absolute end address
    - LP: #1087264
  * dm: fix deadlock with request based dm and queue request_fn recursion
    - LP: #1087264
  * futex: avoid wake_futex() for a PI futex_q
    - LP: #1087264
  * mac80211: deinitialize ibss-internals after emptiness check
    - LP: #1087264
  * radeon: add AGPMode 1 quirk for RV250
    - LP: #1087264
  * can: bcm: initialize ifindex for timeouts without previous frame
    reception
    - LP: #1087264
  * jbd: Fix lock ordering bug in journal_unmap_buffer()
    - LP: #1087264
  * sparc64: not any error from do_sigaltstack() should fail rt_sigreturn()
    - LP: #1087264
  * ALSA: hda - Add new codec ALC283 ALC290 support
    - LP: #1087264
  * ALSA: hda - Fix missing beep on ASUS X43U notebook
    - LP: #1087264
  * ALSA: hda - Add support for Realtek ALC292
    - LP: #1081466, #1087264
  * bas_gigaset: fix pre_reset handling
    - LP: #1087264
  * ixgbe: add support for X540-AT1
    - LP: #1087264
  * sata_svw: check DMA start bit before reset
    - LP: #1087264
  * ixgbe: add support for new 82599 device
    - LP: #1087264
  * ixgbe: add support for new 82599 device id
    - LP: #1087264
  * get_dvb_firmware: fix download site for tda10046 firmware
    - LP: #1087264
  * USB: mct_u232: fix broken close
    - LP: #1087264
  * watchdog: using u64 in get_sample_period()
    - LP: #1087264
  * Input: bcm5974 - set BUTTONPAD property
    - LP: #1087264
  * mmc: sdhci-s3c: fix the wrong number of max bus clocks
    - LP: #1087264
  * Linux 3.0.54
    - LP: #1087264
  * x86-32: Export ke...

Read more...

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

This bug was fixed in the package linux - 3.2.0-36.57

---------------
linux (3.2.0-36.57) precise-proposed; urgency=low

  [Luis Henriques]

  * Release Tracking Bug
    - LP: #1097389

  [ Chris J Arges ]

  * Revert "SAUCE: fsnotify: dont put marks on temporary list when clearing
    marks by group"
    - LP: #1096137
  * Revert "SAUCE: fsnotify: introduce locked versions of
    fsnotify_add_mark() and fsnotify_remove_mark()"
    - LP: #1096137
  * Revert "SAUCE: fsnotify: pass group to fsnotify_destroy_mark()"
    - LP: #1096137
  * Revert "SAUCE: fsnotify: use a mutex instead of a spinlock to protect a
    groups mark list"
    - LP: #1096137
  * Revert "SAUCE: fanotify: add an extra flag to mark_remove_from_mask
    that indicates wheather a mark should be destroyed"
    - LP: #1096137
  * Revert "SAUCE: fsnotify: take groups mark_lock before mark lock"
    - LP: #1096137
  * Revert "SAUCE: fsnotify: use reference counting for groups"
    - LP: #1096137
  * Revert "SAUCE: fsnotify: introduce fsnotify_get_group()"
    - LP: #1096137

  [ Upstream Kernel Changes ]

  * fsnotify: introduce fsnotify_get_group()
    - LP: #1096137
  * fsnotify: use reference counting for groups
    - LP: #1096137
  * fsnotify: take groups mark_lock before mark lock
    - LP: #1096137
  * fanotify: add an extra flag to mark_remove_from_mask that indicates
    wheather a mark should be destroyed
    - LP: #1096137
  * fsnotify: use a mutex instead of a spinlock to protect a groups mark
    list
    - LP: #1096137
  * fsnotify: pass group to fsnotify_destroy_mark()
    - LP: #1096137
  * fsnotify: introduce locked versions of fsnotify_add_mark() and
    fsnotify_remove_mark()
    - LP: #1096137
  * fsnotify: dont put marks on temporary list when clearing marks by group
    - LP: #1096137
  * fsnotify: change locking order
    - LP: #1096137

linux (3.2.0-36.56) precise-proposed; urgency=low

  [Brad Figg]

  * Release Tracking Bug
    - LP: #1095351

  [ Chris J Arges ]

  * SAUCE: add eeprom_bad_csum_allow module parameter
    - LP: #1070182

  [ Colin Ian King ]

  * SAUCE: samsung-laptop: disable in UEFI mode
    - LP: #1040557

  [ Herton Ronaldo Krzesinski ]

  * SAUCE: usb: cdc-wdm: fix regression on buffer deallocation
    - LP: #1074157

  [ Kees Cook ]

  * SAUCE: exec: do not leave bprm->interp on stack
    - LP: #1068888
    - CVE-2012-4530

  [ Leann Ogasawara ]

  * Add ceph to virtual kernel flavor
    - LP: #1063784

  [ Lino Sanfilippo ]

  * SAUCE: fsnotify: introduce fsnotify_get_group()
    - LP: #922906
  * SAUCE: fsnotify: use reference counting for groups
    - LP: #922906
  * SAUCE: fsnotify: take groups mark_lock before mark lock
    - LP: #922906
  * SAUCE: fanotify: add an extra flag to mark_remove_from_mask that
    indicates wheather a mark should be destroyed
    - LP: #922906
  * SAUCE: fsnotify: use a mutex instead of a spinlock to protect a groups
    mark list
    - LP: #922906
  * SAUCE: fsnotify: pass group to fsnotify_destroy_mark()
    - LP: #922906
  * SAUCE: fsnotify: introduce locked versions of fsnotify_add_mark() and
    fsnotify_remove_mark()
    - LP: #922906
  * SAUCE: fsnotify: dont put marks on temporary list when ...

Changed in linux (Ubuntu Precise):
status: Fix Committed → Fix Released
To post a comment you must log in.