Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-19 Thread Kees Cook
On Wed, Sep 19, 2012 at 7:41 AM, Mimi Zohar  wrote:
> On Wed, 2012-09-19 at 13:08 +0930, Rusty Russell wrote:
>> Mimi Zohar  writes:
>>
>> > On Wed, 2012-09-12 at 17:04 +0930, Rusty Russell wrote:
>> >> "H. Peter Anvin"  writes:
>> >>
>> >> > On 09/06/2012 11:13 AM, Kees Cook wrote:
>> >> >> Instead of (or in addition to) kernel module signing, being able to 
>> >> >> reason
>> >> >> about the origin of a kernel module would be valuable in situations
>> >> >> where an OS already trusts a specific file system, file, etc, due to
>> >> >> things like security labels or an existing root of trust to a partition
>> >> >> through things like dm-verity.
>> >> >>
>> >> >> This introduces a new syscall (currently only on x86), similar to
>> >> >> init_module, that has only two arguments. The first argument is used as
>> >> >> a file descriptor to the module and the second argument is a pointer to
>> >> >> the NULL terminated string of module arguments.
>> >> >>
>> >> >
>> >> > Please use the standard naming convention, which is an f- prefix (i.e.
>> >> > finit_module()).
>> >>
>> >> Good point; I just did a replace here.
>> >
>> > Have you pushed out the changes?  And if so, to where?
>>
>> No, I kept them in my patch series but out of linux-next, since I
>> thought you disliked the placement of the security hooks?
>
> I thought about it some more.  The call to
> security_kernel_module_from_file() from copy_module_from_user() doesn't
> provide any information, not the buffer contents nor the signature.  The
> only thing IMA-appraisal can do is to fail the request with
> INTEGRITY_UNKNOWN.  This is reflected in the IMA-appraisal patch I
> posted http://marc.info/?l=linux-security-module=134739023306344=2.
>
> Please add my Acked-by: Mimi Zohar 

FWIW, this was my intent: it is a way for the LSM to see an attempt to
load a module it can't reason about. If it wants to allow it blindly,
it can, otherwise is has the option to refuse it. I didn't want to
leave the old syscall unhooked.

-Kees

-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-19 Thread Mimi Zohar
On Wed, 2012-09-19 at 13:08 +0930, Rusty Russell wrote:
> Mimi Zohar  writes:
> 
> > On Wed, 2012-09-12 at 17:04 +0930, Rusty Russell wrote:
> >> "H. Peter Anvin"  writes:
> >> 
> >> > On 09/06/2012 11:13 AM, Kees Cook wrote:
> >> >> Instead of (or in addition to) kernel module signing, being able to 
> >> >> reason
> >> >> about the origin of a kernel module would be valuable in situations
> >> >> where an OS already trusts a specific file system, file, etc, due to
> >> >> things like security labels or an existing root of trust to a partition
> >> >> through things like dm-verity.
> >> >>
> >> >> This introduces a new syscall (currently only on x86), similar to
> >> >> init_module, that has only two arguments. The first argument is used as
> >> >> a file descriptor to the module and the second argument is a pointer to
> >> >> the NULL terminated string of module arguments.
> >> >>
> >> >
> >> > Please use the standard naming convention, which is an f- prefix (i.e. 
> >> > finit_module()).
> >> 
> >> Good point; I just did a replace here.
> >
> > Have you pushed out the changes?  And if so, to where?
> 
> No, I kept them in my patch series but out of linux-next, since I
> thought you disliked the placement of the security hooks?

I thought about it some more.  The call to
security_kernel_module_from_file() from copy_module_from_user() doesn't
provide any information, not the buffer contents nor the signature.  The
only thing IMA-appraisal can do is to fail the request with
INTEGRITY_UNKNOWN.  This is reflected in the IMA-appraisal patch I
posted http://marc.info/?l=linux-security-module=134739023306344=2.

Please add my Acked-by: Mimi Zohar  

thanks,

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-19 Thread Mimi Zohar
On Wed, 2012-09-19 at 13:08 +0930, Rusty Russell wrote:
 Mimi Zohar zo...@linux.vnet.ibm.com writes:
 
  On Wed, 2012-09-12 at 17:04 +0930, Rusty Russell wrote:
  H. Peter Anvin h...@zytor.com writes:
  
   On 09/06/2012 11:13 AM, Kees Cook wrote:
   Instead of (or in addition to) kernel module signing, being able to 
   reason
   about the origin of a kernel module would be valuable in situations
   where an OS already trusts a specific file system, file, etc, due to
   things like security labels or an existing root of trust to a partition
   through things like dm-verity.
  
   This introduces a new syscall (currently only on x86), similar to
   init_module, that has only two arguments. The first argument is used as
   a file descriptor to the module and the second argument is a pointer to
   the NULL terminated string of module arguments.
  
  
   Please use the standard naming convention, which is an f- prefix (i.e. 
   finit_module()).
  
  Good point; I just did a replace here.
 
  Have you pushed out the changes?  And if so, to where?
 
 No, I kept them in my patch series but out of linux-next, since I
 thought you disliked the placement of the security hooks?

I thought about it some more.  The call to
security_kernel_module_from_file() from copy_module_from_user() doesn't
provide any information, not the buffer contents nor the signature.  The
only thing IMA-appraisal can do is to fail the request with
INTEGRITY_UNKNOWN.  This is reflected in the IMA-appraisal patch I
posted http://marc.info/?l=linux-security-modulem=134739023306344w=2.

