[PATCH v2] combase: Reorder hstring_private elements.
Nikolay Sivov
nsivov at codeweavers.com
Sat Dec 25 06:34:31 CST 2021
On 12/24/21 15:25, Bernhard wrote:
> Thanks for your review Nikolay,
>
> Nikolay Sivov <nsivov at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20211225/fa663e65/attachment.htm>
More information about the wine-devel
mailing list