Skip to contentSkip to conversation

modules-next

GIT PULL Rusty Russell 3 participants 5 messages Aug 1, 2016 – Aug 4, 2016

lore
Share & tools
lore
#0 lore raw

The following changes since commit 3fc9d690936fb2e20e180710965ba2cc3a0881f8:

Merge branch 'for-4.8/drivers' of git://git.kernel.dk/linux-block (2016-07-26 15:37:51 -0700)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux.git tags/modules-next-for-linus

for you to fetch changes up to 8acc0026b0f3046c9929d1f33cf840cd190d1b12:

extable.h: add stddef.h so "NULL" definition is not implicit (2016-07-28 15:23:47 +0930)

----------------------------------------------------------------
Only interesting thing here is Jessica's patch to add ro_after_init support to modules. The rest are all trivia.

Cheers,
Rusty.

----------------------------------------------------------------
Ben Hutchings (3):
module: Invalidate signatures on force-loaded modules Documentation/module-signing.txt: Note need for version info if reusing a key
module: Disable MODULE_FORCE_LOAD when MODULE_SIG_FORCE is enabled

Jessica Yu (1):
modules: add ro_after_init support

Jiri Kosina (1):
module: fix noreturn attribute for __module_put_and_exit()

Libor Pechacek (1):
module: Issue warnings when tainting kernel

Paul Gortmaker (2):
exceptions: fork exception table content from module.h into extable.h extable.h: add stddef.h so "NULL" definition is not implicit

Prarit Bhargava (1):
modules: Add kernel parameter to blacklist modules

Rusty Russell (2):
module: fix redundant test.
jump_label: disable preemption around __module_text_address().

Steven Rostedt (1):
module: Do a WARN_ON_ONCE() for assert module mutex not held

 Documentation/kernel-parameters.txt |   3 +
 Documentation/module-signing.txt    |   6 ++
 include/linux/extable.h             |  32 ++++++++++
 include/linux/module.h              |  37 +++--------
 include/uapi/linux/elf.h            |   1 +
 init/Kconfig                        |   1 +
 kernel/jump_label.c                 |   5 +-
 kernel/livepatch/core.c             |   2 +-
 kernel/module.c                     | 121 +++++++++++++++++++++++++++++-------
 9 files changed, 156 insertions(+), 52 deletions(-)
 create mode 100644 include/linux/extable.h
#1 lore raw

So this feels wrong to me, can you guys please explain:

Quoted 4 lines · Rusty Russell

Ben Hutchings (3):
module: Invalidate signatures on force-loaded modules
module: Disable MODULE_FORCE_LOAD when MODULE_SIG_FORCE is enabled

