On 12/24/21 15:25, Bernhard wrote:
Thanks for your review Nikolay,
Nikolay Sivov nsivov@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