Skip to content

Fix new-delete type mismatch in MSDK dispatcher#196

Open
NathanGray-ChurchillNavigation wants to merge 1 commit into
intel:mainfrom
NathanGray-ChurchillNavigation:fix-dispatcher-new-delete-mismatch
Open

Fix new-delete type mismatch in MSDK dispatcher#196
NathanGray-ChurchillNavigation wants to merge 1 commit into
intel:mainfrom
NathanGray-ChurchillNavigation:fix-dispatcher-new-delete-mismatch

Conversation

@NathanGray-ChurchillNavigation

Copy link
Copy Markdown

Issue

MFXInitEx allocates dispatcher session handles as MFX_DISP_HANDLE_EX, but MFXClose releases them with delete pHandle through a MFX_DISP_HANDLE *. Since ~MFX_DISP_HANDLE is not virtual, deleting the derived object through the base pointer is undefined behavior ([expr.delete]/3). In practice this aborts any AddressSanitizer-instrumented process that opens and closes a session through the dispatcher (MSVC /fsanitize=address, similarly Clang/GCC ASan):

==66932==ERROR: AddressSanitizer: new-delete-type-mismatch on 0x12236d7a2880 in thread T0:
  object passed to delete has wrong type:
  size of the allocated type:   1072 bytes;
  size of the deallocated type: 1048 bytes.
    #0 operator delete(void *, unsigned __int64)
    #1 MFXClose libvpl/src/windows/main.cpp:609
    ...
allocated by thread T0 here:
    #0 operator new(unsigned __int64)
    #1 MFXInitEx libvpl/src/windows/main.cpp:213

We hit this in our application's ASan CI the first time it exercised MFXInitEx/MFXClose through an instrumented dispatcher.

Solution

Making ~MFX_DISP_HANDLE virtual is not viable: the passthrough functions cast the session pointer directly to struct _mfxSession * and read members assuming the base subobject is at offset zero (e.g. MFXMemory_GetSurfaceForVPP), and mfx_dispatcher.h documents that the handle layout must remain compatible. Adding a vtable pointer would shift the base subobject.

Instead, this change makes the allocation and deallocation types agree on every path:

  • Allocate MFX_DISP_HANDLE_EX at the two remaining creation sites that used the base class (MFXInitEx2-style DLL-name init and AllocateCloneHandle). MFX_DISP_HANDLE_EX only extends the base with POD members (mediaAdapterType + reserved), so this is behavior-neutral for those paths.
  • Spell the matching type at each delete of a base-typed pointer (MFXClose, the DLL-name init error path, and the two clone error paths). MFXInitEx's own local/HandleVector sites were already correctly typed as MFX_DISP_HANDLE_EX *.

No layout, API, or behavior changes; 6 lines total.

How Tested

  • Built on Windows with MSVC (VS 2026, Ninja) from this branch: clean configure and build of libvpl.dll.
  • Deployed the same patch against v2.16.0 in our product tree: with an ASan-instrumented dispatcher DLL, MFXInitEx + MFXQuery/MFXVideoENCODE_Query + MFXClose cycles (via our capability-probing code, executed per encoder/decoder node) now run to completion; unpatched they abort with the report above on the first MFXClose.
  • Changed lines are clang-format clean (git clang-format reports no diff for this change).

MFXInitEx allocates session handles as MFX_DISP_HANDLE_EX, but
MFXClose deletes them through a MFX_DISP_HANDLE pointer.  The base
class destructor is not virtual, so this is undefined behavior and
is reported by AddressSanitizer as new-delete-type-mismatch (size of
the allocated type: 1072 bytes; size of the deallocated type: 1048
bytes), aborting any ASan-instrumented process that opens and closes
a session through the dispatcher.

Making ~MFX_DISP_HANDLE virtual is not an option: the passthrough
functions cast the session pointer directly to struct _mfxSession*
and rely on the base subobject living at offset zero, and the header
documents that the handle layout must stay compatible.  Adding a
vtable pointer would shift the base subobject.

Instead, allocate MFX_DISP_HANDLE_EX at every handle creation site
(it only extends the base with POD members, so this is safe for the
two sites that previously allocated the base class) and spell the
matching type at every delete of a base-typed pointer.  Allocation
and deallocation types now agree on all paths, including handles
reaching MFXClose from MFXInitEx, MFXInitEx2, and MFXCloneSession.

Signed-off-by: Nathan Gray <nathan.gray@shotover.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant