Re: q: Memory modified after free in usb2
On Wed, Mar 25, 2009 at 10:46:54AM +0100, Hans Petter Selasky wrote: > On Wednesday 25 March 2009, Weongyo Jeong wrote: > > Hello Hans :), > > > > I think porting uath(4) to usb almost have done that it works well to > > associate with AP and for WPA but I'm suffered from a strange panic after > > detach as follows: > > > > Memory modified after free 0xc4da3600(508) val=2400 @ 0xc4da3600 > > panic: Most recently used by USBdev > > > > cpuid = 0 > > KDB: enter: panic > > [thread pid 17 tid 100036 ] > > Stopped at kdb_enter+0x3a: movl$0,kdb_why > > > > The detach step is like as follows: > > > > usb2_transfer_unsetup(sc->sc_xfer, UATH_N_XFERS); > > ... > > uath_free_rx_data_list(sc); > > uath_free_tx_data_list(sc); > > uath_free_cmd_list(sc, sc->sc_cmd, UATH_CMD_LIST_COUNT); > > > > that I've checked all memory leaks or calls after freeing memory but it > > looks it's not a driver problem. > > > > To solve this problem I modified codes slightly like below: > > > > usb2_transfer_unsetup(sc->sc_xfer, UATH_N_XFERS); > > usb2_pause_mtx(NULL, 5 * hz); > > ... > > uath_free_rx_data_list(sc); > > uath_free_tx_data_list(sc); > > uath_free_cmd_list(sc, sc->sc_cmd, UATH_CMD_LIST_COUNT); > > > > After adding it I couldn't see `Memory modified after free' messages > > anymore. My question is that I can't understand why adding > > usb2_pause_mtx() helps this symptom? > > Did you drain all the taskqueues before unsetup ? It looks I found a problem that some tasks running by net80211 causes this problem. It looks calling usb2_transfer_unsetup() after ieee80211_ifdetach(ic) solves the problem. Thanks! regards, Weongyo Jeong ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to "freebsd-usb-unsubscr...@freebsd.org"
Re: q: Memory modified after free in usb2
On Thursday 26 March 2009, Weongyo Jeong wrote:
> On Wed, Mar 25, 2009 at 10:46:54AM +0100, Hans Petter Selasky wrote:
> > > To solve this problem I modified codes slightly like below:
> > >
> > > usb2_transfer_unsetup(sc->sc_xfer, UATH_N_XFERS);
> > > usb2_pause_mtx(NULL, 5 * hz);
> > > ...
> > > uath_free_rx_data_list(sc);
> > > uath_free_tx_data_list(sc);
> > > uath_free_cmd_list(sc, sc->sc_cmd, UATH_CMD_LIST_COUNT);
> > >
> > > After adding it I couldn't see `Memory modified after free' messages
> > > anymore. My question is that I can't understand why adding
> > > usb2_pause_mtx() helps this symptom?
> >
> > Did you drain all the taskqueues before unsetup ?
>
> Yes. All I used was two callouts that the driver currently doesn't use
> usb2_proc_create() and tried to drain its before calling
> usb2_transfer_unsetup() but it still encounters `Memory modified after
> free'.
Hi,
Is this the link to the complete code?
http://perforce.freebsd.org/fileViewer.cgi?FSPC=//depot/user/weongyo/wireless/src/sys/dev/usb/wlan/if_uath.c&REV=24
1) The manpage states: "This function MUST NOT be called while holding any
locks ..."
|UATH_LOCK(sc);
|callout_drain(&sc->stat_ch);
|callout_drain(&sc->watchdog_ch);
505: 159831 21 |UATH_UNLOCK(sc);
callout_stop() is not callout_drain() !
Same sematics with USB2. transfer_stop()/transfer_drain()
2) Instead of copying the data twice, use the .ext_buffer=1 flag to instruct
USB to not allocate its own buffer in "struct usb2_config", see "umass.c" for
example, and the and
usb2_set_frame_data(xfer,
urb->transfer_buffer, 0);
Before you start the hardware!
Actually you can save alot of copying if you can exploit the multi-frame
feature of the USB-stack for BULK transfers! Then you have to set "frames >
1" in usb2_config structure. For example you would then copy in the header to
wMaxPacketSize bytes, and use the data pointer for the rest, given that the
IP-packet is not that defragged.
3) There is a chicken egg problem at detach.
I suspect that "uath_bulk_tx_callback()" is called with the USB_ERR_CANCELLED
error. And here you seem to access freed memory. I think you need to re-think
how you get that last node freed. Maybe it should be done by the if_stop()
and not at cancelled, because according to detach, you will do if_stop()
first and then cancel USB and unless you drain, you are not certain that the
callback is complete!
|default:
|data = STAILQ_FIRST(&sc->sc_datatx_active);
|if (data == NULL)
|goto setup;
2605: 159735 3 |if (data->ni != NULL) {
|ieee80211_free_node(data->ni);
|data->ni = NULL;
|ifp->if_oerrors++;
|}
2610: 159733 1 |if (xfer->error != USB_ERR_CANCELLED) {
|xfer->flags.stall_pipe = 1;
|goto setup;
|}
I recommend that you do the usb2_transfer_unsetup() first at detach like in
if_rum.c and the other WLAN drivers. That will solve your problem, and maybe
you have to fix the datatx_active queue, so that the last "data" is not stuck
there for ever ???
--HPS
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "freebsd-usb-unsubscr...@freebsd.org"
Re: usb/120017: [ehci] [patch] CS5536 (AMD Geode) USB 2.0 quirk
Synopsis: [ehci] [patch] CS5536 (AMD Geode) USB 2.0 quirk State-Changed-From-To: open->closed State-Changed-By: gavin State-Changed-When: Thu Mar 26 13:40:14 UTC 2009 State-Changed-Why: Fixed in HEAD and MFC'd http://www.freebsd.org/cgi/query-pr.cgi?pr=120017 ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to "freebsd-usb-unsubscr...@freebsd.org"
Re: q: Memory modified after free in usb2
Weongyo Jeong wrote: On Wed, Mar 25, 2009 at 10:46:54AM +0100, Hans Petter Selasky wrote: On Wednesday 25 March 2009, Weongyo Jeong wrote: Hello Hans :), I think porting uath(4) to usb almost have done that it works well to associate with AP and for WPA but I'm suffered from a strange panic after detach as follows: Memory modified after free 0xc4da3600(508) val=2400 @ 0xc4da3600 panic: Most recently used by USBdev cpuid = 0 KDB: enter: panic [thread pid 17 tid 100036 ] Stopped at kdb_enter+0x3a: movl$0,kdb_why The detach step is like as follows: usb2_transfer_unsetup(sc->sc_xfer, UATH_N_XFERS); ... uath_free_rx_data_list(sc); uath_free_tx_data_list(sc); uath_free_cmd_list(sc, sc->sc_cmd, UATH_CMD_LIST_COUNT); that I've checked all memory leaks or calls after freeing memory but it looks it's not a driver problem. To solve this problem I modified codes slightly like below: usb2_transfer_unsetup(sc->sc_xfer, UATH_N_XFERS); usb2_pause_mtx(NULL, 5 * hz); ... uath_free_rx_data_list(sc); uath_free_tx_data_list(sc); uath_free_cmd_list(sc, sc->sc_cmd, UATH_CMD_LIST_COUNT); After adding it I couldn't see `Memory modified after free' messages anymore. My question is that I can't understand why adding usb2_pause_mtx() helps this symptom? Did you drain all the taskqueues before unsetup ? It looks I found a problem that some tasks running by net80211 causes this problem. It looks calling usb2_transfer_unsetup() after ieee80211_ifdetach(ic) solves the problem. Thanks! ieee80211_ifdetach may call back into the driver (e.g. if you're associated in sta mode and the state machine wants to notify the ap to disassociate). Reordering the calls is the right thing to do. Sam ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to "freebsd-usb-unsubscr...@freebsd.org"
Re: q: Memory modified after free in usb2
On Thu, Mar 26, 2009 at 09:02:02AM +0100, Hans Petter Selasky wrote:
> On Thursday 26 March 2009, Weongyo Jeong wrote:
> > On Wed, Mar 25, 2009 at 10:46:54AM +0100, Hans Petter Selasky wrote:
>
> > > > To solve this problem I modified codes slightly like below:
> > > >
> > > > usb2_transfer_unsetup(sc->sc_xfer, UATH_N_XFERS);
> > > > usb2_pause_mtx(NULL, 5 * hz);
> > > > ...
> > > > uath_free_rx_data_list(sc);
> > > > uath_free_tx_data_list(sc);
> > > > uath_free_cmd_list(sc, sc->sc_cmd, UATH_CMD_LIST_COUNT);
> > > >
> > > > After adding it I couldn't see `Memory modified after free' messages
> > > > anymore. My question is that I can't understand why adding
> > > > usb2_pause_mtx() helps this symptom?
> > >
> > > Did you drain all the taskqueues before unsetup ?
> >
> > Yes. All I used was two callouts that the driver currently doesn't use
> > usb2_proc_create() and tried to drain its before calling
> > usb2_transfer_unsetup() but it still encounters `Memory modified after
> > free'.
>
> Hi,
>
> Is this the link to the complete code?
>
> http://perforce.freebsd.org/fileViewer.cgi?FSPC=//depot/user/weongyo/wireless/src/sys/dev/usb/wlan/if_uath.c&REV=24
Yes.
> 1) The manpage states: "This function MUST NOT be called while holding any
> locks ..."
> |UATH_LOCK(sc);
> |callout_drain(&sc->stat_ch);
> |callout_drain(&sc->watchdog_ch);
> 505: 159831 21 |UATH_UNLOCK(sc);
>
>
> callout_stop() is not callout_drain() !
>
> Same sematics with USB2. transfer_stop()/transfer_drain()
It looks it's my mistake to lock before going into callout_drain(9).
> 2) Instead of copying the data twice, use the .ext_buffer=1 flag to instruct
> USB to not allocate its own buffer in "struct usb2_config", see "umass.c" for
> example, and the and
> usb2_set_frame_data(xfer,
> urb->transfer_buffer, 0);
>
> Before you start the hardware!
>
> Actually you can save alot of copying if you can exploit the multi-frame
> feature of the USB-stack for BULK transfers! Then you have to set "frames >
> 1" in usb2_config structure. For example you would then copy in the header to
> wMaxPacketSize bytes, and use the data pointer for the rest, given that the
> IP-packet is not that defragged.
This is what I want! I'll apply it immediately. Thanks! :-)
> 3) There is a chicken egg problem at detach.
>
> I suspect that "uath_bulk_tx_callback()" is called with the USB_ERR_CANCELLED
> error. And here you seem to access freed memory. I think you need to re-think
> how you get that last node freed. Maybe it should be done by the if_stop()
> and not at cancelled, because according to detach, you will do if_stop()
> first and then cancel USB and unless you drain, you are not certain that the
> callback is complete!
>
> |default:
>|data = STAILQ_FIRST(&sc->sc_datatx_active);
>|if (data == NULL)
>|goto setup;
> 2605: 159735 3 |if (data->ni != NULL) {
>|ieee80211_free_node(data->ni);
>|data->ni = NULL;
>|ifp->if_oerrors++;
>|}
> 2610: 159733 1 |if (xfer->error != USB_ERR_CANCELLED) {
>|xfer->flags.stall_pipe = 1;
>|goto setup;
>|}
>
> I recommend that you do the usb2_transfer_unsetup() first at detach like in
> if_rum.c and the other WLAN drivers. That will solve your problem, and maybe
> you have to fix the datatx_active queue, so that the last "data" is not stuck
> there for ever ???
Thank you for advice again. I'll try to do some more tests. It looks
ordering is important when detaching and with looking other WLAN drivers
it seems calling ieee80211_ifdetach(ic) should be moved before
usb2_transfer_unsetup(9) including if_zyd.c, if_ural.c and if_ral.c
regards,
Weongyo Jeong
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to "freebsd-usb-unsubscr...@freebsd.org"
q: a trivial question of usb2_transfer_unsetup()
Hello Hans, I have a simple question that is there a possibility for .mh.callback of usb2_config to be called after usb2_transfer_unsetup()? regards, Weongyo Jeong ___ freebsd-usb@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-usb To unsubscribe, send any mail to "freebsd-usb-unsubscr...@freebsd.org"
