Oliver Neukum <oliver@xxxxxxxxxx> writes: > On Friday 15 February 2013 08:53:28 Bjørn Mork wrote: >> Oliver Neukum <oliver@xxxxxxxxxx> writes: > >> > We have to let user space recover. To do so we need to indicate when >> > exactly we dropped data. >> >> The problem with that is that this is likely to happen when a client >> just doesn't care. It will just continue happily ignoring the read data, >> writing new commands or whatever. Then the *next* client opening the >> file for reading will see this error. > > Well, this may be a separate bug. Should the buffer be cleared when > we run out of openers? No. A valid use case, currently working just fine, is using e.g. shell commands to sequentially write and read, closing the file inbetween. I don't see any reason to suddenly break that. It is a userspace visible ABI. >> Sorry, but I find that still too fragile. To know whether this solves >> the problem I will have to check every possible site where desc->rerr >> can be reset, including those we may add in the future. I trust that > > But clearing desc->rerr without at least clearing the buffer is a bug. > But we can have a separate flag. Yes, I agree that it is a bug. My hesitation is because it isn't an *obvious* bug. It is so much easier to miss than a simple test for remaining buffer space immediately preceding the buffer copy. But maybe I am being too paranoid now. Don't know how to deal with the fact that we didn't catch the obvious bug before... :-) >> you have verified that this works now, but it is still too easy to break >> without noticing. >> >> There should be an explicit test for buffer space here. Indirect >> testing via some other variable is risky IMHO. > > Very well. Was the initial assumption that a single response cannot > be longer than wMaxCommand valid in practice? Does it matter? FWIW I haven't seen any device with longer responses, but I haven't tried hard to trigger it either. The devices I have all have a fairly large wMaxCommand compared to most of the messagees they send. The smallest I have seen is 1536 for a MBIM device, which is still more than enough for normal MBIM messages. And that device can of course use the MBIM fragmentation support if it needs to send longer messages. If a device lies about the real wMaxCommand then it just creates problems for itself by making us read only part of the message. This is not a problem for the driver as far as I can see. Bjørn -- 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
