From 88d5e723ed33dffe3e5485ff68728d5c97fe3322 Mon Sep 17 00:00:00 2001 From: Giles Atkinson <“gatk555@gmail.com”> Date: Wed, 20 Jul 2022 17:46:41 +0100 Subject: [PATCH] Fix double free in XSPICE CALLBACK functions as desribed here: https://sourceforge.net/p/ngspice/mailman/message/37677362/ and add missing CALLBACKs in models that use STATIC_VARs with pointer values. --- src/xspice/icm/analog/oneshot/cfunc.mod | 16 ++++++++++++++++ src/xspice/icm/analog/pwl/cfunc.mod | 9 ++++++--- src/xspice/icm/digital/d_genlut/cfunc.mod | 15 +++++++++++++++ src/xspice/icm/digital/d_lut/cfunc.mod | 15 +++++++++++++++ src/xspice/icm/digital/d_state/cfunc.mod | 1 + src/xspice/icm/xtradev/pswitch/cfunc.mod | 5 ++++- src/xspice/icm/xtradev/sidiode/cfunc.mod | 5 ++++- src/xspice/icm/xtradev/zener/cfunc.mod | 16 ++++++++++++++++ 8 files changed, 77 insertions(+), 5 deletions(-) diff --git a/src/xspice/icm/analog/oneshot/cfunc.mod b/src/xspice/icm/analog/oneshot/cfunc.mod index 36da2fe55..a3e67252a 100644 --- a/src/xspice/icm/analog/oneshot/cfunc.mod +++ b/src/xspice/icm/analog/oneshot/cfunc.mod @@ -51,6 +51,7 @@ NON-STANDARD FEATURES /*=== INCLUDE FILES ====================*/ #include "oneshot.h" +#include @@ -85,6 +86,20 @@ typedef struct { +static void +oneshot_callback(ARGS, Mif_Callback_Reason_t reason) +{ + switch (reason) { + case MIF_CB_DESTROY: { + Local_Data_t *loc = STATIC_VAR (locdata); + if (loc) { + free(loc); + STATIC_VAR (locdata) = NULL; + } + break; + } + } +} /*============================================================================== @@ -253,6 +268,7 @@ void cm_oneshot(ARGS) /* structure holding parms, /*** allocate static storage for *loc ***/ STATIC_VAR (locdata) = calloc (1 , sizeof ( Local_Data_t )); loc = STATIC_VAR (locdata); + CALLBACK = oneshot_callback; /* Allocate storage for breakpoint domain & pulse width values */ x = loc->control = (double *) calloc((size_t) cntl_size, sizeof(double)); diff --git a/src/xspice/icm/analog/pwl/cfunc.mod b/src/xspice/icm/analog/pwl/cfunc.mod index 2bcfb4b01..855859a1d 100644 --- a/src/xspice/icm/analog/pwl/cfunc.mod +++ b/src/xspice/icm/analog/pwl/cfunc.mod @@ -232,9 +232,12 @@ cm_pwl_callback(ARGS, Mif_Callback_Reason_t reason) double *last_x_value = STATIC_VAR (last_x_value); double *x = STATIC_VAR (x); double *y = STATIC_VAR (y); - free(last_x_value); - free(x); - free(y); + if (last_x_value) + free(last_x_value); + if (x) + free(x); + if (y) + free(y); STATIC_VAR (last_x_value) = NULL; STATIC_VAR (x) = NULL; STATIC_VAR (y) = NULL; diff --git a/src/xspice/icm/digital/d_genlut/cfunc.mod b/src/xspice/icm/digital/d_genlut/cfunc.mod index fe08321be..ff8cd1424 100644 --- a/src/xspice/icm/digital/d_genlut/cfunc.mod +++ b/src/xspice/icm/digital/d_genlut/cfunc.mod @@ -65,6 +65,20 @@ NON-STANDARD FEATURES +static void +genlut_callback(ARGS, Mif_Callback_Reason_t reason) +{ + switch (reason) { + case MIF_CB_DESTROY: { + Digital_t *loc = STATIC_VAR (locdata); + if (loc) { + free(loc); + STATIC_VAR (locdata) = NULL; + } + break; + } + } +} /*============================================================================== @@ -170,6 +184,7 @@ void cm_d_genlut(ARGS) /* allocate storage for the lookup table */ STATIC_VAR (locdata) = calloc((size_t) tablelen, sizeof(Digital_t)); lookup_table = STATIC_VAR (locdata); + CALLBACK = genlut_callback; /* allocate storage for the outputs */ cm_event_alloc(0, osize * (int) sizeof(Digital_t)); diff --git a/src/xspice/icm/digital/d_lut/cfunc.mod b/src/xspice/icm/digital/d_lut/cfunc.mod index 515e6cde4..19c3879eb 100644 --- a/src/xspice/icm/digital/d_lut/cfunc.mod +++ b/src/xspice/icm/digital/d_lut/cfunc.mod @@ -65,6 +65,20 @@ NON-STANDARD FEATURES +static void +lut_callback(ARGS, Mif_Callback_Reason_t reason) +{ + switch (reason) { + case MIF_CB_DESTROY: { + Digital_State_t *loc = STATIC_VAR (locdata); + if (loc) { + free(loc); + STATIC_VAR (locdata) = NULL; + } + break; + } + } +} /*============================================================================== @@ -136,6 +150,7 @@ void cm_d_lut(ARGS) /* allocate storage for the lookup table */ STATIC_VAR (locdata) = calloc((size_t) tablelen, sizeof(Digital_State_t)); lookup_table = STATIC_VAR (locdata); + CALLBACK = lut_callback; /* allocate storage for the outputs */ cm_event_alloc(0, sizeof(Digital_State_t)); diff --git a/src/xspice/icm/digital/d_state/cfunc.mod b/src/xspice/icm/digital/d_state/cfunc.mod index f0244400c..472738f98 100644 --- a/src/xspice/icm/digital/d_state/cfunc.mod +++ b/src/xspice/icm/digital/d_state/cfunc.mod @@ -1592,6 +1592,7 @@ static void callback(ARGS, Mif_Callback_Reason_t reason) if (table->next_state) free(table->next_state); free(table); + STATIC_VAR(table) = NULL; } } diff --git a/src/xspice/icm/xtradev/pswitch/cfunc.mod b/src/xspice/icm/xtradev/pswitch/cfunc.mod index 741eddabb..bdaa52f96 100644 --- a/src/xspice/icm/xtradev/pswitch/cfunc.mod +++ b/src/xspice/icm/xtradev/pswitch/cfunc.mod @@ -85,7 +85,10 @@ cm_pswitch_callback(ARGS, Mif_Callback_Reason_t reason) switch (reason) { case MIF_CB_DESTROY: { Local_Data_t *loc = STATIC_VAR (locdata); - free(loc); + if (loc) { + free(loc); + STATIC_VAR (locdata) = NULL; + } break; } } diff --git a/src/xspice/icm/xtradev/sidiode/cfunc.mod b/src/xspice/icm/xtradev/sidiode/cfunc.mod index 1daab7756..8406f1253 100644 --- a/src/xspice/icm/xtradev/sidiode/cfunc.mod +++ b/src/xspice/icm/xtradev/sidiode/cfunc.mod @@ -96,7 +96,10 @@ cm_sidiode_callback(ARGS, Mif_Callback_Reason_t reason) switch (reason) { case MIF_CB_DESTROY: { Local_Data_t *loc = STATIC_VAR (locdata); - free(loc); + if (loc) { + free(loc); + STATIC_VAR (locdata) = NULL; + } break; } } diff --git a/src/xspice/icm/xtradev/zener/cfunc.mod b/src/xspice/icm/xtradev/zener/cfunc.mod index 49a454eab..169cc28db 100644 --- a/src/xspice/icm/xtradev/zener/cfunc.mod +++ b/src/xspice/icm/xtradev/zener/cfunc.mod @@ -48,6 +48,7 @@ NON-STANDARD FEATURES /*=== INCLUDE FILES ====================*/ #include +#include @@ -70,6 +71,20 @@ NON-STANDARD FEATURES +static void +cm_zener_callback(ARGS, Mif_Callback_Reason_t reason) +{ + switch (reason) { + case MIF_CB_DESTROY: { + double *loc = STATIC_VAR(previous_voltage); + if (loc) { + free(loc); + STATIC_VAR(previous_voltage) = NULL; + } + break; + } + } +} /*============================================================================== @@ -180,6 +195,7 @@ void cm_zener(ARGS) /* structure holding parms, /* Set previous_voltage value to zero... */ *previous_voltage = 0.0; + CALLBACK = cm_zener_callback; } else {