cx23885: Incorrect argument passed to videobuf_dma_unmap

[Jeremy Kerr]

In the maverick kernel, the commit bcbfc24 introduced the following change to the cx23885 driver:

@@ -1142,7 +1142,7 @@ void cx23885_free_buffer
        BUG_ON(in_interrupt());
        videobuf_waiton(&buf->vb, 0, 0);
- videobuf_dma_unmap(q, dma);
+ videobuf_dma_unmap(q->dev, dma);
        videobuf_dma_free(dma);
        btcx_riscmem_free(to_pci_dev(q->dev), &buf->risc);

However, videobuf_dma_unmap expects a struct videobuf_queue * as its first argument, not struct device *. This change is for a new API introduced after 2.6.35, and so is not in the v4l code in maverick.

[Jeremy Kerr (jk-ozlabs)]

Patch coming...

[Jeremy Kerr (jk-ozlabs)]

Path sent to k-t: https://lists.ubuntu.com/archives/kernel-team/2011-June/015945.html

[Jeremy Kerr (jk-ozlabs)]

v2: https://lists.ubuntu.com/archives/kernel-team/2011-June/015947.html

[Jeremy Kerr (jk-ozlabs)]

Just to clarify: bcbfc24 is a sauce patch that backports some IR stuff from post-2.6.35. The cx23885-core changes must have snuck in unintentionally.

[Herton R. Krzesinski (herton)]

This bug is missing the SRU Justification text in the description or in a comment, please update it.

Also it 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-maverick' to 'verification-done-maverick'.

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!

[Jeremy Kerr (jk-ozlabs)]

SRU Justification:

Impact: Currently, the kernel build produces a warning on the cx23885-core.c:

drivers/media/video/cx23885/cx23885-core.c: In function ‘cx23885_free_buffer’:
drivers/media/video/cx23885/cx23885-core.c:1146:2: warning: passing argument 1 of ‘videobuf_dma_unmap’ from incompatible pointer type
include/media/videobuf-dma-sg.h:100:5: note: expected ‘struct videobuf_queue *’ but argument is of type ‘struct device *’

- this *may* indicate that video buffers are not properly freed after use

Fix: With this fix applied, the warning is no longer present

[Jeremy Kerr (jk-ozlabs)]

I can confirm that the current git HEAD (4419a4ff8) does not produce this warning. Do you need further verification though?

[Jeremy Kerr (jk-ozlabs)]

Assuming yes. I will track down some hardware to build a testcase.

[Jeremy Kerr (jk-ozlabs)]

I've only had access to 32-bit installs so far, with which this error looks fairly innocuous (looks like the dev argument isn't used to establish mappings on 32-bit). However, on 64-bit, it appears that the incorrect dev pointer code causes an oops (specified in bug 659348).

I've put a call for testing on that bug, that should show a fix with the -proposed kernel.

[Steve Conklin (sconklin)]

We're going to let this go until Monday, and then we'll revert the patch if we don't have verification that the kernel in -proposed resolves the issue.

[Jeremy Kerr (jk-ozlabs)]

Steve - understood. I have no HW to test this on (looks like we need a 64-bit machine with an IOMMU), so can't help out here. So, we're waiting on the folks on bug 659348 to test.

[Steve Conklin (sconklin)]

After discussion among the kernel team, and inspection of the patch, here is the concensus:

This patch looks obviously correct by inspection, and should only affect users of this device.
This kernel has been tested with a number of other PCI devices, and is not broken for any other device.

Therefore, we will leave this patch in the kernel and not revert it.

[Launchpad Janitor (janitor)]

This bug was fixed in the package linux - 2.6.35-30.56

---------------
linux (2.6.35-30.56) maverick-proposed; urgency=low

  [Herton R. Krzesinski]

  * Release Tracking Bug
    - LP: #808934

  [ Herton Ronaldo Krzesinski ]

  * Revert "SAUCE: mmc: Enable MMC card reader for RICOH [1180:e823]"

  [ Upstream Kernel Changes ]

  * Revert "x86: Flush TLB if PGD entry is changed in i386 PAE mode"
    - LP: #805209

linux (2.6.35-30.55) maverick-proposed; urgency=low

  [Steve Conklin]

  * Release Tracking Bug
    - LP: #801690

  [ Jeremy Kerr ]

  * SAUCE: cx23885: Fix argument to videobuf_dma_unmap
    - LP: #800527

  [ Manoj Iyer ]

  * SAUCE: mmc: Enable MMC card reader for RICOH [1180:e823]
    - LP: #790754

  [ Upstream Kernel Changes ]

  * agp: fix OOM and buffer overflow
    - LP: #791918
    - CVE-2011-1746
  * tty: icount changeover for other main devices, CVE-2010-4076,
    CVE-2010-4077
    - LP: #720189
    - CVE-2010-4077
  * fs/partitions/efi.c: corrupted GUID partition tables can cause kernel
    oops
    - LP: #795418
    - CVE-2011-1577
  * Fix corrupted OSF partition table parsing
    - LP: #796606
    - CVE-2011-1163
  * can: Add missing socket check in can/bcm release.
    - LP: #796502
    - CVE-2011-1598
  * nfs4: Ensure that ACL pages sent over NFS were not allocated from the
    slab (v3) CVE-2011-1090
    - LP: #800775
    - CVE-2011-1090
 -- Herton Ronaldo Krzesinski <email address hidden> Mon, 11 Jul 2011 15:17:32 -0300