Page MenuHomeFreeBSD

db_search_symbol: prevent pollution from bogus symbols
ClosedPublic

Authored by vangyzen on Oct 21 2020, 8:09 PM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 16 2022, 1:20 PM
Unknown Object (File)
Dec 13 2022, 1:01 AM
Unknown Object (File)
Dec 10 2022, 6:56 PM
Unknown Object (File)
Nov 27 2022, 10:21 AM
Subscribers

Details

Summary

Assembly files often provide bogus symbols with very small values.
This pollution confounds disassembly by replacing object offsets
with bogus symbols. Since the kernel will never map anything in
the first page, do not query the symbol table for an address smaller
than the page size.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Seems reasonable, although I think the explanation from your description of this change should really go in the code comment rather than the rather terse and enigmatic "Prevent pollution from bogus symbols".

In D26895#599937, @dab wrote:

Seems reasonable, although I think the explanation from your description of this change should really go in the code comment rather than the rather terse and enigmatic "Prevent pollution from bogus symbols".

I agree. It would also be nice to see an example of where the preexisting logic goes wrong.

This looks fine to me. I agree that the comment could be less terse, perhaps something like

/*

  • The kernel will never map the first page, so any symbols in that range cannot refer to addresses.
  • Some third-party assembly files define internal constants which appear in their symbol table.
  • Avoiding the lookup for those symbols avoids replacing small offsets with those symbols
  • during disassembly. */

The particular instance I noticed this time around was from Intel's ISA-L_crypto library, at
https://github.com/01org/isa-l_crypto.git; the line "rot34 equ 23" caused ddb to disassemble
some operands as, e.g., "rot34+4(%rax)" when that file was included in a kernel build.

This revision is now accepted and ready to land.Oct 22 2020, 2:06 PM
This revision now requires review to proceed.Oct 22 2020, 7:50 PM

So the comparison with PAGE_SIZE is arbitrary, right? There's no reason the aforementioned code could emit symbols with a value between, say, PAGE_SIZE and PAGE_SIZE * 2?

This revision is now accepted and ready to land.Oct 23 2020, 2:58 PM

So the comparison with PAGE_SIZE is arbitrary, right? There's no reason the aforementioned code could emit symbols with a value between, say, PAGE_SIZE and PAGE_SIZE * 2?

That's correct. We've only _seen_ values that small, so I was taking a cautious approach. Maybe KERNBASE would be more appropriate?

So the comparison with PAGE_SIZE is arbitrary, right? There's no reason the aforementioned code could emit symbols with a value between, say, PAGE_SIZE and PAGE_SIZE * 2?

That's correct. We've only _seen_ values that small, so I was taking a cautious approach. Maybe KERNBASE would be more appropriate?

I'm not sure if we can assume on all platforms that KLDs don't get loaded below KERNBASE.