On Tue, Nov 15, 2005 at 09:08:53AM -0500, David Safford wrote:
> EVM securityfs files:
> /sys/kernel/security/evm/config - configuration input
> /sys/kernel/security/evm/debug/cache - debugging control
> /sys/kernel/security/evm/debug/xattr - debugging control
> /sys/kernel/security/evm/debug/crypto - debugging control
Why not use debugfs for your debugging stuff? That's the proper place
for it.
> +extern int sandbox_avail; /* can't use MODULE_STATE_GOING notification */
That's a pretty horrible global symbol name. What exactly are you
trying to test for here?
> +typedef enum {
> + EVM_PERMIT = 0, EVM_SANDBOX = -1, EVM_DENY = -2, EVM_NOLABEL = -3
> +} evm_status;
No new typedefs please.
> +extern evm_status evm_analyze_cacheinfo(struct dentry *dentry);
> +extern int evm_idx;
> +extern int evm_setxattr (struct dentry *d, char *name, void *value,
> + size_t size, int flags);
> +extern int evm_update_hmac(struct dentry *dentry, int flags);
> +extern int mem2hex(char *mem, char *buffer, int count);
Heh, anything wrong with sprintf()?
> + char hashStr[MD5_STR_SIZE + 1]; /* MD5 MAC as a string */
Wrong coding style for variable names. Please read
Documentation/CodingStyle again...
> +static int evm_config_datasize = 0; /* number of extended attributes */
> +static int evm_config_version; /* using a timestamp as a config version no. */
Do not initialize static variables to 0, it's redundant and takes up
more memory.
> +int evm_get_isec_size(void)
> +{
> + return evm_isec_size;
> +}
I've seen some foolish wrapper functions before, but...
> +int evm_get_config_info(struct evm_xattr_config **head, int *size,
> + time_t * version)
> +{
> + *head = evm_config_data;
> + *size = evm_config_datasize;
> + *version = evm_config_version;
> + return 0;
> +}
Same here, why is this needed?
> diff -rpuN linux-2.6.14.2.orig/security/evm/evm.h linux-2.6.14.2.tc/security/evm/evm.h
> --- linux-2.6.14.2.orig/security/evm/evm.h 1969-12-31 19:00:00.000000000 -0500
> +++ linux-2.6.14.2.tc/security/evm/evm.h 2005-11-11 13:45:18.000000000 -0500
> @@ -0,0 +1,133 @@
> +/*
> + * evm.h - extended verification module
> + *
> +*/
> +#include <linux/security.h>
> +#include <linux/version.h>
> +#include <linux/security-stack.h>
> +
> +#define CONFIG_SECURITY_STACKER 1
> +
> +#define SHA1_STR_SIZE 40 /* String value */
> +#define SHA1_DIGEST_SIZE 20 /* SHA1 is 160-bits */
> +#define MD5_STR_SIZE 32 /* String value */
> +#define MD5_DIGEST_SIZE 16 /* MD5 is 128-bits */
> +#define EVM_XATTR_SIZE 256 /* XATTR_SIZE_MAX causes kernel to crash */
> +
> +/*
> + * EVM configuration policies as defined in /etc/evm.config are
> + * stored in struct evm_config *evm_config_data.
> + */
> +typedef enum {
> + EVM_TYPE_ENUM = 1, EVM_TYPE_HMAC, EVM_TYPE_MD5, EVM_TYPE_SHA1,
> + EVM_TYPE_STRING
> +} evm_configtype_t;
> +typedef enum {
> + EVM_VALUE_NOLABEL = '0', EVM_VALUE_FAIL = '1', EVM_VALUE_PASS = '2',
> + EVM_VALUE_EXPIRED = '3'
> +} evm_configvalue_t;
No new typedefs please.
> +extern void evm_init_secfs(void);
> +void evm_cleanup_secfs(void);
Why switch between the "extern" and "no extern" style here?
> +#define dprintk(level, format, a...) \
> + if (evm_debug & level) \
> + printk(KERN_INFO format, ##a)
Will not build on older versions of gcc :(
> +/*
> + * Replacing module_use with the global sandbox_avail,
> + * since we don't get MODULE_STATE_GOING notification,
> + * however using a global creates a dependency on EVM for other modules.
> + */
> +int sandbox_avail = 0;
I still don't understand what you are trying to do here.
See above comment about initializing stuff to 0 not being needed...
> + int xattrFlags = 0;
StudlyCapsIsNotTheLinuxKernelVariableStyle.
thanks,
greg k-h
This archive was generated by hypermail 2.1.3 : Tue Nov 15 2005 - 16:16:47 PST