On Tue, Jan 12, 2021 at 04:45:35PM +0530, Chandan Babu R wrote:
>
> 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.
Oops, good catch! Will fix.
--D
> > + }
> > +
> > /* Same normalization? */
> > if (new_entry->normstrlen == entry->normstrlen &&
> > !u_strcmp(new_entry->normstr, entry->normstr) &&
>
>
> --
> chandan
