ftdi_sio.c triggers size overflow in kfifo_unused

[7LL]

With grsecurity-3.0-3.2.55-201403300851.patch (and at least since 20140224) I'm running into this problem when writing a burst of data to a device handled by drivers/usb/serial/ftdi_sio.c:

Code: Select all

kern.err 2000-01-07T01:54:43.104074+01:00 kernel [   99.038830] PAX: size overflow detected in function kfifo_unused kernel/kfifo.c:35 cicus.402_13 min, count: 2
kern.warning 2000-01-07T01:54:43.106584+01:00 kernel [   99.039344] Pid: 161, comm: test-burst Not tainted 3.2.55-grsec-TEST #1
kern.warning 2000-01-07T01:54:43.106793+01:00 kernel [   99.039630] Call Trace:
kern.warning 2000-01-07T01:54:43.106935+01:00 kernel [   99.039917]  [<000704dd>] report_size_overflow+0x21/0x2b
kern.warning 2000-01-07T01:54:43.107071+01:00 kernel [   99.040023]  [<0002c7dd>] kfifo_unused+0x4a/0x55
kern.warning 2000-01-07T01:54:43.107265+01:00 kernel [   99.040023]  [<0002cdbf>] __kfifo_in+0x14/0x32
kern.warning 2000-01-07T01:54:43.107436+01:00 kernel [   99.040023]  [<001e2c10>] usb_serial_generic_write+0x76/0x9b
kern.warning 2000-01-07T01:54:43.107573+01:00 kernel [   99.040023]  [<001e10cf>] serial_write+0x5e/0x6e
kern.warning 2000-01-07T01:54:43.107709+01:00 kernel [   99.040023]  [<00173742>] n_tty_write+0x26d/0x2dc
kern.warning 2000-01-07T01:54:43.107888+01:00 kernel [   99.040023]  [<00016814>] ? try_to_wake_up+0x95/0x95
kern.warning 2000-01-07T01:54:43.108024+01:00 kernel [   99.040023]  [<001734d5>] ? copy_from_read_buf+0x1e9/0x1e9
kern.warning 2000-01-07T01:54:43.108160+01:00 kernel [   99.040023]  [<001704d9>] tty_write+0x17a/0x1eb
kern.warning 2000-01-07T01:54:43.108295+01:00 kernel [   99.040023]  [<001734d5>] ? copy_from_read_buf+0x1e9/0x1e9
kern.warning 2000-01-07T01:54:43.108463+01:00 kernel [   99.040023]  [<0017035f>] ? tty_write_lock+0x3e/0x3e
kern.warning 2000-01-07T01:54:43.108600+01:00 kernel [   99.040023]  [<0006b5e6>] vfs_write+0xcb/0xf1
kern.warning 2000-01-07T01:54:43.108755+01:00 kernel [   99.040023]  [<0006b6a3>] sys_write+0x37/0x60
kern.warning 2000-01-07T01:54:43.108891+01:00 kernel [   99.040023]  [<002b81f1>] syscall_call+0x7/0xb
kern.warning 2000-01-07T01:54:43.109028+01:00 kernel [   99.040023]  [<002b8211>] ? restore_all_pax+0xc/0xc


This is the code that overflows:

Code: Select all

static inline unsigned int kfifo_unused(struct __kfifo *fifo)
{
        return (fifo->mask + 1) - (fifo->in - fifo->out);
}


I did some debugging to find the values that triggers this::

Code: Select all

mask=4095
in=9646
out=5550


With GCC 4.7.2 (which I used before) the assembler output is:

Code: Select all

   0:   55                      push   %ebp
   1:   8b 50 08                mov    0x8(%eax),%edx
   4:   03 50 04                add    0x4(%eax),%edx
   7:   89 e5                   mov    %esp,%ebp
   9:   5d                      pop    %ebp
   a:   42                      inc    %edx
   b:   2b 10                   sub    (%eax),%edx
   d:   89 d0                   mov    %edx,%eax
   f:   c3                      ret


Trying to parse this with my limited x86 asm knowledge, I think this translates to computing: mask + out + 1 - in = 4095 + 5550 + 1 - 9646 = 0 with no wrapping anywhere. Everything is fine.

With GCC 4.8.2 (which I now use) the assembler output is:

Code: Select all

   0:   55                      push   %ebp
   1:   8b 50 04                mov    0x4(%eax),%edx
   4:   03 50 08                add    0x8(%eax),%edx
   7:   2b 10                   sub    (%eax),%edx
   9:   89 e5                   mov    %esp,%ebp
   b:   5d                      pop    %ebp
   c:   8d 42 01                lea    0x1(%edx),%eax
   f:   c3                      ret


This seems to translate to computing: out + mask - in + 1 = 5550 + 4095 - 9646 + 1 = 0, but this will underflow and wrap at the subtraction, before wrapping back to 0 after adding the last one.

I can workaround this issue by removing kfifo_unused from the hash table, and marking it with the __intentional_overflow(-1) attribute, but I'm unsure if this really is the proper way to fix this.

Let me know if I can supply you with any more information.

[PaX Team]

thanks for all the investigation, it was really helpful. what happens here is one of those special cases where gcc introduces an intentional integer overflow and the way the size overflow plugin deals with constants it encounters during stmt duplication. the gcc induced overflow wouldn't be a problem per se as it works just fine with the double wide integer type (u64 in this case) as well, however constant handling had been problematic in the past so we stopped duplicating such stmts long ago and instead insert an overflow check just beforehand. this check is what triggered for you on the intermediate value that did overflow and would not be representable in an u32. as for the way out of this, i guess Emese will have to look into it to see whether the old reasons we had for this constant handling still exist or perhaps we can do something smarter these days. in the meantime the big hammer of the intentional_overflow attribute can be used to disable overflow checking here.

[ephox]

Thanks for the report. It will be fixed in the next PaX version.