Please add my Acked-by: Mimi Zohar zo...@us.ibm.com 

thanks,

Mimi

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-19 Thread Kees Cook
On Wed, Sep 19, 2012 at 7:41 AM, Mimi Zohar zo...@linux.vnet.ibm.com wrote:
 On Wed, 2012-09-19 at 13:08 +0930, Rusty Russell wrote:
 Mimi Zohar zo...@linux.vnet.ibm.com writes:

  On Wed, 2012-09-12 at 17:04 +0930, Rusty Russell wrote:
  H. Peter Anvin h...@zytor.com writes:
 
   On 09/06/2012 11:13 AM, Kees Cook wrote:
   Instead of (or in addition to) kernel module signing, being able to 
   reason
   about the origin of a kernel module would be valuable in situations
   where an OS already trusts a specific file system, file, etc, due to
   things like security labels or an existing root of trust to a partition
   through things like dm-verity.
  
   This introduces a new syscall (currently only on x86), similar to
   init_module, that has only two arguments. The first argument is used as
   a file descriptor to the module and the second argument is a pointer to
   the NULL terminated string of module arguments.
  
  
   Please use the standard naming convention, which is an f- prefix (i.e.
   finit_module()).
 
  Good point; I just did a replace here.
 
  Have you pushed out the changes?  And if so, to where?

 No, I kept them in my patch series but out of linux-next, since I
 thought you disliked the placement of the security hooks?

 I thought about it some more.  The call to
 security_kernel_module_from_file() from copy_module_from_user() doesn't
 provide any information, not the buffer contents nor the signature.  The
 only thing IMA-appraisal can do is to fail the request with
 INTEGRITY_UNKNOWN.  This is reflected in the IMA-appraisal patch I
 posted http://marc.info/?l=linux-security-modulem=134739023306344w=2.

 Please add my Acked-by: Mimi Zohar zo...@us.ibm.com

FWIW, this was my intent: it is a way for the LSM to see an attempt to
load a module it can't reason about. If it wants to allow it blindly,
it can, otherwise is has the option to refuse it. I didn't want to
leave the old syscall unhooked.

-Kees

-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-18 Thread Rusty Russell
Mimi Zohar  writes:

> On Wed, 2012-09-12 at 17:04 +0930, Rusty Russell wrote:
>> "H. Peter Anvin"  writes:
>> 
>> > On 09/06/2012 11:13 AM, Kees Cook wrote:
>> >> Instead of (or in addition to) kernel module signing, being able to reason
>> >> about the origin of a kernel module would be valuable in situations
>> >> where an OS already trusts a specific file system, file, etc, due to
>> >> things like security labels or an existing root of trust to a partition
>> >> through things like dm-verity.
>> >>
>> >> This introduces a new syscall (currently only on x86), similar to
>> >> init_module, that has only two arguments. The first argument is used as
>> >> a file descriptor to the module and the second argument is a pointer to
>> >> the NULL terminated string of module arguments.
>> >>
>> >
>> > Please use the standard naming convention, which is an f- prefix (i.e. 
>> > finit_module()).
>> 
>> Good point; I just did a replace here.
>
> Have you pushed out the changes?  And if so, to where?

No, I kept them in my patch series but out of linux-next, since I
thought you disliked the placement of the security hooks?

Cheers,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-18 Thread Rusty Russell
Mimi Zohar zo...@linux.vnet.ibm.com writes:

 On Wed, 2012-09-12 at 17:04 +0930, Rusty Russell wrote:
 H. Peter Anvin h...@zytor.com writes:
 
  On 09/06/2012 11:13 AM, Kees Cook wrote:
  Instead of (or in addition to) kernel module signing, being able to reason
  about the origin of a kernel module would be valuable in situations
  where an OS already trusts a specific file system, file, etc, due to
  things like security labels or an existing root of trust to a partition
  through things like dm-verity.
 
  This introduces a new syscall (currently only on x86), similar to
  init_module, that has only two arguments. The first argument is used as
  a file descriptor to the module and the second argument is a pointer to
  the NULL terminated string of module arguments.
 
 
  Please use the standard naming convention, which is an f- prefix (i.e. 
  finit_module()).
 
 Good point; I just did a replace here.

 Have you pushed out the changes?  And if so, to where?

No, I kept them in my patch series but out of linux-next, since I
thought you disliked the placement of the security hooks?

Cheers,
Rusty.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-13 Thread Mimi Zohar
On Wed, 2012-09-12 at 17:04 +0930, Rusty Russell wrote:
> "H. Peter Anvin"  writes:
> 
> > On 09/06/2012 11:13 AM, Kees Cook wrote:
> >> Instead of (or in addition to) kernel module signing, being able to reason
> >> about the origin of a kernel module would be valuable in situations
> >> where an OS already trusts a specific file system, file, etc, due to
> >> things like security labels or an existing root of trust to a partition
> >> through things like dm-verity.
> >>
> >> This introduces a new syscall (currently only on x86), similar to
> >> init_module, that has only two arguments. The first argument is used as
> >> a file descriptor to the module and the second argument is a pointer to
> >> the NULL terminated string of module arguments.
> >>
> >
> > Please use the standard naming convention, which is an f- prefix (i.e. 
> > finit_module()).
> 
> Good point; I just did a replace here.

Have you pushed out the changes?  And if so, to where?

