tech-multimedia archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Problems with uvideo(4) on a Asus Eee PC 900



Hi Leonardo (and tech-multimedia),

I've cooked up a possible cause of this fault, although it's speculative
and a long shot. Many thanks to Leonardo for providing a crash dump,
it'd be great if you could test the attached patch. Dump is also
available if other developers want to look at it

Unfortunately the backtrace for the crash is just an invalid call to
0x7f4, but after an evening of inspecting random parts of the stack and
other pain I get this: if you look at the dmesg mailed earlier, you see:

uvideo_stream_init: probelen=34074

This is supposed to be the length of the probe data uvideo
sends/receives from the webcam, but the huge length is invalid. In
uvideo_set_format, uvideo_stream_probe is supposed to store the
default/max configuration in a uvideo_probe_and_commit_data_t struct.
However, uvideo_stream_probe_and_commit starts a usb transfer to copy
into that struct, with the large probelen value as the transfer size.

That leads to a) the SHORT_XFER error, and b) a stack overflow when usb
writes the transfered data back, more than uvideo_probe_commit_data_t
can hold. Eventually uvideo_set_format returns onto the overflow data,
and explodes.

So why is probelen invalid - looking at uvideo_stream_init:

        uWord len;
[...]
        err = uvideo_stream_probe(vs, UR_GET_LEN, len);
[...]
        vs->vs_probelen = UGETW(len);

Where the last argument to uvideo_stream_probe should be a pointer to
receive the length. The code ends up using an uninitialized variable as
vs_probelen. To fix this, I propose the attached patch, which correctly
passes the address of len rather than len itself.

-- 
Thanks,
Jeremy

Index: uvideo.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/uvideo.c,v
retrieving revision 1.21.10.4
diff -u -p -r1.21.10.4 uvideo.c
--- uvideo.c    2 Feb 2009 20:39:46 -0000       1.21.10.4
+++ uvideo.c    9 Feb 2009 08:41:14 -0000
@@ -1070,16 +1070,20 @@ uvideo_stream_init(struct uvideo_stream 
        /* Initialize probe and commit data size.  This value is
         * dependent on the version of the spec the hardware
         * implements. */
-       err = uvideo_stream_probe(vs, UR_GET_LEN, len);
+       err = uvideo_stream_probe(vs, UR_GET_LEN, &len);
        if (err != USBD_NORMAL_COMPLETION) {
                DPRINTF(("uvideo_stream_init: "
                         "error getting probe data len: "
                         "%s (%d)\n",
                         usbd_errstr(err), err));
                vs->vs_probelen = 26; /* conservative v1.0 length */
-       } else {
+       } else if (UGETW(len) <= sizeof(uvideo_probe_and_commit_data_t)) {
                DPRINTFN(15,("uvideo_stream_init: probelen=%d\n", UGETW(len)));
                vs->vs_probelen = UGETW(len);
+       } else {
+               DPRINTFN(15,("uvideo_stream_init: device returned invalid probe"
+                               " len %d, using default\n", UGETW(len)));
+               vs->vs_probelen = 26;
        }
        
        return USBD_NORMAL_COMPLETION;

Attachment: signature.asc
Description: OpenPGP digital signature



Home | Main Index | Thread Index | Old Index