Re: kmod: provide wrappers for kmod_concurrent inc/dec

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



+++ Luis R. Rodriguez [16/12/16 09:05 +0100]:
On Thu, Dec 15, 2016 at 01:46:25PM +0100, Petr Mladek wrote:
On Thu 2016-12-08 22:08:59, Luis R. Rodriguez wrote:
> On Thu, Dec 08, 2016 at 12:29:42PM -0800, Kees Cook wrote:
> > On Thu, Dec 8, 2016 at 11:48 AM, Luis R. Rodriguez <mcgrof@xxxxxxxxxx> wrote:
> > > kmod_concurrent is used as an atomic counter for enabling
> > > the allowed limit of modprobe calls, provide wrappers for it
> > > to enable this to be expanded on more easily. This will be done
> > > later.
> > >
> > > Signed-off-by: Luis R. Rodriguez <mcgrof@xxxxxxxxxx>
> > > ---
> > >  kernel/kmod.c | 27 +++++++++++++++++++++------
> > >  1 file changed, 21 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/kernel/kmod.c b/kernel/kmod.c
> > > index cb6f7ca7b8a5..049d7eabda38 100644
> > > --- a/kernel/kmod.c
> > > +++ b/kernel/kmod.c
> > > @@ -108,6 +111,20 @@ static int call_modprobe(char *module_name, int wait)
> > >         return -ENOMEM;
> > >  }
> > >
> > > +static int kmod_umh_threads_get(void)
> > > +{
> > > +       atomic_inc(&kmod_concurrent);

This approach might actually cause false failures. If we
are on the limit and more processes do this increment
in parallel, it makes the number bigger that it should be.

This approach is *exactly* what the existing code does :P
I just provided wrappers. I agree with the old approach though,
reason is it acts as a lock in for the bump.

I think what Petr meant was that we could run into false failures when multiple
atomic increments happen between the first increment and the subsequent
atomic_read.

Say max_modprobes is 64 -

      atomic_inc(&kmod_concurrent); // thread 1: kmod_concurrent is 63
           atomic_inc(&kmod_concurrent); // thread 2: kmod_concurrent is 64
                atomic_inc(&kmod_concurrent); // thread 3: kmod_concurrent is 65
      if (atomic_read(&kmod_concurrent) < max_modprobes) // if all threads read 65 here, then all will error out
              return 0;                                  // when the first two should have succeeded (false failures)
      atomic_dec(&kmod_concurrent);
      return -ENOMEM;

But yeah, I think this issue was already in the existing kmod code..

Jessica
--
To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux