New subject: [PATCH v2] combase: Reorder hstring_private elements.

25 Dec 2021


      On 12/24/21 15:25, Bernhard wrote:
...
Thanks for your review Nikolay,
Nikolay Sivov <nsivov(a)codeweavers.com> schrieb am Fr., 24. Dez. 2021, 
09:03:
On 12/24/21 01:48, Bernhard Kölbl wrote:
    > +#define HSTRING_REFERENCE_FLAG 1
    > +
    >   struct hstring_private
    >   {
    > -    LPWSTR buffer;
    > +    UINT32 flags;
    >       UINT32 length;
    > -    BOOL   reference;
    > -    LONG   refcount;
    > +    LONG refcount;
    > +    LPWSTR ptr;
    >   };
     From what I can tell this still doesn't match native layout. It also
    might be slightly different for references.
Yes it actually doesn't match but I don't know if it really matters in 
this case, because HSTRINGs seem to be passed with this data only. 
E.g. in the provided bug, the RoGetActivationFactory function takes a 
HSTRING as parameter, but WinRT actually passes a HSTRING_HEADER.
The first field in the HSTRING struct seems to only point at this 
header struct. WinRT dereferences this pointer when passing the String.
I don't follow. What do you mean by "WinRT passing a header" and "WinRT 
dereferencing this pointer" ? Where is this code?
...
>   static BOOL alloc_string(UINT32 len, HSTRING *out)
    >   {
    >       struct hstring_private *priv;
    > -    priv = HeapAlloc(GetProcessHeap(), 0, sizeof(*priv) + (len
    + 1) * sizeof(*priv->buffer));
    > +    priv = HeapAlloc(GetProcessHeap(), 0, sizeof(*priv));
    >       if (!priv)
    >           return FALSE;
    > -    priv->buffer = (LPWSTR)(priv + 1);
    > +    priv->ptr = (LPWSTR)HeapAlloc(GetProcessHeap(), 0, (len +
    1) * sizeof(*priv->ptr));
    I don't see this behaviour on Windows.
Could explain this one a bit more? (I'm not much into how Windows 
libraries allocate memory.) Maybe I'm overseeing a obvious mistake.
I don't think it's justified to have separate allocation for string buffer.
...
But anyway, do you have any idea why application would care? Is it
    some
    statically linked code depending on it, or some native module
    bundled or
    installed separately?
Well, the programs (also WinRT) from the Wine-bug suffer from the same 
issue, which to me looks like is caused by the misarranged struct, 
which I changed in this patch. They try to dereference the flags + 
length field, which causes a crash or invalid data to be read.
I actually don't know why it worked in Wine before. Maybe by luck?
(Also: The code that arranges the HSTRING struct members in memory 
this way, is compiled into the binary from the WinRT headers)
Which headers in particular? I'd like to read through that, maybe we can 
easily match it.
...
If the layout is stable across Windows releases, we'll need tests
    for it.
I think test are the best solution for our open questions.
Happy holidays

Re: [PATCH v2] combase: Reorder hstring_private elements.

Nikolay Sivov

Bernhard

tags

participants (2)