-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 tags 693663 + confirmed thanks
Hello Ben, Thank you for your work to document all these issues. If there would be an alternative, I would not hesitate to get rid of this module. Unfortunately, there isn't. This module was originally written for Linux 2.6.8, including (almost) all the bugs, by the manufacturer of the hardware. I adjusted it so it uses dkms and can compile on newer kernels. I noticed the bad coding, but didn't start fixing all the bugs so far. If I would give priority to this package, I would try to rewrite it so that it conforms to v4l2. As it is now, the only reasonable (and documented) way to use it is through a non-free library provided by the hardware producer. Now that I think about it, this means it really belongs in contrib. I am using this module, and I (manually) make sure that I don't have any races. I agree that this should be done by technical means. Also, I think I see some of the other problems you describe (in particular the memory leakage). I'm interested to get this fixed, but not so interested that I've started doing it already. Summarizing: - - I agree that this module should not be in a stable release. - - I think it does deserve to be in unstable, because people with this hardware want to use it on Debian; this support is better than no support. - - I very much invite people to help and fix the issues. - - I'll make some changes to the description to make clear that the module is buggy. Thanks, Bas On 19-11-12 04:20, Ben Hutchings wrote: > Package: pvcam-dkms Version: 4.1.0-2 Severity: grave > > There is a complete lack of locking, memory barriers or anything > that could protect against races: > > - Two tasks calling device_open() on the same camera at the same > time may race and succeed, which violates the assumption that: /* > With the Linux driver - each camera */ /* is totally exclusive use! > */ - Two tasks may race in device_ioctl() on the same file. - The > ISR is not synchronised with the tasks requesting I/O. > > A failed device_ioctl() may mark the camera closed, but doesn't > prevent the same file handle from being used, so again there can be > two file handles for the same camera. > > Various functions can return positive numbers (= success) for > errors. > > device_ioctl() doesn't consistently check whether copy_from_user() > or copy_to_user() succeeded. > > pvcam_create_buffer() doesn't map memory correctly: virt_to_bus() > doesn't generally work for PCI devices, and won't translate NULL to > 0, so in case memory allocation fails it will not abort. > > pvcam_write_read() assumes little-endian byte order on the host. > > loadPCIflash() reads directly from user addresses without using > copy_from_user(). > > Unimplemented functions quietly do nothing instead of returning a > meaningful error. > > In most error cases that the driver actually bothers to check for, > it immediately returns without clearing up resources that have been > allocated. This can result in a crash or resource leak. > > Ben. > > -- System Information: Debian Release: wheezy/sid APT prefers > stable-updates APT policy: (500, 'stable-updates'), (500, > 'proposed-updates'), (500, 'unstable'), (500, 'stable'), (1, > 'experimental') Architecture: i386 (x86_64) Foreign Architectures: > amd64 > > Kernel: Linux 3.2.0-4-amd64 (SMP w/2 CPU cores) Locale: > LANG=en_GB.utf8, LC_CTYPE=en_GB.utf8 (charmap=UTF-8) Shell: /bin/sh > linked to /bin/dash > > Versions of packages pvcam-dkms depends on: ii dkms 2.2.0.3-1.2 > > Versions of packages pvcam-dkms recommends: pn libpvcam <none> > > pvcam-dkms suggests no packages. -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) iEYEARECAAYFAlC3SK8ACgkQFShl+2J8z5WOsACfV2hhEWamjOPcO9+E2ssA8mDM V2EAoJZ+1tgbVlCQI75wyZN5qfF2Fov9 =kFtF -----END PGP SIGNATURE----- -- To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
