I ran into an overflow problem in the module tag area. With profiling
toggled off, the overflow check in reserve_module_tags() did not
run, a module could load with more tags than the page flags can
address, and re-enabling profiling then silently corrupted
/proc/allocinfo. After discussion with Suren and Andrew the fix went
for a graceful approach: on overflow shut profiling down, release the
reservation and return -EAGAIN, and retry the load with profiling
disabled, so the codetag section lands as regular module data and
the module loads without profiling.

Review of that series by Sashiko turned up two more problems.

One is a race. layout_sections() and move_module() both asked
codetag_needs_module_section() where a codetag section goes, and
mem_profiling_support can change between the two calls, for instance
when another module load overflows the tag index and shuts profiling
down. move_module() then copies the codetag section to offset 0 of
its regular destination and clobbers the first section placed in
that region.

The other is a maple tree entry leak. The failure paths of
reserve_module_tags() return with the reservation still stored, and
a failed load never unloads the module, so nothing releases it.

v6 fixes both by reworking where codetag sections are allocated, on
a prototype by Petr Pavlu [1]. The allocation now runs before
layout_sections() and the placement is decided in one step, so
nothing re-asks the question and the race is gone. The retry is gone
too, on -EAGAIN the section is laid out as regular module data right
in the same load, and both failure paths release the reservation.

[1] https://lore.kernel.org/all/[email protected]/

Sending this as an RFC since 2/2 is a rework of Petr's prototype and
the approach changed quite a bit from v5, feedback on the direction
would be welcome.

Patch 1 moves release_module_tags() above reserve_module_tags(),
since the overflow path now has to call it and the helper sits below
it.

Patch 2 moves the codetag allocation out of move_module() in front
of layout_sections(). On overflow reserve_module_tags() shuts
profiling down, releases the reservation and returns -EAGAIN, and
the section is laid out as regular module data, so the module loads
without profiling instead of failing. Any other error fails the
load.

Tested on an x86_64 virtual machine:

Booted without sysctl.vm.mem_profiling=1,compressed:
# cat /proc/allocinfo is fine

Booted with sysctl.vm.mem_profiling=1,compressed:
# cat /proc/allocinfo is fine
# insmod overflow_tag.ko
# dmesg
  With module overflow_tag there are too many tags to fit in 13 page
  flag bits. Memory allocation profiling is disabled!
# rmmod overflow_tag
The module loads without profiling and unloads cleanly.

Changes in v6:
- rework 2/2 on Petr's prototype and allocate codetag sections
  before layout_sections(), the retry and its state resets are gone
- fix the layout_sections()/move_module() race (Found by Sashiko)
- release the reservation on populate failure as well (Found by
  Sashiko)
- only -EAGAIN keeps the fallback, other errors fail the load

Changes in v5:
- add Fixes: and Cc: stable to patch 1/2 as well, since 2/2 does not
  compile without it (Andrew Morton)
- restore frob-adjusted mem[type].size on retry instead of zeroing,
  as s390 and parisc add GOT/PLT space there in
  module_frob_arch_sections() (Reported by Sashiko)
- drop the load_module() mem_profiling_support check; the percpu
  counter leak is pre-existing and orthogonal to this fix

Changes in v4:
- add a new patch (1/2) to move release_module_tags() above
  reserve_module_tags(); the overflow fix is 2/2
- release the reservation on the -EAGAIN path
- return -EAGAIN instead of -ENOMEM so the module can still load
  without profiling (Suren)
- reset sh_addr, mem[type].size and sym/str SHF_ALLOC before retry
- skip percpu counters in load_module() when profiling is off

Changes in v3:
- use pr_warn_once() instead of pr_warn()
- return -ENOMEM instead of -ENOSPC (Suren)
- expand the commit message to describe the /proc/allocinfo impact
  (Andrew)

Changes in v2:
- return an error after shutdown_mem_profiling() to skip
  vm_module_tags_populate()

v1: https://lore.kernel.org/all/[email protected]/
v2: https://lore.kernel.org/all/[email protected]/
v3: https://lore.kernel.org/all/[email protected]/
v4: https://lore.kernel.org/all/[email protected]/
v5: https://lore.kernel.org/all/[email protected]/

Hao Ge (2):
  alloc_tag: move release_module_tags() above reserve_module_tags()
  module: allocate codetag sections before the regular module layout

 include/linux/module.h   |   2 +
 kernel/module/internal.h |   4 ++
 kernel/module/main.c     | 120 ++++++++++++++++++++-------------------
 mm/alloc_tag.c           | 101 ++++++++++++++++----------------
 4 files changed, 121 insertions(+), 106 deletions(-)

-- 
2.25.1


Reply via email to