Re: [PATCH 03/14] LSM/SELinux: inode_{get,set,notify}secctx hooksto access LSM security context information.

[James Morris]

On Wed, 26 Nov 2008, David P. Quigley wrote:

> + * @inode_getsecctx:
> + * 	Returns a string containing all relavent security context information
> + *
> + * 	@inode we wish to set the security context of.
> + *	@ctx is a pointer in which to place the allocated security context.
> + *	@ctxlen points to the place to put the length of @ctx.
>   * This is the main security structure.
>   */
>  struct security_operations {
> @@ -1479,6 +1514,10 @@ struct security_operations {
>  	int (*secctx_to_secid) (const char *secdata, u32 seclen, u32 *secid);
>  	void (*release_secctx) (char *secdata, u32 seclen);
>  
> +	int (*inode_notifysecctx)(struct inode *inode, void *ctx, u32 ctxlen);
> +	int (*inode_setsecctx)(struct dentry *dentry, void *ctx, u32 ctxlen);
> +	int (*inode_getsecctx)(struct inode *inode, void **ctx, u32 *ctxlen);

For inode_getsecctx(), you're returning the length via the return value, 
so you should not also need to pass in a pointer to ctxlen, right?

IMHO, it's clearer and simpler to always only return error status from 
these kinds of functions, and to pass things like size back via pointer 
args, although it seems that a few mixed return functions have crept in to 
the code over time.  My preference would be to convert it to return value 
is error status only, with the length entirely separate as a pointer arg.


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