After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 108066 - Bad integer cast = broken read(2) calls
Bad integer cast = broken read(2) calls
Status: RESOLVED FIXED
Product: vte
Classification: Core
Component: general
0.10.x
Other Linux
: Normal major
: ---
Assigned To: VTE Maintainers
Nalin Dahyabhai
: 107992 108660 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2003-03-11 02:28 UTC by Javier Kohen
Modified: 2004-12-22 21:47 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Javier Kohen 2003-03-11 02:28:41 UTC
Some ncurses enabled programs were causing hangs in gnome-terminal. I found
the cause to be an invalid call to read(2), more
+specifically, the length being passed was 4294965249. The man page states
that anything besides SSIZE_MAX (defined as 32767
+here) causes undefined behavior.
Thanks to the error message that was being -infinitely- logged I traced the
problem to the vte_terminal_io_read function in
+vte.c.
The exact problem was that the value returned from _vte_buffer_length(...)
was larger than sizeof (buf), and being both size_t+(unsigned) values the
result of the substraction ended up being always positive (and quite
large). I'm almost sure my patch
+fixes this issue, but honestly I couldn't even compile it because I don't
have all of the required development packages
+installed.
                                                                          
                                                   
diff -ur vte-0.10.26.orig/src/vte.c vte-0.10.26/src/vte.c
--- vte-0.10.26.orig/src/vte.c  2003-03-09 22:41:55.000000000 -0300
+++ vte-0.10.26/src/vte.c       2003-03-09 22:51:21.000000000 -0300
@@ -7318,9 +7318,15 @@
        /* Read some data in from this channel. */
        bcount = 0;
        if (condition & G_IO_IN) {
-               bcount = sizeof(buf) -
-                        _vte_buffer_length(terminal->pvt->incoming);
-               bcount = read(fd, buf, MAX(bcount, sizeof(buf) / 2));
+               size_t incoming_len;
+
+               incoming_len = _vte_buffer_length(terminal->pvt->incoming);
+               if (incoming_len > sizeof (buf) / 2)
+                       bcount = sizeof (buf) / 2;
+               else
+                       bcount = sizeof (buf) - incoming_len;
+               g_assert(bcount > 0);
+               bcount = read(fd, buf, bcount);
        }
        eof = FALSE;
        if (condition & G_IO_HUP) {


Sorry, but after I created this patch I realized that the assert would be
better expressed as:
g_assert(sizeof (buf) / 2 <= bcount && bcount <= sizeof (buf));
g_assert(sizeof (buf) <= SSIZE_MAX); /* Source: READ(2) -- 1997-07-12 --
Linux 2.0.32 -- Linux Programmer's Manual */
Comment 1 Christian Marillat 2003-03-12 15:31:29 UTC
*** Bug 107992 has been marked as a duplicate of this bug. ***
Comment 2 Ondřej Surý 2003-03-18 08:32:30 UTC
There is buffer sized with VTE_INPUT_CHUNK_BUFFER, it's size is
0x1000.  There is also input data buffer where are pipe input data stored.

Real problem is hidden in function _vte_iso2022_substitute which is
called from vte_terminal_process_incoming(), which is called from
vte_terminal_io_read() when enough data is gathered.

_vte_iso2022_substitute check if it has enough data to transform it
properly according some rules (I still don't understand fully all
functionality and data structures, so I will describe it briefly) and
when it hasn't it returns -1 and no data is processed and input buffer
is left untouched.  It could work OK on small data, but it seems that
new kernel (2.5.x) feeds data faster then authors expected and we end
up in situation when input buffer is full (ie. bigger then
VTE_INPUT_CHUNK_SIZE) and _vte_iso2022_substitute complains that it
has to read another data to complete transformations.  This creates
infinite loop in vte_terminal_io_read() when we are calling read()
with negative buffer size (it returns -1 and sets EFAULT errno).

Quick workaround is to set VTE_INPUT_CHUNK_SIZE to something bigger, I
tried 0xf000 and it worked for me, but of course this is not fix, but
just ugly hack which would only trigger this bug in the other future.

Proper fix would need to rewrite those routines to be able to process
data as far as they can and leave only unprocessable data in input
buffer. (This could also lead to DOS if somebody could arrange to send
unprocessable data longer then input buffer, but I really don't know
if this is possible...?)

Those asserts in patch up here is right, but it doesn't solve REAL
problem.
Comment 3 Nalin Dahyabhai 2003-04-21 21:58:02 UTC
Fixed in CVS.
Comment 4 Nalin Dahyabhai 2003-04-22 21:47:17 UTC
*** Bug 108660 has been marked as a duplicate of this bug. ***