[Mono-bugs] [Bug 375580] New: Buffer overrun plus infinite loop on partial write to serial port - patch included
bugzilla_noreply at novell.com
bugzilla_noreply at novell.com
Mon Mar 31 15:46:40 EDT 2008
Summary: Buffer overrun plus infinite loop on partial write to
serial port - patch included
Product: Mono: Class Libraries
Priority: P5 - None
AssignedTo: miguel at novell.com
ReportedBy: avillaci at ceibo.fiec.espol.edu.ec
QAContact: mono-bugs at lists.ximian.com
Found By: Customer
Created an attachment (id=205123)
Fix buffer overrun and endless loop on partial write
This patch fixes two bugs in the write_serial() function in the Mono POSIX
>From what I understand of the Mono sources, managed code in SerialPortStream
checks constraints on the data to be sent to the serial port, and then calls
the following function in the POSIX helper:
int write_serial (int fd, guchar *buffer, int offset, int count, int timeout)
The 'offset' parameter is supposed to contain the byte offset from which the
data to be written starts (relative to 'buffer'), and 'count' is supposed to
be the number of bytes to write.
The first bug is at line 98 of support/serial.c:
> n = count - offset;
This subtraction is, IMHO, pointless in this function. Later in the function,
'n' is treated like a count of remaining bytes to send, but this subtraction
treats 'count' as the length of the entire buffer, which is inconsistent with
later treatement of the buffer. The subtraction is a noop when offset is 0, but
breaks when offset > count (for example, sending 20 bytes from offset 40 of a
100-byte buffer), which is allowed by the checks in managed code. Since 'n'
is of type guint32, the subtraction results in a very large unsigned integer,
which immediately causes a buffer overflow when writing to the serial port.
The second bug is at lie 116 of support/serial.c:
> t = write(fd, buffer + offset, count);
In case where write() informs of a partial read (when t > 0 && t < count), this
code is incorrect, because 'count' is invariant inside the write() loop. As
evidenced at line 127, 'n' is decremented by the amount reported by write(), so
'n' is intended to hold the amount of remaining bytes to write. In case of a
short write, the write is retried with 'count' bytes as before, but with a
modified 'offset'. Depending of the values of 'offset' and 'count', this will
eventually write data past the end of 'buffer' to the serial port. In
particular, with 'offset' = 0 and 'count' equal to the full length of 'buffer',
the second write after the first short write WILL read past the end of
'buffer'. Meanwhile, 'n' keeps being decremented. Since the second read uses
'count' > 'n', a full write (or a very big short write) will probably cause 't'
to be greater than 'n'.
Unless the writes happen to reduce 'n' exactly to 0, the subtraction eventually
wraps around (since guint32 is unsigned) into a very large positive integer,
which results in an endless loop of writes of data past the end of buffer, and
an eventual program crash.
This patch fixes both issues by removing the subtraction of 'offset' and
assigning 'n' to the value of 'count' directly, and by using 'n' instead of
'count' as the count of remaining bytes to write, as intended.
This patch applies against mono 1.2.6 .
Configure bugmail: https://bugzilla.novell.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
More information about the mono-bugs