thanks,

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-13 Thread Mimi Zohar
On Wed, 2012-09-12 at 17:04 +0930, Rusty Russell wrote:
 H. Peter Anvin h...@zytor.com writes:
 
  On 09/06/2012 11:13 AM, Kees Cook wrote:
  Instead of (or in addition to) kernel module signing, being able to reason
  about the origin of a kernel module would be valuable in situations
  where an OS already trusts a specific file system, file, etc, due to
  things like security labels or an existing root of trust to a partition
  through things like dm-verity.
 
  This introduces a new syscall (currently only on x86), similar to
  init_module, that has only two arguments. The first argument is used as
  a file descriptor to the module and the second argument is a pointer to
  the NULL terminated string of module arguments.
 
 
  Please use the standard naming convention, which is an f- prefix (i.e. 
  finit_module()).
 
 Good point; I just did a replace here.

Have you pushed out the changes?  And if so, to where?

thanks,

Mimi

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-12 Thread Kees Cook
On Wed, Sep 12, 2012 at 12:34 AM, Rusty Russell  wrote:
> "H. Peter Anvin"  writes:
>
>> On 09/06/2012 11:13 AM, Kees Cook wrote:
>>> Instead of (or in addition to) kernel module signing, being able to reason
>>> about the origin of a kernel module would be valuable in situations
>>> where an OS already trusts a specific file system, file, etc, due to
>>> things like security labels or an existing root of trust to a partition
>>> through things like dm-verity.
>>>
>>> This introduces a new syscall (currently only on x86), similar to
>>> init_module, that has only two arguments. The first argument is used as
>>> a file descriptor to the module and the second argument is a pointer to
>>> the NULL terminated string of module arguments.
>>>
>>
>> Please use the standard naming convention, which is an f- prefix (i.e.
>> finit_module()).
>
> Good point; I just did a replace here.

Ah, yes. Thanks!

-Kees

-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-12 Thread Rusty Russell
"H. Peter Anvin"  writes:

> On 09/06/2012 11:13 AM, Kees Cook wrote:
>> Instead of (or in addition to) kernel module signing, being able to reason
>> about the origin of a kernel module would be valuable in situations
>> where an OS already trusts a specific file system, file, etc, due to
>> things like security labels or an existing root of trust to a partition
>> through things like dm-verity.
>>
>> This introduces a new syscall (currently only on x86), similar to
>> init_module, that has only two arguments. The first argument is used as
>> a file descriptor to the module and the second argument is a pointer to
>> the NULL terminated string of module arguments.
>>
>
> Please use the standard naming convention, which is an f- prefix (i.e. 
> finit_module()).

Good point; I just did a replace here.

Thanks,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-12 Thread Rusty Russell
H. Peter Anvin h...@zytor.com writes:

 On 09/06/2012 11:13 AM, Kees Cook wrote:
 Instead of (or in addition to) kernel module signing, being able to reason
 about the origin of a kernel module would be valuable in situations
 where an OS already trusts a specific file system, file, etc, due to
 things like security labels or an existing root of trust to a partition
 through things like dm-verity.

 This introduces a new syscall (currently only on x86), similar to
 init_module, that has only two arguments. The first argument is used as
 a file descriptor to the module and the second argument is a pointer to
 the NULL terminated string of module arguments.


 Please use the standard naming convention, which is an f- prefix (i.e. 
 finit_module()).

Good point; I just did a replace here.

Thanks,
Rusty.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-12 Thread Kees Cook
On Wed, Sep 12, 2012 at 12:34 AM, Rusty Russell ru...@rustcorp.com.au wrote:
 H. Peter Anvin h...@zytor.com writes:

 On 09/06/2012 11:13 AM, Kees Cook wrote:
 Instead of (or in addition to) kernel module signing, being able to reason
 about the origin of a kernel module would be valuable in situations
 where an OS already trusts a specific file system, file, etc, due to
 things like security labels or an existing root of trust to a partition
 through things like dm-verity.

 This introduces a new syscall (currently only on x86), similar to
 init_module, that has only two arguments. The first argument is used as
 a file descriptor to the module and the second argument is a pointer to
 the NULL terminated string of module arguments.


 Please use the standard naming convention, which is an f- prefix (i.e.
 finit_module()).

 Good point; I just did a replace here.

Ah, yes. Thanks!

-Kees

-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-11 Thread H. Peter Anvin

On 09/06/2012 11:13 AM, Kees Cook wrote:

Instead of (or in addition to) kernel module signing, being able to reason
about the origin of a kernel module would be valuable in situations
where an OS already trusts a specific file system, file, etc, due to
things like security labels or an existing root of trust to a partition
through things like dm-verity.

This introduces a new syscall (currently only on x86), similar to
init_module, that has only two arguments. The first argument is used as
a file descriptor to the module and the second argument is a pointer to
the NULL terminated string of module arguments.



Please use the standard naming convention, which is an f- prefix (i.e. 
finit_module()).


-hpa

--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-11 Thread James Morris
On Mon, 10 Sep 2012, Rusty Russell wrote:

> Kees Cook  writes:
> > On Fri, Sep 7, 2012 at 10:12 AM, Mimi Zohar  
> > wrote:
> >> This method is a consistent and extensible approach to verifying the
> >> integrity of file data/metadata, including kernel modules. The only
> >> downside to this approach, I think, is that it requires changes to the
> >> userspace tool.
> >
> > I'm fine with this -- it's an expected change that I'll pursue with
> > glibc, kmod, etc. Without the userspace changes, nothing will use the
> > new syscall. :) I've already got kmod (and older module-init-tools)
> > patched to do this locally.
> 
> A syscall is the right way to do this.  But does it need to be done?
> 
> 1) Do the LSM guys really want this hook?

