Re: Panic with ugen

2003-12-02 Thread Jay Cornwall
Martin wrote:

Could you try the attached patch (rm -f sys/dev/usb/ugen.c, cvs up 
sys/dev/usb/ugen.c, patch ...) to see if it alleviates the panic? It 
should at least give a more specific panic, if it doesn't fix the problem.

Sorry for the delay, I got a busy weekend.
And sorry for the delay here too. Had a busy start-of-week. :\

I still got a panic after starting the small piece of code, but it looks
like this now:
fatal trap 12
fault virtual address = 0x5
supervisor read, page not present
[...]
trace shows me "ugen_set_config()" at top of the stack.
Hmm. I'm afraid I'm not really sure why my patch doesn't fix the problem.

Unfortunately, to fix this I think I'd need to be able to reproduce the panic 
locally and do some debugging. Otherwise, I'd just be guessing at what's 
causing the problem.

If you could provide a full backtrace with a debug-enabled kernel 
('makeoptions DEBUG=-g' in your kernel configuration file, with GDB using 
kernel.debug to obtain symbols - I think onlamp.com had a tutorial on doing 
this) I may be able to locate where things are going wrong. But if it's too 
much hassle for you, I think it'd be best to pass this on to a developer with 
a machine to test the panic on.

Thanks for trying the patches, though. :)

--
Cheers,
Jay
http://www.evilrealms.net/ - Systems Administrator & Developer
http://www.imperial.ac.uk/ - 3rd year CS student
___
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "[EMAIL PROTECTED]"


Re: Panic with ugen

2003-11-30 Thread Martin
On Fri, 2003-11-28 at 14:43, Jay Cornwall wrote:

> Could you try the attached patch (rm -f sys/dev/usb/ugen.c, cvs up 
> sys/dev/usb/ugen.c, patch ...) to see if it alleviates the panic? It 
> should at least give a more specific panic, if it doesn't fix the problem.

Sorry for the delay, I got a busy weekend.

I still got a panic after starting the small piece of code, but it looks
like this now:

fatal trap 12
fault virtual address = 0x5
supervisor read, page not present
[...]

trace shows me "ugen_set_config()" at top of the stack.

Martin


___
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "[EMAIL PROTECTED]"


Re: Panic with ugen

2003-11-28 Thread Jay Cornwall
Martin wrote:

Could you try the attached patch, to see if it fixes the panic on the second 
run of your program?

No success. Still same panic.
Oops, my bad. I misread your backtrace, and fixed a similar bug in 
ugen_set_interface rather than ugen_set_config.

Could you try the attached patch (rm -f sys/dev/usb/ugen.c, cvs up 
sys/dev/usb/ugen.c, patch ...) to see if it alleviates the panic? It 
should at least give a more specific panic, if it doesn't fix the problem.

