Patch for arch/um/drivers/line.c to fix buffer error
All,
I have been fighting a buffering error when sending large amounts of data out pty devices from UML. I have tried a couple of patches that did not really fix the problem (and were rightly rejected by the group), but have finally found the real bug.
In /arch/um/drivers/line.c there is a function "buffer_data" that is responsible for storing data into the lines ring buffer. The function is "supposed" to return the number of characters actually buffered. Unfortunately, in the case where the buffer wraps, the "len" variable is decremented by "end" before it is used as the return parameter.
Here is a patch that fixes this.
arch/um/drivers/line.c 2004-06-28 01:45:35.000000000 -0400 renamed from arch/um/drivers/line.c 2004-06-28 01:46:21.000000000 -0400 +3 -3
@@ -72,9 +72,9 @@ else { memcpy(line->tail, buf, end); buf += end; - len -= end; - memcpy(line->buffer, buf, len); - line->tail = line->buffer + len; + //len -= end; + memcpy(line->buffer, buf, len - end ); + line->tail = line->buffer + len - end; } return(len);
There is actually another bug in this area, but I am still working on a "good" fix for this. The scenario is this:
o An application opens an output device (ie /dev/serial/13)
o It pushes stuff do the device
o It closes the device
This would be like:
cat /etc/termcap > /dev/serial/13
The problem is that the device will close while there is still data buffered in the ring buffer (and the ring-buffered data will not be transfered to the device). This is a pretty complicated scenario to handle completely, so I have not tried to put together a patch for this (plus it does not directly impact my customers at this point). In theory, handling all of these situations might require changes to how the ring-buffering is implemented (or maybe can the ring-buffer be eliminated all together). Suggestions on how to implement this would be appreciated.
--------------------------------------------------------------------
Doug Dumitru 800-470-2756 (610-237-2000) EasyCo LLC [email protected] http://easyco.com
--------------------------------------------------------------------
-------------------------------------------------------
This SF.Net email sponsored by Black Hat Briefings & Training.
Attend Black Hat Briefings & Training, Las Vegas July 24-29 - digital self defense, top technical experts, no vendor pitches, unmatched networking opportunities. Visit www.blackhat.com
_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel
Alle 08:06, lunedì 28 giugno 2004, Doug Dumitru ha scritto:
Quoted 22 lines
All,
I have been fighting a buffering error when sending large amounts of data out pty devices from UML. I have tried a couple of patches that did not really fix the problem (and were rightly rejected by the group), but have finally found the real bug.
In /arch/um/drivers/line.c there is a function "
buffer_data" that is responsible for storing data into the lines ring buffer. The function is "supposed" to return the number of characters actually buffered. Unfortunately, in the case where the buffer wraps, the "len" variable is decremented by "end" before it is used as the return parameter.Here is a patch that fixes this.
---
linux-2.4.26-um/arch/um/drivers/line.c2004-06-28
01:46:21.000000000 -0400
+++linux-2.4.23-um/arch/um/drivers/line.c2004-06-28
01:45:35.000000000 -0400arch/um/drivers/line.c 2004-06-28 01:45:35.000000000 -0400 renamed from arch/um/drivers/line.c 2004-06-28 01:46:21.000000000 -0400 +0
-0@@ -72,9 +72,9 @@ else { memcpy(line->tail, buf, end);
arch/um/drivers/line.c 2004-06-28 01:45:35.000000000 -0400 line 80
Quoted 10 lines
arch/um/drivers/line.c 2004-06-28 01:45:35.000000000 -0400 renamed from arch/um/drivers/line.c 2004-06-28 01:46:21.000000000 -0400 +3
-3buf += end; - len -= end; - memcpy(line->buffer, buf, len); - line->tail = line->buffer + len; + //len -= end; + memcpy(line->buffer, buf, len - end ); + line->tail = line->buffer + len - end; } return(len);
Since it was not collected in 2.4.26-2um, I'm collecting it for my 2.4 and 2.6 patchsets. I would like a test case for this, if possible (i.e. the patch looks right, but I don't notice any problem on consoles, which are handled by the same code).
Quoted 10 lines
There is actually another bug in this area, but I am still working on a "good" fix for this. The scenario is this:
o An application opens an output device (ie
/dev/serial/13)
o It pushes stuff do the device
o It closes the deviceThis would be like:
cat
/etc/termcap>/dev/serial/13
Without DevFS it maps to cat /etc/termcap > /dev/ttyS0, right? I've just checked that this *should* be true (there seems to be a UML-specific symlink from /dev/serial to /dev/tts).
Quoted 3 lines
The problem is that the device will close while there is still data buffered in the ring buffer (and the ring-buffered data will not be transfered to the device).
Ehr, probably I don't understand you well, because it seems me that the fix would be as easy as calling flush_buffer() during line_close(). Or not?
I guess that there are some synchronization issue (there is an interrupt doing the same thing, but it's disable in line_disable) - and there is some refcount kludge commented about in line_close(). But that's the idea, right? However, for now I didn't handle this thing. So please remind this me.
Quoted 7 lines
This is a pretty complicated scenario to
handle completely, so I have not tried to put together a patch for this (plus it does not directly impact my customers at this point). In theory, handling all of these situations might require changes to how the ring-buffering is implemented (or maybe can the ring-buffer be eliminated all together). Suggestions on how to implement this would be appreciated.
Signature · 14 lines
Paolo Giarrusso, aka Blaisorblade Linux registered user n. 292729 ------------------------------------------------------- SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media 100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33 Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift. http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285 _______________________________________________ User-mode-linux-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel