db_search_symbol: prevent pollution from bogus symbols

[vangyzen]

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.

[dab]

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".

[markj]

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.

[rang_acm.org]

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.

[vangyzen]

• borrow Anton's comment

[markj]

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?

[vangyzen]

In D26895#600356, @markj wrote:

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?

[markj]

In D26895#600361, @vangyzen wrote:

In D26895#600356, @markj wrote:

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.