Re: [Devel] [PATCH RH7 v2] pfcache: hide trusted.pfcache from listxattr

2016-10-04 Thread Maxim Patlasov 

The patch itself looks fine, so:

Reviewed-by: Maxim Patlasov 


As for patch description, its last paragraph looks misleading. We seem 
to be safe only until RHEL reuses name_index == 9 (what was 
EXT4_XATTR_INDEX_TRUSTED_CSUM in the v1 of the patch). I think it would 
be more honest to state it clearly: we do not support pfcache-ed ploop 
images from rh7-3.10.0-327.28.2.vz7.17.10.


Thanks,
Maxim

On 09/27/2016 08:31 AM, Pavel Tikhomirov wrote:

Need it to be able to rsync xattrs for encripted containers which
have pfcache_csum disabled on superblock.

When there is no PFCACHE_CSUM on superblock or we are not
capable(CAP_SYS_ADMIN), we do not allow get/set trusted.pfcache.
So hide trusted.pfcache from list also in thouse two cases.

Tested that: list/get xattr "trusted.pfcache" is OK on file
setxattred on vz7.17.11 kernel, xattr entry on which had wrong
e_name_index (reverted EXT4_XATTR_INDEX_TRUSTED_CSUM), works as
there is no such entry at all. As in ext4_xattr_list_entries
-> ext4_xattr_handler where is special check for it.

v2: do checks in ext4_xattr_trusted_list which is used for
listing trusted.xxx xattrs

https://jira.sw.ru/browse/PSBM-52180
Signed-off-by: Pavel Tikhomirov 
---
  fs/ext4/xattr_trusted.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/fs/ext4/xattr_trusted.c b/fs/ext4/xattr_trusted.c
index 49dd83f..131b6b8 100644
--- a/fs/ext4/xattr_trusted.c
+++ b/fs/ext4/xattr_trusted.c
@@ -19,6 +19,11 @@ ext4_xattr_trusted_list(struct dentry *dentry, char *list, 
size_t list_size,
const size_t prefix_len = XATTR_TRUSTED_PREFIX_LEN;
const size_t total_len = prefix_len + name_len + 1;
  
+	if (!strcmp(name, EXT4_DATA_CSUM_NAME) &&

+   (!capable(CAP_SYS_ADMIN) ||
+!test_opt2(dentry->d_inode->i_sb, PFCACHE_CSUM)))
+   return 0;
+
if (!ve_capable(CAP_SYS_ADMIN))
return 0;
  


___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH RH7 v2] pfcache: hide trusted.pfcache from listxattr

2016-09-27 Thread Pavel Tikhomirov 
Need it to be able to rsync xattrs for encripted containers which
have pfcache_csum disabled on superblock.

When there is no PFCACHE_CSUM on superblock or we are not
capable(CAP_SYS_ADMIN), we do not allow get/set trusted.pfcache.
So hide trusted.pfcache from list also in thouse two cases.

Tested that: list/get xattr "trusted.pfcache" is OK on file
setxattred on vz7.17.11 kernel, xattr entry on which had wrong
e_name_index (reverted EXT4_XATTR_INDEX_TRUSTED_CSUM), works as
there is no such entry at all. As in ext4_xattr_list_entries
-> ext4_xattr_handler where is special check for it.

v2: do checks in ext4_xattr_trusted_list which is used for
listing trusted.xxx xattrs

https://jira.sw.ru/browse/PSBM-52180
Signed-off-by: Pavel Tikhomirov 
---
 fs/ext4/xattr_trusted.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/fs/ext4/xattr_trusted.c b/fs/ext4/xattr_trusted.c
index 49dd83f..131b6b8 100644
--- a/fs/ext4/xattr_trusted.c
+++ b/fs/ext4/xattr_trusted.c
@@ -19,6 +19,11 @@ ext4_xattr_trusted_list(struct dentry *dentry, char *list, 
size_t list_size,
const size_t prefix_len = XATTR_TRUSTED_PREFIX_LEN;
const size_t total_len = prefix_len + name_len + 1;
 
+   if (!strcmp(name, EXT4_DATA_CSUM_NAME) &&
+   (!capable(CAP_SYS_ADMIN) ||
+!test_opt2(dentry->d_inode->i_sb, PFCACHE_CSUM)))
+   return 0;
+
if (!ve_capable(CAP_SYS_ADMIN))
return 0;
 
-- 
2.5.5

___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH RH7 v2] pfcache: hide trusted.pfcache from listxattr

2016-09-23 Thread Pavel Tikhomirov 
In SyS_listxattr -> listxattr -> ext4_listxattr ->
ext4_xattr_list_entries we choose list handler for
each ext4_xattr_entry based on e_name_index, and as
for trusted.pfcache index is EXT4_XATTR_INDEX_TRUSTED,
we chouse ext4_xattr_trusted_list which always prints
xattr to the list.

When there is no PFCACHE_CSUM on superblock we do
not allow get/set trusted.pfcache, so hide it from
list too. To do so - change it's e_name_index to new
EXT4_XATTR_INDEX_TRUSTED_CSUM and thus use
ext4_xattr_trusted_csum_list. Check PFCACHE_CSUM
in it, and if no - do not print xattr to list.

Also do not allow list without global CAP_SYS_ADMIN,
as we do not allow set/get without it.

Test:

