On 12/13/2012 08:56 PM, Eric Sandeen wrote:
> On 4/9/12 11:34 PM, Eric Sandeen wrote:
>> xfs_metadump currently obfuscates entire symlinks without regard
>> to path components; this can lead to a corrupt image when restoring
>> a metadump containing extremely long symlinks:
>>
>> Phase 3 - for each AG...
>> - scan and clear agi unlinked lists...
>> - process known inodes and perform inode discovery...
>> - agno = 0
>> component of symlink in inode 145 too long
>> problem with symbolic link in inode 145
>> cleared inode 145
>> ... <more trail of woe>
>>
>> Fix this by consolidating symlink obfuscation into a new
>> function which obfuscates one path component at a time.
>>
>> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
>
> ping? :)
You know, these things take time.
What you have looks good to me, but I rewrote it, below. :)
Even if you don't change anything...
Reviewed-by: Alex Elder <elder@xxxxxxxxxxx>
>> ---
>>
>> Yes, I need to do an xfstest for this. metadumping a filesystem
>> after fsstressing it works well.
>>
>> diff --git a/db/metadump.c b/db/metadump.c
>> index c5ffddb..9f15d9e 100644
>> --- a/db/metadump.c
>> +++ b/db/metadump.c
>> @@ -956,6 +956,40 @@ obfuscate_sf_dir(
>> }
>>
>> static void
>> +obfuscate_path_components(
>> + char *buf,
>> + __uint64_t len)
>> +{
>> + uchar_t *comp;
>> + xfs_dahash_t hash;
>> +
>> + comp = (uchar_t *)buf;
>> + while (comp < (uchar_t *)buf + len) {
>> + char *slash;
>> + int namelen;
>> +
Maybe instead of handling it later, you could check for
leading and duplicate slashes here:
while (*comp == '/')
comp++;
In fact, maybe it could be more like:
while (*comp == '/')
comp++;
slash = strchrnul((char *) comp, '/');
namlen = slash - comp;
if (!namelen)
return;
hash = libxfs_da_hashname(comp, namelen);
obfuscate_name(hash, namelen, comp);
comp += namelen + 1;
Note that these all assume the original buffer is NUL-terminated
(yours did too).
>> + /* find slash at end of this component */
>> + slash = strchr((char *)comp, '/');
>> + if (!slash) {
>> + /* last (or single) component */
>> + namelen = strlen((char *)comp);
>> + hash = libxfs_da_hashname(comp, namelen);
>> + obfuscate_name(hash, namelen, comp);
>> + break;
>> + }
>> + namelen = slash - (char *)comp;
>> + /* handle leading or consecutive slashes */
>> + if (!namelen) {
>> + comp++;
>> + continue;
>> + }
>> + hash = libxfs_da_hashname(comp, namelen);
>> + obfuscate_name(hash, namelen, comp);
>> + comp += namelen + 1;
>> + }
>> +}
>> +
>> +static void
>> obfuscate_sf_symlink(
>> xfs_dinode_t *dip)
>> {
>> @@ -971,8 +1005,7 @@ obfuscate_sf_symlink(
>> }
>>
>> buf = (char *)XFS_DFORK_DPTR(dip);
>> - while (len > 0)
>> - buf[--len] = random() % 127 + 1;
>> + obfuscate_path_components(buf, len);
>> }
>>
>> static void
>> @@ -1176,11 +1209,8 @@ obfuscate_symlink_blocks(
>> char *block,
>> xfs_dfilblks_t count)
>> {
>> - int i;
>> -
>> count <<= mp->m_sb.sb_blocklog;
>> - for (i = 0; i < count; i++)
>> - block[i] = random() % 127 + 1;
>> + obfuscate_path_components(block, count);
This is interesting. Is there any guarantee that the hashed
result here will match the hashed target file? I don't think
there is, necessarily (mostly if the target is subject to a
hash collision). It's a corner case though so it can most
likely be ignored.
>> }
>>
>> #define MAX_REMOTE_VALS 4095
>>
>> _______________________________________________
>> xfs mailing list
>> xfs@xxxxxxxxxxx
>> http://oss.sgi.com/mailman/listinfo/xfs
>>
>
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs
>
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
