Bug 202612 - another libteken assert() fail on teken.c line 231
Summary: another libteken assert() fail on teken.c line 231
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Ed Schouten
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-08-24 07:27 UTC by kcwu
Modified: 2015-09-14 09:31 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description kcwu 2015-08-24 07:27:25 UTC
Because syscons (a kernel driver) uses libteken, an assertion failure in libteken would result in kernel panic.

How to reproduce:
To reproduce the assertion reliably, I modified src/sys/teken/stress/teken_stress.c to take input from stdin.
$ echo -e '\e[?6h\e[5r\e[144441344d' | ./teken_stress
Assertion failed: (t->t_cursor.tp_row >= t->t_originreg.ts_begin), function teken_input_char, file /usr/src/sys/teken/teken.c, line 231.
Abort trap (core dumped)

Looks like this is very similar to https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=202540

This issue is found by afl-fuzz
Comment 1 commit-hook freebsd_committer 2015-08-24 07:50:24 UTC
A commit references this bug:

Author: ed
Date: Mon Aug 24 07:49:27 UTC 2015
New revision: 287098
URL: https://svnweb.freebsd.org/changeset/base/287098

Log:
  Sync HPA and VPA implementations with CUP.

  After fixing the 16-bits integer arithmetic overflow in 286981, we
  should also make sure to fix the VPA sequence. Bring HPA and VPA in sync
  with how we now implement CUP.

  PR:		202612
  Reported by:	kcwu csie org
  MFC after:	1 month

Changes:
  head/sys/teken/teken_subr.h
Comment 2 Ed Schouten freebsd_committer 2015-08-24 07:51:06 UTC
Ah, good catch. Fixed!
Comment 3 Mark Linimon freebsd_committer 2015-09-01 17:47:44 UTC
Fix already committed.
Comment 4 kcwu 2015-09-01 18:00:11 UTC
I guess it remain open because ed@ want to track MFC?
Comment 5 Ed Schouten freebsd_committer 2015-09-04 10:41:22 UTC
Yes, that was the intent. GNATS used to have this 'PATCHED' state, but it looks like Buganizer does not. That's why I left it as opened. But it doesn't matter that much. I'll patiently wait until I get the MFC reminders. :-)
Comment 6 commit-hook freebsd_committer 2015-09-14 09:13:12 UTC
A commit references this bug:

Author: ed
Date: Mon Sep 14 09:12:30 UTC 2015
New revision: 287776
URL: https://svnweb.freebsd.org/changeset/base/287776

Log:
  MFC r286798 and r286827:

    Stop parsing digits if the value already exceeds UINT_MAX / 100.

    There is no need for us to support parsing values that are larger than
    the maximum terminal window size. In this case that would be the maximum
    of unsigned short.

    The problem with parsing larger values is that they can cause integer
    overflows when adjusting the cursor position, leading to all sorts of
    failing assertions.

  MFC r286981 and r287098:

    Don't truncate cursor arithmetic to 16 bits.

    When updating the row number when the cursor position escape sequence is
    issued, we should make sure to store the intermediate result in a 32-bit
    integer. If we fail to do this, the cursor may be set above the origin
    region, which is bad.

    This could cause libteken to crash when INVARIANTS is enabled, due to
    the strict set of assertions that libteken has.

  PR:		202326, 202540, 202612
  Submitted by:	kwcu csie org

Changes:
_U  stable/10/
  stable/10/sys/teken/teken.c
  stable/10/sys/teken/teken_subr.h
Comment 7 commit-hook freebsd_committer 2015-09-14 09:31:17 UTC
A commit references this bug:

Author: ed
Date: Mon Sep 14 09:31:02 UTC 2015
New revision: 287777
URL: https://svnweb.freebsd.org/changeset/base/287777

Log:
  MFC r286798 and r286827:

    Stop parsing digits if the value already exceeds UINT_MAX / 100.

    There is no need for us to support parsing values that are larger than
    the maximum terminal window size. In this case that would be the maximum
    of unsigned short.

    The problem with parsing larger values is that they can cause integer
    overflows when adjusting the cursor position, leading to all sorts of
    failing assertions.

  MFC r286981 and r287098:

    Don't truncate cursor arithmetic to 16 bits.

    When updating the row number when the cursor position escape sequence is
    issued, we should make sure to store the intermediate result in a 32-bit
    integer. If we fail to do this, the cursor may be set above the origin
    region, which is bad.

    This could cause libteken to crash when INVARIANTS is enabled, due to
    the strict set of assertions that libteken has.

  PR:		202326, 202540, 202612
  Submitted by:	kwcu csie org

Changes:
_U  stable/9/
_U  stable/9/sys/
  stable/9/sys/teken/teken.c
  stable/9/sys/teken/teken_subr.h