Project

General

Profile

Actions

Bug #10565

closed

man -w segfaults on .IX macro

Added by Tim Mooney about 3 years ago. Updated 4 months ago.

Status:
Closed
Priority:
Normal
Category:
manpage - manual pages
Start date:
2019-03-18
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage
Gerrit CR:

Description

I'm attempting to rebuild the 'whatis' database on my OpenIndiana hipster system.

I've discovered a particular man page for some local software (part of a perl module I installed) that causes 'man -w' to segfault:

$ mkdir -p /tmp/test/share/man/man3
$ cp -p /local/share/man/man3/Font\:\:TTF\:\:AATutils.3 /tmp/test/share/man/man3/
$ man -M /tmp/test/share/man -w
Segmentation Fault

I'll attach the man page that triggers the segfault. It's from the perl module Font-TTF-1.04, and was generated with perl's 'pod2man' command, from an older perl 5.18.1.


Files

Font_TTF_AATutils.3 (6.02 KB) Font_TTF_AATutils.3 man page (generated by pod2man) from perl's Font-TTF 1.04 module Tim Mooney, 2019-03-18 08:21 PM
Actions #2

Updated by Robert Mustacchi 4 months ago

  • Category set to manpage - manual pages
  • Assignee set to Robert Mustacchi

I preloaded libumem and enabled umem debugging. This made it very easy to find the culprit:

rm@turin:~$ LD_PRELOAD=libumem.so UMEM_DEBUG=default man -M /tmp/test/share/man -w
Abort (core dumped)
rm@turin:~$ mdb core 
Loading modules: [ libumem.so.1 libc.so.1 ld.so.1 ]
> $C
08039ef8 libc.so.1`_lwp_kill+0x15(1, 6, 0, 1, fefa3000, fef9127d)
08039f18 libc.so.1`raise+0x2b(6)
08039f38 libumem.so.1`umem_do_abort+0x53()
08039f48 libumem.so.1`__umem_assert_failed(fef9127d, fef914fc, 889df34)
08039f88 libumem.so.1`process_free+0x74(889df34, 0, 8039fa8)
08039fc8 libumem.so.1`realloc+0x3d(889df34, 100)
0803a018 libc.so.1`getdelim+0x16e(803a064, 803a068, a, 80690c0)
0803a038 libc.so.1`getline+0x33(803a064, 803a068, 80690c0)
0803a088 process_page+0x67(8874ee8, 803a10c, 0, 80524d4)
0803a0d8 process_section+0xf7(803a10c)
0803a528 mwpath+0xd4(886ef10, 0, 0, 803a610, 803a610, 8878dc8)
0803a548 do_makewhatis+0x5e(886ef40, 0, 803a5a8, 80565e2, fed90548, 0)
0803a5a8 main+0x496(803a5ac, fef035c8, 803a5e8, 805286b)
0803a5e8 _start_crt+0x9a(4, 803a610, f7ee301f, 0, 0, 0)
0803a604 _start+0x1a(4, 803a6f4, 803a6f8, 803a6fb, 803a70f, 0)
> ::umem_status
Status:         ready and active
Concurrency:    16
Logs:           (inactive)
Message buffer:
realloc(889df34): invalid or corrupted buffer
stack trace:
libumem.so.1'umem_err_recoverable+0x37
libumem.so.1'process_free+0x74
libumem.so.1'realloc+0x3d
libc.so.1'getdelim+0x16e
libc.so.1'getline+0x33
man'process_page+0x67
man'process_section+0xf7
man'mwpath+0xd4
man'do_makewhatis+0x5e
man'main+0x496
man'_start_crt+0x9a
man'_start+0x1a
> 803a10c/s
0x803a10c:      /tmp/test/share/man/man3
> 889df34::whatis
889df34 is 889df28+c, allocated from umem_alloc_160:
            ADDR          BUFADDR        TIMESTAMP           THREAD
                            CACHE          LASTLOG         CONTENTS
         889eeb0          889df28     3aafd83ba063                1
                          884e010                0                0
                 libumem.so.1`umem_cache_alloc_debug+0x216
                 libumem.so.1`umem_cache_alloc+0x1f5
                 libumem.so.1`umem_alloc+0x5c
                 libumem.so.1`umem_malloc+0x25
                 libumem.so.1`realloc+0xa2
                 libc.so.1`getdelim+0x4d
                 libc.so.1`getline+0x33
                 process_page+0x67
                 process_section+0xf7
                 mwpath+0xd4
                 do_makewhatis+0x5e
                 main+0x496
                 _start_crt+0x9a
                 _start+0x1a

Note how the buffer that getline/getdlim tried to free is offset at +c from an expected address. The getline function expects to operate by freeing the same string. Critically this means that while the contents of the string that is realloced can be changed, the actual pointer itself should not move. Unfortunately, if we look at the case of it trying to handle the .IX macro it proceeds to increment the line pointer that is fed into getdelim breaking things. The attached manual happens to use this in a few places:

grep IX *
.de IX
.        de IX
.IX Title "Font::TTF::AATutils 3" 
.IX Subsection "($classes, $states) = AAT_read_subtable($fh, $baseOffset, $subtableStart, $limits)" 
.IX Subsection "$length = AAT_write_state_table($fh, $classes, $states, $numExtraTables, $packEntry)" 
.IX Subsection "($classes, $states, $entries) = AAT_read_state_table($fh, $numActionWords)" 
.IX Subsection "($format, $lookup) = AAT_read_lookup($fh, $valueSize, $length, $default)" 
.IX Subsection "AAT_write_lookup($fh, $format, $lookup, $valueSize, $default)" 
.IX Header "AUTHOR" 
.IX Header "LICENSING" 

illumos doesn't normally use this macro in its old sgml->man converted pages and doesn't in mandoc. The fiix here is to simply use a temporary variable to track these modifications of the starting address.

Actions #3

Updated by Robert Mustacchi 4 months ago

  • Subject changed from man -w segfaults on this man page to man -w segfaults on .IX macro
Actions #4

Updated by Robert Mustacchi 4 months ago

To test this I did two things:

  1. Verified that man -w no longer crashed on the attached file using the reproduction steps outlined in the issue.
  2. Copied all of /usr/share/man from an installed system into two copies under /tmp. I pointed the unpatched man binary at one folder then the patched one at the other. I made a whatis database via LD_PRELOAD=libumem.so UMEM_DEBUG=default man -M /tmp/test/old/share/man -w. and LD_PRELOAD=libumem.so UMEM_DEBUG=default ./man -M /tmp/test/new/share/man -w. Then I proceeded to diff everything there via for f in $(find . -type f); do diff $f ../new/$f; done. There were no differences.
Actions #5

Updated by Electric Monk 4 months ago

  • Gerrit CR set to 1952
Actions #6

Updated by Electric Monk 4 months ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit 186c806e1377410853ef84873c5f064d8e980262

commit  186c806e1377410853ef84873c5f064d8e980262
Author: Robert Mustacchi <rm@fingolfin.org>
Date:   2022-01-18T20:08:32.000Z

    10565 man -w segfaults on .IX macro
    Reviewed by: Rich Lowe <richlowe@richlowe.net>
    Reviewed by: Yuri Pankov <ypankov@tintri.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

Actions

Also available in: Atom PDF