From 63916161b479479dac5250f80e7c451762d5bdc7 Mon Sep 17 00:00:00 2001 From: tinysec Date: Wed, 8 Jul 2026 14:35:00 +0800 Subject: [PATCH] fix: Activity action/eligibility callback ABI (dropped AnalysisContext) The C core invokes an Activity's action as void(*)(void*, BNAnalysisContext*) and its eligibility as bool(*)(void*, BNActivity*, BNAnalysisContext*) (workflow.py: ctypes.CFUNCTYPE(None, c_void_p, POINTER(BNAnalysisContext)) and CFUNCTYPE(c_bool, c_void_p, POINTER(BNActivity), POINTER(BNAnalysisContext))). The binding marshaled both as Action (one argument, void return), so: - the action never received its AnalysisContext (the whole point of the callback); - the eligibility handler received neither handle AND returned garbage bool (reading whatever happened to be in the return register), so eligibility was effectively random. Mirror the established MatchDataDelegate pattern: new public delegates ActivityAction(AnalysisContext) and ActivityEligibility(Activity, AnalysisContext), native-shaped ActivityActionDelegate/ActivityEligibilityDelegate, and holder contexts that wrap the raw handles via AnalysisContext/Activity.NewFromHandle (borrowed; the wrapper owns the reference it adds). Create/CreateWithEligibility now wrap the user delegates and root the wrappers (not the user delegates) on the instance, since the wrapper's function pointer is what the core invokes. BNCreateActivity/WithEligibility P/Invoke signatures are unchanged (config, ctxt, action[, eligibility]); only the delegate marshaling was wrong. --- Delegate/ActivityActionDelegate.cs | 52 +++++++++++++++++++ Delegate/ActivityEligibilityDelegate.cs | 51 +++++++++++++++++++ Handle/BNActivity.cs | 67 ++++++++++++++++--------- 3 files changed, 146 insertions(+), 24 deletions(-) create mode 100644 Delegate/ActivityActionDelegate.cs create mode 100644 Delegate/ActivityEligibilityDelegate.cs diff --git a/Delegate/ActivityActionDelegate.cs b/Delegate/ActivityActionDelegate.cs new file mode 100644 index 0000000..2a70f43 --- /dev/null +++ b/Delegate/ActivityActionDelegate.cs @@ -0,0 +1,52 @@ +using System; + +namespace BinaryNinja +{ + // Action invoked when an Activity runs. The core hands the action the AnalysisContext the + // activity is executing under. Mirrors Python Activity.action (workflow.py): the raw + // BNAnalysisContext* is wrapped before reaching managed code. + public delegate void ActivityAction(AnalysisContext analysisContext); + + internal static partial class NativeDelegates + { + // void (*actionCallback)(void* ctxt, BNAnalysisContext* ac) + public delegate void ActivityActionDelegate(IntPtr context, IntPtr analysisContext); + } + + internal static partial class UnsafeUtils + { + // Adapts a public ActivityAction into the native action-callback shape, wrapping the raw + // BNAnalysisContext* via AnalysisContext.NewFromHandle. The core passes a borrowed handle; + // NewFromHandle adds a reference so the wrapper owns its own and frees it on dispose. + internal sealed class ActivityActionContext + { + private readonly ActivityAction m_callback; + + internal ActivityActionContext(ActivityAction callback) + { + this.m_callback = callback; + } + + internal void OnAction(IntPtr context, IntPtr analysisContextHandle) + { + AnalysisContext? analysisContext = AnalysisContext.NewFromHandle(analysisContextHandle); + + // The core always provides a non-null AnalysisContext to a running activity; skip + // defensively if it does not. + if (null == analysisContext) + { + return; + } + + this.m_callback(analysisContext); + } + } + + internal static NativeDelegates.ActivityActionDelegate WrapActivityAction(ActivityAction callback) + { + ActivityActionContext context = new ActivityActionContext(callback); + + return new NativeDelegates.ActivityActionDelegate(context.OnAction); + } + } +} diff --git a/Delegate/ActivityEligibilityDelegate.cs b/Delegate/ActivityEligibilityDelegate.cs new file mode 100644 index 0000000..4123cc3 --- /dev/null +++ b/Delegate/ActivityEligibilityDelegate.cs @@ -0,0 +1,51 @@ +using System; + +namespace BinaryNinja +{ + // Eligibility check for an Activity: given the Activity and the AnalysisContext it would run + // under, return whether it may run. Mirrors Python Activity eligibility (workflow.py). + public delegate bool ActivityEligibility(Activity activity, AnalysisContext analysisContext); + + internal static partial class NativeDelegates + { + // bool (*eligibilityCallback)(void* ctxt, BNActivity* activity, BNAnalysisContext* ac) + public delegate bool ActivityEligibilityDelegate(IntPtr context, IntPtr activity, IntPtr analysisContext); + } + + internal static partial class UnsafeUtils + { + // Adapts a public ActivityEligibility into the native eligibility-callback shape, wrapping + // the raw BNActivity* and BNAnalysisContext* via their NewFromHandle factories (borrowed + // handles; each wrapper owns the reference it adds and frees it on dispose). + internal sealed class ActivityEligibilityContext + { + private readonly ActivityEligibility m_callback; + + internal ActivityEligibilityContext(ActivityEligibility callback) + { + this.m_callback = callback; + } + + internal bool OnEligibility(IntPtr context, IntPtr activityHandle, IntPtr analysisContextHandle) + { + Activity? activity = Activity.NewFromHandle(activityHandle); + AnalysisContext? analysisContext = AnalysisContext.NewFromHandle(analysisContextHandle); + + // If either handle is missing the activity is treated as not eligible. + if (null == activity || null == analysisContext) + { + return false; + } + + return this.m_callback(activity, analysisContext); + } + } + + internal static NativeDelegates.ActivityEligibilityDelegate WrapActivityEligibility(ActivityEligibility callback) + { + ActivityEligibilityContext context = new ActivityEligibilityContext(callback); + + return new NativeDelegates.ActivityEligibilityDelegate(context.OnEligibility); + } + } +} diff --git a/Handle/BNActivity.cs b/Handle/BNActivity.cs index eaf8095..33b2f47 100644 --- a/Handle/BNActivity.cs +++ b/Handle/BNActivity.cs @@ -11,9 +11,12 @@ public sealed class Activity : AbstractSafeHandle // Rooting the caller-supplied delegates on the instance keeps them alive as long as the // Activity; without this, a temporary delegate passed to Create would be GC-eligible after // Create returns and the next activity callback would dereference freed memory. - private Action? m_action = null; + // Holds the NATIVE wrapper delegate (not the user delegate): the wrapper owns the GC root + // for the user callback via its holder context, and its function pointer is what the core + // invokes. Rooting the wrapper for the Activity's lifetime keeps the callback reachable. + private NativeDelegates.ActivityActionDelegate? m_action = null; - private Action? m_eligibilityHandler = null; + private NativeDelegates.ActivityEligibilityDelegate? m_eligibilityHandler = null; internal Activity(IntPtr handle , bool owner) : base(handle , owner) @@ -114,16 +117,22 @@ public string Name /// The configuration name for the activity. /// The action callback delegate, or null for no action. /// A new owned Activity, or null on failure. - public static Activity? Create(string configuration , Action? action = null) + public static Activity? Create(string configuration , ActivityAction? action = null) { - // 1. Marshal the action delegate to a function pointer if provided. - IntPtr actionPtr = IntPtr.Zero; + // 1. Wrap the action into the native (ctxt, analysisContext) callback shape, then take a + // function pointer from the wrapper. The wrapper carries the AnalysisContext the core + // hands the callback (Action dropped it, so the action never saw its context). + NativeDelegates.ActivityActionDelegate? actionWrapper = null; if (null != action) { - actionPtr = Marshal.GetFunctionPointerForDelegate>(action); + actionWrapper = UnsafeUtils.WrapActivityAction(action); } + IntPtr actionPtr = null != actionWrapper + ? Marshal.GetFunctionPointerForDelegate(actionWrapper) + : IntPtr.Zero; + // 2. Create the activity via the native API. Activity? activity = Activity.TakeHandle( NativeMethods.BNCreateActivity( @@ -133,12 +142,12 @@ public string Name ) ); - // 3. Root the action delegate on the instance for the Activity's lifetime; otherwise the - // function pointer handed to the core would dangle once Create returns and the caller's - // delegate is collected (see m_action). - if (null != activity && null != action) + // 3. Root the wrapper (not the user delegate) on the instance for the Activity's lifetime; + // the wrapper holds the user callback via its context, and its function pointer is what the + // core invokes (see m_action). + if (null != activity && null != actionWrapper) { - activity.m_action = action; + activity.m_action = actionWrapper; } return activity; @@ -153,26 +162,36 @@ public string Name /// A new owned Activity, or null on failure. public static Activity? CreateWithEligibility( string configuration , - Action? action = null , - Action? eligibilityHandler = null + ActivityAction? action = null , + ActivityEligibility? eligibilityHandler = null ) { - // 1. Marshal the action delegate to a function pointer if provided. - IntPtr actionPtr = IntPtr.Zero; + // 1. Wrap the action into the native (ctxt, analysisContext) callback shape. + NativeDelegates.ActivityActionDelegate? actionWrapper = null; if (null != action) { - actionPtr = Marshal.GetFunctionPointerForDelegate>(action); + actionWrapper = UnsafeUtils.WrapActivityAction(action); } - // 2. Marshal the eligibility handler delegate to a function pointer if provided. - IntPtr eligibilityPtr = IntPtr.Zero; + IntPtr actionPtr = null != actionWrapper + ? Marshal.GetFunctionPointerForDelegate(actionWrapper) + : IntPtr.Zero; + + // 2. Wrap the eligibility handler into the native (ctxt, activity, analysisContext) -> bool + // callback shape. Action dropped both handles and returned garbage bool, so the + // handler never received its arguments and eligibility was effectively random. + NativeDelegates.ActivityEligibilityDelegate? eligibilityWrapper = null; if (null != eligibilityHandler) { - eligibilityPtr = Marshal.GetFunctionPointerForDelegate>(eligibilityHandler); + eligibilityWrapper = UnsafeUtils.WrapActivityEligibility(eligibilityHandler); } + IntPtr eligibilityPtr = null != eligibilityWrapper + ? Marshal.GetFunctionPointerForDelegate(eligibilityWrapper) + : IntPtr.Zero; + // 3. Create the activity with eligibility via the native API. Activity? activity = Activity.TakeHandle( NativeMethods.BNCreateActivityWithEligibility( @@ -183,14 +202,14 @@ public string Name ) ); - // 4. Root both delegates on the instance for the Activity's lifetime; otherwise the - // function pointers handed to the core would dangle once CreateWithEligibility returns - // and the caller's delegates are collected (see m_action / m_eligibilityHandler). + // 4. Root both wrappers on the instance for the Activity's lifetime; the wrappers hold the + // user callbacks via their contexts, and their function pointers are what the core invokes + // (see m_action / m_eligibilityHandler). if (null != activity) { - activity.m_action = action; + activity.m_action = actionWrapper; - activity.m_eligibilityHandler = eligibilityHandler; + activity.m_eligibilityHandler = eligibilityWrapper; } return activity;