[5/7] ntdll: Implement RtlDecompressFragment.

Matteo Bruni matteo.mystral at gmail.com
Thu Jul 9 07:04:47 CDT 2015


Hi,

a bunch of nitpicks which you can take or ignore:

2015-07-09 3:07 GMT+02:00 Sebastian Lackner <sebastian at fds-team.de>:
> Based on a patch by Michael Müller.
>
> For https://bugs.winehq.org/show_bug.cgi?id=37449
>
> ---
>  dlls/ntdll/ntdll.spec               |    2
>  dlls/ntdll/rtl.c                    |  214 ++++++++++++++++++++++++++++++++++--
>  dlls/ntdll/tests/rtl.c              |    3
>  dlls/ntoskrnl.exe/ntoskrnl.exe.spec |    2
>  4 files changed, 210 insertions(+), 11 deletions(-)
>
> diff --git a/dlls/ntdll/ntdll.spec b/dlls/ntdll/ntdll.spec
> index 75dc647..ca3561d 100644
> --- a/dlls/ntdll/ntdll.spec
> +++ b/dlls/ntdll/ntdll.spec
> @@ -513,7 +513,7 @@
>  @ stdcall RtlDecodePointer(ptr)
>  # @ stub RtlDecodeSystemPointer
>  @ stdcall RtlDecompressBuffer(long ptr long ptr long ptr)
> -@ stub RtlDecompressFragment
> +@ stdcall RtlDecompressFragment(long ptr long ptr long long ptr ptr)
>  @ stub RtlDefaultNpAcl
>  @ stub RtlDelete
>  @ stdcall RtlDeleteAce(ptr long)
> diff --git a/dlls/ntdll/rtl.c b/dlls/ntdll/rtl.c
> index d9e448a..9a196d2 100644
> --- a/dlls/ntdll/rtl.c
> +++ b/dlls/ntdll/rtl.c
> @@ -1309,17 +1309,219 @@ NTSTATUS WINAPI RtlCompressBuffer(USHORT format, PUCHAR uncompressed, ULONG unco
>      }
>  }
>
> +/* decompress a single LZNT1 chunk */
> +static PUCHAR lznt1_decompress_chunk(UCHAR *dst, ULONG dst_size, UCHAR *src, ULONG src_size)

PUCHAR -> UCHAR *

