On 09 Jan 2021 at 11:58, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@xxxxxxxxxx>
>
> The name scanner in xfs_scrub cannot lock a namespace (dirent or xattr)
> and the kernel does not provide a stable cursor interface, which means
> that we can see the same byte sequence multiple times during a scan.
> This isn't a confusing name error since the kernel enforces uniqueness
> on the byte sequence, so all we need to do here is update the old entry.
>
> Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> ---
> scrub/unicrash.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
>
> diff --git a/scrub/unicrash.c b/scrub/unicrash.c
> index de3217c2..f5407b5e 100644
> --- a/scrub/unicrash.c
> +++ b/scrub/unicrash.c
> @@ -68,7 +68,7 @@ struct name_entry {
>
> xfs_ino_t ino;
>
> - /* Raw UTF8 name */
> + /* Raw dirent name */
> size_t namelen;
> char name[0];
> };
> @@ -627,6 +627,20 @@ unicrash_add(
> uc->buckets[bucket] = new_entry;
>
> while (entry != NULL) {
> + /*
> + * If we see the same byte sequence then someone's modifying
> + * the namespace while we're scanning it. Update the existing
> + * entry's inode mapping and erase the new entry from existence.
> + */
> + if (new_entry->namelen == entry->namelen &&
> + !memcmp(new_entry->name, entry->name, entry->namelen)) {
> + entry->ino = new_entry->ino;
> + uc->buckets[bucket] = new_entry->next;
> + name_entry_free(new_entry);
> + *badflags = 0;
> + continue;
If the above condition evaluates to true, the memory pointed to by "new_entry"
is freed. The "continue" statement would cause the while loop to be executed
once more. At this stage, "entry" will still have the previously held non-NULL
value and hence the while loop is executed once more causing the invalid
address in "new_entry" to be dereferenced.
> + }
> +
> /* Same normalization? */
> if (new_entry->normstrlen == entry->normstrlen &&
> !u_strcmp(new_entry->normstr, entry->normstr) &&
--
chandan