TEST_FILE=/vz/root/101/testfile
rm -f $TEST_FILE
touch $TEST_FILE
TEST_SHA1=`sha1sum $TEST_FILE | awk '{print $1}'`
setfattr -n trusted.pfcache -v $TEST_SHA1 $TEST_FILE
setfattr -n trusted.test -v test $TEST_FILE
getfattr -d -m trusted $TEST_FILE

  trusted.pfcache="da39a3ee5e6b4b0d3255bfef95601890afd80709"
  trusted.test="test"

mount -oremount,nopfcache_csum /vz/root/101
getfattr -d -m trusted $TEST_FILE

  trusted.test="test"

v2: hide only if nopfcache_csum

https://jira.sw.ru/browse/PSBM-52180
Signed-off-by: Pavel Tikhomirov 
---
 fs/ext4/pfcache.c | 45 ++---
 fs/ext4/xattr.c   |  1 +
 fs/ext4/xattr.h   |  1 +
 3 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/fs/ext4/pfcache.c b/fs/ext4/pfcache.c
index ff2300b..52bd886 100644
--- a/fs/ext4/pfcache.c
+++ b/fs/ext4/pfcache.c
@@ -441,8 +441,8 @@ int ext4_load_data_csum(struct inode *inode)
 {
int ret;
 
-   ret = ext4_xattr_get(inode, EXT4_XATTR_INDEX_TRUSTED,
-   EXT4_DATA_CSUM_NAME, EXT4_I(inode)->i_data_csum,
+   ret = ext4_xattr_get(inode, EXT4_XATTR_INDEX_TRUSTED_CSUM,
+   "", EXT4_I(inode)->i_data_csum,
EXT4_DATA_CSUM_SIZE);
if (ret < 0)
return ret;
@@ -482,8 +482,8 @@ static int ext4_save_data_csum(struct inode *inode, u8 
*csum)
if (ret)
return ret;
 
-   return ext4_xattr_set(inode, EXT4_XATTR_INDEX_TRUSTED,
-   EXT4_DATA_CSUM_NAME, EXT4_I(inode)->i_data_csum,
+   return ext4_xattr_set(inode, EXT4_XATTR_INDEX_TRUSTED_CSUM,
+   "", EXT4_I(inode)->i_data_csum,
EXT4_DATA_CSUM_SIZE, 0);
 }
 
@@ -492,8 +492,8 @@ void ext4_load_dir_csum(struct inode *inode)
char value[EXT4_DIR_CSUM_VALUE_LEN];
int ret;
 
-   ret = ext4_xattr_get(inode, EXT4_XATTR_INDEX_TRUSTED,
-EXT4_DATA_CSUM_NAME, value, sizeof(value));
+   ret = ext4_xattr_get(inode, EXT4_XATTR_INDEX_TRUSTED_CSUM,
+"", value, sizeof(value));
if (ret == EXT4_DIR_CSUM_VALUE_LEN &&
!strncmp(value, EXT4_DIR_CSUM_VALUE, sizeof(value)))
ext4_set_inode_state(inode, EXT4_STATE_PFCACHE_CSUM);
@@ -502,8 +502,8 @@ void ext4_load_dir_csum(struct inode *inode)
 void ext4_save_dir_csum(struct inode *inode)
 {
ext4_set_inode_state(inode, EXT4_STATE_PFCACHE_CSUM);
-   ext4_xattr_set(inode, EXT4_XATTR_INDEX_TRUSTED,
-   EXT4_DATA_CSUM_NAME,
+   ext4_xattr_set(inode, EXT4_XATTR_INDEX_TRUSTED_CSUM,
+   "",
EXT4_DIR_CSUM_VALUE,
EXT4_DIR_CSUM_VALUE_LEN, 0);
 }
@@ -516,8 +516,8 @@ void ext4_truncate_data_csum(struct inode *inode, loff_t 
pos)
 
if (EXT4_I(inode)->i_data_csum_end < 0) {
WARN_ON(journal_current_handle());
-   ext4_xattr_set(inode, EXT4_XATTR_INDEX_TRUSTED,
-   EXT4_DATA_CSUM_NAME, NULL, 0, 0);
+   ext4_xattr_set(inode, EXT4_XATTR_INDEX_TRUSTED_CSUM,
+   "", NULL, 0, 0);
ext4_close_pfcache(inode);
}
spin_lock(>i_lock);
@@ -658,8 +658,8 @@ static int ext4_xattr_trusted_csum_get(struct dentry 
*dentry, const char *name,
return -EPERM;
 
if (S_ISDIR(inode->i_mode))
-   return ext4_xattr_get(inode, EXT4_XATTR_INDEX_TRUSTED,
- EXT4_DATA_CSUM_NAME, buffer, size);
+   return ext4_xattr_get(inode, EXT4_XATTR_INDEX_TRUSTED_CSUM,
+ "", buffer, size);
 
if (!S_ISREG(inode->i_mode))
return -ENODATA;
@@ -717,8 +717,8 @@ static int ext4_xattr_trusted_csum_set(struct dentry 
*dentry, const char *name,
else
return -EINVAL;
 
-   return ext4_xattr_set(inode, EXT4_XATTR_INDEX_TRUSTED,
- EXT4_DATA_CSUM_NAME, value, size, flags);
+   return ext4_xattr_set(inode, EXT4_XATTR_INDEX_TRUSTED_CSUM,
+ "", value, size, flags);
}
 
if