Re: [RFC 10/10] kmod: add a sanity check on module loading

2017-01-09 Thread Luis R. Rodriguez
On Tue, Jan 10, 2017 at 05:17:22AM +1030, Rusty Russell wrote: > "Luis R. Rodriguez" writes: > > On Tue, Jan 03, 2017 at 10:34:53AM +1030, Rusty Russell wrote: > >> "Luis R. Rodriguez" writes: > >> > Right, out of ~350 request_module() calls (not included

Re: [RFC 10/10] kmod: add a sanity check on module loading

2017-01-09 Thread Luis R. Rodriguez
On Tue, Jan 10, 2017 at 05:17:22AM +1030, Rusty Russell wrote: > "Luis R. Rodriguez" writes: > > On Tue, Jan 03, 2017 at 10:34:53AM +1030, Rusty Russell wrote: > >> "Luis R. Rodriguez" writes: > >> > Right, out of ~350 request_module() calls (not included try requests) > >> > only ~46 check the

Re: [RFC 10/10] kmod: add a sanity check on module loading

2017-01-06 Thread Luis R. Rodriguez
On Tue, Jan 03, 2017 at 10:34:53AM +1030, Rusty Russell wrote: > "Luis R. Rodriguez" writes: > > Right, out of ~350 request_module() calls (not included try requests) > > only ~46 check the return value. Hence a validation check, and come to > > think of it, *this* was the

Re: [RFC 10/10] kmod: add a sanity check on module loading

2017-01-06 Thread Luis R. Rodriguez
On Tue, Jan 03, 2017 at 10:34:53AM +1030, Rusty Russell wrote: > "Luis R. Rodriguez" writes: > > Right, out of ~350 request_module() calls (not included try requests) > > only ~46 check the return value. Hence a validation check, and come to > > think of it, *this* was the issue that originally

Re: [RFC 10/10] kmod: add a sanity check on module loading

2017-01-02 Thread Rusty Russell
"Luis R. Rodriguez" writes: >> Maybe a similar hack for try_then_request_module(), but many places seem >> to open-code request_module() so it's not as trivial... Hi Luis, Jessica (who is the main module maintainer now), Back from break, sorry about delay. > Right,

Re: [RFC 10/10] kmod: add a sanity check on module loading

2017-01-02 Thread Rusty Russell
"Luis R. Rodriguez" writes: >> Maybe a similar hack for try_then_request_module(), but many places seem >> to open-code request_module() so it's not as trivial... Hi Luis, Jessica (who is the main module maintainer now), Back from break, sorry about delay. > Right, out of ~350

Re: [RFC 10/10] kmod: add a sanity check on module loading

2016-12-21 Thread Luis R. Rodriguez
On Tue, Dec 20, 2016 at 8:21 PM, Rusty Russell wrote: > "Luis R. Rodriguez" writes: >> OK -- if userspace messes up again it may be a bit hard to prove >> unless we have a validation debug thing in place, would such a thing >> in debug form be reasonable

Re: [RFC 10/10] kmod: add a sanity check on module loading

2016-12-21 Thread Luis R. Rodriguez
On Tue, Dec 20, 2016 at 8:21 PM, Rusty Russell wrote: > "Luis R. Rodriguez" writes: >> OK -- if userspace messes up again it may be a bit hard to prove >> unless we have a validation debug thing in place, would such a thing >> in debug form be reasonable ? > > That makes perfect sense. Untested

Re: [RFC 10/10] kmod: add a sanity check on module loading

2016-12-20 Thread Rusty Russell
"Luis R. Rodriguez" writes: > OK -- if userspace messes up again it may be a bit hard to prove > unless we have a validation debug thing in place, would such a thing > in debug form be reasonable ? That makes perfect sense. Untested hack: diff --git a/fs/filesystems.c

Re: [RFC 10/10] kmod: add a sanity check on module loading

2016-12-20 Thread Rusty Russell
"Luis R. Rodriguez" writes: > OK -- if userspace messes up again it may be a bit hard to prove > unless we have a validation debug thing in place, would such a thing > in debug form be reasonable ? That makes perfect sense. Untested hack: diff --git a/fs/filesystems.c b/fs/filesystems.c index

Re: [RFC 10/10] kmod: add a sanity check on module loading

2016-12-20 Thread Luis R. Rodriguez
On Mon, Dec 19, 2016 at 6:53 PM, Rusty Russell wrote: > Where does this NULL-deref is the module isn't correctly loaded? No you are right, sorry -- I had confused a failure to mount over null deref, my mistake. >> *Iff* we want a sanity check to verify kmod's umh is not

Re: [RFC 10/10] kmod: add a sanity check on module loading

2016-12-20 Thread Luis R. Rodriguez
On Mon, Dec 19, 2016 at 6:53 PM, Rusty Russell wrote: > Where does this NULL-deref is the module isn't correctly loaded? No you are right, sorry -- I had confused a failure to mount over null deref, my mistake. >> *Iff* we want a sanity check to verify kmod's umh is not lying to us we >> need

Re: [RFC 10/10] kmod: add a sanity check on module loading

2016-12-19 Thread Rusty Russell
"Luis R. Rodriguez" writes: > On Dec 16, 2016 9:54 PM, "Rusty Russell" wrote: > > AFAICT the mistake here is that kmod is returning "done, OK" when the > > module it is trying to load is already loading (but not finished > > loading). That's the root

Re: [RFC 10/10] kmod: add a sanity check on module loading

2016-12-19 Thread Rusty Russell
"Luis R. Rodriguez" writes: > On Dec 16, 2016 9:54 PM, "Rusty Russell" wrote: > > AFAICT the mistake here is that kmod is returning "done, OK" when the > > module it is trying to load is already loading (but not finished > > loading). That's the root problem; it's an attempt at optimization by

Re: [RFC 10/10] kmod: add a sanity check on module loading

2016-12-16 Thread Rusty Russell
"Luis R. Rodriguez" writes: > On Thu, Dec 15, 2016 at 10:57:42AM +1030, Rusty Russell wrote: >> "Luis R. Rodriguez" writes: >> > kmod has an optimization in place whereby if a some kernel code >> > uses request_module() on a module already loaded we never

Re: [RFC 10/10] kmod: add a sanity check on module loading

2016-12-16 Thread Rusty Russell
"Luis R. Rodriguez" writes: > On Thu, Dec 15, 2016 at 10:57:42AM +1030, Rusty Russell wrote: >> "Luis R. Rodriguez" writes: >> > kmod has an optimization in place whereby if a some kernel code >> > uses request_module() on a module already loaded we never bother >> > userspace as the module

Re: [RFC 10/10] kmod: add a sanity check on module loading

2016-12-16 Thread Luis R. Rodriguez
On Thu, Dec 15, 2016 at 10:57:42AM +1030, Rusty Russell wrote: > "Luis R. Rodriguez" writes: > > kmod has an optimization in place whereby if a some kernel code > > uses request_module() on a module already loaded we never bother > > userspace as the module already is loaded.

Re: [RFC 10/10] kmod: add a sanity check on module loading

2016-12-16 Thread Luis R. Rodriguez
On Thu, Dec 15, 2016 at 10:57:42AM +1030, Rusty Russell wrote: > "Luis R. Rodriguez" writes: > > kmod has an optimization in place whereby if a some kernel code > > uses request_module() on a module already loaded we never bother > > userspace as the module already is loaded. This is not true for

Re: [RFC 10/10] kmod: add a sanity check on module loading

2016-12-15 Thread Luis R. Rodriguez
On Fri, Dec 09, 2016 at 12:56:21PM -0800, Linus Torvalds wrote: > On Fri, Dec 9, 2016 at 12:03 PM, Martin Wilck wrote: > > On Thu, 2016-12-08 at 11:49 -0800, Luis R. Rodriguez wrote: > >> > >> Although this does get us in the business of keeping alias maps in > >> kernel, the the

Re: [RFC 10/10] kmod: add a sanity check on module loading

2016-12-15 Thread Luis R. Rodriguez
On Fri, Dec 09, 2016 at 12:56:21PM -0800, Linus Torvalds wrote: > On Fri, Dec 9, 2016 at 12:03 PM, Martin Wilck wrote: > > On Thu, 2016-12-08 at 11:49 -0800, Luis R. Rodriguez wrote: > >> > >> Although this does get us in the business of keeping alias maps in > >> kernel, the the work to support

Re: [RFC 10/10] kmod: add a sanity check on module loading

2016-12-14 Thread Rusty Russell
"Luis R. Rodriguez" writes: > kmod has an optimization in place whereby if a some kernel code > uses request_module() on a module already loaded we never bother > userspace as the module already is loaded. This is not true for > get_fs_type() though as it uses aliases. Well,

Re: [RFC 10/10] kmod: add a sanity check on module loading

2016-12-14 Thread Rusty Russell
"Luis R. Rodriguez" writes: > kmod has an optimization in place whereby if a some kernel code > uses request_module() on a module already loaded we never bother > userspace as the module already is loaded. This is not true for > get_fs_type() though as it uses aliases. Well, the obvious thing to

Re: [RFC 10/10] kmod: add a sanity check on module loading

2016-12-09 Thread Linus Torvalds
On Fri, Dec 9, 2016 at 12:03 PM, Martin Wilck wrote: > On Thu, 2016-12-08 at 11:49 -0800, Luis R. Rodriguez wrote: >> >> Although this does get us in the business of keeping alias maps in >> kernel, the the work to support and maintain this is trivial. > > You've implemented a

Re: [RFC 10/10] kmod: add a sanity check on module loading

2016-12-09 Thread Linus Torvalds
On Fri, Dec 9, 2016 at 12:03 PM, Martin Wilck wrote: > On Thu, 2016-12-08 at 11:49 -0800, Luis R. Rodriguez wrote: >> >> Although this does get us in the business of keeping alias maps in >> kernel, the the work to support and maintain this is trivial. > > You've implemented a special treatment

Re: [RFC 10/10] kmod: add a sanity check on module loading

2016-12-09 Thread Martin Wilck
On Thu, 2016-12-08 at 11:49 -0800, Luis R. Rodriguez wrote: > > Although this does get us in the business of keeping alias maps in > kernel, the the work to support and maintain this is trivial. You've implemented a special treatment for request_module("fs-$X")in finished_kmod_load(), but there

Re: [RFC 10/10] kmod: add a sanity check on module loading

2016-12-09 Thread Martin Wilck
On Thu, 2016-12-08 at 11:49 -0800, Luis R. Rodriguez wrote: > > Although this does get us in the business of keeping alias maps in > kernel, the the work to support and maintain this is trivial. You've implemented a special treatment for request_module("fs-$X")in finished_kmod_load(), but there

[RFC 10/10] kmod: add a sanity check on module loading

2016-12-08 Thread Luis R. Rodriguez
kmod has an optimization in place whereby if a some kernel code uses request_module() on a module already loaded we never bother userspace as the module already is loaded. This is not true for get_fs_type() though as it uses aliases. Additionally kmod <= v19 was broken -- it returns 0 to modprobe

[RFC 10/10] kmod: add a sanity check on module loading

2016-12-08 Thread Luis R. Rodriguez
kmod has an optimization in place whereby if a some kernel code uses request_module() on a module already loaded we never bother userspace as the module already is loaded. This is not true for get_fs_type() though as it uses aliases. Additionally kmod <= v19 was broken -- it returns 0 to modprobe