--
Cheers,
Jay
http://www.imperial.ac.uk/ - 3rd year CS student
Index: sys/dev/usb/ugen.c
===
RCS file: /home/ncvs/src/sys/dev/usb/ugen.c,v
retrieving revision 1.81
diff -u -3 -p -r1.81 ugen.c
--- sys/dev/usb/ugen.c  9 Nov 2003 09:17:22 -   1.81
+++ sys/dev/usb/ugen.c  28 Nov 2003 13:36:27 -
@@ -322,10 +322,6 @@ ugen_set_config(struct ugen_softc *sc, i
DPRINTFN(1,("ugen_set_config: %s to configno %d, sc=%p\n",
USBDEVNAME(sc->sc_dev), configno, sc));
 
-#if defined(__FreeBSD__)
-   ugen_destroy_devnodes(sc);
-#endif
-
/* We start at 1, not 0, because we don't care whether the
 * control endpoint is open or not. It is always present.
 */
@@ -347,15 +343,22 @@ ugen_set_config(struct ugen_softc *sc, i
err = usbd_interface_count(dev, &niface);
if (err)
return (err);
+
+#if defined(__FreeBSD__)
+   ugen_destroy_devnodes(sc);
+#endif
+
memset(sc->sc_endpoints, 0, sizeof sc->sc_endpoints);
for (ifaceno = 0; ifaceno < niface; ifaceno++) {
DPRINTFN(1,("ugen_set_config: ifaceno %d\n", ifaceno));
err = usbd_device2interface_handle(dev, ifaceno, &iface);
-   if (err)
-   return (err);
+   if (err) {
+   panic("ugen_set_config: can't obtain interface handle");
+   }
err = usbd_endpoint_count(iface, &nendpt);
-   if (err)
-   return (err);
+   if (err) {
+   panic("ugen_set_config: endpoint count failed");
+   }
for (endptno = 0; endptno < nendpt; endptno++) {
ed = usbd_interface2endpoint_descriptor(iface,endptno);
endpt = ed->bEndpointAddress;
@@ -1014,8 +1017,8 @@ ugen_set_interface(struct ugen_softc *sc
usbd_interface_handle iface;
usb_endpoint_descriptor_t *ed;
usbd_status err;
-   struct ugen_endpoint *sce;
-   u_int8_t niface, nendpt, endptno, endpt;
+   struct ugen_endpoint *sce, **sce_cache;
+   u_int8_t niface, nendpt, nendpt_cache, endptno, endpt;
int dir;
 
DPRINTFN(15, ("ugen_set_interface %d %d\n", ifaceidx, altno));
@@ -1033,30 +1036,46 @@ ugen_set_interface(struct ugen_softc *sc
if (err)
return (err);
 
-#if defined(__FreeBSD__)
-   /* destroy the existing devices, we remake the new ones in a moment */
-   ugen_destroy_devnodes(sc);
-#endif
+   /* store an array of endpoint descriptors to destroy if the interface
+* change succeeds - these aren't available afterwards */
+   sce_cache = malloc(sizeof(struct ugen_endpoint *) * nendpt, M_TEMP,
+   M_WAITOK);
+   nendpt_cache = nendpt;
 
-   /* XXX should only do this after setting new altno has succeeded */
for (endptno = 0; endptno < nendpt; endptno++) {
ed = usbd_interface2endpoint_descriptor(iface,endptno);
endpt = ed->bEndpointAddress;
dir = UE_GET_DIR(endpt) == UE_DIR_IN ? IN : OUT;
-   sce = &sc->sc_endpoints[UE_GET_ADDR(endpt)][dir];
-   sce->sc = 0;
-   sce->edesc = 0;
-   sce->iface = 0;
+   sce_cache[endptno] = &sc->sc_endpoints[UE_GET_ADDR(endpt)][dir];
}
 
/* change setting */
err = usbd_set_interface(iface, altno);
-   if (err)
+   if (err) {
+   free(sce_cache, M_TEMP);
return (err);
+   }
 
err = usbd_endpoint_count(iface, &nendpt);
-   if (err)
-   return (err);
+   if (err) {
+   panic("ugen_set_interface: endpoint count failed");
+   }
+
+#if defined(__FreeBSD__)
+   /* destroy the existing devices, we remake the new ones in a moment */
+   ugen_destroy_devnodes(sc);
+#endif
+
+   /* now we can clear the old interface's ugen_endpoints */
+   for (endptno = 0; endptno < nendpt_cache; endptno++) {
+   sce = sce_cache[endptno];
+   sce->sc = 0;
+   sce->edesc = 0;
+   sce->iface = 0;
+   }
+   free(sce_cache, M_TEMP);
+
+   /* set the new interface's ugen_endpoints */
for (endptno = 0; endptno < nendpt; endptno++) {
ed = usbd_interface2endpoint_descriptor(iface,endptno);
endpt = ed->

Re: Panic with ugen

2003-11-27 Thread Martin
On Thu, 2003-11-27 at 15:24, Jay Cornwall wrote:

> Could you try the attached patch, to see if it fixes the panic on the second 
> run of your program?

No success. Still same panic.

Martin


___
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "[EMAIL PROTECTED]"


Re: Panic with ugen

2003-11-27 Thread Daan Vreeken [PA4DAN]
On Thursday 27 November 2003 15:33, Jay Cornwall wrote:
> Daan Vreeken [PA4DAN] wrote:
> > If you have time left, could you perhaps also have a look at kern/51186?
> > I have filed it back in March and it's still open. (Fixes a memory
> > corruption bug in ugen).
>
> I'm not a committer, I'm afraid, so it's probably best to get in touch with
> the code maintainer. ([EMAIL PROTECTED], if the PR is correct - he did reply
> at the bottom of the PR)
>
> But from a brief look at the code, I can't see anything getting past this
> line: if(sce->fill < sce->cur && sce->cur <= sce->fill + count)
>
> If sce->fill is less than sce->cur, then sce->cur can only be <= (sce->fill
> + count) if count is negative. But I haven't studied the code that closely,
> so maybe I'm just missing something obvious. :)
It can. Imagine a buffer of 1000 bytes.
sce->fill=980 and sce->cur=990.
If we have to store 40 bytes, sce->fill (980) is smaller than sce->cur (990).
And sce->cur (990) is smaller or equal to sce->fill + count (980+40=1020).

After that count gets added to sce->cur ( sce->cur=990+40=1030).
Now sce->cur is bigger than sce->limit so this line of code get execute :
sce->cur = sce->ibuf + (sce->limit - sce->cur);
Leading to :
sce->cur = sce->ibuf + ( 1000 - 1030 ) =
  beginning-of-buffer - 30 !
In stead of :
sce->cur = sce->ibuf + ( 1030 - 1000 ) =
   beginning-of-buffer + 30

grtz,
Daan
___
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "[EMAIL PROTECTED]"


Re: Panic with ugen

2003-11-27 Thread Jay Cornwall
Daan Vreeken [PA4DAN] wrote:

If you have time left, could you perhaps also have a look at kern/51186?
I have filed it back in March and it's still open. (Fixes a memory corruption 
bug in ugen).
I'm not a committer, I'm afraid, so it's probably best to get in touch with 
the code maintainer. ([EMAIL PROTECTED], if the PR is correct - he did reply at 
the bottom of the PR)

But from a brief look at the code, I can't see anything getting past this line:
  if(sce->fill < sce->cur && sce->cur <= sce->fill + count)
If sce->fill is less than sce->cur, then sce->cur can only be <= (sce->fill + 
count) if count is negative. But I haven't studied the code that closely, so 
maybe I'm just missing something obvious. :)

--
Cheers,
Jay
http://www.evilrealms.net/ - Systems Administrator & Developer
http://www.imperial.ac.uk/ - 3rd year CS student
___
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "[EMAIL PROTECTED]"


Re: Panic with ugen

2003-11-27 Thread Jay Cornwall
Martin wrote:

panic()
destroy_dev()
ugen_destroy_devnodes()
ugen_set_config()
Could you try the attached patch, to see if it fixes the panic on the second 
run of your program?

--
Cheers,
Jay
http://www.evilrealms.net/ - Systems Administrator & Developer
http://www.imperial.ac.uk/ - 3rd year CS student
Index: sys/dev/usb/ugen.c
===
RCS file: /home/ncvs/src/sys/dev/usb/ugen.c,v
retrieving revision 1.81
diff -u -3 -p -r1.81 ugen.c
--- sys/dev/usb/ugen.c  9 Nov 2003 09:17:22 -   1.81
+++ sys/dev/usb/ugen.c  27 Nov 2003 14:06:25 -
@@ -1014,8 +1014,8 @@ ugen_set_interface(struct ugen_softc *sc
usbd_interface_handle iface;
usb_endpoint_descriptor_t *ed;
usbd_status err;
-   struct ugen_endpoint *sce;
-   u_int8_t niface, nendpt, endptno, endpt;
+   struct ugen_endpoint *sce, **sce_cache;
+   u_int8_t niface, nendpt, nendpt_cache, endptno, endpt;
int dir;
 
DPRINTFN(15, ("ugen_set_interface %d %d\n", ifaceidx, altno));
@@ -1033,30 +1033,47 @@ ugen_set_interface(struct ugen_softc *sc
if (err)
return (err);
 
-#if defined(__FreeBSD__)
-   /* destroy the existing devices, we remake the new ones in a moment */
-   ugen_destroy_devnodes(sc);
-#endif
+   /* store an array of endpoint descriptors to destroy if the interface
+* change succeeds - these aren't available afterwards */
+   sce_cache = malloc(sizeof(struct ugen_endpoint *) * nendpt, M_TEMP,
+   M_WAITOK);
+   nendpt_cache = nendpt;
 
-   /* XXX should only do this after setting new altno has succeeded */
for (endptno = 0; endptno < nendpt; endptno++) {
ed = usbd_interface2endpoint_descriptor(iface,endptno);
endpt = ed->bEndpointAddress;
dir = UE_GET_DIR(endpt) == UE_DIR_IN ? IN : OUT;
-   sce = &sc->sc_endpoints[UE_GET_ADDR(endpt)][dir];
-   sce->sc = 0;
-   sce->edesc = 0;
-   sce->iface = 0;
+   sce_cache[endptno] = &sc->sc_endpoints[UE_GET_ADDR(endpt)][dir];
}
 
/* change setting */
err = usbd_set_interface(iface, altno);
-   if (err)
+   if (err) {
+   free(sce_cache, M_TEMP);
return (err);
+   }
 
err = usbd_endpoint_count(iface, &nendpt);
-   if (err)
+   if (err) {
+   free(sce_cache, M_TEMP);
return (err);
+   }
+
+#if defined(__FreeBSD__)
+   /* destroy the existing devices, we remake the new ones in a moment */
+   ugen_destroy_devnodes(sc);
+#endif
+
+   /* now we can clear the old interface's ugen_endpoints */
+   for (endptno = 0; endptno < nendpt_cache; endptno++) {
+   sce = sce_cache[endptno];
+   sce->sc = 0;
+   sce->edesc = 0;
+   sce->iface = 0;
+   }
+   free(sce_cache, M_TEMP);
+
+   /* set the new interface's ugen_endpoints */
for (endptno = 0; endptno < nendpt; endptno++) {
ed = usbd_interface2endpoint_descriptor(iface,endptno);
endpt = ed->bEndpointAddress;
___
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "[EMAIL PROTECTED]"


Re: Panic with ugen

2003-11-27 Thread Daan Vreeken [PA4DAN]
On Thursday 27 November 2003 01:08, Jay Cornwall wrote:
> > It looks like this:
> >
> > panic()
> > destroy_dev()
> > ugen_destroy_devnodes()
> > ugen_set_config()
>
> Yes, that's the one, and I think I can see why. The existing code fixed
> devfs problems for normal ugen_set_config calls, but doesn't account for
> what happens when an error occurs (which is presumably happening in your
> example program, as you said it gives an error the first time round) - the
> devfs stuff only half completes.
>
> (actually, looking at that error handling code, it doesn't look like it's
> been thought through well anyway - /* XXX should only do this after setting
> new altno has succeeded */ - maybe time to clean this code up?)
>
> After the device endpoints are destroyed (sys/dev/ugen.c:1038), the returns
> on lines 1055 and 1058 need to be covered by a devnode recovery procedure -
> particularly tricky given we just wiped the endpoint descriptors clean.
>
> I'll look at restructuring this code tomorrow, if Bernd doesn't beat me to
> it.
If you have time left, could you perhaps also have a look at kern/51186?
I have filed it back in March and it's still open. (Fixes a memory corruption 
bug in ugen).

grtz,
Daan
___
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "[EMAIL PROTECTED]"


Re: Panic with ugen

2003-11-26 Thread Jay Cornwall
Martin wrote:

It looks like this:

panic()
destroy_dev()
ugen_destroy_devnodes()
ugen_set_config()
Yes, that's the one, and I think I can see why. The existing code fixed devfs 
problems for normal ugen_set_config calls, but doesn't account for what 
happens when an error occurs (which is presumably happening in your example 
program, as you said it gives an error the first time round) - the devfs stuff 
only half completes.

(actually, looking at that error handling code, it doesn't look like it's been 
thought through well anyway - /* XXX should only do this after setting new 
altno has succeeded */ - maybe time to clean this code up?)

After the device endpoints are destroyed (sys/dev/ugen.c:1038), the returns on 
lines 1055 and 1058 need to be covered by a devnode recovery procedure - 
particularly tricky given we just wiped the endpoint descriptors clean.

I'll look at restructuring this code tomorrow, if Bernd doesn't beat me to it.

--
Cheers,
Jay
http://www.evilrealms.net/ - Systems Administrator & Developer
http://www.imperial.ac.uk/ - 3rd year CS student
___
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "[EMAIL PROTECTED]"


Re: Panic with ugen

2003-11-26 Thread Martin
On Wed, 2003-11-26 at 20:44, Jay Cornwall wrote:

> Can you (or someone) provide a backtrace to help trace the cause?

OK. Here is what I found out, I didn't write down any register contents
and stuff, sorry.

It looks like this:

WARNING: Driver mistake: destroy_dev on 114/1
panic: don't do that
db> trace
Debugger()
panic()
destroy_dev()
ugen_destroy_devnodes()
ugen_set_config()
ugen_do_ioctl()
ugenioctl()
spec_ioctl()
spec_vnoperate()
vn_ioctl()
ioctl()
syscall()
Xint0x80_syscall()

This was the latest kernel:
5.2-BETA FreeBSD 5.2-BETA #0: Wed Nov 26 22:24:43 CET 2003 i386

Martin


___
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "[EMAIL PROTECTED]"


Re: Panic with ugen

2003-11-26 Thread Jay Cornwall
Martin wrote:

Run it once, you'll get an error. The second run
will cause a panic.
Additional info:
5.1-CURRENT FreeBSD 5.1-CURRENT #1: Tue Nov 18 00:30:15 CET 2003 i386
usb0:  on uhci0
ugen0: WINBOND W9967CF, rev 1.10/1.10, addr 3
This sounds similar to a bug which (I thought) I fixed earlier in the year 
[multiple calls of USB_SET_CONFIG conflicted with devfs, fixed in 
sys/dev/ugen.c rev 1.71].

I have a modem attached to the USB connector at the moment, so it's a bit 
tricky to test. Can you (or someone) provide a backtrace to help trace the cause?

(and is the panic "don't do that", perchance?)

--
Cheers,
Jay
http://www.evilrealms.net/ - Systems Administrator & Developer
http://www.imperial.ac.uk/ - 3rd year CS student
___
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "[EMAIL PROTECTED]"


Panic with ugen

2003-11-26 Thread Martin

Hi,

I'm still trying to write a webcam application for my
"Creative Videoblaster Webcam Go". I have not much luck, but
I accidently discovered how to cause panic with few lines
of code while using ugen.

Here is the code:

>--<
#include 
#include 
#include 
#include 
#include 
#include 
#include 

#define DEVNAME "/dev/ugen0"

static char dev_name[MAXPATHLEN];

int main(int argc, char *argv[]) {
 
int fdout;
int i;
 
sprintf(dev_name,"%s",DEVNAME);
 
fdout=open(dev_name, O_RDWR, 0);
if (fdout<0) {
  perror("open");
  fprintf(stderr, "Cannot open device: %s\n",DEVNAME);
  exit(-1);
}

i=0;

/* PANIC here in ioctl (during second run) */
if(ioctl(fdout, USB_SET_CONFIG, &i) < 0) {
  perror("ioctl(USB_SET_CONFIG)");
  exit(-1);
}
close(fdout);

return 0;
}
>--<

Run it once, you'll get an error. The second run
will cause a panic.

Additional info:
5.1-CURRENT FreeBSD 5.1-CURRENT #1: Tue Nov 18 00:30:15 CET 2003 i386
usb0:  on uhci0
ugen0: WINBOND W9967CF, rev 1.10/1.10, addr 3

Martin


___
[EMAIL PROTECTED] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "[EMAIL PROTECTED]"