Hi Eric,

Eric Biggers wrote:
> [Resending with only the script attached, since the original apparently didn't
> go through]
>
> Hi,
>
> I finally have more information, and a potential solution, for the apparent 
> NTFS
> corruption bug I've been encountering during randomized tests.  The bug, as 
> I've
> been experiencing it, results in an unreadable directory where readdir fails
> with EIO.  I've attached a script that creates a small volume exhibiting this
> bug.
>
> Based on my analysis, the bug is actually read-side.  In the example volume, 
> the
> unreadable directory has an ATTRIBUTE_LIST attribute and an INDEX_ALLOCATION
> attribute occupying two clusters, each in a different extent.  Therefore, the
> first INDEX_ALLOCATION extent has lowest_vcn=0 and highest_vcn=0, and the 
> second
> has lowest_vcn=1 and highest_vcn=1.

Good catch.

>
> This unusual case, which is apparently created by the combination of a small
> volume and near-full MFT records, triggers some special-case behavior in
> ntfs_mapping_pairs_decompress_i() near line 950 in runlist.c:

Looks good.

Apparently the "small volume" condition causes the
first run to fit into 5 bytes, and the second one
to require 4 more bytes, so there is a double overflow
first in the index (need for a new run) and then in
the MFT (need for a new extent).

Thanks

Jean-Pierre

>
>>                /*
>>                 * A highest_vcn of zero means this is a single extent
>>                 * attribute so simply terminate the runlist with LCN_ENOENT).
>>                 */
>
> That behavior is incorrect if the attribute's first extent only contains a
> single cluster, since in that case highest_vcn=0 as well.
>
> For what it's worth, I tested the volume on Windows and it *is* able to
> successfully read the directory.  This supports the hypothesis that the volume
> is valid and NTFS-3g has a bug on the read side.
>
> I think that this bug could, in theory, occur with any non-resident attribute,
> not just INDEX_ALLOCATION attributes.
>
> Finally, here is a proposed patch to fix the bug; please read it carefully 
> since
> a lot of the code has been new to me:
>
> diff --git a/libntfs-3g/runlist.c b/libntfs-3g/runlist.c
> index 7e158d4..7bb9da9 100644
> --- a/libntfs-3g/runlist.c
> +++ b/libntfs-3g/runlist.c
> @@ -939,40 +939,39 @@ mpa_err:
>                               "attribute.\n");
>               goto err_out;
>       }
> -     /* Setup not mapped runlist element if this is the base extent. */
> +
> +     /* If this is the base extent (if 'lowest_vcn' is 0), then
> +      * 'allocated_size' is valid, and we can use it to compute the total
> +      * number of clusters across all extents.  If the runlist covers all
> +      * clusters, then there was just a single extent and we can terminate
> +      * the runlist with LCN_NOENT.  Otherwise, we must terminate the runlist
> +      * with LCN_RL_NOT_MAPPED and let the caller look for more extents.  */
>       if (!attr->lowest_vcn) {
> -             VCN max_cluster;
> +             VCN num_clusters;
>
> -             max_cluster = ((sle64_to_cpu(attr->allocated_size) +
> +             num_clusters = ((sle64_to_cpu(attr->allocated_size) +
>                               vol->cluster_size - 1) >>
> -                             vol->cluster_size_bits) - 1;
> -             /*
> -              * A highest_vcn of zero means this is a single extent
> -              * attribute so simply terminate the runlist with LCN_ENOENT).
> -              */
> -             if (deltaxcn) {
> -                     /*
> -                      * If there is a difference between the highest_vcn and
> -                      * the highest cluster, the runlist is either corrupt
> -                      * or, more likely, there are more extents following
> -                      * this one.
> -                      */
> -                     if (deltaxcn < max_cluster) {
> -                             ntfs_log_debug("More extents to follow; 
> deltaxcn = "
> -                                             "0x%llx, max_cluster = 
> 0x%llx\n",
> -                                             (long long)deltaxcn,
> -                                             (long long)max_cluster);
> -                             rl[rlpos].vcn = vcn;
> -                             vcn += rl[rlpos].length = max_cluster - 
> deltaxcn;
> -                             rl[rlpos].lcn = (LCN)LCN_RL_NOT_MAPPED;
> -                             rlpos++;
> -                     } else if (deltaxcn > max_cluster) {
> -                             ntfs_log_debug("Corrupt attribute. deltaxcn = "
> -                                             "0x%llx, max_cluster = 
> 0x%llx\n",
> -                                             (long long)deltaxcn,
> -                                             (long long)max_cluster);
> -                             goto mpa_err;
> -                     }
> +                             vol->cluster_size_bits);
> +
> +             if (num_clusters > vcn) {
> +                     /* The runlist doesn't cover all the clusters, so there
> +                      * must be more extents.  */
> +                     ntfs_log_debug("More extents to follow; vcn = 0x%llx, "
> +                                    "num_clusters = 0x%llx\n",
> +                                     (long long)vcn,
> +                                     (long long)num_clusters);
> +                     rl[rlpos].vcn = vcn;
> +                     vcn += rl[rlpos].length = num_clusters - vcn;
> +                     rl[rlpos].lcn = (LCN)LCN_RL_NOT_MAPPED;
> +                     rlpos++;
> +             } else if (vcn > num_clusters) {
> +                     /* There are more VCNs in the runlist than expected, so
> +                      * the runlist is corrupt.  */
> +                     ntfs_log_debug("Corrupt attribute. vcn = 0x%llx, "
> +                                    "num_clusters = 0x%llx\n",
> +                                     (long long)vcn,
> +                                     (long long)num_clusters);
> +                     goto mpa_err;
>               }
>               rl[rlpos].lcn = (LCN)LCN_ENOENT;
>       } else /* Not the base extent. There may be more extents to follow. */
>
>
>
> ------------------------------------------------------------------------------
>
>
>
> _______________________________________________
> ntfs-3g-devel mailing list
> ntfs-3g-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/ntfs-3g-devel
>


------------------------------------------------------------------------------
Presto, an open source distributed SQL query engine for big data, initially
developed by Facebook, enables you to easily query your data on Hadoop in a 
more interactive manner. Teradata is also now providing full enterprise
support for Presto. Download a free open source copy now.
http://pubads.g.doubleclick.net/gampad/clk?id=250295911&iu=/4140
_______________________________________________
ntfs-3g-devel mailing list
ntfs-3g-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ntfs-3g-devel

Reply via email to