[PATCH] xfs_metadump: properly handle obfuscation of all remote attribute blocks

[PATCH] xfs_metadump: properly handle obfuscation of all remote attribute blocks

[Eric Sandeen]

add_remote_vals assumes that it can subtract blocksize
from each block that it processes, but with CRCs, there
is a header on each block, so the assumption that each
block consumes $BLOCKSIZE of the value length is incorrect.

This causes us to stop adding remote blocks too soon, and
the missed blocks do not get obfuscated.

Fix this by accounting for the header size as appropriate,
depending on whether or not we have a CRC filesystem.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

diff --git a/db/metadump.c b/db/metadump.c
index 96641e0..c8abfeb 100644
--- a/db/metadump.c
+++ b/db/metadump.c
@@ -1602,14 +1602,20 @@ static struct attr_data_s {
 
 static inline void
 add_remote_vals(
+	struct xfs_mount	*mp,
 	xfs_dablk_t 		blockidx,
 	int			length)
 {
+	int			hdrsize = 0;
+
+	if (xfs_sb_version_hascrc(&mp->m_sb))
+		hdrsize = sizeof(struct xfs_attr3_rmt_hdr);
+
 	while (length > 0 && attr_data.remote_val_count < MAX_REMOTE_VALS) {
 		attr_data.remote_vals[attr_data.remote_val_count] = blockidx;
 		attr_data.remote_val_count++;
 		blockidx++;
-		length -= mp->m_sb.sb_blocksize;
+		length -= (mp->m_sb.sb_blocksize - hdrsize);
 	}
 
 	if (attr_data.remote_val_count >= MAX_REMOTE_VALS) {
@@ -1716,7 +1722,8 @@ process_attr_block(
 			if (obfuscate) {
 				generate_obfuscated_name(0, remote->namelen,
 							 &remote->name[0]);
-				add_remote_vals(be32_to_cpu(remote->valueblk),
+				add_remote_vals(mp,
+						be32_to_cpu(remote->valueblk),
 						be32_to_cpu(remote->valuelen));
 			}
 			/* zero from end of name[] to next name start */

Re: [PATCH] xfs_metadump: properly handle obfuscation of all remote attribute blocks

[Brian Foster]

On Wed, Jul 26, 2017 at 03:35:37PM -0500, Eric Sandeen wrote:
> add_remote_vals assumes that it can subtract blocksize
> from each block that it processes, but with CRCs, there
> is a header on each block, so the assumption that each
> block consumes $BLOCKSIZE of the value length is incorrect.
> 
> This causes us to stop adding remote blocks too soon, and
> the missed blocks do not get obfuscated.
> 
> Fix this by accounting for the header size as appropriate,
> depending on whether or not we have a CRC filesystem.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/db/metadump.c b/db/metadump.c
> index 96641e0..c8abfeb 100644
> --- a/db/metadump.c
> +++ b/db/metadump.c
> @@ -1602,14 +1602,20 @@ static struct attr_data_s {
>  
>  static inline void
>  add_remote_vals(
> +	struct xfs_mount	*mp,
>  	xfs_dablk_t 		blockidx,
>  	int			length)
>  {
> +	int			hdrsize = 0;
> +
> +	if (xfs_sb_version_hascrc(&mp->m_sb))
> +		hdrsize = sizeof(struct xfs_attr3_rmt_hdr);
> +
>  	while (length > 0 && attr_data.remote_val_count < MAX_REMOTE_VALS) {
>  		attr_data.remote_vals[attr_data.remote_val_count] = blockidx;
>  		attr_data.remote_val_count++;
>  		blockidx++;
> -		length -= mp->m_sb.sb_blocksize;
> +		length -= (mp->m_sb.sb_blocksize - hdrsize);

XFS_ATTR3_RMT_BUF_SPACE()?

Brian

>  	}
>  
>  	if (attr_data.remote_val_count >= MAX_REMOTE_VALS) {
> @@ -1716,7 +1722,8 @@ process_attr_block(
>  			if (obfuscate) {
>  				generate_obfuscated_name(0, remote->namelen,
>  							 &remote->name[0]);
> -				add_remote_vals(be32_to_cpu(remote->valueblk),
> +				add_remote_vals(mp,
> +						be32_to_cpu(remote->valueblk),
>  						be32_to_cpu(remote->valuelen));
>  			}
>  			/* zero from end of name[] to next name start */
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] xfs_metadump: properly handle obfuscation of all remote attribute blocks

[Eric Sandeen]

On 7/28/17 7:52 AM, Brian Foster wrote:
> On Wed, Jul 26, 2017 at 03:35:37PM -0500, Eric Sandeen wrote:
>> add_remote_vals assumes that it can subtract blocksize
>> from each block that it processes, but with CRCs, there
>> is a header on each block, so the assumption that each
>> block consumes $BLOCKSIZE of the value length is incorrect.
>>
>> This causes us to stop adding remote blocks too soon, and
>> the missed blocks do not get obfuscated.
>>
>> Fix this by accounting for the header size as appropriate,
>> depending on whether or not we have a CRC filesystem.
>>
>> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
>> ---
>>
>> diff --git a/db/metadump.c b/db/metadump.c
>> index 96641e0..c8abfeb 100644
>> --- a/db/metadump.c
>> +++ b/db/metadump.c
>> @@ -1602,14 +1602,20 @@ static struct attr_data_s {
>>  
>>  static inline void
>>  add_remote_vals(
>> +	struct xfs_mount	*mp,
>>  	xfs_dablk_t 		blockidx,
>>  	int			length)
>>  {
>> +	int			hdrsize = 0;
>> +
>> +	if (xfs_sb_version_hascrc(&mp->m_sb))
>> +		hdrsize = sizeof(struct xfs_attr3_rmt_hdr);
>> +
>>  	while (length > 0 && attr_data.remote_val_count < MAX_REMOTE_VALS) {
>>  		attr_data.remote_vals[attr_data.remote_val_count] = blockidx;
>>  		attr_data.remote_val_count++;
>>  		blockidx++;
>> -		length -= mp->m_sb.sb_blocksize;
>> +		length -= (mp->m_sb.sb_blocksize - hdrsize);
> 
> XFS_ATTR3_RMT_BUF_SPACE()?

Oh, yeah, why didn't I use that!

Thanks,
-Eric

> Brian

[PATCH V2] xfs_metadump: properly handle obfuscation of all remote attribute blocks

[Eric Sandeen]

add_remote_vals assumes that it can subtract blocksize
from each block that it processes, but with CRCs, there
is a header on each block, so the assumption that each
block consumes $BLOCKSIZE of the value length is incorrect.

This causes us to stop adding remote blocks too soon, and
the missed blocks do not get obfuscated.

Fix this by accounting for the header size as appropriate,
depending on whether or not we have a CRC filesystem.

Signed-off-by: Eric Sandeen <sandeen@redhat.com>
---

V2: Use helper macro, don't send mp as arg (it's a global (!))

diff --git a/db/metadump.c b/db/metadump.c
index 96641e0..f60bd32 100644
--- a/db/metadump.c
+++ b/db/metadump.c
@@ -1609,7 +1609,7 @@ add_remote_vals(
 		attr_data.remote_vals[attr_data.remote_val_count] = blockidx;
 		attr_data.remote_val_count++;
 		blockidx++;
-		length -= mp->m_sb.sb_blocksize;
+		length -= XFS_ATTR3_RMT_BUF_SPACE(mp, mp->m_sb.sb_blocksize);
 	}
 
 	if (attr_data.remote_val_count >= MAX_REMOTE_VALS) {

Re: [PATCH V2] xfs_metadump: properly handle obfuscation of all remote attribute blocks

[Brian Foster]

On Tue, Aug 01, 2017 at 11:39:19AM -0500, Eric Sandeen wrote:
> add_remote_vals assumes that it can subtract blocksize
> from each block that it processes, but with CRCs, there
> is a header on each block, so the assumption that each
> block consumes $BLOCKSIZE of the value length is incorrect.
> 
> This causes us to stop adding remote blocks too soon, and
> the missed blocks do not get obfuscated.
> 
> Fix this by accounting for the header size as appropriate,
> depending on whether or not we have a CRC filesystem.
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 

Reviewed-by: Brian Foster <bfoster@redhat.com>

> V2: Use helper macro, don't send mp as arg (it's a global (!))
> 
> diff --git a/db/metadump.c b/db/metadump.c
> index 96641e0..f60bd32 100644
> --- a/db/metadump.c
> +++ b/db/metadump.c
> @@ -1609,7 +1609,7 @@ add_remote_vals(
>  		attr_data.remote_vals[attr_data.remote_val_count] = blockidx;
>  		attr_data.remote_val_count++;
>  		blockidx++;
> -		length -= mp->m_sb.sb_blocksize;
> +		length -= XFS_ATTR3_RMT_BUF_SPACE(mp, mp->m_sb.sb_blocksize);
>  	}
>  
>  	if (attr_data.remote_val_count >= MAX_REMOTE_VALS) {
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html