Page 1 of 1

PAX: size overflow detected in function n_tty_receive_buf_co

PostPosted: Wed Dec 16, 2015 5:48 pm
by mathias
Hello,

Just compiled Linux 4.3.3 with grsecurity-3.1-4.3.3-201512151908.patch System boots, but when I try to login either locally or via SSH the system crashes with the error

Code: Select all
PAX: size overflow detected in function n_tty_receive_buf_common drivers/tty/n_tty.c:1706 cicus.492_529 min, count: 104, decl: read_head; num: 0; context: n_tty_data;


I've uploaded a photo of the complete error and call trace to my site: https://calenhad.com/grsec-error.jpg

Re: PAX: size overflow detected in function n_tty_receive_bu

PostPosted: Wed Dec 16, 2015 7:31 pm
by ephox
Thanks for the report, it will be fixed in the next grsec patch.

Re: PAX: size overflow detected in function n_tty_receive_bu

PostPosted: Mon Apr 25, 2016 7:16 pm
by marcan
This is not fixed. I just got the same thing, running 4.4.8-hardened (Gentoo), when pasting a very long line into a terminal emulator:
Code: Select all
PAX: size overflow detected in function n_tty_receive_buf_common drivers/tty/n_tty.c:1726 cicus.417_504 min, count: 90, decl: read_head; num: 0; context: n_tty_data;


The line that crashes is:
Code: Select all
room--;


In fact, the grsec patch is broken here. It does:
Code: Select all
-   int room, n, rcvd = 0, overflow;
+   size_t room, n, rcvd = 0, overflow;


but room is expected to go negative (as the code immediately checks for zero or negative after the decrement) - but size_t is unsigned, so the overflow plugin rightly catches it and panics the kernel. You probably want ssize_t here.

Also, overflow is a boolean, so using size_t there is arguably wrong.

Re: PAX: size overflow detected in function n_tty_receive_bu

PostPosted: Mon Apr 25, 2016 8:49 pm
by PaX Team
thanks for the report, however your analysis is wrong, room represents a non-negative quantity, it's just an 'optimization' that abuses signed integer arithmetic in a corner case (when room as originally computed is 0) that we managed to overlook. the correct fix isn't to introduce even more integer types but this:
Code: Select all
--- a/drivers/tty/n_tty.c        2016-02-18 21:48:22.542295565 +0100
+++ b/drivers/tty/n_tty.c 2016-04-26 02:25:51.258630991 +0200
@@ -1723,15 +1723,16 @@
                room = N_TTY_BUF_SIZE - (ldata->read_head - tail);
                if (I_PARMRK(tty))
                        room = (room + 2) / 3;
-               room--;
-               if (room <= 0) {
+               if (room <= 1) {
                        overflow = ldata->icanon && ldata->canon_head == tail;
-                       if (overflow && room < 0)
+                       if (overflow && room == 0)
                                ldata->read_head--;
                        room = overflow;
                        ldata->no_room = flow && !room;
-               } else
+               } else {
+                       room--;
                        overflow = 0;
+               }

                n = min(count, room);
                if (!n)


as for using a size_t for a boolean, it doesn't matter, in C any integer type has enough range to express a bool value (the original code didn't use bool either but int).