All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Tim Chen <tim.c.chen@linux.intel.com>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org,
	ak <ak@linux.intel.com>, "H. Peter Anvin" <hpa@zytor.com>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>
Subject: Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.
Date: Sat, 20 Jul 2013 02:00:44 +0200	[thread overview]
Message-ID: <2745724.Y86yAFA8gy@vostro.rjw.lan> (raw)
In-Reply-To: <20130719231630.GC1701@kroah.com>

On Friday, July 19, 2013 04:16:30 PM Greg Kroah-Hartman wrote:
> On Fri, Jul 19, 2013 at 11:38:04PM +0200, Rafael J. Wysocki wrote:
> > Alas, this is not the one I'd like to apply.
> > 
> > With that patch applied, new device objects are created to avoid binding the
> > processor driver directly to the cpu system device objects, because that
> > apparently confuses udev and it starts to ignore the cpu modalias once the
> > driver has been bound to any of those objects.
> > 
> > I've verified in the meantime that this indeed is the case.
> > 
> > A link to the patch in question: https://patchwork.kernel.org/patch/2830561/
> > 
> > Greg, I asked you some time ago whether or not it was possible for udev to stop
> > autoloading modules that matched the cpu modalias after a driver had been bound
> > to the cpu system device objects and you said "no".  However, this time I can
> > say with certainty that that really is the case.  So, the question now is
> > whether or not we can do anything in the kernel to avoid that confusion in udev
> > instead of applying the patch linked above (which is beyond ugly in my not so
> > humble opinion)?
> 
> udev isn't doing any module loading, 'modprobe' is just being called for
> any new module alias that shows up in the system, and all of the drivers
> that match it then get loaded.

The problem is that that doesn't happen when a driver is bound to the
cpu system device objects.  modprobe is just not called for modules that
match the cpu modalias in that case.

If I call modprobe manually for any of the modules in question, it loads
and works no problem.

> How is it a problem if a module is attempted to be loaded that is
> already loaded?  How is it a problem if a different module is loaded for
> a device already bound to a driver?  Both of those should be total
> "no-ops" for the kernel.

Precisely, but that's not what happens in practice, hence my question.

The situation is this:

- With 3.11-rc1 modules that match the CPU modalias are not loaded
  automatically (that is, modprobe is not called for them by udev) after
  the processor module is loaded.
- With 3.10 they are loaded automatically at any time.
- With the patch at https://patchwork.kernel.org/patch/2830561/ on top of
  3.11-rc1 the situation is the same as for 3.10.

Therefore we have a kernel regression from 3.10 (with respect to the existing
user space which is udev) in 3.11-rc1 3.10 and the patch at
https://patchwork.kernel.org/patch/2830561/ makes that regression go away.

However, that patch is totally voodoo programming, so I wonder if there is any
saner alternative or we really need to do voodoo programming in the kernel to
work around udev's confusion.

> But, I don't know anything about the cpu code, how is loading a module
> causing problems?  That sounds like it needs to be fixes, as any root
> user can load modules whenever they want, you can't protect the kernel
> from doing that.

Yes, that needs to be fixed, but I don't know *why* it is a problem in the
first place.  I'd like to understand what's going on, because I don't
right now.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

WARNING: multiple messages have this Message-ID (diff)
From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Tim Chen <tim.c.chen@linux.intel.com>,
	Herbert Xu <herbert@gondor.hengli.com.au>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org,
	ak <ak@linux.intel.com>, "H. Peter Anvin" <hpa@zytor.com>,
	ACPI Devel Maling List <linux-acpi@vger.kernel.org>
Subject: Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.
Date: Sat, 20 Jul 2013 02:00:44 +0200	[thread overview]
Message-ID: <2745724.Y86yAFA8gy@vostro.rjw.lan> (raw)
In-Reply-To: <20130719231630.GC1701@kroah.com>