Yes.

Acked-by: James Morris 



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-11 Thread James Morris
On Mon, 10 Sep 2012, Rusty Russell wrote:

 Kees Cook keesc...@chromium.org writes:
  On Fri, Sep 7, 2012 at 10:12 AM, Mimi Zohar zo...@linux.vnet.ibm.com 
  wrote:
  This method is a consistent and extensible approach to verifying the
  integrity of file data/metadata, including kernel modules. The only
  downside to this approach, I think, is that it requires changes to the
  userspace tool.
 
  I'm fine with this -- it's an expected change that I'll pursue with
  glibc, kmod, etc. Without the userspace changes, nothing will use the
  new syscall. :) I've already got kmod (and older module-init-tools)
  patched to do this locally.
 
 A syscall is the right way to do this.  But does it need to be done?
 
 1) Do the LSM guys really want this hook?

Yes.

Acked-by: James Morris james.l.mor...@oracle.com



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-11 Thread H. Peter Anvin

On 09/06/2012 11:13 AM, Kees Cook wrote:

Instead of (or in addition to) kernel module signing, being able to reason
about the origin of a kernel module would be valuable in situations
where an OS already trusts a specific file system, file, etc, due to
things like security labels or an existing root of trust to a partition
through things like dm-verity.

This introduces a new syscall (currently only on x86), similar to
init_module, that has only two arguments. The first argument is used as
a file descriptor to the module and the second argument is a pointer to
the NULL terminated string of module arguments.



Please use the standard naming convention, which is an f- prefix (i.e. 
finit_module()).


-hpa

--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-10 Thread Kees Cook
On Sun, Sep 9, 2012 at 6:46 PM, Rusty Russell  wrote:
> Kees Cook  writes:
>> On Fri, Sep 7, 2012 at 10:12 AM, Mimi Zohar  wrote:
>>> This method is a consistent and extensible approach to verifying the
>>> integrity of file data/metadata, including kernel modules. The only
>>> downside to this approach, I think, is that it requires changes to the
>>> userspace tool.
>>
>> I'm fine with this -- it's an expected change that I'll pursue with
>> glibc, kmod, etc. Without the userspace changes, nothing will use the
>> new syscall. :) I've already got kmod (and older module-init-tools)
>> patched to do this locally.
>
> A syscall is the right way to do this.  But does it need to be done?
>
> 1) Do the LSM guys really want this hook?

The LSM hook half has already been acked by Serge and Eric, and I want
to use it in Yama as well.

> 2) Do we have a userspace which uses it?

Chrome OS will be using it; I have patches for kmod and
module-init-tools already.

> If yes to both, and noone comes up with any creative complaints, I will
> take the patch.

Sound good; thanks!

-Kees

-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-10 Thread Kees Cook
On Sun, Sep 9, 2012 at 6:46 PM, Rusty Russell ru...@rustcorp.com.au wrote:
 Kees Cook keesc...@chromium.org writes:
 On Fri, Sep 7, 2012 at 10:12 AM, Mimi Zohar zo...@linux.vnet.ibm.com wrote:
 This method is a consistent and extensible approach to verifying the
 integrity of file data/metadata, including kernel modules. The only
 downside to this approach, I think, is that it requires changes to the
 userspace tool.

 I'm fine with this -- it's an expected change that I'll pursue with
 glibc, kmod, etc. Without the userspace changes, nothing will use the
 new syscall. :) I've already got kmod (and older module-init-tools)
 patched to do this locally.

 A syscall is the right way to do this.  But does it need to be done?

 1) Do the LSM guys really want this hook?

The LSM hook half has already been acked by Serge and Eric, and I want
to use it in Yama as well.

 2) Do we have a userspace which uses it?

Chrome OS will be using it; I have patches for kmod and
module-init-tools already.

 If yes to both, and noone comes up with any creative complaints, I will
 take the patch.

Sound good; thanks!

-Kees

-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-09 Thread Rusty Russell
Kees Cook  writes:
> On Fri, Sep 7, 2012 at 10:12 AM, Mimi Zohar  wrote:
>> This method is a consistent and extensible approach to verifying the
>> integrity of file data/metadata, including kernel modules. The only
>> downside to this approach, I think, is that it requires changes to the
>> userspace tool.
>
> I'm fine with this -- it's an expected change that I'll pursue with
> glibc, kmod, etc. Without the userspace changes, nothing will use the
> new syscall. :) I've already got kmod (and older module-init-tools)
> patched to do this locally.

A syscall is the right way to do this.  But does it need to be done?

1) Do the LSM guys really want this hook?
2) Do we have a userspace which uses it?

If yes to both, and noone comes up with any creative complaints, I will
take the patch.

Cheers,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-09 Thread Rusty Russell
Kees Cook keesc...@chromium.org writes:
 On Fri, Sep 7, 2012 at 10:12 AM, Mimi Zohar zo...@linux.vnet.ibm.com wrote:
 This method is a consistent and extensible approach to verifying the
 integrity of file data/metadata, including kernel modules. The only
 downside to this approach, I think, is that it requires changes to the
 userspace tool.

 I'm fine with this -- it's an expected change that I'll pursue with
 glibc, kmod, etc. Without the userspace changes, nothing will use the
 new syscall. :) I've already got kmod (and older module-init-tools)
 patched to do this locally.

