* [PATCH v1.2 0/5] IMA: making i_readcount a first class inode citizen
@ 2010-11-02 17:08 Mimi Zohar
2010-11-02 17:08 ` [PATCH v1.2 1/5] IMA: convert i_readcount to atomic Mimi Zohar
` (5 more replies)
0 siblings, 6 replies; 9+ messages in thread
From: Mimi Zohar @ 2010-11-02 17:08 UTC (permalink / raw)
To: linux-kernel
Cc: Mimi Zohar, linux-security-module, linux-fsdevel, jmorris, akpm,
torvalds, eparis, viro, Dave Chinner
Changelog:
- renamed iget/iput_readcount to i_readcount_inc/dec (Dave Chinner's suggestion)
- finished cleaning up the locking, so that: (based on Eric's comment.)
- i_lock is not required
- i_mutex is taken when making measurement decisions to prevent
file metadata(eg. permissions, xattr) changing from under it.
- iint->mutex is taken when accessing/modifying the iint structure.
- Based on the previous posting discussion, i_readcount is now defined as
atomic.
This patchset separates the incrementing/decrementing of the i_readcount,
in the VFS layer, from other IMA functionality, by replacing the current
ima_counts_get() call with i_readcount_inc(). Its unclear whether this
call to increment i_readcount should be made earlier, like i_writecount.
The patch ordering is a bit redundant in order to leave removing the ifdef
around i_readcount until the last patch. The first four patches: redefines
i_readcount as atomic, defines i_readcount_inc/dec(), moves the IMA
functionality in ima_counts_get() to ima_file_check(), and removes the
IMA imbalance code, simplifying IMA. The last patch removes the ifdef
around i_readcount, making i_readcount into a "first class inode citizen".
The generic_setlease code could then take advantage of i_readcount.
Mimi
Mimi Zohar (5):
IMA: convert i_readcount to atomic
IMA: define readcount functions
IMA: maintain i_readcount in the VFS layer
IMA: remove IMA imbalance checking
IMA: making i_readcount a first class inode citizen
fs/file_table.c | 5 +-
fs/open.c | 3 +-
include/linux/fs.h | 16 ++++-
include/linux/ima.h | 6 --
security/integrity/ima/ima_iint.c | 5 --
security/integrity/ima/ima_main.c | 130 ++++---------------------------------
6 files changed, 31 insertions(+), 134 deletions(-)
--
1.7.2.2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v1.2 1/5] IMA: convert i_readcount to atomic
2010-11-02 17:08 [PATCH v1.2 0/5] IMA: making i_readcount a first class inode citizen Mimi Zohar
@ 2010-11-02 17:08 ` Mimi Zohar
2010-11-02 17:08 ` [PATCH v1.2 2/5] IMA: define readcount functions Mimi Zohar
` (4 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Mimi Zohar @ 2010-11-02 17:08 UTC (permalink / raw)
To: linux-kernel
Cc: Mimi Zohar, linux-security-module, linux-fsdevel, jmorris, akpm,
torvalds, eparis, viro, Dave Chinner, Mimi Zohar
Convert the inode's i_readcount from an unsigned int to atomic.
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
---
include/linux/fs.h | 3 +--
security/integrity/ima/ima_iint.c | 7 ++++---
security/integrity/ima/ima_main.c | 11 ++++++-----
3 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 334d68a..442fceb 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -787,8 +787,7 @@ struct inode {
unsigned int i_flags;
#ifdef CONFIG_IMA
- /* protected by i_lock */
- unsigned int i_readcount; /* struct files open RO */
+ atomic_t i_readcount; /* struct files open RO */
#endif
atomic_t i_writecount;
#ifdef CONFIG_SECURITY
diff --git a/security/integrity/ima/ima_iint.c b/security/integrity/ima/ima_iint.c
index c442e47..f005355 100644
--- a/security/integrity/ima/ima_iint.c
+++ b/security/integrity/ima/ima_iint.c
@@ -137,10 +137,11 @@ void ima_inode_free(struct inode *inode)
{
struct ima_iint_cache *iint;
- if (inode->i_readcount)
- printk(KERN_INFO "%s: readcount: %u\n", __func__, inode->i_readcount);
+ if (atomic_read(&inode->i_readcount))
+ printk(KERN_INFO "%s: readcount: %u\n", __func__,
+ atomic_read(&inode->i_readcount));
- inode->i_readcount = 0;
+ atomic_set(&inode->i_readcount, 0);
if (!IS_IMA(inode))
return;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 203de97..a189197 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -113,7 +113,7 @@ void ima_counts_get(struct file *file)
goto out;
if (mode & FMODE_WRITE) {
- if (inode->i_readcount && IS_IMA(inode))
+ if (atomic_read(&inode->i_readcount) && IS_IMA(inode))
send_tomtou = true;
goto out;
}
@@ -127,7 +127,7 @@ void ima_counts_get(struct file *file)
out:
/* remember the vfs deals with i_writecount */
if ((mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ)
- inode->i_readcount++;
+ atomic_inc(&inode->i_readcount);
spin_unlock(&inode->i_lock);
@@ -149,15 +149,16 @@ static void ima_dec_counts(struct inode *inode, struct file *file)
assert_spin_locked(&inode->i_lock);
if ((mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) {
- if (unlikely(inode->i_readcount == 0)) {
+ if (unlikely(atomic_read(&inode->i_readcount) == 0)) {
if (!ima_limit_imbalance(file)) {
printk(KERN_INFO "%s: open/free imbalance (r:%u)\n",
- __func__, inode->i_readcount);
+ __func__,
+ atomic_read(&inode->i_readcount));
dump_stack();
}
return;
}
- inode->i_readcount--;
+ atomic_dec(&inode->i_readcount);
}
}
--
1.7.2.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v1.2 2/5] IMA: define readcount functions
2010-11-02 17:08 [PATCH v1.2 0/5] IMA: making i_readcount a first class inode citizen Mimi Zohar
2010-11-02 17:08 ` [PATCH v1.2 1/5] IMA: convert i_readcount to atomic Mimi Zohar
@ 2010-11-02 17:08 ` Mimi Zohar
2010-11-02 17:08 ` [PATCH v1.2 3/5] IMA: maintain i_readcount in the VFS layer Mimi Zohar
` (3 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Mimi Zohar @ 2010-11-02 17:08 UTC (permalink / raw)
To: linux-kernel
Cc: Mimi Zohar, linux-security-module, linux-fsdevel, jmorris, akpm,
torvalds, eparis, viro, Dave Chinner, Mimi Zohar
Define i_readcount_inc/dec() functions to be called from the VFS layer.
Changelog:
- renamed iget/iput_readcount to i_readcount_inc/dec (Dave Chinner's suggestion)
- removed i_lock in iput_readcount() (based on comments:Dave Chinner,Eric Paris)
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
---
include/linux/fs.h | 20 ++++++++++++++++++++
1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 442fceb..6b7e2fb 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2176,6 +2176,26 @@ static inline void allow_write_access(struct file *file)
if (file)
atomic_inc(&file->f_path.dentry->d_inode->i_writecount);
}
+#ifdef CONFIG_IMA
+static inline void i_readcount_dec(struct inode *inode)
+{
+ BUG_ON(!atomic_read(&inode->i_readcount));
+ atomic_dec(&inode->i_readcount);
+}
+static inline void i_readcount_inc(struct inode *inode)
+{
+ atomic_inc(&inode->i_readcount);
+}
+#else
+static inline void i_readcount_dec(struct inode *inode)
+{
+ return;
+}
+static inline void i_readcount_inc(struct inode *inode)
+{
+ return;
+}
+#endif
extern int do_pipe_flags(int *, int);
extern struct file *create_read_pipe(struct file *f, int flags);
extern struct file *create_write_pipe(int flags);
--
1.7.2.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v1.2 3/5] IMA: maintain i_readcount in the VFS layer
2010-11-02 17:08 [PATCH v1.2 0/5] IMA: making i_readcount a first class inode citizen Mimi Zohar
2010-11-02 17:08 ` [PATCH v1.2 1/5] IMA: convert i_readcount to atomic Mimi Zohar
2010-11-02 17:08 ` [PATCH v1.2 2/5] IMA: define readcount functions Mimi Zohar
@ 2010-11-02 17:08 ` Mimi Zohar
2010-11-02 17:08 ` [PATCH v1.2 4/5] IMA: remove IMA imbalance checking Mimi Zohar
` (2 subsequent siblings)
5 siblings, 0 replies; 9+ messages in thread
From: Mimi Zohar @ 2010-11-02 17:08 UTC (permalink / raw)
To: linux-kernel
Cc: Mimi Zohar, linux-security-module, linux-fsdevel, jmorris, akpm,
torvalds, eparis, viro, Dave Chinner, Mimi Zohar
ima_counts_get() updated the readcount and invalidated the PCR,
as necessary. Only update the i_readcount in the VFS layer.
Move the PCR invalidation checks to ima_file_check(), where it
belongs.
Maintaining the i_readcount in the VFS layer, will allow other
subsystems to use i_readcount.
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
---
fs/file_table.c | 5 ++++-
fs/open.c | 3 ++-
include/linux/ima.h | 6 ------
security/integrity/ima/ima_iint.c | 2 --
security/integrity/ima/ima_main.c | 25 ++++++++-----------------
5 files changed, 14 insertions(+), 27 deletions(-)
diff --git a/fs/file_table.c b/fs/file_table.c
index c3dee38..0c724de 100644
--- a/fs/file_table.c
+++ b/fs/file_table.c
@@ -190,7 +190,8 @@ struct file *alloc_file(struct path *path, fmode_t mode,
file_take_write(file);
WARN_ON(mnt_clone_write(path->mnt));
}
- ima_counts_get(file);
+ if ((mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ)
+ i_readcount_inc(path->dentry->d_inode);
return file;
}
EXPORT_SYMBOL(alloc_file);
@@ -251,6 +252,8 @@ static void __fput(struct file *file)
fops_put(file->f_op);
put_pid(file->f_owner.pid);
file_sb_list_del(file);
+ if ((file->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ)
+ i_readcount_dec(inode);
if (file->f_mode & FMODE_WRITE)
drop_file_write_access(file);
file->f_path.dentry = NULL;
diff --git a/fs/open.c b/fs/open.c
index 4197b9e..0d485c5 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -688,7 +688,8 @@ static struct file *__dentry_open(struct dentry *dentry, struct vfsmount *mnt,
if (error)
goto cleanup_all;
}
- ima_counts_get(f);
+ if ((f->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ)
+ i_readcount_inc(inode);
f->f_flags &= ~(O_CREAT | O_EXCL | O_NOCTTY | O_TRUNC);
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 975837e..09e6e62 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -20,7 +20,6 @@ extern void ima_inode_free(struct inode *inode);
extern int ima_file_check(struct file *file, int mask);
extern void ima_file_free(struct file *file);
extern int ima_file_mmap(struct file *file, unsigned long prot);
-extern void ima_counts_get(struct file *file);
#else
static inline int ima_bprm_check(struct linux_binprm *bprm)
@@ -53,10 +52,5 @@ static inline int ima_file_mmap(struct file *file, unsigned long prot)
return 0;
}
-static inline void ima_counts_get(struct file *file)
-{
- return;
-}
-
#endif /* CONFIG_IMA_H */
#endif /* _LINUX_IMA_H */
diff --git a/security/integrity/ima/ima_iint.c b/security/integrity/ima/ima_iint.c
index f005355..68efe3b 100644
--- a/security/integrity/ima/ima_iint.c
+++ b/security/integrity/ima/ima_iint.c
@@ -141,8 +141,6 @@ void ima_inode_free(struct inode *inode)
printk(KERN_INFO "%s: readcount: %u\n", __func__,
atomic_read(&inode->i_readcount));
- atomic_set(&inode->i_readcount, 0);
-
if (!IS_IMA(inode))
return;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index a189197..a0626bc 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -86,17 +86,16 @@ out:
}
/*
- * ima_counts_get - increment file counts
+ * ima_rdwr_violation_check
*
- * Maintain read/write counters for all files, but only
- * invalidate the PCR for measured files:
+ * Only invalidate the PCR for measured files:
* - Opening a file for write when already open for read,
* results in a time of measure, time of use (ToMToU) error.
* - Opening a file for read when already open for write,
* could result in a file measurement error.
*
*/
-void ima_counts_get(struct file *file)
+static void ima_rdwr_violation_check(struct file *file)
{
struct dentry *dentry = file->f_path.dentry;
struct inode *inode = dentry->d_inode;
@@ -104,13 +103,10 @@ void ima_counts_get(struct file *file)
int rc;
bool send_tomtou = false, send_writers = false;
- if (!S_ISREG(inode->i_mode))
+ if (!S_ISREG(inode->i_mode) || !ima_initialized)
return;
- spin_lock(&inode->i_lock);
-
- if (!ima_initialized)
- goto out;
+ mutex_lock(&inode->i_mutex); /* file metadata: permissions, xattr */
if (mode & FMODE_WRITE) {
if (atomic_read(&inode->i_readcount) && IS_IMA(inode))
@@ -125,11 +121,7 @@ void ima_counts_get(struct file *file)
if (atomic_read(&inode->i_writecount) > 0)
send_writers = true;
out:
- /* remember the vfs deals with i_writecount */
- if ((mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ)
- atomic_inc(&inode->i_readcount);
-
- spin_unlock(&inode->i_lock);
+ mutex_unlock(&inode->i_mutex);
if (send_tomtou)
ima_add_violation(inode, dentry->d_name.name, "invalid_pcr",
@@ -158,7 +150,6 @@ static void ima_dec_counts(struct inode *inode, struct file *file)
}
return;
}
- atomic_dec(&inode->i_readcount);
}
}
@@ -203,8 +194,7 @@ static void ima_file_free_noiint(struct inode *inode, struct file *file)
* ima_file_free - called on __fput()
* @file: pointer to file structure being freed
*
- * Flag files that changed, based on i_version;
- * and decrement the i_readcount.
+ * Flag files that changed, based on i_version
*/
void ima_file_free(struct file *file)
{
@@ -318,6 +308,7 @@ int ima_file_check(struct file *file, int mask)
{
int rc;
+ ima_rdwr_violation_check(file);
rc = process_measurement(file, file->f_dentry->d_name.name,
mask & (MAY_READ | MAY_WRITE | MAY_EXEC),
FILE_CHECK);
--
1.7.2.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v1.2 4/5] IMA: remove IMA imbalance checking
2010-11-02 17:08 [PATCH v1.2 0/5] IMA: making i_readcount a first class inode citizen Mimi Zohar
` (2 preceding siblings ...)
2010-11-02 17:08 ` [PATCH v1.2 3/5] IMA: maintain i_readcount in the VFS layer Mimi Zohar
@ 2010-11-02 17:08 ` Mimi Zohar
2010-11-02 17:08 ` [PATCH v1.2 5/5] IMA: making i_readcount a first class inode citizen Mimi Zohar
2010-11-02 18:03 ` [PATCH v1.2 0/5] " Eric Paris
5 siblings, 0 replies; 9+ messages in thread
From: Mimi Zohar @ 2010-11-02 17:08 UTC (permalink / raw)
To: linux-kernel
Cc: Mimi Zohar, linux-security-module, linux-fsdevel, jmorris, akpm,
torvalds, eparis, viro, Dave Chinner, Mimi Zohar
Now that i_readcount is maintained by the VFS layer, remove the
imbalance checking in IMA. Cleans up the IMA code nicely.
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
---
security/integrity/ima/ima_iint.c | 4 --
security/integrity/ima/ima_main.c | 104 ++-----------------------------------
2 files changed, 4 insertions(+), 104 deletions(-)
diff --git a/security/integrity/ima/ima_iint.c b/security/integrity/ima/ima_iint.c
index 68efe3b..4ae7304 100644
--- a/security/integrity/ima/ima_iint.c
+++ b/security/integrity/ima/ima_iint.c
@@ -137,10 +137,6 @@ void ima_inode_free(struct inode *inode)
{
struct ima_iint_cache *iint;
- if (atomic_read(&inode->i_readcount))
- printk(KERN_INFO "%s: readcount: %u\n", __func__,
- atomic_read(&inode->i_readcount));
-
if (!IS_IMA(inode))
return;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index a0626bc..2df9021 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -36,55 +36,6 @@ static int __init hash_setup(char *str)
}
__setup("ima_hash=", hash_setup);
-struct ima_imbalance {
- struct hlist_node node;
- unsigned long fsmagic;
-};
-
-/*
- * ima_limit_imbalance - emit one imbalance message per filesystem type
- *
- * Maintain list of filesystem types that do not measure files properly.
- * Return false if unknown, true if known.
- */
-static bool ima_limit_imbalance(struct file *file)
-{
- static DEFINE_SPINLOCK(ima_imbalance_lock);
- static HLIST_HEAD(ima_imbalance_list);
-
- struct super_block *sb = file->f_dentry->d_sb;
- struct ima_imbalance *entry;
- struct hlist_node *node;
- bool found = false;
-
- rcu_read_lock();
- hlist_for_each_entry_rcu(entry, node, &ima_imbalance_list, node) {
- if (entry->fsmagic == sb->s_magic) {
- found = true;
- break;
- }
- }
- rcu_read_unlock();
- if (found)
- goto out;
-
- entry = kmalloc(sizeof(*entry), GFP_NOFS);
- if (!entry)
- goto out;
- entry->fsmagic = sb->s_magic;
- spin_lock(&ima_imbalance_lock);
- /*
- * we could have raced and something else might have added this fs
- * to the list, but we don't really care
- */
- hlist_add_head_rcu(&entry->node, &ima_imbalance_list);
- spin_unlock(&ima_imbalance_lock);
- printk(KERN_INFO "IMA: unmeasured files on fsmagic: %lX\n",
- entry->fsmagic);
-out:
- return found;
-}
-
/*
* ima_rdwr_violation_check
*
@@ -131,65 +82,20 @@ out:
"open_writers");
}
-/*
- * Decrement ima counts
- */
-static void ima_dec_counts(struct inode *inode, struct file *file)
-{
- mode_t mode = file->f_mode;
-
- assert_spin_locked(&inode->i_lock);
-
- if ((mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ) {
- if (unlikely(atomic_read(&inode->i_readcount) == 0)) {
- if (!ima_limit_imbalance(file)) {
- printk(KERN_INFO "%s: open/free imbalance (r:%u)\n",
- __func__,
- atomic_read(&inode->i_readcount));
- dump_stack();
- }
- return;
- }
- }
-}
-
static void ima_check_last_writer(struct ima_iint_cache *iint,
struct inode *inode,
struct file *file)
{
mode_t mode = file->f_mode;
- BUG_ON(!mutex_is_locked(&iint->mutex));
- assert_spin_locked(&inode->i_lock);
-
+ mutex_lock(&iint->mutex);
if (mode & FMODE_WRITE &&
atomic_read(&inode->i_writecount) == 1 &&
iint->version != inode->i_version)
iint->flags &= ~IMA_MEASURED;
-}
-
-static void ima_file_free_iint(struct ima_iint_cache *iint, struct inode *inode,
- struct file *file)
-{
- mutex_lock(&iint->mutex);
- spin_lock(&inode->i_lock);
-
- ima_dec_counts(inode, file);
- ima_check_last_writer(iint, inode, file);
-
- spin_unlock(&inode->i_lock);
mutex_unlock(&iint->mutex);
}
-static void ima_file_free_noiint(struct inode *inode, struct file *file)
-{
- spin_lock(&inode->i_lock);
-
- ima_dec_counts(inode, file);
-
- spin_unlock(&inode->i_lock);
-}
-
/**
* ima_file_free - called on __fput()
* @file: pointer to file structure being freed
@@ -205,12 +111,10 @@ void ima_file_free(struct file *file)
return;
iint = ima_iint_find(inode);
+ if (!iint)
+ return;
- if (iint)
- ima_file_free_iint(iint, inode, file);
- else
- ima_file_free_noiint(inode, file);
-
+ ima_check_last_writer(iint, inode, file);
}
static int process_measurement(struct file *file, const unsigned char *filename,
--
1.7.2.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v1.2 5/5] IMA: making i_readcount a first class inode citizen
2010-11-02 17:08 [PATCH v1.2 0/5] IMA: making i_readcount a first class inode citizen Mimi Zohar
` (3 preceding siblings ...)
2010-11-02 17:08 ` [PATCH v1.2 4/5] IMA: remove IMA imbalance checking Mimi Zohar
@ 2010-11-02 17:08 ` Mimi Zohar
2010-11-02 18:03 ` [PATCH v1.2 0/5] " Eric Paris
5 siblings, 0 replies; 9+ messages in thread
From: Mimi Zohar @ 2010-11-02 17:08 UTC (permalink / raw)
To: linux-kernel
Cc: Mimi Zohar, linux-security-module, linux-fsdevel, jmorris, akpm,
torvalds, eparis, viro, Dave Chinner, Mimi Zohar
Finally, remove the ifdef's around i_readcount, making it a full
inode citizen so that other subsystems, such as leases, could use it.
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
---
include/linux/fs.h | 15 ++-------------
1 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 6b7e2fb..1024da1 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -786,9 +786,7 @@ struct inode {
unsigned int i_flags;
-#ifdef CONFIG_IMA
atomic_t i_readcount; /* struct files open RO */
-#endif
atomic_t i_writecount;
#ifdef CONFIG_SECURITY
void *i_security;
@@ -2176,7 +2174,7 @@ static inline void allow_write_access(struct file *file)
if (file)
atomic_inc(&file->f_path.dentry->d_inode->i_writecount);
}
-#ifdef CONFIG_IMA
+
static inline void i_readcount_dec(struct inode *inode)
{
BUG_ON(!atomic_read(&inode->i_readcount));
@@ -2186,16 +2184,7 @@ static inline void i_readcount_inc(struct inode *inode)
{
atomic_inc(&inode->i_readcount);
}
-#else
-static inline void i_readcount_dec(struct inode *inode)
-{
- return;
-}
-static inline void i_readcount_inc(struct inode *inode)
-{
- return;
-}
-#endif
+
extern int do_pipe_flags(int *, int);
extern struct file *create_read_pipe(struct file *f, int flags);
extern struct file *create_write_pipe(int flags);
--
1.7.2.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v1.2 0/5] IMA: making i_readcount a first class inode citizen
2010-11-02 17:08 [PATCH v1.2 0/5] IMA: making i_readcount a first class inode citizen Mimi Zohar
` (4 preceding siblings ...)
2010-11-02 17:08 ` [PATCH v1.2 5/5] IMA: making i_readcount a first class inode citizen Mimi Zohar
@ 2010-11-02 18:03 ` Eric Paris
2010-11-05 16:35 ` J. Bruce Fields
5 siblings, 1 reply; 9+ messages in thread
From: Eric Paris @ 2010-11-02 18:03 UTC (permalink / raw)
To: Mimi Zohar
Cc: linux-kernel, linux-security-module, linux-fsdevel, jmorris, akpm,
torvalds, viro, Dave Chinner
On Tue, 2010-11-02 at 13:08 -0400, Mimi Zohar wrote:
> Changelog:
> - renamed iget/iput_readcount to i_readcount_inc/dec (Dave Chinner's suggestion)
> - finished cleaning up the locking, so that: (based on Eric's comment.)
> - i_lock is not required
> - i_mutex is taken when making measurement decisions to prevent
> file metadata(eg. permissions, xattr) changing from under it.
> - iint->mutex is taken when accessing/modifying the iint structure.
> - Based on the previous posting discussion, i_readcount is now defined as
> atomic.
>
> This patchset separates the incrementing/decrementing of the i_readcount,
> in the VFS layer, from other IMA functionality, by replacing the current
> ima_counts_get() call with i_readcount_inc(). Its unclear whether this
> call to increment i_readcount should be made earlier, like i_writecount.
>
> The patch ordering is a bit redundant in order to leave removing the ifdef
> around i_readcount until the last patch. The first four patches: redefines
> i_readcount as atomic, defines i_readcount_inc/dec(), moves the IMA
> functionality in ima_counts_get() to ima_file_check(), and removes the
> IMA imbalance code, simplifying IMA. The last patch removes the ifdef
> around i_readcount, making i_readcount into a "first class inode citizen".
>
> The generic_setlease code could then take advantage of i_readcount.
>
> Mimi
> Mimi Zohar (5):
> IMA: convert i_readcount to atomic
> IMA: define readcount functions
> IMA: maintain i_readcount in the VFS layer
> IMA: remove IMA imbalance checking
> IMA: making i_readcount a first class inode citizen
Ack on patches 1-4. Patch 5 I'm not so sure about. As long as IMA is
the only user I don't think we really want to unconditionally compile
i_readcount in. I say we either leave it wrapped in CONFIG_IMA until
there is a second user. Then we either unconditionally compile it in,
or we wrap it in something that either user can select in the Kconfig.
I'll let others voice their opinion.
-Eric
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1.2 0/5] IMA: making i_readcount a first class inode citizen
2010-11-02 18:03 ` [PATCH v1.2 0/5] " Eric Paris
@ 2010-11-05 16:35 ` J. Bruce Fields
0 siblings, 0 replies; 9+ messages in thread
From: J. Bruce Fields @ 2010-11-05 16:35 UTC (permalink / raw)
To: Eric Paris
Cc: Mimi Zohar, linux-kernel, linux-security-module, linux-fsdevel,
jmorris, akpm, torvalds, viro, Dave Chinner
On Tue, Nov 02, 2010 at 02:03:29PM -0400, Eric Paris wrote:
> On Tue, 2010-11-02 at 13:08 -0400, Mimi Zohar wrote:
> > Changelog:
> > - renamed iget/iput_readcount to i_readcount_inc/dec (Dave Chinner's suggestion)
> > - finished cleaning up the locking, so that: (based on Eric's comment.)
> > - i_lock is not required
> > - i_mutex is taken when making measurement decisions to prevent
> > file metadata(eg. permissions, xattr) changing from under it.
> > - iint->mutex is taken when accessing/modifying the iint structure.
> > - Based on the previous posting discussion, i_readcount is now defined as
> > atomic.
> >
> > This patchset separates the incrementing/decrementing of the i_readcount,
> > in the VFS layer, from other IMA functionality, by replacing the current
> > ima_counts_get() call with i_readcount_inc(). Its unclear whether this
> > call to increment i_readcount should be made earlier, like i_writecount.
> >
> > The patch ordering is a bit redundant in order to leave removing the ifdef
> > around i_readcount until the last patch. The first four patches: redefines
> > i_readcount as atomic, defines i_readcount_inc/dec(), moves the IMA
> > functionality in ima_counts_get() to ima_file_check(), and removes the
> > IMA imbalance code, simplifying IMA. The last patch removes the ifdef
> > around i_readcount, making i_readcount into a "first class inode citizen".
> >
> > The generic_setlease code could then take advantage of i_readcount.
> >
> > Mimi
> > Mimi Zohar (5):
> > IMA: convert i_readcount to atomic
> > IMA: define readcount functions
> > IMA: maintain i_readcount in the VFS layer
> > IMA: remove IMA imbalance checking
> > IMA: making i_readcount a first class inode citizen
>
>
> Ack on patches 1-4. Patch 5 I'm not so sure about. As long as IMA is
> the only user I don't think we really want to unconditionally compile
> i_readcount in. I say we either leave it wrapped in CONFIG_IMA until
> there is a second user. Then we either unconditionally compile it in,
> or we wrap it in something that either user can select in the Kconfig.
>
> I'll let others voice their opinion.
I think it solves a real problem for the lease code.
--b.
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v1.2 5/5] IMA: making i_readcount a first class inode citizen
2010-11-18 23:02 [PATCH v1.2 0/5] IMA: making i_readcount a first class inode citizen (reposting) Mimi Zohar
@ 2010-11-18 23:03 ` Mimi Zohar
0 siblings, 0 replies; 9+ messages in thread
From: Mimi Zohar @ 2010-11-18 23:03 UTC (permalink / raw)
To: linux-kernel
Cc: Mimi Zohar, linux-security-module, linux-fsdevel, jmorris, akpm,
torvalds, eparis, viro, Dave Chinner, J. Bruce Fields,
David Safford, Mimi Zohar
Finally, remove the ifdef's around i_readcount, making it a full
inode citizen so that other subsystems, such as leases, could use it.
Signed-off-by: Mimi Zohar <zohar@us.ibm.com>
---
include/linux/fs.h | 15 ++-------------
1 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 6b7e2fb..1024da1 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -786,9 +786,7 @@ struct inode {
unsigned int i_flags;
-#ifdef CONFIG_IMA
atomic_t i_readcount; /* struct files open RO */
-#endif
atomic_t i_writecount;
#ifdef CONFIG_SECURITY
void *i_security;
@@ -2176,7 +2174,7 @@ static inline void allow_write_access(struct file *file)
if (file)
atomic_inc(&file->f_path.dentry->d_inode->i_writecount);
}
-#ifdef CONFIG_IMA
+
static inline void i_readcount_dec(struct inode *inode)
{
BUG_ON(!atomic_read(&inode->i_readcount));
@@ -2186,16 +2184,7 @@ static inline void i_readcount_inc(struct inode *inode)
{
atomic_inc(&inode->i_readcount);
}
-#else
-static inline void i_readcount_dec(struct inode *inode)
-{
- return;
-}
-static inline void i_readcount_inc(struct inode *inode)
-{
- return;
-}
-#endif
+
extern int do_pipe_flags(int *, int);
extern struct file *create_read_pipe(struct file *f, int flags);
extern struct file *create_write_pipe(int flags);
--
1.7.2.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-11-18 23:03 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-02 17:08 [PATCH v1.2 0/5] IMA: making i_readcount a first class inode citizen Mimi Zohar
2010-11-02 17:08 ` [PATCH v1.2 1/5] IMA: convert i_readcount to atomic Mimi Zohar
2010-11-02 17:08 ` [PATCH v1.2 2/5] IMA: define readcount functions Mimi Zohar
2010-11-02 17:08 ` [PATCH v1.2 3/5] IMA: maintain i_readcount in the VFS layer Mimi Zohar
2010-11-02 17:08 ` [PATCH v1.2 4/5] IMA: remove IMA imbalance checking Mimi Zohar
2010-11-02 17:08 ` [PATCH v1.2 5/5] IMA: making i_readcount a first class inode citizen Mimi Zohar
2010-11-02 18:03 ` [PATCH v1.2 0/5] " Eric Paris
2010-11-05 16:35 ` J. Bruce Fields
-- strict thread matches above, loose matches on Subject: below --
2010-11-18 23:02 [PATCH v1.2 0/5] IMA: making i_readcount a first class inode citizen (reposting) Mimi Zohar
2010-11-18 23:03 ` [PATCH v1.2 5/5] IMA: making i_readcount a first class inode citizen Mimi Zohar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).