On Friday, July 19, 2013 04:16:30 PM Greg Kroah-Hartman wrote:
> On Fri, Jul 19, 2013 at 11:38:04PM +0200, Rafael J. Wysocki wrote:
> > Alas, this is not the one I'd like to apply.
> > 
> > With that patch applied, new device objects are created to avoid binding the
> > processor driver directly to the cpu system device objects, because that
> > apparently confuses udev and it starts to ignore the cpu modalias once the
> > driver has been bound to any of those objects.
> > 
> > I've verified in the meantime that this indeed is the case.
> > 
> > A link to the patch in question: https://patchwork.kernel.org/patch/2830561/
> > 
> > Greg, I asked you some time ago whether or not it was possible for udev to stop
> > autoloading modules that matched the cpu modalias after a driver had been bound
> > to the cpu system device objects and you said "no".  However, this time I can
> > say with certainty that that really is the case.  So, the question now is
> > whether or not we can do anything in the kernel to avoid that confusion in udev
> > instead of applying the patch linked above (which is beyond ugly in my not so
> > humble opinion)?
> 
> udev isn't doing any module loading, 'modprobe' is just being called for
> any new module alias that shows up in the system, and all of the drivers
> that match it then get loaded.

The problem is that that doesn't happen when a driver is bound to the
cpu system device objects.  modprobe is just not called for modules that
match the cpu modalias in that case.

If I call modprobe manually for any of the modules in question, it loads
and works no problem.

> How is it a problem if a module is attempted to be loaded that is
> already loaded?  How is it a problem if a different module is loaded for
> a device already bound to a driver?  Both of those should be total
> "no-ops" for the kernel.

Precisely, but that's not what happens in practice, hence my question.

The situation is this:

- With 3.11-rc1 modules that match the CPU modalias are not loaded
  automatically (that is, modprobe is not called for them by udev) after
  the processor module is loaded.
- With 3.10 they are loaded automatically at any time.
- With the patch at https://patchwork.kernel.org/patch/2830561/ on top of
  3.11-rc1 the situation is the same as for 3.10.

Therefore we have a kernel regression from 3.10 (with respect to the existing
user space which is udev) in 3.11-rc1 3.10 and the patch at
https://patchwork.kernel.org/patch/2830561/ makes that regression go away.

However, that patch is totally voodoo programming, so I wonder if there is any
saner alternative or we really need to do voodoo programming in the kernel to
work around udev's confusion.

> But, I don't know anything about the cpu code, how is loading a module
> causing problems?  That sounds like it needs to be fixes, as any root
> user can load modules whenever they want, you can't protect the kernel
> from doing that.