> +{
> +    UCHAR *src_cur = src, *src_end = src + src_size;
> +    UCHAR *dst_cur = dst, *dst_end = dst + dst_size;
> +    ULONG displacement_bits, length_bits;
> +    ULONG code_displacement, code_length;
> +    WORD flags, code;
> +
> +    while (src_cur < src_end && dst_cur < dst_end)
> +    {
> +        flags = 0x8000 | *src_cur++;
> +        while ((flags & 0xFF00) && src_cur < src_end)

It looks like we're mostly using lowercase in hex literals.

> +        {
> +            if (flags & 1)
> +            {
> +                /* backwards reference */
> +                if (src_cur + sizeof(WORD) > src_end)
> +                    return NULL;
> +
> +                code = *(WORD *)src_cur;
> +                src_cur += sizeof(WORD);
> +
> +                /* find length / displacement bits */
> +                for (displacement_bits = 12; displacement_bits > 4; displacement_bits--)
> +                    if ((1 << (displacement_bits - 1)) < dst_cur - dst) break;
> +
> +                length_bits       = 16 - displacement_bits;
> +                code_length       = (code & ((1 << length_bits) - 1)) + 3;
> +                code_displacement = (code >> length_bits) + 1;
> +
> +                if (dst_cur < dst + code_displacement)
> +                    return NULL;
> +
> +                /* copy bytes of chunk - we can't use memcpy()
> +                 * since source and dest can be overlapping */

I would also mention that you can't use memmove() either because you
actually want to copy any overlapping byte over and over again, as
necessary.

> +                while (code_length--)
> +                {
> +                    if (dst_cur >= dst_end) return dst_cur;
> +                    *dst_cur = *(dst_cur - code_displacement);
> +                    dst_cur++;
> +                }
> +            }
> +            else
> +            {
> +                /* uncompressed data */
> +                if (dst_cur >= dst_end) return dst_cur;
> +                *dst_cur++ = *src_cur++;
> +            }
> +            flags >>= 1;
> +        }
> +    }
> +
> +    return dst_cur;
> +}
> +
> +/* decompress data encoded with LZNT1 */
> +static NTSTATUS lznt1_decompress(UCHAR *dst, ULONG dst_size, UCHAR *src, ULONG src_size,
> +                                 ULONG offset, ULONG *final_size, UCHAR *workspace)
> +{
> +    UCHAR *src_cur = src, *src_end = src + src_size;
> +    UCHAR *dst_cur = dst, *dst_end = dst + dst_size;
> +    ULONG chunk_size, block_size;
> +    WORD chunk_header;
> +    UCHAR *ptr;
> +
> +    if (src_cur + sizeof(WCHAR) > src_end)
> +        return STATUS_BAD_COMPRESSION_BUFFER;
> +
> +    /* skip over chunks which have a distance >= 0x1000 to the destination offset */
> +    while (offset >= 0x1000 && src_cur + sizeof(WCHAR) <= src_end)
> +    {
> +        chunk_header = *(WORD *)src_cur;
> +        src_cur += sizeof(WCHAR);

Why this mixing of WORD and WCHAR? They do match in practice but still...

> +        if (!chunk_header) goto out;
> +        chunk_size = (chunk_header & 0xFFF) + 1;
> +
> +        if (src_cur + chunk_size > src_end)
> +            return STATUS_BAD_COMPRESSION_BUFFER;
> +
> +        src_cur += chunk_size;
> +        offset  -= 0x1000;
> +    }
> +
> +    /* check if a chunk is included partially */
> +    if (offset && src_cur + sizeof(WCHAR) <= src_end)
> +    {
> +        chunk_header = *(WORD *)src_cur;
> +        src_cur += sizeof(WCHAR);
> +        if (!chunk_header) goto out;
> +        chunk_size = (chunk_header & 0xFFF) + 1;
> +
> +        if (src_cur + chunk_size > src_end)
> +            return STATUS_BAD_COMPRESSION_BUFFER;
> +
> +        if (dst_cur >= dst_end)
> +            goto out;
> +
> +        if (chunk_header & 0x8000)
> +        {
> +            /* compressed chunk */
> +            if (!workspace) return STATUS_ACCESS_VIOLATION;
> +            ptr = lznt1_decompress_chunk(workspace, 0x1000, src_cur, chunk_size);

I guess in theory you could avoid decompressing the whole chunk if you
need less than the whole chunk to be decompressed. It's not strictly
required though so if doing that makes the code too convoluted then
ignore it.

> +            if (!ptr) return STATUS_BAD_COMPRESSION_BUFFER;
> +            if (ptr - workspace > offset)
> +            {
> +                block_size = min((ptr - workspace) - offset, dst_end - dst_cur);
> +                memcpy(dst_cur, workspace + offset, block_size);
> +                dst_cur += block_size;
> +            }
> +        }
> +        else
> +        {
> +            /* uncompressed chunk */
> +            if (chunk_size > offset)
> +            {
> +                block_size = min(chunk_size - offset, dst_end - dst_cur);
> +                memcpy(dst_cur, src_cur + offset, block_size);
> +                dst_cur += block_size;
> +            }
> +        }
> +
> +        src_cur += chunk_size;
> +    }
> +
> +    /* remaining blocks */
> +    while (src_cur + sizeof(WCHAR) <= src_end)
> +    {
> +        chunk_header = *(WORD *)src_cur;
> +        src_cur += sizeof(WCHAR);
> +        if (!chunk_header) goto out;
> +        chunk_size = (chunk_header & 0xFFF) + 1;
> +
> +        if (src_cur + chunk_size > src_end)
> +            return STATUS_BAD_COMPRESSION_BUFFER;
> +
> +        /* fill space with padding */
> +        block_size = ((dst_cur - dst) + offset) & 0xFFF;
> +        if (block_size)
> +        {
> +            block_size = 0x1000 - block_size;
> +            if (dst_cur + block_size >= dst_end)
> +                goto out;
> +            memset(dst_cur, 0, block_size);
> +            dst_cur += block_size;
> +        }

I'm probably dense but I don't follow this padding part. Don't you
still need to memset to 0 the "remaining part" even if dst_end <=
dst_cur + block_size (obviously stopping at dst_end) and then proceed
to decompress the chunk? Unless you're not supposed to decompress the
last chunk in that case?

> +
> +        if (dst_cur >= dst_end)
> +            goto out;
> +
> +        if (chunk_header & 0x8000)
> +        {
> +            /* compressed chunk */
> +            dst_cur = lznt1_decompress_chunk(dst_cur, dst_end - dst_cur, src_cur, chunk_size);
> +            if (!dst_cur) return STATUS_BAD_COMPRESSION_BUFFER;
> +        }
> +        else
> +        {
> +            /* uncompressed chunk */
> +            block_size = min(chunk_size, dst_end - dst_cur);
> +            memcpy(dst_cur, src_cur, block_size);
> +            dst_cur += block_size;
> +        }
> +
> +        src_cur += chunk_size;
> +    }
> +
> +out:
> +    if (final_size)
> +        *final_size = dst_cur - dst;
> +
> +    return STATUS_SUCCESS;
> +
> +}
> +
> +/******************************************************************************
> + *  RtlDecompressFragment      [NTDLL.@]
> + */
> +NTSTATUS RtlDecompressFragment(USHORT format, PUCHAR uncompressed, ULONG uncompressed_size,
> +                               PUCHAR compressed, ULONG compressed_size, ULONG offset,
> +                               PULONG final_size, PVOID workspace)
> +{
> +    TRACE("0x%04x, %p, %u, %p, %u, %u, %p, %p\n", format, uncompressed,
> +          uncompressed_size, compressed, compressed_size, offset, final_size, workspace);
> +
> +    switch (format & ~COMPRESSION_ENGINE_MAXIMUM)
> +    {
> +        case COMPRESSION_FORMAT_LZNT1:
> +            return lznt1_decompress(uncompressed, uncompressed_size, compressed,
> +                                    compressed_size, offset, final_size, workspace);
> +
> +        case COMPRESSION_FORMAT_NONE:
> +        case COMPRESSION_FORMAT_DEFAULT:
> +            return STATUS_INVALID_PARAMETER;
> +
> +        default:
> +            FIXME("format %u not implemented\n", format);
> +            return STATUS_UNSUPPORTED_COMPRESSION;
> +    }
> +}
> +
> +
>  /******************************************************************************
>   *  RtlDecompressBuffer                [NTDLL.@]
>   */
> -NTSTATUS WINAPI RtlDecompressBuffer(USHORT CompressionFormat, PUCHAR UncompressedBuffer,
> -                                    ULONG UncompressedBufferSize, PUCHAR CompressedBuffer,
> -                                    ULONG CompressedBufferSize, PULONG FinalUncompressedSize)
> +NTSTATUS WINAPI RtlDecompressBuffer(USHORT format, PUCHAR uncompressed, ULONG uncompressed_size,
> +                                    PUCHAR compressed, ULONG compressed_size, PULONG final_size)
>  {
> -    FIXME("0x%04x, %p, %u, %p, %u, %p :stub\n", CompressionFormat, UncompressedBuffer, UncompressedBufferSize,
> -         CompressedBuffer, CompressedBufferSize, FinalUncompressedSize);
> +    TRACE("0x%04x, %p, %u, %p, %u, %p\n", format, uncompressed,
> +        uncompressed_size, compressed, compressed_size, final_size);
>
> -    return STATUS_NOT_IMPLEMENTED;
> +    return RtlDecompressFragment(format, uncompressed, uncompressed_size,
> +                                 compressed, compressed_size, 0, final_size, NULL);
>  }
>
>  /***********************************************************************
> diff --git a/dlls/ntdll/tests/rtl.c b/dlls/ntdll/tests/rtl.c
> index c6a7023..99fd413 100644
> --- a/dlls/ntdll/tests/rtl.c
> +++ b/dlls/ntdll/tests/rtl.c
> @@ -1663,11 +1663,8 @@ static void test_RtlCompressBuffer(void)
>      memset(buf2, 0x11, sizeof(buf2));
>      status = pRtlDecompressBuffer(COMPRESSION_FORMAT_LZNT1, buf2, sizeof(buf2),
>                                    buf1, buf_size, &final_size);
> -    todo_wine
>      ok(status == STATUS_SUCCESS, "got wrong status 0x%08x\n", status);
> -    todo_wine
>      ok(final_size == sizeof(test_buffer), "got wrong final_size %u\n", final_size);
> -    todo_wine
>      ok(!memcmp(buf2, test_buffer, sizeof(test_buffer)), "got wrong decoded data\n");
>      ok(buf2[sizeof(test_buffer)] == 0x11, "too many bytes written\n");
>
> diff --git a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec
> index 11d1c62..0bb7d04 100644
> --- a/dlls/ntoskrnl.exe/ntoskrnl.exe.spec
> +++ b/dlls/ntoskrnl.exe/ntoskrnl.exe.spec
> @@ -977,7 +977,7 @@
>  @ stub RtlCustomCPToUnicodeN
>  @ stdcall RtlDecompressBuffer(long ptr long ptr long ptr) ntdll.RtlDecompressBuffer
>  @ stub RtlDecompressChunks
> -@ stub RtlDecompressFragment
> +@ stdcall RtlDecompressFragment(long ptr long ptr long long ptr ptr) ntdll.RtlDecompressFragment
>  @ stub RtlDelete
>  @ stdcall RtlDeleteAce(ptr long) ntdll.RtlDeleteAce
>  @ stdcall RtlDeleteAtomFromAtomTable(ptr long) ntdll.RtlDeleteAtomFromAtomTable
> --
> 2.4.5
>
>



More information about the wine-devel mailing list