Re: [RFC 00/12] module: avoid userspace pressure on unwanted allocations

From: Luis Chamberlain
Date: Fri Mar 24 2023 - 16:00:17 EST


On Fri, Mar 24, 2023 at 12:11:07PM -0700, Linus Torvalds wrote:
> On Fri, Mar 24, 2023 at 10:54 AM Luis Chamberlain <mcgrof@xxxxxxxxxx> wrote:
> >
> > +/*
> > + * This clutch ensures we only allow a certain number concurrent threads at a
>
> kludge, not clutch.
>
> And it's much worse than a kludge. It's just wrong and disgusting.

I wasn't happy with it either...

> > + pr_warn_ratelimited("kread_concurrent_max (%u) close to 0 (max_loads: %u), throttling...",
> > + atomic_read(&kread_concurrent_max),
> > + MAX_KREAD_CONCURRENT);
>
> This is also wrong, since it's not kernel_read_file() that is the
> problem, but whatever broken caller.
>
> Yeah, yeah, in practice it's presumably always just finit_module()
> doing kernel_read_file_from_fd(), but it's still *completely* wrong to
> just say "function X is throttling" when "X" isn't the problem, and
> doesn't tell what the _real_ problem is.

True.

> I really think this all needs some core fixing at the module layer,
> not these kinds of horrific hacks.

On the modules side of things we can be super defensive on the second
vmalloc allocation defensive [0] but other than this the initial kread
also needs care too.

To address the kread abuse within finit_module we could just move the
kludge to the modules side of things until each free happens as in the
below alternative. That just means any easy user interfacing call with
kernel_read*() would likely have to be as careful. Untested below.

[0] https://lkml.kernel.org/r/20230319214926.1794108-4-mcgrof@xxxxxxxxxx

Luis