[apparmor] [RFC v2] "features" directory for version/capability information
Seth Arnold
seth.arnold at gmail.com
Sat Dec 31 02:44:16 UTC 2011
On Thu, Dec 29, 2011 at 4:46 PM, Kees Cook <kees at ubuntu.com> wrote:
> AppArmor: implement static feature reporting interface
>
> This adds the ability to query the internal versions of the various
> policy features of AppArmor.
>
> Signed-off-by: Kees Cook <kees at ubuntu.com>
> ---
> v2:
> - start using enum/union for display values.
> v1:
> - initial patch.
> ---
>
> diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
> index 0848292..9f54625 100644
> --- a/security/apparmor/apparmorfs.c
> +++ b/security/apparmor/apparmorfs.c
> @@ -18,6 +18,7 @@
> #include <linux/seq_file.h>
> #include <linux/uaccess.h>
> #include <linux/namei.h>
> +#include <linux/capability.h>
Why is this header now required? I didn't spot it in the code.
> #include "include/apparmor.h"
> #include "include/apparmorfs.h"
> @@ -146,11 +147,104 @@ static const struct file_operations aa_fs_profile_remove = {
>
> static struct dentry *aa_fs_dentry __initdata;
>
> -static void __init aafs_remove(const char *name)
> +enum aa_fs_value {
> + AA_FS_STRING,
> + AA_FS_BOOLEAN,
> + AA_FS_U64,
> +};
> +
> +struct aa_fs_file {
> + char *name;
> + enum aa_fs_value v_type;
> + union {
> + char *string;
> + unsigned long u64;
> + bool boolean;
> + } v;
> + const struct file_operations *file_ops;
> +};
> +
> +struct aa_fs_dir {
> + const char *name;
> + struct dentry *dentry;
> + struct aa_fs_file *files;
> +};
> +
> +static int aa_fs_seq_show(struct seq_file *seq, void *v)
> +{
> + struct aa_fs_file *fs_file = seq->private;
> +
> + if (!fs_file)
> + return 0;
> +
> + switch (fs_file->v_type) {
> + case AA_FS_STRING:
> + seq_printf(seq, "%s\n", fs_file->v.string);
> + break;
> + case AA_FS_U64:
> + seq_printf(seq, "%#08lx\n", fs_file->v.u64);
> + break;
> + case AA_FS_BOOLEAN:
> + seq_printf(seq, "%s\n", fs_file->v.boolean ?
> + "yes" : "no");
> + break;
> + }
> +
> + return 0;
> +}
> +
> +static int aa_fs_seq_open(struct inode *inode, struct file *file)
> +{
> + return single_open(file, aa_fs_seq_show, inode->i_private);
> +}
> +
> +static const struct file_operations aa_fs_seq_file_ops = {
> + .owner = THIS_MODULE,
> + .open = aa_fs_seq_open,
> + .read = seq_read,
> + .llseek = seq_lseek,
> + .release = single_release,
> +};
> +
> +#define AA_FS_STRING_FILE(name, value) \
> + { name, \
> + .v_type = AA_FS_STRING, .v.string = value, \
> + &aa_fs_seq_file_ops }
> +#define AA_FS_BOOLEAN_FILE(name, value) \
> + { name, \
> + .v_type = AA_FS_BOOLEAN, .v.boolean = value, \
> + &aa_fs_seq_file_ops }
> +#define AA_FS_U64_FILE(name, value) \
> + { name, \
> + .v_type = AA_FS_U64, .v.u64 = value, \
> + &aa_fs_seq_file_ops }
I would rather the name and value have () around them in the expansion:
#define AA_FS_STRING_FILE(name, value) \
{ (name), \
.v_type = AA_FS_STRING, .v.string = (value), \
&aa_fs_seq_file_ops }
#define AA_FS_BOOLEAN_FILE(name, value) \
{ (name), \
.v_type = AA_FS_BOOLEAN, .v.boolean = (value), \
&aa_fs_seq_file_ops }
#define AA_FS_U64_FILE(name, value) \
{ (name), \
.v_type = AA_FS_U64, .v.u64 = (value), \
&aa_fs_seq_file_ops }
It won't make a difference now, but if these are ever called with
expressions as arguments in the future, the results will be more
predictable.
> +static struct aa_fs_file aa_fs_features_files[] = {
> + AA_FS_BOOLEAN_FILE("change_hat", 1),
> + AA_FS_BOOLEAN_FILE("change_hatv", 1),
> + AA_FS_BOOLEAN_FILE("change_onexec", 1),
> + AA_FS_BOOLEAN_FILE("change_profile", 1),
> + AA_FS_BOOLEAN_FILE("namespaces", 1),
> + AA_FS_BOOLEAN_FILE("network", 0),
> + AA_FS_STRING_FILE("file", "3.1"),
> + AA_FS_STRING_FILE("rlimit", "1.1"),
> + AA_FS_U64_FILE("capability", VFS_CAP_FLAGS_MASK),
> + { }
> +};
> +#undef AA_FS_STRING_FILE
Only AA_FS_STRING_FILE is undefed here -- why not also include
AA_FS_U64_FILE and AA_FS_BOOLEAN_FILE?
> +static struct aa_fs_dir aa_fs_dirs[] = {
> + { "features", NULL, aa_fs_features_files },
> + { }
> +};
> +
> +static void __init aafs_remove(const char *name, struct dentry *dir_dentry)
> {
> struct dentry *dentry;
>
> - dentry = lookup_one_len(name, aa_fs_dentry, strlen(name));
> + if (!dir_dentry)
> + return;
> +
> + dentry = lookup_one_len(name, dir_dentry, strlen(name));
This looks like a bugfix; should that be rolled in separately?
> if (!IS_ERR(dentry)) {
> securityfs_remove(dentry);
> dput(dentry);
> @@ -166,12 +260,14 @@ static void __init aafs_remove(const char *name)
> * Used aafs_remove to remove entries created with this fn.
> */
> static int __init aafs_create(const char *name, int mask,
> + struct dentry *dir_dentry,
> + void *data,
> const struct file_operations *fops)
> {
> struct dentry *dentry;
>
> - dentry = securityfs_create_file(name, S_IFREG | mask, aa_fs_dentry,
> - NULL, fops);
> + dentry = securityfs_create_file(name, S_IFREG | mask, dir_dentry,
> + data, fops);
>
> return IS_ERR(dentry) ? PTR_ERR(dentry) : 0;
> }
> @@ -184,9 +280,21 @@ static int __init aafs_create(const char *name, int mask,
> void __init aa_destroy_aafs(void)
> {
> if (aa_fs_dentry) {
> - aafs_remove(".remove");
> - aafs_remove(".replace");
> - aafs_remove(".load");
> + struct aa_fs_dir *fs_dir;
> +
> + aafs_remove(".remove", aa_fs_dentry);
> + aafs_remove(".replace", aa_fs_dentry);
> + aafs_remove(".load", aa_fs_dentry);
> +
> + for (fs_dir = aa_fs_dirs; fs_dir->name; ++fs_dir) {
> + struct aa_fs_file *fs_file;
> +
> + for (fs_file = fs_dir->files; fs_file->name; ++fs_file)
> + aafs_remove(fs_file->name, fs_dir->dentry);
> +
> + aafs_remove(fs_dir->name, aa_fs_dentry);
> + fs_dir->dentry = NULL;
> + }
>
> securityfs_remove(aa_fs_dentry);
> aa_fs_dentry = NULL;
> @@ -203,6 +311,7 @@ void __init aa_destroy_aafs(void)
> int __init aa_create_aafs(void)
> {
> int error;
> + struct aa_fs_dir *fs_dir;
>
> if (!apparmor_initialized)
> return 0;
> @@ -219,16 +328,39 @@ int __init aa_create_aafs(void)
> goto error;
> }
>
> - error = aafs_create(".load", 0640, &aa_fs_profile_load);
> + error = aafs_create(".load", 0640, aa_fs_dentry, NULL,
> + &aa_fs_profile_load);
> if (error)
> goto error;
> - error = aafs_create(".replace", 0640, &aa_fs_profile_replace);
> + error = aafs_create(".replace", 0640, aa_fs_dentry, NULL,
> + &aa_fs_profile_replace);
> if (error)
> goto error;
> - error = aafs_create(".remove", 0640, &aa_fs_profile_remove);
> + error = aafs_create(".remove", 0640, aa_fs_dentry, NULL,
> + &aa_fs_profile_remove);
> if (error)
> goto error;
>
> + for (fs_dir = aa_fs_dirs; fs_dir->name; ++fs_dir) {
> + struct aa_fs_file *fs_file;
> +
> + fs_dir->dentry = securityfs_create_dir(fs_dir->name,
> + aa_fs_dentry);
> + if (IS_ERR(fs_dir->dentry)) {
> + error = PTR_ERR(fs_dir->dentry);
> + fs_dir->dentry = NULL;
> + goto error;
> + }
> +
> + for (fs_file = fs_dir->files; fs_file->name; ++fs_file) {
> + error = aafs_create(fs_file->name, 0444,
> + fs_dir->dentry, fs_file,
> + fs_file->file_ops);
> + if (error)
> + goto error;
> + }
> + }
> +
> /* TODO: add support for apparmorfs_null and apparmorfs_mnt */
>
> /* Report that AppArmor fs is enabled */
>
Thanks Kees!
More information about the AppArmor
mailing list