Yes, that needs to be fixed, but I don't know *why* it is a problem in the
first place.  I'd like to understand what's going on, because I don't
right now.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

  parent reply	other threads:[~2013-07-19 23:50 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-16 11:53 [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency Tetsuo Handa
2013-07-16 11:55 ` Herbert Xu
2013-07-16 13:49   ` [PATCH 3.11-rc1] crypto: Fix boot failure due to moduledependency Tetsuo Handa
2013-07-16 16:23     ` Tim Chen
2013-07-16 16:23       ` Tim Chen
2013-07-17 11:52       ` [PATCH 3.11-rc1] crypto: Fix boot failure due tomoduledependency Tetsuo Handa
2013-07-17 11:52         ` Tetsuo Handa
2013-07-17 16:46         ` Tim Chen
2013-07-17 16:46           ` Tim Chen
2013-07-17 20:50           ` [PATCH 3.11-rc1] crypto: Fix boot failure duetomoduledependency Tetsuo Handa
2013-07-17 20:50             ` Tetsuo Handa
2013-07-17 21:53             ` Tim Chen
2013-07-17 21:53               ` Tim Chen
2013-07-17 22:08             ` Tim Chen
2013-07-17 22:08               ` Tim Chen
2013-07-18  3:47               ` [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency Tetsuo Handa
2013-07-18  3:47                 ` Tetsuo Handa
2013-07-18 21:00                 ` Tim Chen
2013-07-18 21:00                   ` Tim Chen
2013-07-18 22:17                   ` Rafael J. Wysocki
2013-07-18 22:17                     ` Rafael J. Wysocki
2013-07-18 23:08                     ` Tim Chen
2013-07-18 23:08                       ` Tim Chen
2013-07-19 13:03                       ` Rafael J. Wysocki
2013-07-19 13:03                         ` Rafael J. Wysocki
2013-07-19 14:49                         ` Rafael J. Wysocki
2013-07-19 14:49                           ` Rafael J. Wysocki
2013-07-19 18:08                           ` Tim Chen
2013-07-19 18:08                             ` Tim Chen
2013-07-19 21:38                             ` Rafael J. Wysocki
2013-07-19 21:38                               ` Rafael J. Wysocki
2013-07-19 23:16                               ` Greg Kroah-Hartman
2013-07-19 23:16                                 ` Greg Kroah-Hartman
2013-07-19 23:21                                 ` H. Peter Anvin
2013-07-19 23:24                                   ` Herbert Xu
2013-07-19 23:24                                     ` Herbert Xu
2013-07-19 23:37                                     ` Tim Chen
2013-07-19 23:37                                       ` Tim Chen
2013-07-20  1:31                                       ` Tim Chen
2013-07-20  1:31                                         ` Tim Chen
2013-07-20  2:19                                         ` [PATCH 3.11-rc1] crypto: Fix boot failure due to moduledependency Tetsuo Handa
2013-07-20  2:19                                           ` Tetsuo Handa
2013-07-20  5:30                                         ` [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency Herbert Xu
2013-07-20  5:30                                           ` Herbert Xu
2013-07-20  5:56                                           ` [PATCH 3.11-rc1] crypto: Fix boot failure due to moduledependency Tetsuo Handa
2013-07-20  5:56                                             ` Tetsuo Handa
2013-09-11 11:43                                             ` [3.12-rc1] Dependency on module-init-tools >= 3.11 ? Tetsuo Handa
2013-09-12  4:26                                               ` Herbert Xu
2013-09-12  5:03                                                 ` Tetsuo Handa
2013-09-12  5:28                                                   ` Herbert Xu
2013-09-12  5:28                                                     ` Herbert Xu
2013-09-12 10:20                                                     ` Tetsuo Handa
2013-09-12 10:20                                                       ` Tetsuo Handa
2013-09-12 10:29                                                       ` Herbert Xu
2013-09-12 10:29                                                         ` Herbert Xu
2013-09-12 14:26                                                         ` Waiman Long
2013-09-12 14:26                                                           ` Waiman Long
2013-09-13 13:27                                                           ` Tetsuo Handa
2013-09-13 13:27                                                             ` Tetsuo Handa
2013-09-12 11:12                                                 ` Arthur Marsh
2013-07-19 23:26                                   ` [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency Greg Kroah-Hartman
2013-07-19 23:28                                     ` H. Peter Anvin
2013-07-20  0:00                                 ` Rafael J. Wysocki [this message]
2013-07-20  0:00                                   ` Rafael J. Wysocki
2013-07-20  3:06                                   ` Rafael J. Wysocki
2013-07-20  3:06                                     ` Rafael J. Wysocki
2013-07-20  9:51                                     ` Rafael J. Wysocki
2013-07-20  9:51                                       ` Rafael J. Wysocki
2013-07-20 11:01                                       ` Rafael J. Wysocki
2013-07-20 11:01                                         ` Rafael J. Wysocki
2013-07-18 23:44                     ` H. Peter Anvin
2013-07-19 12:57                       ` Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2745724.Y86yAFA8gy@vostro.rjw.lan \
    --to=rjw@rjwysocki.net \
    --cc=ak@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=hpa@zytor.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=rafael.j.wysocki@intel.com \
    --cc=tim.c.chen@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.