A syscall is the right way to do this.  But does it need to be done?

1) Do the LSM guys really want this hook?
2) Do we have a userspace which uses it?

If yes to both, and noone comes up with any creative complaints, I will
take the patch.

Cheers,
Rusty.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-07 Thread Mimi Zohar
On Fri, 2012-09-07 at 10:19 -0700, Kees Cook wrote:
> On Fri, Sep 7, 2012 at 10:12 AM, Mimi Zohar  wrote:
> > On Fri, 2012-09-07 at 09:45 +0930, Rusty Russell wrote:
> >> Kees Cook  writes:
> >> > Instead of (or in addition to) kernel module signing, being able to 
> >> > reason
> >> > about the origin of a kernel module would be valuable in situations
> >> > where an OS already trusts a specific file system, file, etc, due to
> >> > things like security labels or an existing root of trust to a partition
> >> > through things like dm-verity.
> >> >
> >> > This introduces a new syscall (currently only on x86), similar to
> >> > init_module, that has only two arguments. The first argument is used as
> >> > a file descriptor to the module and the second argument is a pointer to
> >> > the NULL terminated string of module arguments.
> >>
> >> Thanks.  Minor comments follow:
> >
> > Rusty, sorry for bringing this up again, but with Kees' new syscall,
> > which passes in the file descriptor, appraising the integrity of kernel
> > modules could be like appraising the integrity of any other file on the
> > filesystem.  All that would be needed is a new security hook, which is
> > needed in anycase for IMA measurement.
> 
> The second patch in this series provides such a hook.

Thanks! Don't know how I missed it.

> 
> > [...]
> > This method is a consistent and extensible approach to verifying the
> > integrity of file data/metadata, including kernel modules. The only
> > downside to this approach, I think, is that it requires changes to the
> > userspace tool.
> 
> I'm fine with this -- it's an expected change that I'll pursue with
> glibc, kmod, etc. Without the userspace changes, nothing will use the
> new syscall. :) I've already got kmod (and older module-init-tools)
> patched to do this locally.

Great!

Mimi


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-07 Thread Kees Cook
On Fri, Sep 7, 2012 at 10:12 AM, Mimi Zohar  wrote:
> On Fri, 2012-09-07 at 09:45 +0930, Rusty Russell wrote:
>> Kees Cook  writes:
>> > Instead of (or in addition to) kernel module signing, being able to reason
>> > about the origin of a kernel module would be valuable in situations
>> > where an OS already trusts a specific file system, file, etc, due to
>> > things like security labels or an existing root of trust to a partition
>> > through things like dm-verity.
>> >
>> > This introduces a new syscall (currently only on x86), similar to
>> > init_module, that has only two arguments. The first argument is used as
>> > a file descriptor to the module and the second argument is a pointer to
>> > the NULL terminated string of module arguments.
>>
>> Thanks.  Minor comments follow:
>
> Rusty, sorry for bringing this up again, but with Kees' new syscall,
> which passes in the file descriptor, appraising the integrity of kernel
> modules could be like appraising the integrity of any other file on the
> filesystem.  All that would be needed is a new security hook, which is
> needed in anycase for IMA measurement.

The second patch in this series provides such a hook.

> [...]
> This method is a consistent and extensible approach to verifying the
> integrity of file data/metadata, including kernel modules. The only
> downside to this approach, I think, is that it requires changes to the
> userspace tool.

I'm fine with this -- it's an expected change that I'll pursue with
glibc, kmod, etc. Without the userspace changes, nothing will use the
new syscall. :) I've already got kmod (and older module-init-tools)
patched to do this locally.

-Kees

-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-07 Thread Mimi Zohar
On Fri, 2012-09-07 at 09:45 +0930, Rusty Russell wrote:
> Kees Cook  writes:
> > Instead of (or in addition to) kernel module signing, being able to reason
> > about the origin of a kernel module would be valuable in situations
> > where an OS already trusts a specific file system, file, etc, due to
> > things like security labels or an existing root of trust to a partition
> > through things like dm-verity.
> >
> > This introduces a new syscall (currently only on x86), similar to
> > init_module, that has only two arguments. The first argument is used as
> > a file descriptor to the module and the second argument is a pointer to
> > the NULL terminated string of module arguments.
> 
> Thanks.  Minor comments follow:

Rusty, sorry for bringing this up again, but with Kees' new syscall,
which passes in the file descriptor, appraising the integrity of kernel
modules could be like appraising the integrity of any other file on the
filesystem.  All that would be needed is a new security hook, which is
needed in anycase for IMA measurement.

The concerns with this approach, expressed in the past by David Howells,
are that it requires IMA-appraisal to be enabled, extended attribute
support, and changes to userspace tools.  Normally I wouldn't be too
concerned about filesystems that don't support extended attributes, but
the initramfs is currently cpio.  Perhaps this isn't an issue in
anycase, as the initramfs would be appraised in the secure boot
environment.

The first two concerns could be addressed by passing in a digital
signature, which the new syscall supports. The signature could be stored
as an extended attribute, appended to the kernel module or, as
originally described by Dmitry, in a .sig file. Where/how the signature
is stored would be left up to the distro's and userspace tool's
discretion.

