On Wed, 28 Aug 2013, Alan Stern wrote:

> > Aug 26 16:11:36 localhost kernel: ------------[ cut here ]------------
> > Aug 26 16:11:36 localhost kernel: WARNING: CPU: 0 PID: 54 at
> > lib/dma-debug.c:950 check_for_stack+0xa0/0x100()
> > Aug 26 16:11:36 localhost kernel: ehci-pci 0000:00:1a.0: DMA-API: device
> > driver maps memory fromstack [addr=ffff880427629376]
> > Aug 26 16:11:36 localhost kernel: Modules linked in: floppy(+) scsi_dh_rdac
> > scsi_dh_hp_sw scsi_dh_emc scsi_dh_alua iscsi_tcp libiscsi_tcp libiscsi
> > scsi_transport_iscsi squashfs cramfs edd dm_multipath
> > Aug 26 16:11:36 localhost kernel: CPU: 0 PID: 54 Comm: khubd Not tainted
> > 3.11.0-0.rc6.git4.1.fc20.x86_64 #1
> > Aug 26 16:11:36 localhost kernel: Hardware name: LENOVO 7745/7745, BIOS
> > DUKT31AUS 05/23/2011
> > Aug 26 16:11:36 localhost kernel:  0000000000000009 ffff880427629028
> > ffffffff81723846 ffff880427629070
> > Aug 26 16:11:36 localhost kernel:  ffff880427629060 ffffffff8107464d
> > ffff880427f9c5b8 ffff88042752bae0
> > Aug 26 16:11:36 localhost kernel:  ffff880427629376 ffff880427f9c5b8
> > ffffffff81c27440 ffff8804276290c0
> > Aug 26 16:11:36 localhost kernel: Call Trace:
> > Aug 26 16:11:36 localhost kernel:  [<ffffffff81723846>] dump_stack+0x54/0x74
> > Aug 26 16:11:36 localhost kernel:  [<ffffffff8107464d>]
> > warn_slowpath_common+0x7d/0xa0
> > Aug 26 16:11:36 localhost kernel:  [<ffffffff810746bc>]
> > warn_slowpath_fmt+0x4c/0x50
> > Aug 26 16:11:36 localhost kernel:  [<ffffffff81392550>]
> > check_for_stack+0xa0/0x100
> > Aug 26 16:11:36 localhost kernel:  [<ffffffff813928f0>]
> > debug_dma_map_page+0x100/0x140
> > Aug 26 16:11:36 localhost kernel:  [<ffffffff815029d0>]
> > usb_hcd_map_urb_for_dma+0x5d0/0x700
> > Aug 26 16:11:36 localhost kernel:  [<ffffffff81502d95>]
> > usb_hcd_submit_urb+0x295/0x8f0
> > Aug 26 16:11:36 localhost kernel:  [<ffffffff810e434c>] ?
> > lockdep_init_map+0xac/0x4a0
> > Aug 26 16:11:36 localhost kernel:  [<ffffffff815043b5>]
> > usb_submit_urb+0x155/0x3d0
> > Aug 26 16:11:36 localhost kernel:  [<ffffffff81504dc4>]
> > usb_start_wait_urb+0x74/0x190
> > Aug 26 16:11:36 localhost kernel:  [<ffffffff811cfe41>] ?
> > kmem_cache_alloc_trace+0x111/0x350
> > Aug 26 16:11:36 localhost kernel:  [<ffffffff81504fa5>]
> > usb_control_msg+0xc5/0x110
> > Aug 26 16:11:36 localhost kernel:  [<ffffffff815b2164>]
> > usbhid_get_raw_report+0x94/0xb0
> > Aug 26 16:11:36 localhost kernel:  [<ffffffff815a6fbd>]
> > hidinput_get_battery_property+0x6d/0x100
> 
> This is where the bug lies.  hidinput_get_battery_property() needs to 
> allocate a buffer with kmalloc() instead of using on that's on the 
> stack.

Indeed. Could you please test with the patch below, so that I could add 
your Tested-by?

Thanks.



From: Jiri Kosina <jkos...@suse.cz>
Subject: [PATCH] HID: battery: don't do DMA from stack

Instead of using data from stack for DMA in hidinput_get_battery_property(),
allocate the buffer dynamically.

Reported-by: Richard Ryniker <ryni...@alum.mit.edu>
Reported-by: Alan Stern <st...@rowland.harvard.edu>
Signed-off-by: Jiri Kosina <jkos...@suse.cz>
---
 drivers/hid/hid-input.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 7480799..3fc4034 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -340,7 +340,7 @@ static int hidinput_get_battery_property(struct 
power_supply *psy,
 {
        struct hid_device *dev = container_of(psy, struct hid_device, battery);
        int ret = 0;
-       __u8 buf[2] = {};
+       __u8 *buf;
 
        switch (prop) {
        case POWER_SUPPLY_PROP_PRESENT:
@@ -349,12 +349,19 @@ static int hidinput_get_battery_property(struct 
power_supply *psy,
                break;
 
        case POWER_SUPPLY_PROP_CAPACITY:
+
+               buf = kmalloc(2 * sizeof(__u8), GFP_KERNEL);
+               if (!buf) {
+                       ret = -ENOMEM;
+                       break;
+               }
                ret = dev->hid_get_raw_report(dev, dev->battery_report_id,
-                                             buf, sizeof(buf),
+                                             buf, 2,
                                              dev->battery_report_type);
 
                if (ret != 2) {
                        ret = -ENODATA;
+                       kfree(buf);
                        break;
                }
                ret = 0;
@@ -364,6 +371,7 @@ static int hidinput_get_battery_property(struct 
power_supply *psy,
                    buf[1] <= dev->battery_max)
                        val->intval = (100 * (buf[1] - dev->battery_min)) /
                                (dev->battery_max - dev->battery_min);
+               kfree(buf);
                break;
 
        case POWER_SUPPLY_PROP_MODEL_NAME:

-- 
Jiri Kosina
SUSE Labs
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to