On 8/2/17 8:54 AM, Brian Foster wrote:
> On Tue, Aug 01, 2017 at 09:35:27PM -0500, Eric Sandeen wrote:
>> xfs_metadump attempts to zero out unused regions of metadata
>> blocks to prevent data leaks when sharing metadata images.
>>
>> However, Stefan Ring reported a significant number of leaked
>> strings when dumping his 1T filesystem. Based on a reduced
>> metadata set, I was able to identify "leaf" directories
>> (with XFS_DIR2_LEAF1_MAGIC magic) as the primary culprit;
>> the region between the end of the entries array and the start
>> of the bests array was not getting zeroed out. This patch
>> seems to remedy that problem.
>>
>> Reported-by: Stefan Ring <stefanrin@xxxxxxxxx>
>> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
>> ---
>>
>> I may have missed some handy macro to work out some of the
>> math below, if so I'd be perfectly happy to hear about it ;)
>>
>
> Looks good to me. A couple nits...
>
>> diff --git a/db/metadump.c b/db/metadump.c
>> index 96641e0..6d77d61 100644
>> --- a/db/metadump.c
>> +++ b/db/metadump.c
>> @@ -1459,6 +1459,37 @@ process_dir_data_block(
>> int wantmagic;
>> struct xfs_dir2_data_hdr *datahdr;
>>
>> + if (offset >= mp->m_dir_geo->freeblk) {
>> + /* TODO */
>> + return;
>
> TODO what exactly? Zero from the end of the bests array to the end of
> the block? We could at least add the if (!zero_stale_data) check here
> too.
TODO: see if there's any work to do here ;) Yes, end of bests
to end of block, I think.
>> + } else if (offset >= mp->m_dir_geo->leafblk) {
>> + struct xfs_dir2_leaf *leaf;
>> + struct xfs_dir2_leaf_tail *ltp;
>> + struct xfs_dir3_icleaf_hdr leafhdr;
>> +
>> + if (!zero_stale_data)
>> + return;
>> +
>> + leaf = (struct xfs_dir2_leaf *)block;
>> + ltp = xfs_dir2_leaf_tail_p(mp->m_dir_geo, leaf);
>
> ltp isn't used until the block of code below (which already has locally
> scoped vars).
Ok, oops, moved most. thanks.
Dave also points out that I should be checking DIR3_LEAF1_MAGIC
as well.
Thanks,
-Eric
>
> Brian
>
>> + M_DIROPS(mp)->leaf_hdr_from_disk(&leafhdr, leaf);
>> +
>> + /* Zero out space from end of ents[] to bests */
>> + if (leafhdr.magic == XFS_DIR2_LEAF1_MAGIC) {
>> + struct xfs_dir2_leaf_entry *ents;
>> + __be16 *lbp;
>> + char *start; /* end of ents */
>> +
>> + ents = M_DIROPS(mp)->leaf_ents_p(leaf);
>> + start = (char *)&ents[leafhdr.count + leafhdr.stale];
>> + lbp = xfs_dir2_leaf_bests_p(ltp);
>> + memset(start, 0, (char *)lbp - start);
>> + iocur_top->need_crc = 1;
>> + }
>> + return;
>> + }
>> +
>> + /* It's a data block. */
>> datahdr = (struct xfs_dir2_data_hdr *)block;
>>
>> if (is_block_format) {
>> @@ -1800,9 +1831,6 @@ process_single_fsb_objects(
>> dp = iocur_top->data;
>> switch (btype) {
>> case TYP_DIR2:
>> - if (o >= mp->m_dir_geo->leafblk)
>> - break;
>> -
>> process_dir_data_block(dp, o,
>> last == mp->m_dir_geo->fsbcount);
>> iocur_top->need_crc = 1;
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
