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

Submitted by Pavel Tikhomirov on Sept. 27, 2016, 3:31 p.m.

Details

Message ID 1474990308-27655-1-git-send-email-ptikhomirov@virtuozzo.com
State New
Series "pfcache: hide trusted.pfcache from listxattr"
Headers show

Commit Message

Pavel Tikhomirov Sept. 27, 2016, 3:31 p.m.
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 <ptikhomirov@virtuozzo.com>
---
 fs/ext4/xattr_trusted.c | 5 +++++
 1 file changed, 5 insertions(+)

Patch hide | download patch | download mbox

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;
 

Comments

Maxim Patlasov Oct. 4, 2016, 7:56 p.m.
The patch itself looks fine, so:

Reviewed-by: Maxim Patlasov <mpatlasov@virtuozzo.com>


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 <ptikhomirov@virtuozzo.com>
> ---
>   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;
>