xfs
[Top] [All Lists]

Re: [PATCH] xfs_repair: junk last entry in sf dir if name starts beyond

To: Eric Sandeen <sandeen@xxxxxxxxxxx>
Subject: Re: [PATCH] xfs_repair: junk last entry in sf dir if name starts beyond dir size
From: Brian Foster <bfoster@xxxxxxxxxx>
Date: Mon, 9 Mar 2015 21:17:45 -0400
Cc: Rui Gomes <rgomes@xxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <54FDFEDC.5090106@xxxxxxxxxxx>
References: <54FDFEDC.5090106@xxxxxxxxxxx>
User-agent: Mutt/1.5.23 (2014-03-12)
On Mon, Mar 09, 2015 at 04:13:16PM -0400, Eric Sandeen wrote:
> When process_sf_dir2() is trying to salvage entries in a corrupted
> short form directory, it may attempt to shorten the last entry in
> the dir if it extends beyond the directory size.
> 
> However, if the name already starts past the dir size, no amount
> of name-shortening will make it fit, but the code doesn't realize
> this.  The namelen variable comes out to be negative, and things
> go downhill from there, resulting in a segfault when we try to
> memmove a negative number of bytes.
> 
> If no amount of name-shortening can make the last dir entry fit
> in the dir size, simply junk the entry.
> 
> Reported by: Rui Gomes <rgomes@xxxxxx>
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---
> 
> This adds a bit more spaghetti to an existing pot, but I think
> it clearly fixes the problem; I might try to rework these cases
> to coalesce some of the code.
> 
> (I also wonder about the tradeoff between shortening entries and
> increasing the dir size, but for now I'm just following the
> direction the repair code already takes).
> 

Seems Ok on a first glance. The fix is associated with the specific
namelen calculation. Are we susceptible to a similar problem in the
previous branch where we also calculate namelen from the dir size (the
namelen == 0 case)? It looks like we could set a bad value there.

Brian

> diff --git a/repair/dir2.c b/repair/dir2.c
> index 6b8964d..308808d 100644
> --- a/repair/dir2.c
> +++ b/repair/dir2.c
> @@ -911,6 +911,20 @@ _("zero length entry in shortform dir %" PRIu64 ""),
>                               namelen = ino_dir_size -
>                                       ((__psint_t) &sfep->name[0] -
>                                        (__psint_t) sfp);
> +                             /* Entry name starts past size of dir; junk it 
> */
> +                             if (namelen <= 0) {
> +                                     do_warn(
> +_("start of last entry name overflows space left in in shortform dir %" 
> PRIu64 ", "),
> +                                             ino);
> +                                     if (!no_modify)
> +                                             do_warn(
> +                                             _("junking entry #%d\n"), i);
> +                                     else
> +                                             do_warn(
> +                                             _("would junk entry #%d\n"), i);
> +                                     break;
> +                             }
> +                             /* Ok, truncate this entry to end of dir */
>                               do_warn(
>  _("size of last entry overflows space left in in shortform dir %" PRIu64 ", 
> "),
>                                       ino);
> 
> _______________________________________________
> xfs mailing list
> xfs@xxxxxxxxxxx
> http://oss.sgi.com/mailman/listinfo/xfs

<Prev in Thread] Current Thread [Next in Thread>