On Fri, 4 Oct 2013, Markus Rechberger wrote: > > The biggest problem is that your proc_alloc_memory() routine doesn't > > call usbfs_increase_memory_usage(). Without that, there's nothing to > > prevent a user from allocating all the available kernel memory. > > > > only root is supposed to have raw USB access, Not true at all. Any user may be allowed to access a USB device, depending on the permissions for the usbfs device file. > which limit would you suggest? The limit is already in place. All you have to do is call the routine to make sure that you don't exceed the limit. Check the other places where usbfs_increase_memory_usage() is used and you'll see how it works. > >> Version 0.4: > >> * overriding SG transfers once memory mapped buffers are allocated for > >> BULK > >> * adding pgprot_noncached to ensure that the IO memory is not cached > >> (thanks to Ming Lei for mentioning that) > > > > I don't understand this. Why shouldn't the buffer memory be cached? > > Won't uncached buffers be a lot slower? > > > > I was only testing reading the data so I didn't see any caching > effects since I don't have a device or driver which I can send a lot > data out. > As far as I understand pgprot_noncached would only be required when > sending data (writing data to the DMA'able buffer). > > This question is still open. The buffer should be cached. The userspace program will have to make sure that it doesn't try to access the buffer while DMA is in progress. As long as that restriction is obeyed, the USB core will take care of mapping the buffer for DMA (which flushes the cache). > > As I understand it, you wrote this in order to solve problems where > > memory couldn't be allocated for USB transfers. If the system is > > running short on memory, won't your USBDEVFS_ALLOC_MEMORY ioctl also > > fail? > > > > No, the buffers will be kept in a list and re-used (aside of that they > are also kept in a list in devio.c and re-used). When the buffers are > allocated initially there's no chance to fail this process during > run-time. Why not? What if the memory is already almost full? The call to alloc_pages_exact() could fail. I agree this is less likely than a failure with the current system, but it is still possible. > please note the transfer buffer are allocated and freed permanently > during run-time with the old mechanism, this mechanism usually fails > during run-time on low end systems. It's true that the existing system allocates and deallocates the buffer for each transfer. With your patch, the buffer is allocated only once. In addition, the same buffer is used in userspace and in the kernel, removing the need to allocate two buffers and copy everything over. This will be a significant saving of both time and space. By the way, have you considered that the user program might want the I/O transfer to start at some position other than the beginning of the buffer? Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
