2007-07-18 07:49:43

by Michael Wu

[permalink] [raw]
Subject: Re: p54usb: Don't run DMA off code segment

On Monday 16 July 2007 23:10, Pete Zaitcev wrote:
> +???????if ((err = p54u_bulk_msg(priv, P54U_PIPE_DATA, buf, 4)) != 0) {
Please follow the style used elsewhere in this function and do it like:

err = p54u_bulk_msg(priv, P54U_PIPE_DATA, buf, 4);
if (err) {

The rest of the patch looks fine. Thanks for fixing this.

-Michael Wu


Attachments:
(No filename) (335.00 B)
(No filename) (189.00 B)
Download all attachments

2007-07-20 05:25:59

by Pete Zaitcev

[permalink] [raw]
Subject: Re: p54usb: Don't run DMA off code segment

On Wed, 18 Jul 2007 00:48:31 -0700, Michael Wu <[email protected]=
> wrote:

> On Monday 16 July 2007 23:10, Pete Zaitcev wrote:
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0if ((err =3D p54u_bulk_m=
sg(priv, P54U_PIPE_DATA, buf, 4)) !=3D 0) {
> Please follow the style used elsewhere in this function and do it lik=
e:
>=20
> err =3D p54u_bulk_msg(priv, P54U_PIPE_DATA, buf, 4);
> if (err) {
>=20
> The rest of the patch looks fine. Thanks for fixing this.

How about this, then?

---

Passing a pointer into the code segment to DMA API is invalid. Most of =
the
time, you just DMA garbage into the device, but sometimes the controlle=
r
gets a bad address and then you see this:

usbcore: registered new interface driver prism54usb
usb 3-1: new full speed USB device using uhci_hcd and address 2
usb 3-1: not running at top speed; connect to a high speed hub
usb 3-1: configuration #1 chosen from 1 choice
uhci_hcd 0000:00:1d.2: host system error, PCI problems?
uhci_hcd 0000:00:1d.2: host controller halted, very bad!
uhci_hcd 0000:00:1d.2: HC died; cleaning up
p54usb: reset failed!
prism54usb: probe of 3-1:1.0 failed with error -110

See:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=3D242638

Signed-off-by: Pete Zaitcev <[email protected]>

--- linux-2.6.23-0.15.rc0.git1.fc8/drivers/net/wireless/mac80211/p54/pr=
ism54usb.c 2007-07-16 20:16:00.000000000 -0700
+++ linux-2.6.23-0.15.rc0.git1.fc8-p54u/drivers/net/wireless/mac80211/p=
54/prism54usb.c 2007-07-19 21:46:41.000000000 -0700
@@ -375,22 +375,24 @@ static int p54u_upload_firmware_3887(str
struct x2_header *hdr;
unsigned long timeout;
=20
- err =3D p54u_bulk_msg(priv, P54U_PIPE_DATA, start_string, 4);
- if (err) {
- printk(KERN_ERR "p54usb: reset failed!\n");
- return err;
- }
-
tmp =3D buf =3D kmalloc(P54U_FW_BLOCK, GFP_KERNEL);
if (!buf) {
printk(KERN_ERR "p54usb: cannot allocate firmware upload buffer!\n")=
;
- return -ENOMEM;
+ err =3D -ENOMEM;
+ goto err_bufalloc;
+ }
+
+ memcpy(buf, start_string, 4);
+ err =3D p54u_bulk_msg(priv, P54U_PIPE_DATA, buf, 4);
+ if (err) {
+ printk(KERN_ERR "p54usb: reset failed! (%d)\n", err);
+ goto err_reset;
}
=20
err =3D request_firmware(&fw_entry, "isl3887usb_bare", &priv->udev->d=
ev);
if (err) {
printk(KERN_ERR "p54usb: cannot find firmware (isl3887usb_bare)!\n")=
;
- return err;
+ goto err_req_fw_failed;
}
=20
p54_parse_firmware(dev, fw_entry);
@@ -499,7 +501,10 @@ static int p54u_upload_firmware_3887(str
=20
err_upload_failed:
release_firmware(fw_entry);
+ err_req_fw_failed:
+ err_reset:
kfree(buf);
+ err_bufalloc:
return err;
}
=20

2007-07-20 15:03:39

by John W. Linville

[permalink] [raw]
Subject: Re: p54usb: Don't run DMA off code segment

On Thu, Jul 19, 2007 at 10:27:16PM -0700, Pete Zaitcev wrote:
> On Wed, 18 Jul 2007 00:48:31 -0700, Michael Wu <[email protected]=
et> wrote:
>=20
> > On Monday 16 July 2007 23:10, Pete Zaitcev wrote:
> > > +=A0=A0=A0=A0=A0=A0=A0if ((err =3D p54u_bulk_msg(priv, P54U_PIPE_=
DATA, buf, 4)) !=3D 0) {
> > Please follow the style used elsewhere in this function and do it l=
ike:
> >=20
> > err =3D p54u_bulk_msg(priv, P54U_PIPE_DATA, buf, 4);
> > if (err) {
> >=20
> > The rest of the patch looks fine. Thanks for fixing this.
>=20
> How about this, then?

<snip>

That is exactly the way I committed it. Thanks, Pete!

John
--=20
John W. Linville
[email protected]