On 6/3/19 7:14 AM, Evgenii Shatokhin wrote:
Hi,
It is possible that keeping each new and patched function in a separate
ELF section in a livepatch kernel module could cause problems.
During the testing of binary patches for the kernels used in Virtuozzo
7, I found that the kernel may try to allocate a relatively large (for
kmalloc) memory area to support sysfs files
/sys/module/<module_name>/sections/* for the patch modules. The size was
16 - 35 KB, depending on the patch, i.e. 3rd and 4th order allocations.
The numbers do not look very big but still increase the chance that the
patch module will fail to load when the memory is fragmented.
kernel/module.c, add_sect_attrs():
/* Count loaded sections and allocate structures */
for (i = 0; i < info->hdr->e_shnum; i++)
if (!sect_empty(&info->sechdrs[i]))
nloaded++;
size[0] = ALIGN(sizeof(*sect_attrs)
+ nloaded * sizeof(sect_attrs->attrs[0]),
sizeof(sect_attrs->grp.attrs[0]));
size[1] = (nloaded + 1) * sizeof(sect_attrs->grp.attrs[0]);
sect_attrs = kzalloc(size[0] + size[1], GFP_KERNEL);
So, in our case, the size of the requested memory chunk was
48 + 80 * <number_of_loaded_ELF_sections>.
Both livepatch and old-style KPatch kernel modules place each new or
patched function in a separate section, same for the new global and
static data...
Hi Evgenii,
To be specific, I think this is a kpatch-related issue, right? I don't
recall there being any section specifications in the upstream livepatch
code base (aside from arch-specific relocations for alt/para inst, etc.)
> ... So, if we patch 200+ kernel functions (which not that
unusual in cumulative patches), the kernel will try to allocate around
16 KB of memory for these sysfs data only. The largest of our binary
patches have around 400 new and patched functions, plus a few dozens of
new data items, which results in a wasted kernel memory chunk of 35 KB
in size.
Here are the questions.
1. The files /sys/module/<module_name>/sections/* currently contain the
start addresses of the relevant sections, i.e. the addresses of new and
patched functions among other things. Is this info really needed for
livepatch kernel modules after they have been loaded?
I think this is generic module debugging info and that livepatch isn't
interested in these. I suppose there might be some userspace tools that
look at those sysfs files for quick verification purposes... though
there appears to be some filtering of sections that are listed here.
Not sure which meet the criteria.
2. Of course, create-diff-object relies heavily upon placing each new or
patched function in a separate section. But is it needed to keep the
functions there after the diff object files have been prepared?
Does the code that loads/unloads the patches require that each function
is kept that way? Looks like no, but I am not 100% sure.
Josh will know better than me, but I suspect these sections are just an
artifact of -ffunction-sections used for the kpatch ELF comparison and
extraction.
As an experiment, I added the following to kmod/patch/kpatch.lds.S to
merge all .text.* sections into .text.livepatch in the resulting patch
module:
SECTIONS
{
+ .text.livepatch : {
+ *(.text.*)
+ }
My test patch module with around 200 changed functions was built OK with
that, the functions were actually placed into .text.livepatch section as
requested. The patch was loaded fine but I haven't tested it much yet.
It also might be reasonable to merge .rodata.__func__.* the same way.
Nice. If you have success with this approach, please post up a PR on
the kpatch github for review.
Also, in the interest of minimizing sections, perhaps the kpatch
callback and force sections can be omitted if they are unused. Do they
count towards the kmalloc allocation you listed above or are they
skipped when empty?
Are there any pitfalls in such merging of sections? Am I missing
something obvious?
Regards,
-- Joe
