On 2/28/14, 12:25 AM, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> In testing the previous metadump changes, fsstress generated an
> inline symlink of 336 bytes in length. This caused corruption of the
> restored filesystem that wasn't present in the original filesystem -
> it corrupted the magic number of the next inode in the chunk. The
> reason being that the symlink data is not null terminated in the
> inode literal area, and hence when the symlink consumes the entire
> literal area like so:
>
> xfs_db> daddr 0x42679
> xfs_db> p
> 000: 494ea1ff 03010000 00000000 00000000 00000001 00000000 00000000 00000000
> 020: 53101af9 1678d2a8 53101af9 15fec0a8 53101af9 15fec0a8 00000000 00000150
> 040: 00000000 00000000 00000000 00000000 00000002 00000000 00000000 d868b52d
> 060: ffffffff 0ce5477a 00000000 00000002 00000002 0000041c 00000000 00000000
> 080: 00000000 00000000 00000000 00000000 53101af9 15fec0a8 00000000 00042679
> 0a0: 6c4e9d4e 84944986 a074cffd 0ea042a8 78787878 78787878 78782f78 78787878
> 0c0: 78787878 2f787878 78787878 78782f78 78787878 78787878 2f787878 78787878
> 0e0: 78782f78 78787878 78787878 2f787878 78787878 78782f78 78787878 78787878
> 100: 2f787878 78787878 78782f78 78787878 78787878 2f787878 78787878 78782f78
> 120: 78787878 78787878 2f787878 78787878 78782f78 78787878 78787878 2f787878
> 140: 78787878 78782f78 78787878 78787878 2f787878 78787878 78782f78 78787878
> 160: 78787878 2f787878 78787878 78782f78 78787878 78787878 2f787878 78787878
> 180: 78782f78 78787878 78787878 2f787878 78787878 78782f78 78787878 78787878
> 1a0: 2f787878 78787878 78782f78 78787878 78787878 2f787878 78787878 78782f78
> 1c0: 78787878 78787878 2f787878 78787878 78782f78 78787878 78787878 2f787878
> 1e0: 78787878 78782f78 78787878 78787878 2f787878 78787878 78782f78 78787878
>
> the symlink data butts right up agains the magic number of the next
> inode in the chunk. And then, when obfuscation gets to the final
> pathname component, it gets it's length via:
>
> /* last (or single) component */
> namelen = strlen((char *)comp);
> hash = libxfs_da_hashname(comp, namelen);
> obfuscate_name(hash, namelen, comp);
>
> strlen(), which looks for a null terminator and finds it several
> bytes into the next inode. It then proceeds to obfuscate that
> length, including the inode magic number of the next inode....
My fault!
> Fix this by ensuring we can't overrun the symlink buffer length
> by assuming that the symlink is not null terminated.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> ---
> db/metadump.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/db/metadump.c b/db/metadump.c
> index 3248009..cd489a6 100644
> --- a/db/metadump.c
> +++ b/db/metadump.c
> @@ -1015,16 +1015,23 @@ obfuscate_sf_dir(
> }
> }
>
> +/*
> + * The pathname may not be null terminated. It name be terminated by the end of
can you fix up the start of that 2nd sentence?
Otherwise,
Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> + * a buffer or inode literal area, and the start of the next region may contain
> + * any data at all. Hence when we get to the last component of the symlink, we
> + * cannot assume that strlen() will give us the right result. Hence we need to
> + * track the remaining pathname length and use that instead.
> + */
> static void
> obfuscate_path_components(
> char *buf,
> __uint64_t len)
> {
> - uchar_t *comp;
> + uchar_t *comp = (uchar_t *)buf;
> + uchar_t *end = comp + len;
> xfs_dahash_t hash;
>
> - comp = (uchar_t *)buf;
> - while (comp < (uchar_t *)buf + len) {
> + while (comp < end) {
> char *slash;
> int namelen;
>
> @@ -1032,7 +1039,7 @@ obfuscate_path_components(
> slash = strchr((char *)comp, '/');
> if (!slash) {
> /* last (or single) component */
> - namelen = strlen((char *)comp);
> + namelen = strnlen((char *)comp, len);
> hash = libxfs_da_hashname(comp, namelen);
> obfuscate_name(hash, namelen, comp);
> break;
> @@ -1041,11 +1048,13 @@ obfuscate_path_components(
> /* handle leading or consecutive slashes */
> if (!namelen) {
> comp++;
> + len--;
> continue;
> }
> hash = libxfs_da_hashname(comp, namelen);
> obfuscate_name(hash, namelen, comp);
> comp += namelen + 1;
> + len -= namelen + 1;
> }
> }
>
>
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