When EVM/IMA-appraisal is enabled, it would either appraise the kernel
module based on the xattr, if available, or the supplied signature.
Otherwise, without EVM/IMA-appraisal enabled, the stub hook would
appraise the kernel module based on the supplied signature, calling
integrity_digsig_verify() directly.

This method is a consistent and extensible approach to verifying the
integrity of file data/metadata, including kernel modules. The only
downside to this approach, I think, is that it requires changes to the
userspace tool.

thanks,

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-07 Thread Kees Cook
On Thu, Sep 6, 2012 at 5:15 PM, Rusty Russell  wrote:
> Kees Cook  writes:
>> Instead of (or in addition to) kernel module signing, being able to reason
>> about the origin of a kernel module would be valuable in situations
>> where an OS already trusts a specific file system, file, etc, due to
>> things like security labels or an existing root of trust to a partition
>> through things like dm-verity.
>>
>> This introduces a new syscall (currently only on x86), similar to
>> init_module, that has only two arguments. The first argument is used as
>> a file descriptor to the module and the second argument is a pointer to
>> the NULL terminated string of module arguments.
>
> Thanks.  Minor comments follow:
>
>> +350  i386init_module_from_fd sys_init_module_from_fd
>
> The from_ seems redundant.

I'm happy to drop it.

>> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
>> index 19439c7..5386629 100644
>> --- a/include/linux/syscalls.h
>> +++ b/include/linux/syscalls.h
>> @@ -860,4 +860,5 @@ asmlinkage long sys_process_vm_writev(pid_t pid,
>>
>>  asmlinkage long sys_kcmp(pid_t pid1, pid_t pid2, int type,
>>unsigned long idx1, unsigned long idx2);
>> +asmlinkage long sys_init_module_from_fd(int len, const char __user *uargs);
>>  #endif
>
> You mean, "int fd"?

Whoops. Thanks, I'll fix that.

>> diff --git a/kernel/module.c b/kernel/module.c
>> index 4edbd9c..b080cf8 100644
>> --- a/kernel/module.c
>> +++ b/kernel/module.c
> ...
>> +/* Sets info->hdr and info->len. */
>> +int copy_module_from_fd(int fd, struct load_info *info)
>> +{
>> + struct file *file;
>> + int err;
>> + struct kstat stat;
>> + unsigned long size;
>> + off_t pos;
>> + ssize_t bytes = 0;
>> +
>> + file = fget(fd);
>> + if (!file)
>> + return -ENOEXEC;
>> +
>> + err = vfs_getattr(file->f_vfsmnt, file->f_dentry, );
>> + if (err)
>> + goto out;
>> +
>> + if (stat.size > INT_MAX) {
>> + err = -ENOMEM;
>> + goto out;
>>   }
>> + size = stat.size;
>>
>> - if (hdr->e_shoff >= len ||
>> - hdr->e_shnum * sizeof(Elf_Shdr) > len - hdr->e_shoff) {
>> - err = -ENOEXEC;
>> - goto free_hdr;
>> + info->hdr = vmalloc(size);
>> + if (!info->hdr) {
>> + err = -ENOMEM;
>> + goto out;
>
> The stat.size > INT_MAX is redundant: vmalloc is quite careful on its
> checking of the size param.
>
> (We removed a similar test from the module.c code a few years ago).

Fair enough, I'll remove that too and send an updated version.

Thanks!

-Kees

-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-07 Thread Kees Cook
On Thu, Sep 6, 2012 at 5:15 PM, Rusty Russell ru...@rustcorp.com.au wrote:
 Kees Cook keesc...@chromium.org writes:
 Instead of (or in addition to) kernel module signing, being able to reason
 about the origin of a kernel module would be valuable in situations
 where an OS already trusts a specific file system, file, etc, due to
 things like security labels or an existing root of trust to a partition
 through things like dm-verity.

 This introduces a new syscall (currently only on x86), similar to
 init_module, that has only two arguments. The first argument is used as
 a file descriptor to the module and the second argument is a pointer to
 the NULL terminated string of module arguments.

 Thanks.  Minor comments follow:

 +350  i386init_module_from_fd sys_init_module_from_fd

 The from_ seems redundant.

I'm happy to drop it.

 diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
 index 19439c7..5386629 100644
 --- a/include/linux/syscalls.h
 +++ b/include/linux/syscalls.h
 @@ -860,4 +860,5 @@ asmlinkage long sys_process_vm_writev(pid_t pid,

  asmlinkage long sys_kcmp(pid_t pid1, pid_t pid2, int type,
unsigned long idx1, unsigned long idx2);
 +asmlinkage long sys_init_module_from_fd(int len, const char __user *uargs);
  #endif

 You mean, int fd?

Whoops. Thanks, I'll fix that.

 diff --git a/kernel/module.c b/kernel/module.c
 index 4edbd9c..b080cf8 100644
 --- a/kernel/module.c
 +++ b/kernel/module.c
 ...
 +/* Sets info-hdr and info-len. */
 +int copy_module_from_fd(int fd, struct load_info *info)
 +{
 + struct file *file;
 + int err;
 + struct kstat stat;
 + unsigned long size;
 + off_t pos;
 + ssize_t bytes = 0;
 +
 + file = fget(fd);
 + if (!file)
 + return -ENOEXEC;
 +
 + err = vfs_getattr(file-f_vfsmnt, file-f_dentry, stat);
 + if (err)
 + goto out;
 +
 + if (stat.size  INT_MAX) {
 + err = -ENOMEM;
 + goto out;
   }
 + size = stat.size;

 - if (hdr-e_shoff = len ||
 - hdr-e_shnum * sizeof(Elf_Shdr)  len - hdr-e_shoff) {
 - err = -ENOEXEC;
 - goto free_hdr;
 + info-hdr = vmalloc(size);
 + if (!info-hdr) {
 + err = -ENOMEM;
 + goto out;

 The stat.size  INT_MAX is redundant: vmalloc is quite careful on its
 checking of the size param.

 (We removed a similar test from the module.c code a few years ago).

Fair enough, I'll remove that too and send an updated version.

Thanks!

-Kees

-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-07 Thread Mimi Zohar
On Fri, 2012-09-07 at 09:45 +0930, Rusty Russell wrote:
 Kees Cook keesc...@chromium.org writes:
  Instead of (or in addition to) kernel module signing, being able to reason
  about the origin of a kernel module would be valuable in situations
  where an OS already trusts a specific file system, file, etc, due to
  things like security labels or an existing root of trust to a partition
  through things like dm-verity.
 
  This introduces a new syscall (currently only on x86), similar to
  init_module, that has only two arguments. The first argument is used as
  a file descriptor to the module and the second argument is a pointer to
  the NULL terminated string of module arguments.
 
 Thanks.  Minor comments follow:

Rusty, sorry for bringing this up again, but with Kees' new syscall,
which passes in the file descriptor, appraising the integrity of kernel
modules could be like appraising the integrity of any other file on the
filesystem.  All that would be needed is a new security hook, which is
needed in anycase for IMA measurement.

The concerns with this approach, expressed in the past by David Howells,
are that it requires IMA-appraisal to be enabled, extended attribute
support, and changes to userspace tools.  Normally I wouldn't be too
concerned about filesystems that don't support extended attributes, but
the initramfs is currently cpio.  Perhaps this isn't an issue in
anycase, as the initramfs would be appraised in the secure boot
environment.

The first two concerns could be addressed by passing in a digital
signature, which the new syscall supports. The signature could be stored
as an extended attribute, appended to the kernel module or, as
originally described by Dmitry, in a .sig file. Where/how the signature
is stored would be left up to the distro's and userspace tool's
discretion.

When EVM/IMA-appraisal is enabled, it would either appraise the kernel
module based on the xattr, if available, or the supplied signature.
Otherwise, without EVM/IMA-appraisal enabled, the stub hook would
appraise the kernel module based on the supplied signature, calling
integrity_digsig_verify() directly.

This method is a consistent and extensible approach to verifying the
integrity of file data/metadata, including kernel modules. The only
downside to this approach, I think, is that it requires changes to the
userspace tool.

thanks,

Mimi

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-07 Thread Kees Cook
On Fri, Sep 7, 2012 at 10:12 AM, Mimi Zohar zo...@linux.vnet.ibm.com wrote:
 On Fri, 2012-09-07 at 09:45 +0930, Rusty Russell wrote:
 Kees Cook keesc...@chromium.org writes:
  Instead of (or in addition to) kernel module signing, being able to reason
  about the origin of a kernel module would be valuable in situations
  where an OS already trusts a specific file system, file, etc, due to
  things like security labels or an existing root of trust to a partition
  through things like dm-verity.
 
  This introduces a new syscall (currently only on x86), similar to
  init_module, that has only two arguments. The first argument is used as
  a file descriptor to the module and the second argument is a pointer to
  the NULL terminated string of module arguments.

 Thanks.  Minor comments follow:

 Rusty, sorry for bringing this up again, but with Kees' new syscall,
 which passes in the file descriptor, appraising the integrity of kernel
 modules could be like appraising the integrity of any other file on the
 filesystem.  All that would be needed is a new security hook, which is
 needed in anycase for IMA measurement.

The second patch in this series provides such a hook.

 [...]
 This method is a consistent and extensible approach to verifying the
 integrity of file data/metadata, including kernel modules. The only
 downside to this approach, I think, is that it requires changes to the
 userspace tool.

I'm fine with this -- it's an expected change that I'll pursue with
glibc, kmod, etc. Without the userspace changes, nothing will use the
new syscall. :) I've already got kmod (and older module-init-tools)
patched to do this locally.

-Kees

-- 
Kees Cook
Chrome OS Security
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-07 Thread Mimi Zohar
On Fri, 2012-09-07 at 10:19 -0700, Kees Cook wrote:
 On Fri, Sep 7, 2012 at 10:12 AM, Mimi Zohar zo...@linux.vnet.ibm.com wrote:
  On Fri, 2012-09-07 at 09:45 +0930, Rusty Russell wrote:
  Kees Cook keesc...@chromium.org writes:
   Instead of (or in addition to) kernel module signing, being able to 
   reason
   about the origin of a kernel module would be valuable in situations
   where an OS already trusts a specific file system, file, etc, due to
   things like security labels or an existing root of trust to a partition
   through things like dm-verity.
  
   This introduces a new syscall (currently only on x86), similar to
   init_module, that has only two arguments. The first argument is used as
   a file descriptor to the module and the second argument is a pointer to
   the NULL terminated string of module arguments.
 
  Thanks.  Minor comments follow:
 
  Rusty, sorry for bringing this up again, but with Kees' new syscall,
  which passes in the file descriptor, appraising the integrity of kernel
  modules could be like appraising the integrity of any other file on the
  filesystem.  All that would be needed is a new security hook, which is
  needed in anycase for IMA measurement.
 
 The second patch in this series provides such a hook.

Thanks! Don't know how I missed it.

 
  [...]
  This method is a consistent and extensible approach to verifying the
  integrity of file data/metadata, including kernel modules. The only
  downside to this approach, I think, is that it requires changes to the
  userspace tool.
 
 I'm fine with this -- it's an expected change that I'll pursue with
 glibc, kmod, etc. Without the userspace changes, nothing will use the
 new syscall. :) I've already got kmod (and older module-init-tools)
 patched to do this locally.

Great!

Mimi


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-06 Thread Rusty Russell
Kees Cook  writes:
> Instead of (or in addition to) kernel module signing, being able to reason
> about the origin of a kernel module would be valuable in situations
> where an OS already trusts a specific file system, file, etc, due to
> things like security labels or an existing root of trust to a partition
> through things like dm-verity.
>
> This introduces a new syscall (currently only on x86), similar to
> init_module, that has only two arguments. The first argument is used as
> a file descriptor to the module and the second argument is a pointer to
> the NULL terminated string of module arguments.

Thanks.  Minor comments follow:

> +350  i386init_module_from_fd sys_init_module_from_fd

The from_ seems redundant.

> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 19439c7..5386629 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -860,4 +860,5 @@ asmlinkage long sys_process_vm_writev(pid_t pid,
>  
>  asmlinkage long sys_kcmp(pid_t pid1, pid_t pid2, int type,
>unsigned long idx1, unsigned long idx2);
> +asmlinkage long sys_init_module_from_fd(int len, const char __user *uargs);
>  #endif

You mean, "int fd"?

> diff --git a/kernel/module.c b/kernel/module.c
> index 4edbd9c..b080cf8 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
...
> +/* Sets info->hdr and info->len. */
> +int copy_module_from_fd(int fd, struct load_info *info)
> +{
> + struct file *file;
> + int err;
> + struct kstat stat;
> + unsigned long size;
> + off_t pos;
> + ssize_t bytes = 0;
> +
> + file = fget(fd);
> + if (!file)
> + return -ENOEXEC;
> +
> + err = vfs_getattr(file->f_vfsmnt, file->f_dentry, );
> + if (err)
> + goto out;
> +
> + if (stat.size > INT_MAX) {
> + err = -ENOMEM;
> + goto out;
>   }
> + size = stat.size;
>  
> - if (hdr->e_shoff >= len ||
> - hdr->e_shnum * sizeof(Elf_Shdr) > len - hdr->e_shoff) {
> - err = -ENOEXEC;
> - goto free_hdr;
> + info->hdr = vmalloc(size);
> + if (!info->hdr) {
> + err = -ENOMEM;
> + goto out;

The stat.size > INT_MAX is redundant: vmalloc is quite careful on its
checking of the size param.

(We removed a similar test from the module.c code a few years ago).

Cheers,
Rusty.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 1/2] module: add syscall to load module from fd

2012-09-06 Thread Rusty Russell
Kees Cook keesc...@chromium.org writes:
 Instead of (or in addition to) kernel module signing, being able to reason
 about the origin of a kernel module would be valuable in situations
 where an OS already trusts a specific file system, file, etc, due to
 things like security labels or an existing root of trust to a partition
 through things like dm-verity.

 This introduces a new syscall (currently only on x86), similar to
 init_module, that has only two arguments. The first argument is used as
 a file descriptor to the module and the second argument is a pointer to
 the NULL terminated string of module arguments.

Thanks.  Minor comments follow:

 +350  i386init_module_from_fd sys_init_module_from_fd

The from_ seems redundant.

 diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
 index 19439c7..5386629 100644
 --- a/include/linux/syscalls.h
 +++ b/include/linux/syscalls.h
 @@ -860,4 +860,5 @@ asmlinkage long sys_process_vm_writev(pid_t pid,
  
  asmlinkage long sys_kcmp(pid_t pid1, pid_t pid2, int type,
unsigned long idx1, unsigned long idx2);
 +asmlinkage long sys_init_module_from_fd(int len, const char __user *uargs);
  #endif

You mean, int fd?

 diff --git a/kernel/module.c b/kernel/module.c
 index 4edbd9c..b080cf8 100644
 --- a/kernel/module.c
 +++ b/kernel/module.c
...
 +/* Sets info-hdr and info-len. */
 +int copy_module_from_fd(int fd, struct load_info *info)
 +{
 + struct file *file;
 + int err;
 + struct kstat stat;
 + unsigned long size;
 + off_t pos;
 + ssize_t bytes = 0;
 +
 + file = fget(fd);
 + if (!file)
 + return -ENOEXEC;
 +
 + err = vfs_getattr(file-f_vfsmnt, file-f_dentry, stat);
 + if (err)
 + goto out;
 +
 + if (stat.size  INT_MAX) {
 + err = -ENOMEM;
 + goto out;
   }
 + size = stat.size;
  
 - if (hdr-e_shoff = len ||
 - hdr-e_shnum * sizeof(Elf_Shdr)  len - hdr-e_shoff) {
 - err = -ENOEXEC;
 - goto free_hdr;
 + info-hdr = vmalloc(size);
 + if (!info-hdr) {
 + err = -ENOMEM;
 + goto out;

The stat.size  INT_MAX is redundant: vmalloc is quite careful on its
checking of the size param.

(We removed a similar test from the module.c code a few years ago).

Cheers,
Rusty.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/