Page MenuHomeFreeBSD

Implement the ksyms mmap interface using d_mmap_single.
ClosedPublic

Authored by markj on Jul 31 2017, 1:22 AM.

Details

Summary

This attempts to fix mmap(/dev/ksyms). In particular, we now
preallocate a VM object for the pages backing the symbol table and allow
it to be shared among processes that share the device file handle.

While here, fix some unrelated bugs:

  • don't call the dtor if ksyms_open() fails, since devfs_open() will do that for us
  • check for errors from copyout()
  • use an sx lock instead of a mutex to allow M_WAITOK allocations
Test Plan

Only basic functional testing of the mmap and ioctl interfaces so far.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

markj added reviewers: kib, alc.
sys/dev/ksyms/ksyms.c
396 ↗(On Diff #31363)

Why is this limitation useful ?

560 ↗(On Diff #31363)

Can the addition overflow ?

581 ↗(On Diff #31363)

As was noted in the presentation, close might happen in the context of the process which is different from the process where open occured.

sys/dev/ksyms/ksyms.c
396 ↗(On Diff #31363)

I can't see a reason.

560 ↗(On Diff #31363)

I think so.

581 ↗(On Diff #31363)

Hm, right. I think this behaviour of mapping on open is not really supportable - there is no reliable way to remove the mapping on close.

Further cleanups:

  • Remove the ksyms ioctl interface. lockstat was modified to work without it, and the interface isn't present in Solaris/illumos.
  • Don't map the object into the opening process. We have no good way of removing the mapping when the device is closed.
  • Use uiomove_object() to copy the ELF file into the object, and in ksyms_read().
sys/dev/ksyms/ksyms.c
396 ↗(On Diff #31363)

I think I'll remove this, but would prefer to do so in a separate revision.

581 ↗(On Diff #31363)

I addressed this by simply removing the ioctl interface, which effectively removes the need to manually map the image into the process' address space.

kib added inline comments.
sys/dev/ksyms/ksyms.c
492 ↗(On Diff #31463)

You may remove empty ksyms_close(). kern_conf.c automatically substitutes NULL d_close with the same stub.

This revision is now accepted and ready to land.Aug 2 2017, 8:12 AM
This revision was automatically updated to reflect the committed changes.