* [PATCH] security: cap_inode_getsecctx returning garbage
@ 2013-05-09 1:43 J. Bruce Fields
2013-05-09 5:49 ` Serge Hallyn
0 siblings, 1 reply; 6+ messages in thread
From: J. Bruce Fields @ 2013-05-09 1:43 UTC (permalink / raw)
To: Serge Hallyn
Cc: linux-security-module, linux-nfs, David P. Quigley, Steve Dickson
From: "J. Bruce Fields" <bfields@redhat.com>
We shouldn't be returning success from this function without also
filling in the return values ctx and ctxlen.
Note currently this doesn't appear to cause bugs since the only
inode_getsecctx caller I can find is fs/sysfs/inode.c, which only calls
this if security_inode_setsecurity succeeds. Assuming
security_inode_setsecurity is set to cap_inode_setsecurity whenever
inode_getsecctx is set to cap_inode_getsecctx, this function can never
actually called.
So I noticed this only because the server labeled NFS patches add a real
caller.
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
security/capability.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Also, assuming this is correct, would you be willing to apply it for
3.10?
If you'd prefer it wait till the next merge window: could you ACK it,
and let me merge it through the nfsd tree? (It's a prerequisite for
the labeled NFS patches that I hope to merge for 3.11.)
diff --git a/security/capability.c b/security/capability.c
index d32e16e..32b5157 100644
--- a/security/capability.c
+++ b/security/capability.c
@@ -858,7 +858,7 @@ static int cap_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen)
static int cap_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
{
- return 0;
+ return -EOPNOTSUPP;
}
#ifdef CONFIG_KEYS
static int cap_key_alloc(struct key *key, const struct cred *cred,
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] security: cap_inode_getsecctx returning garbage
2013-05-09 1:43 [PATCH] security: cap_inode_getsecctx returning garbage J. Bruce Fields
@ 2013-05-09 5:49 ` Serge Hallyn
2013-05-09 14:08 ` J. Bruce Fields
0 siblings, 1 reply; 6+ messages in thread
From: Serge Hallyn @ 2013-05-09 5:49 UTC (permalink / raw)
To: J. Bruce Fields
Cc: Serge Hallyn, linux-security-module, linux-nfs, David P. Quigley,
Steve Dickson
Quoting J. Bruce Fields (bfields@fieldses.org):
> From: "J. Bruce Fields" <bfields@redhat.com>
>
> We shouldn't be returning success from this function without also
> filling in the return values ctx and ctxlen.
>
> Note currently this doesn't appear to cause bugs since the only
> inode_getsecctx caller I can find is fs/sysfs/inode.c, which only calls
> this if security_inode_setsecurity succeeds. Assuming
> security_inode_setsecurity is set to cap_inode_setsecurity whenever
> inode_getsecctx is set to cap_inode_getsecctx, this function can never
> actually called.
>
> So I noticed this only because the server labeled NFS patches add a real
> caller.
>
> Signed-off-by: J. Bruce Fields <bfields@redhat.com>
Thanks, the comment in include/linux/security.h doesn't mention the
return value at all, but based on the other implementations this looks
right.
Acked-by: Serge E. Hallyn <serge.hallyn@ubuntu.com>
> ---
> security/capability.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Also, assuming this is correct, would you be willing to apply it for
> 3.10?
>
> If you'd prefer it wait till the next merge window: could you ACK it,
> and let me merge it through the nfsd tree? (It's a prerequisite for
> the labeled NFS patches that I hope to merge for 3.11.)
>
> diff --git a/security/capability.c b/security/capability.c
> index d32e16e..32b5157 100644
> --- a/security/capability.c
> +++ b/security/capability.c
> @@ -858,7 +858,7 @@ static int cap_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen)
>
> static int cap_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
> {
> - return 0;
> + return -EOPNOTSUPP;
> }
> #ifdef CONFIG_KEYS
> static int cap_key_alloc(struct key *key, const struct cred *cred,
> --
> 1.7.9.5
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] security: cap_inode_getsecctx returning garbage
2013-05-09 5:49 ` Serge Hallyn
@ 2013-05-09 14:08 ` J. Bruce Fields
2013-05-09 15:13 ` Serge Hallyn
0 siblings, 1 reply; 6+ messages in thread
From: J. Bruce Fields @ 2013-05-09 14:08 UTC (permalink / raw)
To: Serge Hallyn
Cc: Serge Hallyn, linux-security-module, linux-nfs, David P. Quigley,
Steve Dickson
On Thu, May 09, 2013 at 12:49:26AM -0500, Serge Hallyn wrote:
> Quoting J. Bruce Fields (bfields@fieldses.org):
> > From: "J. Bruce Fields" <bfields@redhat.com>
> >
> > We shouldn't be returning success from this function without also
> > filling in the return values ctx and ctxlen.
> >
> > Note currently this doesn't appear to cause bugs since the only
> > inode_getsecctx caller I can find is fs/sysfs/inode.c, which only calls
> > this if security_inode_setsecurity succeeds. Assuming
> > security_inode_setsecurity is set to cap_inode_setsecurity whenever
> > inode_getsecctx is set to cap_inode_getsecctx, this function can never
> > actually called.
> >
> > So I noticed this only because the server labeled NFS patches add a real
> > caller.
> >
> > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
>
> Thanks, the comment in include/linux/security.h doesn't mention the
> return value at all, but based on the other implementations this looks
> right.
>
> Acked-by: Serge E. Hallyn <serge.hallyn@ubuntu.com>
Thanks! Hm, would something like this help clarify?:
@@ -1412,7 +1412,8 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
* @ctxlen contains the length of @ctx.
*
* @inode_getsecctx:
- * Returns a string containing all relevant security context information
+ * On success, fills out @ctx and @ctxlen with the security context
+ * for the given @inode.
*
* @inode we wish to get the security context of.
* @ctx is a pointer in which to place the allocated security context.
--b.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] security: cap_inode_getsecctx returning garbage
2013-05-09 14:08 ` J. Bruce Fields
@ 2013-05-09 15:13 ` Serge Hallyn
2013-05-09 15:41 ` [PATCH] security: clarify cap_inode_getsecctx description J. Bruce Fields
0 siblings, 1 reply; 6+ messages in thread
From: Serge Hallyn @ 2013-05-09 15:13 UTC (permalink / raw)
To: J. Bruce Fields
Cc: Serge Hallyn, linux-security-module, linux-nfs, David P. Quigley,
Steve Dickson
Quoting J. Bruce Fields (bfields@fieldses.org):
> On Thu, May 09, 2013 at 12:49:26AM -0500, Serge Hallyn wrote:
> > Quoting J. Bruce Fields (bfields@fieldses.org):
> > > From: "J. Bruce Fields" <bfields@redhat.com>
> > >
> > > We shouldn't be returning success from this function without also
> > > filling in the return values ctx and ctxlen.
> > >
> > > Note currently this doesn't appear to cause bugs since the only
> > > inode_getsecctx caller I can find is fs/sysfs/inode.c, which only calls
> > > this if security_inode_setsecurity succeeds. Assuming
> > > security_inode_setsecurity is set to cap_inode_setsecurity whenever
> > > inode_getsecctx is set to cap_inode_getsecctx, this function can never
> > > actually called.
> > >
> > > So I noticed this only because the server labeled NFS patches add a real
> > > caller.
> > >
> > > Signed-off-by: J. Bruce Fields <bfields@redhat.com>
> >
> > Thanks, the comment in include/linux/security.h doesn't mention the
> > return value at all, but based on the other implementations this looks
> > right.
> >
> > Acked-by: Serge E. Hallyn <serge.hallyn@ubuntu.com>
>
> Thanks! Hm, would something like this help clarify?:
>
> @@ -1412,7 +1412,8 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
> * @ctxlen contains the length of @ctx.
> *
> * @inode_getsecctx:
> - * Returns a string containing all relevant security context information
> + * On success, fills out @ctx and @ctxlen with the security context
> + * for the given @inode.
... and returns 0.
That would be great, thanks!
Acked-by: Serge E. Hallyn <serge.hallyn@ubuntu.com>
> *
> * @inode we wish to get the security context of.
> * @ctx is a pointer in which to place the allocated security context.
>
> --b.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] security: clarify cap_inode_getsecctx description
2013-05-09 15:13 ` Serge Hallyn
@ 2013-05-09 15:41 ` J. Bruce Fields
2013-05-12 11:37 ` James Morris
0 siblings, 1 reply; 6+ messages in thread
From: J. Bruce Fields @ 2013-05-09 15:41 UTC (permalink / raw)
To: James Morris
Cc: Serge Hallyn, linux-security-module, linux-nfs, David P. Quigley,
Steve Dickson
From: "J. Bruce Fields" <bfields@redhat.com>
Make it clear that cap_inode_getsecctx shouldn't return success without
filling in the context data.
Acked-by: Serge E. Hallyn <serge.hallyn@ubuntu.com>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>
---
include/linux/security.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/include/linux/security.h b/include/linux/security.h
index 4686491..40560f4 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1392,7 +1392,8 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
* @ctxlen contains the length of @ctx.
*
* @inode_getsecctx:
- * Returns a string containing all relevant security context information
+ * On success, returns 0 and fills out @ctx and @ctxlen with the security
+ * context for the given @inode.
*
* @inode we wish to get the security context of.
* @ctx is a pointer in which to place the allocated security context.
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-05-12 11:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-09 1:43 [PATCH] security: cap_inode_getsecctx returning garbage J. Bruce Fields
2013-05-09 5:49 ` Serge Hallyn
2013-05-09 14:08 ` J. Bruce Fields
2013-05-09 15:13 ` Serge Hallyn
2013-05-09 15:41 ` [PATCH] security: clarify cap_inode_getsecctx description J. Bruce Fields
2013-05-12 11:37 ` James Morris
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox