2013-06-07 05:57:23

by Jaegeuk Kim

[permalink] [raw]
Subject: [PATCH 1/2] f2fs: fix iget/iput of dir during recovery

It is possible that iput is skipped after iget during the recovery.

In recover_dentry(),
dir = f2fs_iget();
...
if (de && inode->i_ino == le32_to_cpu(de->ino))
goto out;

In this case, this dir is not able to be added in dirty_dir_inode_list.
The actual linking is done only when set_page_dirty() is called.

So let's add this newly got inode into the list explicitly, and put it at the
end of the recovery routine.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/checkpoint.c | 55 +++++++++++++++++++++++++++++++++++++---------------
fs/f2fs/f2fs.h | 1 +
fs/f2fs/recovery.c | 1 +
3 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 6f56e57..9a77509 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -450,13 +450,30 @@ fail_no_cp:
return -EINVAL;
}

-void set_dirty_dir_page(struct inode *inode, struct page *page)
+static int __add_dirty_inode(struct inode *inode, struct dir_inode_entry *new)
{
struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
struct list_head *head = &sbi->dir_inode_list;
- struct dir_inode_entry *new;
struct list_head *this;

+ list_for_each(this, head) {
+ struct dir_inode_entry *entry;
+ entry = list_entry(this, struct dir_inode_entry, list);
+ if (entry->inode == inode)
+ return -EEXIST;
+ }
+ list_add_tail(&new->list, head);
+#ifdef CONFIG_F2FS_STAT_FS
+ sbi->n_dirty_dirs++;
+#endif
+ return 0;
+}
+
+void set_dirty_dir_page(struct inode *inode, struct page *page)
+{
+ struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
+ struct dir_inode_entry *new;
+
if (!S_ISDIR(inode->i_mode))
return;
retry:
@@ -469,25 +486,31 @@ retry:
INIT_LIST_HEAD(&new->list);

spin_lock(&sbi->dir_inode_lock);
- list_for_each(this, head) {
- struct dir_inode_entry *entry;
- entry = list_entry(this, struct dir_inode_entry, list);
- if (entry->inode == inode) {
- kmem_cache_free(inode_entry_slab, new);
- goto out;
- }
- }
- list_add_tail(&new->list, head);
-#ifdef CONFIG_F2FS_STAT_FS
- sbi->n_dirty_dirs++;
-#endif
+ if (__add_dirty_inode(inode, new))
+ kmem_cache_free(inode_entry_slab, new);

- BUG_ON(!S_ISDIR(inode->i_mode));
-out:
inc_page_count(sbi, F2FS_DIRTY_DENTS);
inode_inc_dirty_dents(inode);
SetPagePrivate(page);
+ spin_unlock(&sbi->dir_inode_lock);
+}
+
+void add_dirty_dir_inode(struct inode *inode)
+{
+ struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
+ struct dir_inode_entry *new;
+retry:
+ new = kmem_cache_alloc(inode_entry_slab, GFP_NOFS);
+ if (!new) {
+ cond_resched();
+ goto retry;
+ }
+ new->inode = inode;
+ INIT_LIST_HEAD(&new->list);

+ spin_lock(&sbi->dir_inode_lock);
+ if (__add_dirty_inode(inode, new))
+ kmem_cache_free(inode_entry_slab, new);
spin_unlock(&sbi->dir_inode_lock);
}

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 40b137a..d6e63da 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1030,6 +1030,7 @@ void remove_orphan_inode(struct f2fs_sb_info *, nid_t);
int recover_orphan_inodes(struct f2fs_sb_info *);
int get_valid_checkpoint(struct f2fs_sb_info *);
void set_dirty_dir_page(struct inode *, struct page *);
+void add_dirty_dir_inode(struct inode *);
void remove_dirty_dir_inode(struct inode *);
struct inode *check_dirty_dir_inode(struct f2fs_sb_info *, nid_t);
void sync_dirty_dir_inodes(struct f2fs_sb_info *);
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index 539ca32..ddde14f 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -58,6 +58,7 @@ static int recover_dentry(struct page *ipage, struct inode *inode)
goto out;
}
set_inode_flag(F2FS_I(dir), FI_DELAY_IPUT);
+ add_dirty_dir_inode(dir);
}

name.len = le32_to_cpu(raw_inode->i_namelen);
--
1.8.1.3.566.gaa39828


2013-06-07 05:57:39

by Jaegeuk Kim

[permalink] [raw]
Subject: [PATCH 2/2] f2fs: support xattr security labels

This patch adds the support of security labels for f2fs, which will be used
by SElinux.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/Kconfig | 9 +++++++++
fs/f2fs/dir.c | 5 +++++
fs/f2fs/xattr.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
fs/f2fs/xattr.h | 12 +++++++++++-
4 files changed, 82 insertions(+), 3 deletions(-)

diff --git a/fs/f2fs/Kconfig b/fs/f2fs/Kconfig
index fd27e7e..2214cc9 100644
--- a/fs/f2fs/Kconfig
+++ b/fs/f2fs/Kconfig
@@ -51,3 +51,12 @@ config F2FS_FS_POSIX_ACL
Linux website <http://acl.bestbits.at/>.

If you don't know what Access Control Lists are, say N
+
+config F2FS_FS_SECURITY
+ bool "F2FS Security Labels"
+ depends on F2FS_FS_XATTR
+ help
+ Security labels provide acls used by the security modules
+ like SELinux. This option should be used with the xattr mode.
+
+ If you are not using a security module, say N.
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 67e2d13..81a1d6f 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -13,6 +13,7 @@
#include "f2fs.h"
#include "node.h"
#include "acl.h"
+#include "xattr.h"

static unsigned long dir_blocks(struct inode *inode)
{
@@ -334,6 +335,10 @@ static struct page *init_inode_metadata(struct inode *inode,
if (err)
goto error;

+ err = f2fs_init_security(inode, dir, name);
+ if (err)
+ goto error;
+
wait_on_page_writeback(page);
} else {
page = get_node_page(F2FS_SB(dir->i_sb), inode->i_ino);
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index ae61f35..b5292fa 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -20,6 +20,7 @@
*/
#include <linux/rwsem.h>
#include <linux/f2fs_fs.h>
+#include <linux/security.h>
#include "f2fs.h"
#include "xattr.h"

@@ -43,6 +44,10 @@ static size_t f2fs_xattr_generic_list(struct dentry *dentry, char *list,
prefix = XATTR_TRUSTED_PREFIX;
prefix_len = XATTR_TRUSTED_PREFIX_LEN;
break;
+ case F2FS_XATTR_INDEX_SECURITY:
+ prefix = XATTR_SECURITY_PREFIX;
+ prefix_len = XATTR_SECURITY_PREFIX_LEN;
+ break;
default:
return -EINVAL;
}
@@ -70,13 +75,14 @@ static int f2fs_xattr_generic_get(struct dentry *dentry, const char *name,
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
break;
+ case F2FS_XATTR_INDEX_SECURITY:
+ break;
default:
return -EINVAL;
}
if (strcmp(name, "") == 0)
return -EINVAL;
- return f2fs_getxattr(dentry->d_inode, type, name,
- buffer, size);
+ return f2fs_getxattr(dentry->d_inode, type, name, buffer, size);
}

static int f2fs_xattr_generic_set(struct dentry *dentry, const char *name,
@@ -93,6 +99,8 @@ static int f2fs_xattr_generic_set(struct dentry *dentry, const char *name,
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
break;
+ case F2FS_XATTR_INDEX_SECURITY:
+ break;
default:
return -EINVAL;
}
@@ -145,6 +153,40 @@ static int f2fs_xattr_advise_set(struct dentry *dentry, const char *name,
return 0;
}

+#ifdef CONFIG_F2FS_FS_SECURITY
+static int f2fs_initxattrs(struct inode *inode, const struct xattr *xattr_array,
+ void *fs_info)
+{
+ const struct xattr *xattr;
+ char *name;
+ int err = 0;
+
+ for (xattr = xattr_array; xattr->name != NULL; xattr++) {
+ name = kmalloc(XATTR_SECURITY_PREFIX_LEN +
+ strlen(xattr->name) + 1, GFP_NOFS);
+ if (!name) {
+ err = -ENOMEM;
+ break;
+ }
+ strcpy(name, XATTR_SECURITY_PREFIX);
+ strcpy(name + XATTR_SECURITY_PREFIX_LEN, xattr->name);
+ err = f2fs_setxattr(inode, F2FS_XATTR_INDEX_SECURITY, name,
+ xattr->value, xattr->value_len);
+ kfree(name);
+ if (err < 0)
+ break;
+ }
+ return err;
+}
+
+int f2fs_init_security(struct inode *inode, struct inode *dir,
+ const struct qstr *qstr)
+{
+ return security_inode_init_security(inode, dir, qstr,
+ &f2fs_initxattrs, NULL);
+}
+#endif
+
const struct xattr_handler f2fs_xattr_user_handler = {
.prefix = XATTR_USER_PREFIX,
.flags = F2FS_XATTR_INDEX_USER,
@@ -169,6 +211,13 @@ const struct xattr_handler f2fs_xattr_advise_handler = {
.set = f2fs_xattr_advise_set,
};

+const struct xattr_handler f2fs_xattr_security_handler = {
+ .prefix = XATTR_SECURITY_PREFIX,
+ .list = f2fs_xattr_generic_list,
+ .get = f2fs_xattr_generic_get,
+ .set = f2fs_xattr_generic_set,
+};
+
static const struct xattr_handler *f2fs_xattr_handler_map[] = {
[F2FS_XATTR_INDEX_USER] = &f2fs_xattr_user_handler,
#ifdef CONFIG_F2FS_FS_POSIX_ACL
@@ -177,6 +226,9 @@ static const struct xattr_handler *f2fs_xattr_handler_map[] = {
#endif
[F2FS_XATTR_INDEX_TRUSTED] = &f2fs_xattr_trusted_handler,
[F2FS_XATTR_INDEX_ADVISE] = &f2fs_xattr_advise_handler,
+#ifdef CONFIG_F2FS_FS_SECURITY
+ [F2FS_XATTR_INDEX_SECURITY] = &f2fs_xattr_security_handler,
+#endif
};

const struct xattr_handler *f2fs_xattr_handlers[] = {
@@ -187,6 +239,9 @@ const struct xattr_handler *f2fs_xattr_handlers[] = {
#endif
&f2fs_xattr_trusted_handler,
&f2fs_xattr_advise_handler,
+#ifdef CONFIG_F2FS_FS_SECURITY
+ &f2fs_xattr_security_handler,
+#endif
NULL,
};

diff --git a/fs/f2fs/xattr.h b/fs/f2fs/xattr.h
index 49c9558..14e1329 100644
--- a/fs/f2fs/xattr.h
+++ b/fs/f2fs/xattr.h
@@ -112,6 +112,7 @@ extern const struct xattr_handler f2fs_xattr_trusted_handler;
extern const struct xattr_handler f2fs_xattr_acl_access_handler;
extern const struct xattr_handler f2fs_xattr_acl_default_handler;
extern const struct xattr_handler f2fs_xattr_advise_handler;
+extern const struct xattr_handler f2fs_xattr_security_handler;

extern const struct xattr_handler *f2fs_xattr_handlers[];

@@ -121,7 +122,6 @@ extern int f2fs_getxattr(struct inode *inode, int name_index, const char *name,
void *buffer, size_t buffer_size);
extern ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer,
size_t buffer_size);
-
#else

#define f2fs_xattr_handlers NULL
@@ -142,4 +142,14 @@ static inline ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer,
}
#endif

+#ifdef CONFIG_F2FS_FS_SECURITY
+extern int f2fs_init_security(struct inode *inode, struct inode *dir,
+ const struct qstr *qstr);
+#else
+static inline int f2fs_init_security(struct inode *inode, struct inode *dir,
+ const struct qstr *qstr)
+{
+ return 0;
+}
+#endif
#endif /* __F2FS_XATTR_H__ */
--
1.8.1.3.566.gaa39828

2013-06-07 08:11:10

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [PATCH 2/2 v2] f2fs: support xattr security labels

Change log from v1:
o fix a bug

From 901aea226653e26aa3556edd88ca1e48fa89eb5f Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <[email protected]>
Date: Mon, 3 Jun 2013 19:46:19 +0900
Subject: [PATCH 2/2] f2fs: support xattr security labels
Cc: [email protected], [email protected], [email protected]

This patch adds the support of security labels for f2fs, which will be used
by SElinux.

Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/Kconfig | 9 +++++++++
fs/f2fs/dir.c | 5 +++++
fs/f2fs/xattr.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
fs/f2fs/xattr.h | 12 +++++++++++-
4 files changed, 83 insertions(+), 3 deletions(-)

diff --git a/fs/f2fs/Kconfig b/fs/f2fs/Kconfig
index fd27e7e..2214cc9 100644
--- a/fs/f2fs/Kconfig
+++ b/fs/f2fs/Kconfig
@@ -51,3 +51,12 @@ config F2FS_FS_POSIX_ACL
Linux website <http://acl.bestbits.at/>.

If you don't know what Access Control Lists are, say N
+
+config F2FS_FS_SECURITY
+ bool "F2FS Security Labels"
+ depends on F2FS_FS_XATTR
+ help
+ Security labels provide acls used by the security modules
+ like SELinux. This option should be used with the xattr mode.
+
+ If you are not using a security module, say N.
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 67e2d13..81a1d6f 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -13,6 +13,7 @@
#include "f2fs.h"
#include "node.h"
#include "acl.h"
+#include "xattr.h"

static unsigned long dir_blocks(struct inode *inode)
{
@@ -334,6 +335,10 @@ static struct page *init_inode_metadata(struct inode *inode,
if (err)
goto error;

+ err = f2fs_init_security(inode, dir, name);
+ if (err)
+ goto error;
+
wait_on_page_writeback(page);
} else {
page = get_node_page(F2FS_SB(dir->i_sb), inode->i_ino);
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index ae61f35..67ef1a5 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -20,6 +20,7 @@
*/
#include <linux/rwsem.h>
#include <linux/f2fs_fs.h>
+#include <linux/security.h>
#include "f2fs.h"
#include "xattr.h"

@@ -43,6 +44,10 @@ static size_t f2fs_xattr_generic_list(struct dentry *dentry, char *list,
prefix = XATTR_TRUSTED_PREFIX;
prefix_len = XATTR_TRUSTED_PREFIX_LEN;
break;
+ case F2FS_XATTR_INDEX_SECURITY:
+ prefix = XATTR_SECURITY_PREFIX;
+ prefix_len = XATTR_SECURITY_PREFIX_LEN;
+ break;
default:
return -EINVAL;
}
@@ -70,13 +75,14 @@ static int f2fs_xattr_generic_get(struct dentry *dentry, const char *name,
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
break;
+ case F2FS_XATTR_INDEX_SECURITY:
+ break;
default:
return -EINVAL;
}
if (strcmp(name, "") == 0)
return -EINVAL;
- return f2fs_getxattr(dentry->d_inode, type, name,
- buffer, size);
+ return f2fs_getxattr(dentry->d_inode, type, name, buffer, size);
}

static int f2fs_xattr_generic_set(struct dentry *dentry, const char *name,
@@ -93,6 +99,8 @@ static int f2fs_xattr_generic_set(struct dentry *dentry, const char *name,
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
break;
+ case F2FS_XATTR_INDEX_SECURITY:
+ break;
default:
return -EINVAL;
}
@@ -145,6 +153,40 @@ static int f2fs_xattr_advise_set(struct dentry *dentry, const char *name,
return 0;
}

+#ifdef CONFIG_F2FS_FS_SECURITY
+static int f2fs_initxattrs(struct inode *inode, const struct xattr *xattr_array,
+ void *fs_info)
+{
+ const struct xattr *xattr;
+ char *name;
+ int err = 0;
+
+ for (xattr = xattr_array; xattr->name != NULL; xattr++) {
+ name = kmalloc(XATTR_SECURITY_PREFIX_LEN +
+ strlen(xattr->name) + 1, GFP_NOFS);
+ if (!name) {
+ err = -ENOMEM;
+ break;
+ }
+ strcpy(name, XATTR_SECURITY_PREFIX);
+ strcpy(name + XATTR_SECURITY_PREFIX_LEN, xattr->name);
+ err = f2fs_setxattr(inode, F2FS_XATTR_INDEX_SECURITY, name,
+ xattr->value, xattr->value_len);
+ kfree(name);
+ if (err < 0)
+ break;
+ }
+ return err;
+}
+
+int f2fs_init_security(struct inode *inode, struct inode *dir,
+ const struct qstr *qstr)
+{
+ return security_inode_init_security(inode, dir, qstr,
+ &f2fs_initxattrs, NULL);
+}
+#endif
+
const struct xattr_handler f2fs_xattr_user_handler = {
.prefix = XATTR_USER_PREFIX,
.flags = F2FS_XATTR_INDEX_USER,
@@ -169,6 +211,14 @@ const struct xattr_handler f2fs_xattr_advise_handler = {
.set = f2fs_xattr_advise_set,
};

+const struct xattr_handler f2fs_xattr_security_handler = {
+ .prefix = XATTR_SECURITY_PREFIX,
+ .flags = F2FS_XATTR_INDEX_SECURITY,
+ .list = f2fs_xattr_generic_list,
+ .get = f2fs_xattr_generic_get,
+ .set = f2fs_xattr_generic_set,
+};
+
static const struct xattr_handler *f2fs_xattr_handler_map[] = {
[F2FS_XATTR_INDEX_USER] = &f2fs_xattr_user_handler,
#ifdef CONFIG_F2FS_FS_POSIX_ACL
@@ -176,6 +226,9 @@ static const struct xattr_handler *f2fs_xattr_handler_map[] = {
[F2FS_XATTR_INDEX_POSIX_ACL_DEFAULT] = &f2fs_xattr_acl_default_handler,
#endif
[F2FS_XATTR_INDEX_TRUSTED] = &f2fs_xattr_trusted_handler,
+#ifdef CONFIG_F2FS_FS_SECURITY
+ [F2FS_XATTR_INDEX_SECURITY] = &f2fs_xattr_security_handler,
+#endif
[F2FS_XATTR_INDEX_ADVISE] = &f2fs_xattr_advise_handler,
};

@@ -186,6 +239,9 @@ const struct xattr_handler *f2fs_xattr_handlers[] = {
&f2fs_xattr_acl_default_handler,
#endif
&f2fs_xattr_trusted_handler,
+#ifdef CONFIG_F2FS_FS_SECURITY
+ &f2fs_xattr_security_handler,
+#endif
&f2fs_xattr_advise_handler,
NULL,
};
diff --git a/fs/f2fs/xattr.h b/fs/f2fs/xattr.h
index 49c9558..14e1329 100644
--- a/fs/f2fs/xattr.h
+++ b/fs/f2fs/xattr.h
@@ -112,6 +112,7 @@ extern const struct xattr_handler f2fs_xattr_trusted_handler;
extern const struct xattr_handler f2fs_xattr_acl_access_handler;
extern const struct xattr_handler f2fs_xattr_acl_default_handler;
extern const struct xattr_handler f2fs_xattr_advise_handler;
+extern const struct xattr_handler f2fs_xattr_security_handler;

extern const struct xattr_handler *f2fs_xattr_handlers[];

@@ -121,7 +122,6 @@ extern int f2fs_getxattr(struct inode *inode, int name_index, const char *name,
void *buffer, size_t buffer_size);
extern ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer,
size_t buffer_size);
-
#else

#define f2fs_xattr_handlers NULL
@@ -142,4 +142,14 @@ static inline ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer,
}
#endif

+#ifdef CONFIG_F2FS_FS_SECURITY
+extern int f2fs_init_security(struct inode *inode, struct inode *dir,
+ const struct qstr *qstr);
+#else
+static inline int f2fs_init_security(struct inode *inode, struct inode *dir,
+ const struct qstr *qstr)
+{
+ return 0;
+}
+#endif
#endif /* __F2FS_XATTR_H__ */
--
1.8.1.3.566.gaa39828



--
Jaegeuk Kim
Samsung


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2013-06-07 10:50:33

by Namjae Jeon

[permalink] [raw]
Subject: Re: [PATCH 2/2 v2] f2fs: support xattr security labels

2013/6/7, Jaegeuk Kim <[email protected]>:
> Change log from v1:
> o fix a bug
>
> From 901aea226653e26aa3556edd88ca1e48fa89eb5f Mon Sep 17 00:00:00 2001
> From: Jaegeuk Kim <[email protected]>
> Date: Mon, 3 Jun 2013 19:46:19 +0900
> Subject: [PATCH 2/2] f2fs: support xattr security labels
> Cc: [email protected], [email protected],
> [email protected]
>
> This patch adds the support of security labels for f2fs, which will be used
> by SElinux.
>
> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
> fs/f2fs/Kconfig | 9 +++++++++
> fs/f2fs/dir.c | 5 +++++
> fs/f2fs/xattr.c | 60
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> fs/f2fs/xattr.h | 12 +++++++++++-
> 4 files changed, 83 insertions(+), 3 deletions(-)
>
> diff --git a/fs/f2fs/Kconfig b/fs/f2fs/Kconfig
> index fd27e7e..2214cc9 100644
> --- a/fs/f2fs/Kconfig
> +++ b/fs/f2fs/Kconfig
> @@ -51,3 +51,12 @@ config F2FS_FS_POSIX_ACL
> Linux website <http://acl.bestbits.at/>.
>
> If you don't know what Access Control Lists are, say N
> +
> +config F2FS_FS_SECURITY
> + bool "F2FS Security Labels"
> + depends on F2FS_FS_XATTR
Hi Jaegeuk.
Do we need to add "default y" as other xattr options ?

Thanks.
> + help
> + Security labels provide acls used by the security modules
> + like SELinux. This option should be used with the xattr mode.
> +
> + If you are not using a security module, say N.

2013-06-07 22:13:48

by Casey Schaufler

[permalink] [raw]
Subject: Re: [PATCH 2/2] f2fs: support xattr security labels

On 6/6/2013 10:55 PM, Jaegeuk Kim wrote:
> This patch adds the support of security labels for f2fs, which will be used
> by SElinux.

Please be inclusive. Security xattrs are used by LSMs other than SELinux.

> Signed-off-by: Jaegeuk Kim <[email protected]>
> ---
> fs/f2fs/Kconfig | 9 +++++++++
> fs/f2fs/dir.c | 5 +++++
> fs/f2fs/xattr.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> fs/f2fs/xattr.h | 12 +++++++++++-
> 4 files changed, 82 insertions(+), 3 deletions(-)
>
> diff --git a/fs/f2fs/Kconfig b/fs/f2fs/Kconfig
> index fd27e7e..2214cc9 100644
> --- a/fs/f2fs/Kconfig
> +++ b/fs/f2fs/Kconfig
> @@ -51,3 +51,12 @@ config F2FS_FS_POSIX_ACL
> Linux website <http://acl.bestbits.at/>.
>
> If you don't know what Access Control Lists are, say N
> +
> +config F2FS_FS_SECURITY
> + bool "F2FS Security Labels"
> + depends on F2FS_FS_XATTR
> + help
> + Security labels provide acls used by the security modules
> + like SELinux. This option should be used with the xattr mode.

This description missuses the term "acl". Security labels are not
Access Control Lists (ACLs). What is the "xattr mode"? If this option
depends on xattr support "should" is not correct.

> +
> + If you are not using a security module, say N.
> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> index 67e2d13..81a1d6f 100644
> --- a/fs/f2fs/dir.c
> +++ b/fs/f2fs/dir.c
> @@ -13,6 +13,7 @@
> #include "f2fs.h"
> #include "node.h"
> #include "acl.h"
> +#include "xattr.h"
>
> static unsigned long dir_blocks(struct inode *inode)
> {
> @@ -334,6 +335,10 @@ static struct page *init_inode_metadata(struct inode *inode,
> if (err)
> goto error;
>
> + err = f2fs_init_security(inode, dir, name);
> + if (err)
> + goto error;
> +
> wait_on_page_writeback(page);
> } else {
> page = get_node_page(F2FS_SB(dir->i_sb), inode->i_ino);
> diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
> index ae61f35..b5292fa 100644
> --- a/fs/f2fs/xattr.c
> +++ b/fs/f2fs/xattr.c
> @@ -20,6 +20,7 @@
> */
> #include <linux/rwsem.h>
> #include <linux/f2fs_fs.h>
> +#include <linux/security.h>
> #include "f2fs.h"
> #include "xattr.h"
>
> @@ -43,6 +44,10 @@ static size_t f2fs_xattr_generic_list(struct dentry *dentry, char *list,
> prefix = XATTR_TRUSTED_PREFIX;
> prefix_len = XATTR_TRUSTED_PREFIX_LEN;
> break;
> + case F2FS_XATTR_INDEX_SECURITY:
> + prefix = XATTR_SECURITY_PREFIX;
> + prefix_len = XATTR_SECURITY_PREFIX_LEN;
> + break;
> default:
> return -EINVAL;
> }
> @@ -70,13 +75,14 @@ static int f2fs_xattr_generic_get(struct dentry *dentry, const char *name,
> if (!capable(CAP_SYS_ADMIN))
> return -EPERM;
> break;
> + case F2FS_XATTR_INDEX_SECURITY:
> + break;
> default:
> return -EINVAL;
> }
> if (strcmp(name, "") == 0)
> return -EINVAL;
> - return f2fs_getxattr(dentry->d_inode, type, name,
> - buffer, size);
> + return f2fs_getxattr(dentry->d_inode, type, name, buffer, size);
> }
>
> static int f2fs_xattr_generic_set(struct dentry *dentry, const char *name,
> @@ -93,6 +99,8 @@ static int f2fs_xattr_generic_set(struct dentry *dentry, const char *name,
> if (!capable(CAP_SYS_ADMIN))
> return -EPERM;
> break;
> + case F2FS_XATTR_INDEX_SECURITY:
> + break;
> default:
> return -EINVAL;
> }
> @@ -145,6 +153,40 @@ static int f2fs_xattr_advise_set(struct dentry *dentry, const char *name,
> return 0;
> }
>
> +#ifdef CONFIG_F2FS_FS_SECURITY
> +static int f2fs_initxattrs(struct inode *inode, const struct xattr *xattr_array,
> + void *fs_info)
> +{
> + const struct xattr *xattr;
> + char *name;
> + int err = 0;
> +
> + for (xattr = xattr_array; xattr->name != NULL; xattr++) {
> + name = kmalloc(XATTR_SECURITY_PREFIX_LEN +
> + strlen(xattr->name) + 1, GFP_NOFS);
> + if (!name) {
> + err = -ENOMEM;
> + break;
> + }
> + strcpy(name, XATTR_SECURITY_PREFIX);
> + strcpy(name + XATTR_SECURITY_PREFIX_LEN, xattr->name);

sprintf(name, XATTR_SECURITY_PREFIX "%s", xattr->name);

might look simpler.

> + err = f2fs_setxattr(inode, F2FS_XATTR_INDEX_SECURITY, name,
> + xattr->value, xattr->value_len);
> + kfree(name);
> + if (err < 0)
> + break;
> + }
> + return err;
> +}
> +
> +int f2fs_init_security(struct inode *inode, struct inode *dir,
> + const struct qstr *qstr)
> +{
> + return security_inode_init_security(inode, dir, qstr,
> + &f2fs_initxattrs, NULL);
> +}
> +#endif
> +
> const struct xattr_handler f2fs_xattr_user_handler = {
> .prefix = XATTR_USER_PREFIX,
> .flags = F2FS_XATTR_INDEX_USER,
> @@ -169,6 +211,13 @@ const struct xattr_handler f2fs_xattr_advise_handler = {
> .set = f2fs_xattr_advise_set,
> };
>
> +const struct xattr_handler f2fs_xattr_security_handler = {
> + .prefix = XATTR_SECURITY_PREFIX,
> + .list = f2fs_xattr_generic_list,
> + .get = f2fs_xattr_generic_get,
> + .set = f2fs_xattr_generic_set,
> +};
> +
> static const struct xattr_handler *f2fs_xattr_handler_map[] = {
> [F2FS_XATTR_INDEX_USER] = &f2fs_xattr_user_handler,
> #ifdef CONFIG_F2FS_FS_POSIX_ACL
> @@ -177,6 +226,9 @@ static const struct xattr_handler *f2fs_xattr_handler_map[] = {
> #endif
> [F2FS_XATTR_INDEX_TRUSTED] = &f2fs_xattr_trusted_handler,
> [F2FS_XATTR_INDEX_ADVISE] = &f2fs_xattr_advise_handler,
> +#ifdef CONFIG_F2FS_FS_SECURITY
> + [F2FS_XATTR_INDEX_SECURITY] = &f2fs_xattr_security_handler,
> +#endif
> };
>
> const struct xattr_handler *f2fs_xattr_handlers[] = {
> @@ -187,6 +239,9 @@ const struct xattr_handler *f2fs_xattr_handlers[] = {
> #endif
> &f2fs_xattr_trusted_handler,
> &f2fs_xattr_advise_handler,
> +#ifdef CONFIG_F2FS_FS_SECURITY
> + &f2fs_xattr_security_handler,
> +#endif
> NULL,
> };
>
> diff --git a/fs/f2fs/xattr.h b/fs/f2fs/xattr.h
> index 49c9558..14e1329 100644
> --- a/fs/f2fs/xattr.h
> +++ b/fs/f2fs/xattr.h
> @@ -112,6 +112,7 @@ extern const struct xattr_handler f2fs_xattr_trusted_handler;
> extern const struct xattr_handler f2fs_xattr_acl_access_handler;
> extern const struct xattr_handler f2fs_xattr_acl_default_handler;
> extern const struct xattr_handler f2fs_xattr_advise_handler;
> +extern const struct xattr_handler f2fs_xattr_security_handler;
>
> extern const struct xattr_handler *f2fs_xattr_handlers[];
>
> @@ -121,7 +122,6 @@ extern int f2fs_getxattr(struct inode *inode, int name_index, const char *name,
> void *buffer, size_t buffer_size);
> extern ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer,
> size_t buffer_size);
> -
> #else
>
> #define f2fs_xattr_handlers NULL
> @@ -142,4 +142,14 @@ static inline ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer,
> }
> #endif
>
> +#ifdef CONFIG_F2FS_FS_SECURITY
> +extern int f2fs_init_security(struct inode *inode, struct inode *dir,
> + const struct qstr *qstr);
> +#else
> +static inline int f2fs_init_security(struct inode *inode, struct inode *dir,
> + const struct qstr *qstr)
> +{
> + return 0;
> +}
> +#endif
> #endif /* __F2FS_XATTR_H__ */

2013-06-08 00:19:58

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [PATCH 2/2 v2] f2fs: support xattr security labels

2013-06-07 (금), 19:50 +0900, Namjae Jeon:
> 2013/6/7, Jaegeuk Kim <[email protected]>:
> > Change log from v1:
> > o fix a bug
> >
> > From 901aea226653e26aa3556edd88ca1e48fa89eb5f Mon Sep 17 00:00:00 2001
> > From: Jaegeuk Kim <[email protected]>
> > Date: Mon, 3 Jun 2013 19:46:19 +0900
> > Subject: [PATCH 2/2] f2fs: support xattr security labels
> > Cc: [email protected], [email protected],
> > [email protected]
> >
> > This patch adds the support of security labels for f2fs, which will be used
> > by SElinux.
> >
> > Signed-off-by: Jaegeuk Kim <[email protected]>
> > ---
> > fs/f2fs/Kconfig | 9 +++++++++
> > fs/f2fs/dir.c | 5 +++++
> > fs/f2fs/xattr.c | 60
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> > fs/f2fs/xattr.h | 12 +++++++++++-
> > 4 files changed, 83 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/f2fs/Kconfig b/fs/f2fs/Kconfig
> > index fd27e7e..2214cc9 100644
> > --- a/fs/f2fs/Kconfig
> > +++ b/fs/f2fs/Kconfig
> > @@ -51,3 +51,12 @@ config F2FS_FS_POSIX_ACL
> > Linux website <http://acl.bestbits.at/>.
> >
> > If you don't know what Access Control Lists are, say N
> > +
> > +config F2FS_FS_SECURITY
> > + bool "F2FS Security Labels"
> > + depends on F2FS_FS_XATTR
> Hi Jaegeuk.
> Do we need to add "default y" as other xattr options ?

Hi Namjae,
Currently negative.
Most of xattr operations will be fine though, I'd like to test them more
intensively. After then, we'd better set *default y*.
Thanks, :)

>
> Thanks.
> > + help
> > + Security labels provide acls used by the security modules
> > + like SELinux. This option should be used with the xattr mode.
> > +
> > + If you are not using a security module, say N.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Jaegeuk Kim
Samsung


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2013-06-08 00:57:04

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [PATCH 2/2] f2fs: support xattr security labels

Hi,
Thank you for the review.
I agreed all, and will submit v3.
Thanks,

2013-06-07 (금), 15:13 -0700, Casey Schaufler:
> On 6/6/2013 10:55 PM, Jaegeuk Kim wrote:
> > This patch adds the support of security labels for f2fs, which will be used
> > by SElinux.
>
> Please be inclusive. Security xattrs are used by LSMs other than SELinux.

> > Signed-off-by: Jaegeuk Kim <[email protected]>
> > ---
> > fs/f2fs/Kconfig | 9 +++++++++
> > fs/f2fs/dir.c | 5 +++++
> > fs/f2fs/xattr.c | 59 +++++++++++++++++++++++++++++++++++++++++++++++++++++++--
> > fs/f2fs/xattr.h | 12 +++++++++++-
> > 4 files changed, 82 insertions(+), 3 deletions(-)
> >
> > diff --git a/fs/f2fs/Kconfig b/fs/f2fs/Kconfig
> > index fd27e7e..2214cc9 100644
> > --- a/fs/f2fs/Kconfig
> > +++ b/fs/f2fs/Kconfig
> > @@ -51,3 +51,12 @@ config F2FS_FS_POSIX_ACL
> > Linux website <http://acl.bestbits.at/>.
> >
> > If you don't know what Access Control Lists are, say N
> > +
> > +config F2FS_FS_SECURITY
> > + bool "F2FS Security Labels"
> > + depends on F2FS_FS_XATTR
> > + help
> > + Security labels provide acls used by the security modules
> > + like SELinux. This option should be used with the xattr mode.
>
> This description missuses the term "acl". Security labels are not
> Access Control Lists (ACLs). What is the "xattr mode"? If this option
> depends on xattr support "should" is not correct.
>
> > +
> > + If you are not using a security module, say N.
> > diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> > index 67e2d13..81a1d6f 100644
> > --- a/fs/f2fs/dir.c
> > +++ b/fs/f2fs/dir.c
> > @@ -13,6 +13,7 @@
> > #include "f2fs.h"
> > #include "node.h"
> > #include "acl.h"
> > +#include "xattr.h"
> >
> > static unsigned long dir_blocks(struct inode *inode)
> > {
> > @@ -334,6 +335,10 @@ static struct page *init_inode_metadata(struct inode *inode,
> > if (err)
> > goto error;
> >
> > + err = f2fs_init_security(inode, dir, name);
> > + if (err)
> > + goto error;
> > +
> > wait_on_page_writeback(page);
> > } else {
> > page = get_node_page(F2FS_SB(dir->i_sb), inode->i_ino);
> > diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
> > index ae61f35..b5292fa 100644
> > --- a/fs/f2fs/xattr.c
> > +++ b/fs/f2fs/xattr.c
> > @@ -20,6 +20,7 @@
> > */
> > #include <linux/rwsem.h>
> > #include <linux/f2fs_fs.h>
> > +#include <linux/security.h>
> > #include "f2fs.h"
> > #include "xattr.h"
> >
> > @@ -43,6 +44,10 @@ static size_t f2fs_xattr_generic_list(struct dentry *dentry, char *list,
> > prefix = XATTR_TRUSTED_PREFIX;
> > prefix_len = XATTR_TRUSTED_PREFIX_LEN;
> > break;
> > + case F2FS_XATTR_INDEX_SECURITY:
> > + prefix = XATTR_SECURITY_PREFIX;
> > + prefix_len = XATTR_SECURITY_PREFIX_LEN;
> > + break;
> > default:
> > return -EINVAL;
> > }
> > @@ -70,13 +75,14 @@ static int f2fs_xattr_generic_get(struct dentry *dentry, const char *name,
> > if (!capable(CAP_SYS_ADMIN))
> > return -EPERM;
> > break;
> > + case F2FS_XATTR_INDEX_SECURITY:
> > + break;
> > default:
> > return -EINVAL;
> > }
> > if (strcmp(name, "") == 0)
> > return -EINVAL;
> > - return f2fs_getxattr(dentry->d_inode, type, name,
> > - buffer, size);
> > + return f2fs_getxattr(dentry->d_inode, type, name, buffer, size);
> > }
> >
> > static int f2fs_xattr_generic_set(struct dentry *dentry, const char *name,
> > @@ -93,6 +99,8 @@ static int f2fs_xattr_generic_set(struct dentry *dentry, const char *name,
> > if (!capable(CAP_SYS_ADMIN))
> > return -EPERM;
> > break;
> > + case F2FS_XATTR_INDEX_SECURITY:
> > + break;
> > default:
> > return -EINVAL;
> > }
> > @@ -145,6 +153,40 @@ static int f2fs_xattr_advise_set(struct dentry *dentry, const char *name,
> > return 0;
> > }
> >
> > +#ifdef CONFIG_F2FS_FS_SECURITY
> > +static int f2fs_initxattrs(struct inode *inode, const struct xattr *xattr_array,
> > + void *fs_info)
> > +{
> > + const struct xattr *xattr;
> > + char *name;
> > + int err = 0;
> > +
> > + for (xattr = xattr_array; xattr->name != NULL; xattr++) {
> > + name = kmalloc(XATTR_SECURITY_PREFIX_LEN +
> > + strlen(xattr->name) + 1, GFP_NOFS);
> > + if (!name) {
> > + err = -ENOMEM;
> > + break;
> > + }
> > + strcpy(name, XATTR_SECURITY_PREFIX);
> > + strcpy(name + XATTR_SECURITY_PREFIX_LEN, xattr->name);
>
> sprintf(name, XATTR_SECURITY_PREFIX "%s", xattr->name);
>
> might look simpler.
>
> > + err = f2fs_setxattr(inode, F2FS_XATTR_INDEX_SECURITY, name,
> > + xattr->value, xattr->value_len);
> > + kfree(name);
> > + if (err < 0)
> > + break;
> > + }
> > + return err;
> > +}
> > +
> > +int f2fs_init_security(struct inode *inode, struct inode *dir,
> > + const struct qstr *qstr)
> > +{
> > + return security_inode_init_security(inode, dir, qstr,
> > + &f2fs_initxattrs, NULL);
> > +}
> > +#endif
> > +
> > const struct xattr_handler f2fs_xattr_user_handler = {
> > .prefix = XATTR_USER_PREFIX,
> > .flags = F2FS_XATTR_INDEX_USER,
> > @@ -169,6 +211,13 @@ const struct xattr_handler f2fs_xattr_advise_handler = {
> > .set = f2fs_xattr_advise_set,
> > };
> >
> > +const struct xattr_handler f2fs_xattr_security_handler = {
> > + .prefix = XATTR_SECURITY_PREFIX,
> > + .list = f2fs_xattr_generic_list,
> > + .get = f2fs_xattr_generic_get,
> > + .set = f2fs_xattr_generic_set,
> > +};
> > +
> > static const struct xattr_handler *f2fs_xattr_handler_map[] = {
> > [F2FS_XATTR_INDEX_USER] = &f2fs_xattr_user_handler,
> > #ifdef CONFIG_F2FS_FS_POSIX_ACL
> > @@ -177,6 +226,9 @@ static const struct xattr_handler *f2fs_xattr_handler_map[] = {
> > #endif
> > [F2FS_XATTR_INDEX_TRUSTED] = &f2fs_xattr_trusted_handler,
> > [F2FS_XATTR_INDEX_ADVISE] = &f2fs_xattr_advise_handler,
> > +#ifdef CONFIG_F2FS_FS_SECURITY
> > + [F2FS_XATTR_INDEX_SECURITY] = &f2fs_xattr_security_handler,
> > +#endif
> > };
> >
> > const struct xattr_handler *f2fs_xattr_handlers[] = {
> > @@ -187,6 +239,9 @@ const struct xattr_handler *f2fs_xattr_handlers[] = {
> > #endif
> > &f2fs_xattr_trusted_handler,
> > &f2fs_xattr_advise_handler,
> > +#ifdef CONFIG_F2FS_FS_SECURITY
> > + &f2fs_xattr_security_handler,
> > +#endif
> > NULL,
> > };
> >
> > diff --git a/fs/f2fs/xattr.h b/fs/f2fs/xattr.h
> > index 49c9558..14e1329 100644
> > --- a/fs/f2fs/xattr.h
> > +++ b/fs/f2fs/xattr.h
> > @@ -112,6 +112,7 @@ extern const struct xattr_handler f2fs_xattr_trusted_handler;
> > extern const struct xattr_handler f2fs_xattr_acl_access_handler;
> > extern const struct xattr_handler f2fs_xattr_acl_default_handler;
> > extern const struct xattr_handler f2fs_xattr_advise_handler;
> > +extern const struct xattr_handler f2fs_xattr_security_handler;
> >
> > extern const struct xattr_handler *f2fs_xattr_handlers[];
> >
> > @@ -121,7 +122,6 @@ extern int f2fs_getxattr(struct inode *inode, int name_index, const char *name,
> > void *buffer, size_t buffer_size);
> > extern ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer,
> > size_t buffer_size);
> > -
> > #else
> >
> > #define f2fs_xattr_handlers NULL
> > @@ -142,4 +142,14 @@ static inline ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer,
> > }
> > #endif
> >
> > +#ifdef CONFIG_F2FS_FS_SECURITY
> > +extern int f2fs_init_security(struct inode *inode, struct inode *dir,
> > + const struct qstr *qstr);
> > +#else
> > +static inline int f2fs_init_security(struct inode *inode, struct inode *dir,
> > + const struct qstr *qstr)
> > +{
> > + return 0;
> > +}
> > +#endif
> > #endif /* __F2FS_XATTR_H__ */
>

--
Jaegeuk Kim
Samsung


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2013-06-08 01:10:43

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [PATCH 2/2 v2] f2fs: support xattr security labels

Change log from v2:
o fix description and simplify a code line (commented by Casey
Schaufler)

Change log from v1:
o fix a bug

From 9f889afcd74a58d9a538fa02f4af31d11eb17c54 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <[email protected]>
Date: Mon, 3 Jun 2013 19:46:19 +0900
Subject: [PATCH] f2fs: support xattr security labels
Cc: [email protected], [email protected], [email protected]

This patch adds the support of security labels for f2fs, which will be used
by Linus Security Models (LSMs).

Quote from http://en.wikipedia.org/wiki/Linux_Security_Modules:
"Linux Security Modules (LSM) is a framework that allows the Linux kernel to
support a variety of computer security models while avoiding favoritism toward
any single security implementation. The framework is licensed under the terms of
the GNU General Public License and is standard part of the Linux kernel since
Linux 2.6. AppArmor, SELinux, Smack and TOMOYO Linux are the currently accepted
modules in the official kernel.".

Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/Kconfig | 12 +++++++++++
fs/f2fs/dir.c | 5 +++++
fs/f2fs/xattr.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++---
fs/f2fs/xattr.h | 12 ++++++++++-
4 files changed, 87 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/Kconfig b/fs/f2fs/Kconfig
index fd27e7e..e06e099 100644
--- a/fs/f2fs/Kconfig
+++ b/fs/f2fs/Kconfig
@@ -51,3 +51,15 @@ config F2FS_FS_POSIX_ACL
Linux website <http://acl.bestbits.at/>.

If you don't know what Access Control Lists are, say N
+
+config F2FS_FS_SECURITY
+ bool "F2FS Security Labels"
+ depends on F2FS_FS_XATTR
+ help
+ Security labels provide an access control facility to support Linux
+ Security Models (LSMs) accepted by AppArmor, SELinux, Smack and TOMOYO
+ Linux. This option enables an extended attribute handler for file
+ security labels in the f2fs filesystem, so that it requires enabling
+ the extended attribute support in advance.
+
+ If you are not using a security module, say N.
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 67e2d13..81a1d6f 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -13,6 +13,7 @@
#include "f2fs.h"
#include "node.h"
#include "acl.h"
+#include "xattr.h"

static unsigned long dir_blocks(struct inode *inode)
{
@@ -334,6 +335,10 @@ static struct page *init_inode_metadata(struct inode *inode,
if (err)
goto error;

+ err = f2fs_init_security(inode, dir, name);
+ if (err)
+ goto error;
+
wait_on_page_writeback(page);
} else {
page = get_node_page(F2FS_SB(dir->i_sb), inode->i_ino);
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index ae61f35..c06e8a3 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -20,6 +20,7 @@
*/
#include <linux/rwsem.h>
#include <linux/f2fs_fs.h>
+#include <linux/security.h>
#include "f2fs.h"
#include "xattr.h"

@@ -43,6 +44,10 @@ static size_t f2fs_xattr_generic_list(struct dentry *dentry, char *list,
prefix = XATTR_TRUSTED_PREFIX;
prefix_len = XATTR_TRUSTED_PREFIX_LEN;
break;
+ case F2FS_XATTR_INDEX_SECURITY:
+ prefix = XATTR_SECURITY_PREFIX;
+ prefix_len = XATTR_SECURITY_PREFIX_LEN;
+ break;
default:
return -EINVAL;
}
@@ -50,7 +55,7 @@ static size_t f2fs_xattr_generic_list(struct dentry *dentry, char *list,
total_len = prefix_len + name_len + 1;
if (list && total_len <= list_size) {
memcpy(list, prefix, prefix_len);
- memcpy(list+prefix_len, name, name_len);
+ memcpy(list + prefix_len, name, name_len);
list[prefix_len + name_len] = '\0';
}
return total_len;
@@ -70,13 +75,14 @@ static int f2fs_xattr_generic_get(struct dentry *dentry, const char *name,
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
break;
+ case F2FS_XATTR_INDEX_SECURITY:
+ break;
default:
return -EINVAL;
}
if (strcmp(name, "") == 0)
return -EINVAL;
- return f2fs_getxattr(dentry->d_inode, type, name,
- buffer, size);
+ return f2fs_getxattr(dentry->d_inode, type, name, buffer, size);
}

static int f2fs_xattr_generic_set(struct dentry *dentry, const char *name,
@@ -93,6 +99,8 @@ static int f2fs_xattr_generic_set(struct dentry *dentry, const char *name,
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
break;
+ case F2FS_XATTR_INDEX_SECURITY:
+ break;
default:
return -EINVAL;
}
@@ -145,6 +153,40 @@ static int f2fs_xattr_advise_set(struct dentry *dentry, const char *name,
return 0;
}

+#ifdef CONFIG_F2FS_FS_SECURITY
+static int f2fs_initxattrs(struct inode *inode, const struct xattr *xattr_array,
+ void *fs_info)
+{
+ const struct xattr *xattr;
+ char *name;
+ int err = 0;
+
+ for (xattr = xattr_array; xattr->name != NULL; xattr++) {
+ name = kmalloc(XATTR_SECURITY_PREFIX_LEN +
+ strlen(xattr->name) + 1, GFP_NOFS);
+ if (!name) {
+ err = -ENOMEM;
+ break;
+ }
+ strcpy(name, XATTR_SECURITY_PREFIX);
+ sprintf(name, XATTR_SECURITY_PREFIX "%s", xattr->name);
+ err = f2fs_setxattr(inode, F2FS_XATTR_INDEX_SECURITY, name,
+ xattr->value, xattr->value_len);
+ kfree(name);
+ if (err < 0)
+ break;
+ }
+ return err;
+}
+
+int f2fs_init_security(struct inode *inode, struct inode *dir,
+ const struct qstr *qstr)
+{
+ return security_inode_init_security(inode, dir, qstr,
+ &f2fs_initxattrs, NULL);
+}
+#endif
+
const struct xattr_handler f2fs_xattr_user_handler = {
.prefix = XATTR_USER_PREFIX,
.flags = F2FS_XATTR_INDEX_USER,
@@ -169,6 +211,14 @@ const struct xattr_handler f2fs_xattr_advise_handler = {
.set = f2fs_xattr_advise_set,
};

+const struct xattr_handler f2fs_xattr_security_handler = {
+ .prefix = XATTR_SECURITY_PREFIX,
+ .flags = F2FS_XATTR_INDEX_SECURITY,
+ .list = f2fs_xattr_generic_list,
+ .get = f2fs_xattr_generic_get,
+ .set = f2fs_xattr_generic_set,
+};
+
static const struct xattr_handler *f2fs_xattr_handler_map[] = {
[F2FS_XATTR_INDEX_USER] = &f2fs_xattr_user_handler,
#ifdef CONFIG_F2FS_FS_POSIX_ACL
@@ -176,6 +226,9 @@ static const struct xattr_handler *f2fs_xattr_handler_map[] = {
[F2FS_XATTR_INDEX_POSIX_ACL_DEFAULT] = &f2fs_xattr_acl_default_handler,
#endif
[F2FS_XATTR_INDEX_TRUSTED] = &f2fs_xattr_trusted_handler,
+#ifdef CONFIG_F2FS_FS_SECURITY
+ [F2FS_XATTR_INDEX_SECURITY] = &f2fs_xattr_security_handler,
+#endif
[F2FS_XATTR_INDEX_ADVISE] = &f2fs_xattr_advise_handler,
};

@@ -186,6 +239,9 @@ const struct xattr_handler *f2fs_xattr_handlers[] = {
&f2fs_xattr_acl_default_handler,
#endif
&f2fs_xattr_trusted_handler,
+#ifdef CONFIG_F2FS_FS_SECURITY
+ &f2fs_xattr_security_handler,
+#endif
&f2fs_xattr_advise_handler,
NULL,
};
diff --git a/fs/f2fs/xattr.h b/fs/f2fs/xattr.h
index 49c9558..14e1329 100644
--- a/fs/f2fs/xattr.h
+++ b/fs/f2fs/xattr.h
@@ -112,6 +112,7 @@ extern const struct xattr_handler f2fs_xattr_trusted_handler;
extern const struct xattr_handler f2fs_xattr_acl_access_handler;
extern const struct xattr_handler f2fs_xattr_acl_default_handler;
extern const struct xattr_handler f2fs_xattr_advise_handler;
+extern const struct xattr_handler f2fs_xattr_security_handler;

extern const struct xattr_handler *f2fs_xattr_handlers[];

@@ -121,7 +122,6 @@ extern int f2fs_getxattr(struct inode *inode, int name_index, const char *name,
void *buffer, size_t buffer_size);
extern ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer,
size_t buffer_size);
-
#else

#define f2fs_xattr_handlers NULL
@@ -142,4 +142,14 @@ static inline ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer,
}
#endif

+#ifdef CONFIG_F2FS_FS_SECURITY
+extern int f2fs_init_security(struct inode *inode, struct inode *dir,
+ const struct qstr *qstr);
+#else
+static inline int f2fs_init_security(struct inode *inode, struct inode *dir,
+ const struct qstr *qstr)
+{
+ return 0;
+}
+#endif
#endif /* __F2FS_XATTR_H__ */
--
1.8.1.3.566.gaa39828



--
Jaegeuk Kim
Samsung


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2013-06-11 06:56:03

by Jaegeuk Kim

[permalink] [raw]
Subject: Re: [PATCH 2/2 v4] f2fs: support xattr security labels

Change log from v3:
o fix deadlock condition for inode block page (tested on SElinux)

Change log from v2:
o fix description and simplify a code line (commented by Casey
Schaufler)

Change log from v1:
o fix a bug

From 2065a7282f65eb1b395c675d4a1f9c8967e60385 Mon Sep 17 00:00:00 2001
From: Jaegeuk Kim <[email protected]>
Date: Mon, 3 Jun 2013 19:46:19 +0900
Subject: [PATCH] f2fs: support xattr security labels
Cc: [email protected], [email protected],
[email protected]

This patch adds the support of security labels for f2fs, which will be
used
by Linus Security Models (LSMs).

Quote from http://en.wikipedia.org/wiki/Linux_Security_Modules:
"Linux Security Modules (LSM) is a framework that allows the Linux
kernel to
support a variety of computer security models while avoiding favoritism
toward
any single security implementation. The framework is licensed under the
terms of
the GNU General Public License and is standard part of the Linux kernel
since
Linux 2.6. AppArmor, SELinux, Smack and TOMOYO Linux are the currently
accepted
modules in the official kernel.".

Signed-off-by: Jaegeuk Kim <[email protected]>
---
fs/f2fs/Kconfig | 12 +++++++++++
fs/f2fs/dir.c | 5 +++++
fs/f2fs/f2fs.h | 2 +-
fs/f2fs/node.c | 12 +++++++----
fs/f2fs/xattr.c | 64
++++++++++++++++++++++++++++++++++++++++++++++++++-------
fs/f2fs/xattr.h | 24 ++++++++++++++--------
6 files changed, 99 insertions(+), 20 deletions(-)

diff --git a/fs/f2fs/Kconfig b/fs/f2fs/Kconfig
index fd27e7e..e06e099 100644
--- a/fs/f2fs/Kconfig
+++ b/fs/f2fs/Kconfig
@@ -51,3 +51,15 @@ config F2FS_FS_POSIX_ACL
Linux website <http://acl.bestbits.at/>.

If you don't know what Access Control Lists are, say N
+
+config F2FS_FS_SECURITY
+ bool "F2FS Security Labels"
+ depends on F2FS_FS_XATTR
+ help
+ Security labels provide an access control facility to support Linux
+ Security Models (LSMs) accepted by AppArmor, SELinux, Smack and
TOMOYO
+ Linux. This option enables an extended attribute handler for file
+ security labels in the f2fs filesystem, so that it requires enabling
+ the extended attribute support in advance.
+
+ If you are not using a security module, say N.
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 67e2d13..eaea5b5 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -13,6 +13,7 @@
#include "f2fs.h"
#include "node.h"
#include "acl.h"
+#include "xattr.h"

static unsigned long dir_blocks(struct inode *inode)
{
@@ -334,6 +335,10 @@ static struct page *init_inode_metadata(struct
inode *inode,
if (err)
goto error;

+ err = f2fs_init_security(inode, dir, name, page);
+ if (err)
+ goto error;
+
wait_on_page_writeback(page);
} else {
page = get_node_page(F2FS_SB(dir->i_sb), inode->i_ino);
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index d6e63da..4f2c209 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -968,7 +968,7 @@ int get_dnode_of_data(struct dnode_of_data *,
pgoff_t, int);
int truncate_inode_blocks(struct inode *, pgoff_t);
int remove_inode_page(struct inode *);
struct page *new_inode_page(struct inode *, const struct qstr *);
-struct page *new_node_page(struct dnode_of_data *, unsigned int);
+struct page *new_node_page(struct dnode_of_data *, unsigned int, struct
page *);
void ra_node_page(struct f2fs_sb_info *, nid_t);
struct page *get_node_page(struct f2fs_sb_info *, pgoff_t);
struct page *get_node_page_ra(struct page *, int);
diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 5a59780..b02440c 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -433,7 +433,7 @@ int get_dnode_of_data(struct dnode_of_data *dn,
pgoff_t index, int mode)
}

dn->nid = nids[i];
- npage[i] = new_node_page(dn, noffset[i]);
+ npage[i] = new_node_page(dn, noffset[i], NULL);
if (IS_ERR(npage[i])) {
alloc_nid_failed(sbi, nids[i]);
err = PTR_ERR(npage[i]);
@@ -814,10 +814,11 @@ struct page *new_inode_page(struct inode *inode,
const struct qstr *name)
set_new_dnode(&dn, inode, NULL, NULL, inode->i_ino);

/* caller should f2fs_put_page(page, 1); */
- return new_node_page(&dn, 0);
+ return new_node_page(&dn, 0, NULL);
}

-struct page *new_node_page(struct dnode_of_data *dn, unsigned int ofs)
+struct page *new_node_page(struct dnode_of_data *dn,
+ unsigned int ofs, struct page *ipage)
{
struct f2fs_sb_info *sbi = F2FS_SB(dn->inode->i_sb);
struct address_space *mapping = sbi->node_inode->i_mapping;
@@ -850,7 +851,10 @@ struct page *new_node_page(struct dnode_of_data
*dn, unsigned int ofs)
set_cold_node(dn->inode, page);

dn->node_page = page;
- sync_inode_page(dn);
+ if (ipage)
+ update_inode(dn->inode, ipage);
+ else
+ sync_inode_page(dn);
set_page_dirty(page);
if (ofs == 0)
inc_valid_inode_count(sbi);
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c
index ae61f35..3ab07ec 100644
--- a/fs/f2fs/xattr.c
+++ b/fs/f2fs/xattr.c
@@ -20,6 +20,7 @@
*/
#include <linux/rwsem.h>
#include <linux/f2fs_fs.h>
+#include <linux/security.h>
#include "f2fs.h"
#include "xattr.h"

@@ -43,6 +44,10 @@ static size_t f2fs_xattr_generic_list(struct dentry
*dentry, char *list,
prefix = XATTR_TRUSTED_PREFIX;
prefix_len = XATTR_TRUSTED_PREFIX_LEN;
break;
+ case F2FS_XATTR_INDEX_SECURITY:
+ prefix = XATTR_SECURITY_PREFIX;
+ prefix_len = XATTR_SECURITY_PREFIX_LEN;
+ break;
default:
return -EINVAL;
}
@@ -50,7 +55,7 @@ static size_t f2fs_xattr_generic_list(struct dentry
*dentry, char *list,
total_len = prefix_len + name_len + 1;
if (list && total_len <= list_size) {
memcpy(list, prefix, prefix_len);
- memcpy(list+prefix_len, name, name_len);
+ memcpy(list + prefix_len, name, name_len);
list[prefix_len + name_len] = '\0';
}
return total_len;
@@ -70,13 +75,14 @@ static int f2fs_xattr_generic_get(struct dentry
*dentry, const char *name,
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
break;
+ case F2FS_XATTR_INDEX_SECURITY:
+ break;
default:
return -EINVAL;
}
if (strcmp(name, "") == 0)
return -EINVAL;
- return f2fs_getxattr(dentry->d_inode, type, name,
- buffer, size);
+ return f2fs_getxattr(dentry->d_inode, type, name, buffer, size);
}

static int f2fs_xattr_generic_set(struct dentry *dentry, const char
*name,
@@ -93,13 +99,15 @@ static int f2fs_xattr_generic_set(struct dentry
*dentry, const char *name,
if (!capable(CAP_SYS_ADMIN))
return -EPERM;
break;
+ case F2FS_XATTR_INDEX_SECURITY:
+ break;
default:
return -EINVAL;
}
if (strcmp(name, "") == 0)
return -EINVAL;

- return f2fs_setxattr(dentry->d_inode, type, name, value, size);
+ return f2fs_setxattr(dentry->d_inode, type, name, value, size, NULL);
}

static size_t f2fs_xattr_advise_list(struct dentry *dentry, char *list,
@@ -145,6 +153,31 @@ static int f2fs_xattr_advise_set(struct dentry
*dentry, const char *name,
return 0;
}

+#ifdef CONFIG_F2FS_FS_SECURITY
+static int f2fs_initxattrs(struct inode *inode, const struct xattr
*xattr_array,
+ void *page)
+{
+ const struct xattr *xattr;
+ int err = 0;
+
+ for (xattr = xattr_array; xattr->name != NULL; xattr++) {
+ err = f2fs_setxattr(inode, F2FS_XATTR_INDEX_SECURITY,
+ xattr->name, xattr->value,
+ xattr->value_len, (struct page *)page);
+ if (err < 0)
+ break;
+ }
+ return err;
+}
+
+int f2fs_init_security(struct inode *inode, struct inode *dir,
+ const struct qstr *qstr, struct page *ipage)
+{
+ return security_inode_init_security(inode, dir, qstr,
+ &f2fs_initxattrs, ipage);
+}
+#endif
+
const struct xattr_handler f2fs_xattr_user_handler = {
.prefix = XATTR_USER_PREFIX,
.flags = F2FS_XATTR_INDEX_USER,
@@ -169,6 +202,14 @@ const struct xattr_handler
f2fs_xattr_advise_handler = {
.set = f2fs_xattr_advise_set,
};

+const struct xattr_handler f2fs_xattr_security_handler = {
+ .prefix = XATTR_SECURITY_PREFIX,
+ .flags = F2FS_XATTR_INDEX_SECURITY,
+ .list = f2fs_xattr_generic_list,
+ .get = f2fs_xattr_generic_get,
+ .set = f2fs_xattr_generic_set,
+};
+
static const struct xattr_handler *f2fs_xattr_handler_map[] = {
[F2FS_XATTR_INDEX_USER] = &f2fs_xattr_user_handler,
#ifdef CONFIG_F2FS_FS_POSIX_ACL
@@ -176,6 +217,9 @@ static const struct xattr_handler
*f2fs_xattr_handler_map[] = {
[F2FS_XATTR_INDEX_POSIX_ACL_DEFAULT] =
&f2fs_xattr_acl_default_handler,
#endif
[F2FS_XATTR_INDEX_TRUSTED] = &f2fs_xattr_trusted_handler,
+#ifdef CONFIG_F2FS_FS_SECURITY
+ [F2FS_XATTR_INDEX_SECURITY] = &f2fs_xattr_security_handler,
+#endif
[F2FS_XATTR_INDEX_ADVISE] = &f2fs_xattr_advise_handler,
};

@@ -186,6 +230,9 @@ const struct xattr_handler *f2fs_xattr_handlers[] =
{
&f2fs_xattr_acl_default_handler,
#endif
&f2fs_xattr_trusted_handler,
+#ifdef CONFIG_F2FS_FS_SECURITY
+ &f2fs_xattr_security_handler,
+#endif
&f2fs_xattr_advise_handler,
NULL,
};
@@ -300,7 +347,7 @@ cleanup:
}

int f2fs_setxattr(struct inode *inode, int name_index, const char
*name,
- const void *value, size_t value_len)
+ const void *value, size_t value_len, struct page *ipage)
{
struct f2fs_sb_info *sbi = F2FS_SB(inode->i_sb);
struct f2fs_inode_info *fi = F2FS_I(inode);
@@ -339,7 +386,7 @@ int f2fs_setxattr(struct inode *inode, int
name_index, const char *name,
set_new_dnode(&dn, inode, NULL, NULL, fi->i_xattr_nid);
mark_inode_dirty(inode);

- page = new_node_page(&dn, XATTR_NODE_OFFSET);
+ page = new_node_page(&dn, XATTR_NODE_OFFSET, ipage);
if (IS_ERR(page)) {
alloc_nid_failed(sbi, fi->i_xattr_nid);
fi->i_xattr_nid = 0;
@@ -439,7 +486,10 @@ int f2fs_setxattr(struct inode *inode, int
name_index, const char *name,
inode->i_ctime = CURRENT_TIME;
clear_inode_flag(fi, FI_ACL_MODE);
}
- update_inode_page(inode);
+ if (ipage)
+ update_inode(inode, ipage);
+ else
+ update_inode_page(inode);
mutex_unlock_op(sbi, ilock);

return 0;
diff --git a/fs/f2fs/xattr.h b/fs/f2fs/xattr.h
index 49c9558..3c0817b 100644
--- a/fs/f2fs/xattr.h
+++ b/fs/f2fs/xattr.h
@@ -112,21 +112,19 @@ extern const struct xattr_handler
f2fs_xattr_trusted_handler;
extern const struct xattr_handler f2fs_xattr_acl_access_handler;
extern const struct xattr_handler f2fs_xattr_acl_default_handler;
extern const struct xattr_handler f2fs_xattr_advise_handler;
+extern const struct xattr_handler f2fs_xattr_security_handler;

extern const struct xattr_handler *f2fs_xattr_handlers[];

-extern int f2fs_setxattr(struct inode *inode, int name_index, const
char *name,
- const void *value, size_t value_len);
-extern int f2fs_getxattr(struct inode *inode, int name_index, const
char *name,
- void *buffer, size_t buffer_size);
-extern ssize_t f2fs_listxattr(struct dentry *dentry, char *buffer,
- size_t buffer_size);
-
+extern int f2fs_setxattr(struct inode *, int, const char *,
+ const void *, size_t, struct page *);
+extern int f2fs_getxattr(struct inode *, int, const char *, void *,
size_t);
+extern ssize_t f2fs_listxattr(struct dentry *, char *, size_t);
#else

#define f2fs_xattr_handlers NULL
static inline int f2fs_setxattr(struct inode *inode, int name_index,
- const char *name, const void *value, size_t value_len)
+ const char *name, const void *value, size_t value_len)
{
return -EOPNOTSUPP;
}
@@ -142,4 +140,14 @@ static inline ssize_t f2fs_listxattr(struct dentry
*dentry, char *buffer,
}
#endif

+#ifdef CONFIG_F2FS_FS_SECURITY
+extern int f2fs_init_security(struct inode *, struct inode *,
+ const struct qstr *, struct page *);
+#else
+static inline int f2fs_init_security(struct inode *inode, struct inode
*dir,
+ const struct qstr *qstr, struct page *ipage)
+{
+ return 0;
+}
+#endif
#endif /* __F2FS_XATTR_H__ */
--
1.8.1.3.566.gaa39828



--
Jaegeuk Kim
Samsung


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part