[uml-devel] Patch for arch/um/drivers/line.c to fix buffer error

[uml-devel] Patch for arch/um/drivers/line.c to fix buffer error

[Doug Dumitru]

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.c      2004-06-28 
01:46:21.000000000 -0400
+++ linux-2.4.23-um/arch/um/drivers/line.c      2004-06-28 
01:45:35.000000000 -0400
@@ -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       doug@easyco.com  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
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Re: [uml-devel] Patch for arch/um/drivers/line.c to fix buffer error

[BlaisorBlade]

Alle 08:06, lunedì 28 giugno 2004, Doug Dumitru ha scritto:
> 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.c      2004-06-28
> 01:46:21.000000000 -0400
> +++ linux-2.4.23-um/arch/um/drivers/line.c      2004-06-28
> 01:45:35.000000000 -0400
> @@ -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);
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).

> 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

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).

> 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.

> 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.
-- 
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
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel