[PATCH] module: Remove module size limit

[PATCH] module: Remove module size limit

[Sasha Levin]

Module size was limited to 64MB, this was legacy limitation due to vmalloc()
which was removed a while ago.

Limiting module size to 64MB is both pointless and affects real world use
cases.

Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Tim Abbott <tim.abbott@oracle.com>
Cc: stable@vger.kernel.org
Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
---
 kernel/module.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index 2c93276..3d56b6f 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2380,8 +2380,7 @@ static int copy_and_check(struct load_info *info,
 		return -ENOEXEC;
 
 	/* Suck in entire file: we'll want most of it. */
-	/* vmalloc barfs on "unusual" numbers.  Check here */
-	if (len > 64 * 1024 * 1024 || (hdr = vmalloc(len)) == NULL)
+	if ((hdr = vmalloc(len)) == NULL)
 		return -ENOMEM;
 
 	if (copy_from_user(hdr, umod, len) != 0) {
-- 
1.7.8.4

Re: [PATCH] module: Remove module size limit

[Sasha Levin]

Rusty,

I'm not sure why git forgot to add the 'From' line, but if you do merge
it please add:

From: Sasha Levin <sasha.levin@oracle.com>

Thanks!

On Mon, 2012-01-30 at 23:07 -0500, Sasha Levin wrote:
> Module size was limited to 64MB, this was legacy limitation due to vmalloc()
> which was removed a while ago.
> 
> Limiting module size to 64MB is both pointless and affects real world use
> cases.
> 
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Tim Abbott <tim.abbott@oracle.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> ---
>  kernel/module.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/module.c b/kernel/module.c
> index 2c93276..3d56b6f 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2380,8 +2380,7 @@ static int copy_and_check(struct load_info *info,
>  		return -ENOEXEC;
>  
>  	/* Suck in entire file: we'll want most of it. */
> -	/* vmalloc barfs on "unusual" numbers.  Check here */
> -	if (len > 64 * 1024 * 1024 || (hdr = vmalloc(len)) == NULL)
> +	if ((hdr = vmalloc(len)) == NULL)
>  		return -ENOMEM;
>  
>  	if (copy_from_user(hdr, umod, len) != 0) {


-- 

Sasha.

Re: [PATCH] module: Remove module size limit

[Josh Boyer]

On Mon, Jan 30, 2012 at 11:07 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
> Module size was limited to 64MB, this was legacy limitation due to vmalloc()
> which was removed a while ago.
>
> Limiting module size to 64MB is both pointless and affects real world use
> cases.
>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Tim Abbott <tim.abbott@oracle.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> ---
>  kernel/module.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/module.c b/kernel/module.c
> index 2c93276..3d56b6f 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -2380,8 +2380,7 @@ static int copy_and_check(struct load_info *info,
>                return -ENOEXEC;
>
>        /* Suck in entire file: we'll want most of it. */
> -       /* vmalloc barfs on "unusual" numbers.  Check here */
> -       if (len > 64 * 1024 * 1024 || (hdr = vmalloc(len)) == NULL)
> +       if ((hdr = vmalloc(len)) == NULL)
>                return -ENOMEM;
>
>        if (copy_from_user(hdr, umod, len) != 0) {

I could be missing something somewhere, but this is the only upper bounds
check that is in place on the overall module size.  If we remove this without
putting some other kind of sanity check, wouldn't it be possible for someone
to exhaust the entire vmalloc space for the kernel by loading a bloated module?

I would think we still want to have some form of upper bounds check to prevent
that, but maybe I'm paranoid.

As an aside, which real world use cases are blocked by having a 64MB limit?
That is already HUGE.

josh

Re: [PATCH] module: Remove module size limit

[Sasha Levin]

On Tue, 2012-01-31 at 09:07 -0500, Josh Boyer wrote:
> On Mon, Jan 30, 2012 at 11:07 PM, Sasha Levin <levinsasha928@gmail.com> wrote:
> > Module size was limited to 64MB, this was legacy limitation due to vmalloc()
> > which was removed a while ago.
> >
> > Limiting module size to 64MB is both pointless and affects real world use
> > cases.
> >
> > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > Cc: Tim Abbott <tim.abbott@oracle.com>
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Sasha Levin <sasha.levin@oracle.com>
> > ---
> >  kernel/module.c |    3 +--
> >  1 files changed, 1 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/module.c b/kernel/module.c
> > index 2c93276..3d56b6f 100644
> > --- a/kernel/module.c
> > +++ b/kernel/module.c
> > @@ -2380,8 +2380,7 @@ static int copy_and_check(struct load_info *info,
> >                return -ENOEXEC;
> >
> >        /* Suck in entire file: we'll want most of it. */
> > -       /* vmalloc barfs on "unusual" numbers.  Check here */
> > -       if (len > 64 * 1024 * 1024 || (hdr = vmalloc(len)) == NULL)
> > +       if ((hdr = vmalloc(len)) == NULL)
> >                return -ENOMEM;
> >
> >        if (copy_from_user(hdr, umod, len) != 0) {
> 
> I could be missing something somewhere, but this is the only upper bounds
> check that is in place on the overall module size.  If we remove this without
> putting some other kind of sanity check, wouldn't it be possible for someone
> to exhaust the entire vmalloc space for the kernel by loading a bloated module?

That someone needs CAP_SYS_MODULE to load a module in the first place,
it means that the user has to be privileged enough to load modules at
all.
Right now he can exhaust the vmalloc space simply by loading multiple
64MB modules, I don't think it matters much if we allow him to load one
module with over 64MB.
Also, if a malicious user can get a privileged user to load a kernel
module of his choice there are bigger things to worry about than the
vmalloc space.

> I would think we still want to have some form of upper bounds check to prevent
> that, but maybe I'm paranoid.

If there is a valid technical limit it would make sense, but just
scaling the 64MB limit up arbitrarily is pointless.

> As an aside, which real world use cases are blocked by having a 64MB limit?
> That is already HUGE.

When using KSplice, there is a debug option which allows you to generate
debug modules which weigh just a bit over 64MB. We currently patched the
64MB check out using a different KSplice patch, and everything works
quite well.

Re: [PATCH] module: Remove module size limit

[Rusty Russell]

On Mon, 30 Jan 2012 23:07:22 -0500, Sasha Levin <levinsasha928@gmail.com> wrote:
> Module size was limited to 64MB, this was legacy limitation due to vmalloc()
> which was removed a while ago.
> 
> Limiting module size to 64MB is both pointless and affects real world use
> cases.
> 
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> Cc: Tim Abbott <tim.abbott@oracle.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Sasha Levin <sasha.levin@oracle.com>

Thanks, applied.

Cheers,
Rusty.