Bug 224875 - kldxref fails if a mod_depend md_cval is too close to the end of allocated sections
Summary: kldxref fails if a mod_depend md_cval is too close to the end of allocated se...
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: Ed Maste
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-01-03 20:10 UTC by Ed Maste
Modified: 2018-02-13 22:42 UTC (History)
1 user (show)

See Also:
emaste: mfc-stable11+
emaste: mfc-stable10-


Attachments
Reproduction case for kldxref issue (69.00 KB, application/x-tar)
2018-01-04 18:26 UTC, Ed Maste
no flags Details
Module that demonstrates the issue (1.73 KB, application/gzip)
2018-01-04 18:35 UTC, Ed Maste
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ed Maste freebsd_committer freebsd_triage 2018-01-03 20:10:45 UTC
In kldxref.c:read_kld() we have a 33-byte cval buffer:

char ... cval[MAXMODNAME + 1] ...

into which we read a string:

check(EF_SEG_READ(&ef, (Elf_Off)md.md_cval, sizeof(cval), cval));

This requires that a 33-byte read is successful, however it may fail if the string is shorter than 32 characters (plus the NUL) and is located near the highest allocated address.

It appears this has never been an issue with ld.bfd, which places a loadable .comment section after other sections of interest, so kldxref is free to read unrelated data beyond the end of the cval string.

ld.lld however places .comment early in the section list, and so the cval may be in a .data or .rodata section that comes at the end of the section list.

(CTF data may also be after .data/.rodata and would mitigate this issue; it may happen only with CTF disabled)
Comment 1 Ed Maste freebsd_committer freebsd_triage 2018-01-04 18:26:52 UTC
Created attachment 189412 [details]
Reproduction case for kldxref issue
Comment 2 Ed Maste freebsd_committer freebsd_triage 2018-01-04 18:35:41 UTC
Created attachment 189413 [details]
Module that demonstrates the issue
Comment 3 Ed Maste freebsd_committer freebsd_triage 2018-01-04 18:36:14 UTC
Create a directory and uncompress pty.ko.gz there. Then

% cd <dir>
% kldxref -v .
volta% kldxref -v .  
./lhint.YF8PCY
kldxref: elf_open(./lhint.YF8PCY): Inappropriate file type or format
./pty.ko
kldxref: ef_seg_read_rel(./pty.ko): bad offset/len (552:33)
kldxref: error while reading ./pty.ko: Bad address

The "bad offset/len" is the issue described here.
Comment 4 commit-hook freebsd_committer freebsd_triage 2018-01-16 18:20:37 UTC
A commit references this bug:

Author: emaste
Date: Tue Jan 16 18:20:12 UTC 2018
New revision: 328052
URL: https://svnweb.freebsd.org/changeset/base/328052

Log:
  kldxref: handle modules with md_cval at the end of allocated sections

  Attempting to retrieve an md_cval string from a kernel module with
  kldxref would throw a offset error for modules created using lld, since
  this value would be placed at the end of all allocated sections.

  Add an ef_read_seg_string method to the ef interface, to allow reading
  strings of varying size without attempting to read beyond the segment's
  bounds.

  PR:		224875
  Submitted by:	Mitchell Horne <mhorne063@gmail.com>
  Reviewed by:	cem, kib
  Sponsored by:	The FreeBSD Foundation
  Differential Revision:	https://reviews.freebsd.org/D13923

Changes:
  head/usr.sbin/kldxref/ef.c
  head/usr.sbin/kldxref/ef.h
  head/usr.sbin/kldxref/ef_obj.c
  head/usr.sbin/kldxref/kldxref.c
Comment 5 commit-hook freebsd_committer freebsd_triage 2018-02-13 22:40:57 UTC
A commit references this bug:

Author: emaste
Date: Tue Feb 13 22:40:33 UTC 2018
New revision: 329247
URL: https://svnweb.freebsd.org/changeset/base/329247

Log:
  MFC r328052: kldxref: handle modules with md_cval at end of allocated secs

  Attempting to retrieve an md_cval string from a kernel module with
  kldxref would throw a offset error for modules created using lld, since
  this value would be placed at the end of all allocated sections.

  Add an ef_read_seg_string method to the ef interface, to allow reading
  strings of varying size without attempting to read beyond the segment's
  bounds.

  PR:		224875
  Submitted by:	Mitchell Horne <mhorne063@gmail.com>
  Sponsored by:	The FreeBSD Foundation

Changes:
_U  stable/11/
  stable/11/usr.sbin/kldxref/ef.c
  stable/11/usr.sbin/kldxref/ef.h
  stable/11/usr.sbin/kldxref/ef_obj.c
  stable/11/usr.sbin/kldxref/kldxref.c