diff options
author | Dmitry Kasatkin <d.kasatkin@samsung.com> | 2014-11-20 09:31:01 -0500 |
---|---|---|
committer | Mimi Zohar <zohar@linux.vnet.ibm.com> | 2015-05-21 13:28:47 -0400 |
commit | 7c51bb00c40e5608fb2cdac5230f51aeb56a28df (patch) | |
tree | ea228d622a60f43812b50ce7e27b2abbb466f04b | |
parent | 5101a1850bb7ccbf107929dee9af0cd2f400940f (diff) |
evm: fix potential race when removing xattrs
EVM needs to be atomically updated when removing xattrs.
Otherwise concurrent EVM verification may fail in between.
This patch fixes by moving i_mutex unlocking after calling
EVM hook. fsnotify_xattr() is also now called while locked
the same way as it is done in __vfs_setxattr_noperm.
Changelog:
- remove unused 'inode' variable.
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
-rw-r--r-- | fs/xattr.c | 10 | ||||
-rw-r--r-- | security/integrity/evm/evm_main.c | 7 |
2 files changed, 8 insertions, 9 deletions
diff --git a/fs/xattr.c b/fs/xattr.c index 4ef698549e31..072fee1258dd 100644 --- a/fs/xattr.c +++ b/fs/xattr.c | |||
@@ -298,18 +298,18 @@ vfs_removexattr(struct dentry *dentry, const char *name) | |||
298 | 298 | ||
299 | mutex_lock(&inode->i_mutex); | 299 | mutex_lock(&inode->i_mutex); |
300 | error = security_inode_removexattr(dentry, name); | 300 | error = security_inode_removexattr(dentry, name); |
301 | if (error) { | 301 | if (error) |
302 | mutex_unlock(&inode->i_mutex); | 302 | goto out; |
303 | return error; | ||
304 | } | ||
305 | 303 | ||
306 | error = inode->i_op->removexattr(dentry, name); | 304 | error = inode->i_op->removexattr(dentry, name); |
307 | mutex_unlock(&inode->i_mutex); | ||
308 | 305 | ||
309 | if (!error) { | 306 | if (!error) { |
310 | fsnotify_xattr(dentry); | 307 | fsnotify_xattr(dentry); |
311 | evm_inode_post_removexattr(dentry, name); | 308 | evm_inode_post_removexattr(dentry, name); |
312 | } | 309 | } |
310 | |||
311 | out: | ||
312 | mutex_unlock(&inode->i_mutex); | ||
313 | return error; | 313 | return error; |
314 | } | 314 | } |
315 | EXPORT_SYMBOL_GPL(vfs_removexattr); | 315 | EXPORT_SYMBOL_GPL(vfs_removexattr); |
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 582091498819..1334e02ae8f4 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c | |||
@@ -387,17 +387,16 @@ void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name, | |||
387 | * @xattr_name: pointer to the affected extended attribute name | 387 | * @xattr_name: pointer to the affected extended attribute name |
388 | * | 388 | * |
389 | * Update the HMAC stored in 'security.evm' to reflect removal of the xattr. | 389 | * Update the HMAC stored in 'security.evm' to reflect removal of the xattr. |
390 | * | ||
391 | * No need to take the i_mutex lock here, as this function is called from | ||
392 | * vfs_removexattr() which takes the i_mutex. | ||
390 | */ | 393 | */ |
391 | void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name) | 394 | void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name) |
392 | { | 395 | { |
393 | struct inode *inode = d_backing_inode(dentry); | ||
394 | |||
395 | if (!evm_initialized || !evm_protected_xattr(xattr_name)) | 396 | if (!evm_initialized || !evm_protected_xattr(xattr_name)) |
396 | return; | 397 | return; |
397 | 398 | ||
398 | mutex_lock(&inode->i_mutex); | ||
399 | evm_update_evmxattr(dentry, xattr_name, NULL, 0); | 399 | evm_update_evmxattr(dentry, xattr_name, NULL, 0); |
400 | mutex_unlock(&inode->i_mutex); | ||
401 | } | 400 | } |
402 | 401 | ||
403 | /** | 402 | /** |