forcing a load and SIG_FORCE are entirely independent issues, afaik. I think requiring signed modules is just a good idea. But that doesn't necessarily mean that you don't have a signed module that is signed with a key you trust, but you still want to force-load it for the wrong kernel version (ie maybe you have a binary-only module from your IT department (and your IT department is evil,but at least they sign it to show that the module is trust-worthy as coming from them, even if they have some dubious behavior), but you did some kernel updates that still allow the module to work but the version doesn't match any more).

Am I missing something? What's the connection between MODULE_FORCE_LOAD and MODULE_SIG_FORCE? Because it smells like they are independent and that the above changes are very very dubious.

I didn't actually pull the tree, I just reacted to the pull request itself.

Linus

#2 lore raw
Quoted 8 lines · Linus Torvalds

So this feels wrong to me, can you guys please explain:

Quoted 4 lines · Rusty Russell

Ben Hutchings (3):
      module: Invalidate signatures on force-loaded modules
      module: Disable MODULE_FORCE_LOAD when MODULE_SIG_FORCE is enabled

Quoted 10 lines

forcing a load and SIG_FORCE are entirely independent issues, afaik. I think requiring signed modules is just a good idea. But that doesn't necessarily mean that you don't have a signed module that is signed with a key you trust, but you still want to force-load it for the wrong kernel version (ie maybe you have a binary-only module from your IT department (and your IT department is evil,but at least they sign it to show that the module is trust-worthy as coming from them, even if they have some dubious behavior), but you did some kernel updates that still allow the module to work but the version doesn't match any more).

We discussed this before and I thought you were happy with this version.  If the use case you describe is at all common, it could perhaps be handled by having a tool that patches the version information and re-signs the module with a different trusted key.

Quoted 3 lines

Am I missing something? What's the connection between MODULE_FORCE_LOAD and MODULE_SIG_FORCE? Because it smells like they are independent and that the above changes are very very dubious.

As I understand it:
- module signature enforcement means that root is not trusted to load arbitrary code into the kernel; instead the code has to be approved by one of the signing key holders
- force-loading a module means "I promise that this module is ABI compatible, even though it doesn't appear to be"

No-one signs that promise, and if it's false, the ABI differences could mean that an otherwise benign module would compromise the kernel.  So as I see it, the kernel should not trust a force-loaded signed module any more than an unsigned module.

If you still think that module signature enforcement is compatible with force-loading, I would like to know what you consider the purpose of enforcement to be.

Ben.

Quoted 3 lines

I didn't actually pull the tree, I just reacted to the pull request itself.

             Linus

Signature · 2 lines
Ben Hutchings
Sturgeon's Law: Ninety percent of everything is crap.
#3 lore raw
In reply to Linus Torvalds
Quoted 19 lines · Linus Torvalds

So this feels wrong to me, can you guys please explain:

Quoted 4 lines · Rusty Russell

Ben Hutchings (3):
module: Invalidate signatures on force-loaded modules
module: Disable MODULE_FORCE_LOAD when MODULE_SIG_FORCE is enabled

forcing a load and SIG_FORCE are entirely independent issues, afaik. I think requiring signed modules is just a good idea. But that doesn't necessarily mean that you don't have a signed module that is signed with a key you trust, but you still want to force-load it for the wrong kernel version (ie maybe you have a binary-only module from your IT department (and your IT department is evil,but at least they sign it to show that the module is trust-worthy as coming from them, even if they have some dubious behavior), but you did some kernel updates that still allow the module to work but the version doesn't match any more).

Quoted 5 lines

Am I missing something? What's the connection between MODULE_FORCE_LOAD and MODULE_SIG_FORCE? Because it smells like they are independent and that the above changes are very very dubious.

I didn't actually pull the tree, I just reacted to the pull request itself.

Well, MODULE_FORCE_LOAD is really "I am a doing crazy shit", and MODULE_SIG_FORCE is "Don't let me do crazy shit".

You have to contrive pretty hard to get a situation where the combination makes sense, so I tend to let Ben worry about the module signing stuff.

I can pull them out of modules-next if you'd prefer.

Cheers,
Rusty.

#4 lore raw
Quoted 2 lines · Rusty Russell
Quoted 1 line · Linus Torvalds

So this feels wrong to me, can you guys please explain:

...

Quoted 1 line
Quoted 1 line

I didn't actually pull the tree, I just reacted to the pull request itself.

...

Quoted 1 line

I can pull them out of modules-next if you'd prefer.

OK, removed that patch. Here's the update pullreq:

The following changes since commit 3fc9d690936fb2e20e180710965ba2cc3a0881f8:

Merge branch 'for-4.8/drivers' of git://git.kernel.dk/linux-block (2016-07-26 15:37:51 -0700)

are available in the git repository at:

git://git.kernel.org/pub/scm/linux/kernel/git/rusty/linux.git tags/modules-next-for-linus

for you to fetch changes up to 49aadcf1b6f4240751921dad52e86c760d70a5f1:

extable.h: add stddef.h so "NULL" definition is not implicit (2016-08-04 10:16:56 +0930)

----------------------------------------------------------------
Removed the MODULE_SIG_FORCE-means-no-MODULE_FORCE_LOAD patch.

Only interesting thing here is Jessica's patch to add ro_after_init support to modules. The rest are all trivia.

Cheers,
Rusty.

----------------------------------------------------------------
Ben Hutchings (2):
module: Invalidate signatures on force-loaded modules Documentation/module-signing.txt: Note need for version info if reusing a key

Jessica Yu (1):
modules: add ro_after_init support

Jiri Kosina (1):
module: fix noreturn attribute for __module_put_and_exit()

Libor Pechacek (1):
module: Issue warnings when tainting kernel

Paul Gortmaker (2):
exceptions: fork exception table content from module.h into extable.h extable.h: add stddef.h so "NULL" definition is not implicit

Prarit Bhargava (1):
modules: Add kernel parameter to blacklist modules

Rusty Russell (2):
module: fix redundant test.
jump_label: disable preemption around __module_text_address().

Steven Rostedt (1):
module: Do a WARN_ON_ONCE() for assert module mutex not held

 Documentation/kernel-parameters.txt |   3 +
 Documentation/module-signing.txt    |   6 ++
 include/linux/extable.h             |  32 ++++++++++
 include/linux/module.h              |  37 +++--------
 include/uapi/linux/elf.h            |   1 +
 kernel/jump_label.c                 |   5 +-
 kernel/livepatch/core.c             |   2 +-
 kernel/module.c                     | 121 +++++++++++++++++++++++++++++-------
 8 files changed, 155 insertions(+), 52 deletions(-)
 create mode 100644 include/linux/extable.h