From 9cd5b2a16ff5f83711ab9b6d9c5f58371e7c2061 Mon Sep 17 00:00:00 2001 From: Dirk Zimoch Date: Thu, 7 Nov 2024 14:15:55 +0100 Subject: [PATCH] fix recursion bug Previously, linking a field of e.g. a calc record to itself (calc.INPA = calc.A) directly or through a chain of other records that support to follow the link for attribures like units, precision caused an infinite resursion and crashed the ioc. Also added support for seq link 0 fields which had been added meanwhile. --- modules/database/src/std/rec/aSubRecord.c | 66 +++++++++++--------- modules/database/src/std/rec/calcRecord.c | 42 ++++++++----- modules/database/src/std/rec/calcoutRecord.c | 44 ++++++++----- modules/database/src/std/rec/seqRecord.c | 58 ++++++++++------- modules/database/src/std/rec/subRecord.c | 44 ++++++++----- 5 files changed, 151 insertions(+), 103 deletions(-) diff --git a/modules/database/src/std/rec/aSubRecord.c b/modules/database/src/std/rec/aSubRecord.c index c6ba13038b..3e4acb95c9 100644 --- a/modules/database/src/std/rec/aSubRecord.c +++ b/modules/database/src/std/rec/aSubRecord.c @@ -302,6 +302,16 @@ static long get_outlinkNumber(int fieldIndex) { return -1; } +#define do_recursion_safe(func, plink, ...) \ +do { \ + dbScanLock((dbCommon*)prec); \ + DBLINK link = *(plink); \ + (plink)->lset = NULL; \ + func(&link, __VA_ARGS__); \ + (plink)->lset = link.lset; \ + dbScanUnlock((dbCommon*)prec); \ +} while(0) + static long get_units(DBADDR *paddr, char *units) { aSubRecord *prec = (aSubRecord *)paddr->precord; @@ -309,12 +319,12 @@ static long get_units(DBADDR *paddr, char *units) linkNumber = get_inlinkNumber(dbGetFieldIndex(paddr)); if (linkNumber >= 0) { - dbGetUnits(&prec->inpa + linkNumber, units, DB_UNITS_SIZE); + do_recursion_safe(dbGetUnits, &prec->inpa + linkNumber, units, DB_UNITS_SIZE); return 0; } linkNumber = get_outlinkNumber(dbGetFieldIndex(paddr)); if (linkNumber >= 0) { - dbGetUnits(&prec->outa + linkNumber, units, DB_UNITS_SIZE); + do_recursion_safe(dbGetUnits, &prec->outa + linkNumber, units, DB_UNITS_SIZE); } return 0; } @@ -322,27 +332,25 @@ static long get_units(DBADDR *paddr, char *units) static long get_precision(const DBADDR *paddr, long *pprecision) { aSubRecord *prec = (aSubRecord *)paddr->precord; + long status; int fieldIndex = dbGetFieldIndex(paddr); int linkNumber; + short precision; *pprecision = prec->prec; linkNumber = get_inlinkNumber(fieldIndex); if (linkNumber >= 0) { - short precision; - - if (dbGetPrecision(&prec->inpa + linkNumber, &precision) == 0) - *pprecision = precision; + do_recursion_safe(status = dbGetPrecision, &prec->inpa + linkNumber, &precision); + if (status == 0) *pprecision = precision; return 0; } - linkNumber = get_outlinkNumber(fieldIndex); if (linkNumber >= 0) { - short precision; - - if (dbGetPrecision(&prec->outa + linkNumber, &precision) == 0) - *pprecision = precision; - } else - recGblGetPrec(paddr, pprecision); + do_recursion_safe(status = dbGetPrecision, &prec->outa + linkNumber, &precision); + if (status == 0) *pprecision = precision; + return 0; + } + recGblGetPrec(paddr, pprecision); return 0; } @@ -354,16 +362,16 @@ static long get_graphic_double(DBADDR *paddr, struct dbr_grDouble *pgd) linkNumber = get_inlinkNumber(fieldIndex); if (linkNumber >= 0) { - dbGetGraphicLimits(&prec->inpa + linkNumber, - &pgd->lower_disp_limit, - &pgd->upper_disp_limit); + do_recursion_safe(dbGetGraphicLimits, &prec->inpa + linkNumber, + &pgd->lower_disp_limit, + &pgd->upper_disp_limit); return 0; } linkNumber = get_outlinkNumber(fieldIndex); if (linkNumber >= 0) { - dbGetGraphicLimits(&prec->outa + linkNumber, - &pgd->lower_disp_limit, - &pgd->upper_disp_limit); + do_recursion_safe(dbGetGraphicLimits, &prec->outa + linkNumber, + &pgd->lower_disp_limit, + &pgd->upper_disp_limit); } return 0; } @@ -382,20 +390,20 @@ static long get_alarm_double(DBADDR *paddr, struct dbr_alDouble *pad) linkNumber = get_inlinkNumber(fieldIndex); if (linkNumber >= 0) { - dbGetAlarmLimits(&prec->inpa + linkNumber, - &pad->lower_alarm_limit, - &pad->lower_warning_limit, - &pad->upper_warning_limit, - &pad->upper_alarm_limit); + do_recursion_safe(dbGetAlarmLimits, &prec->inpa + linkNumber, + &pad->lower_alarm_limit, + &pad->lower_warning_limit, + &pad->upper_warning_limit, + &pad->upper_alarm_limit); return 0; } linkNumber = get_outlinkNumber(fieldIndex); if (linkNumber >= 0) { - dbGetAlarmLimits(&prec->outa + linkNumber, - &pad->lower_alarm_limit, - &pad->lower_warning_limit, - &pad->upper_warning_limit, - &pad->upper_alarm_limit); + do_recursion_safe(dbGetAlarmLimits, &prec->outa + linkNumber, + &pad->lower_alarm_limit, + &pad->lower_warning_limit, + &pad->upper_warning_limit, + &pad->upper_alarm_limit); return 0; } recGblGetAlarmDouble(paddr, pad); diff --git a/modules/database/src/std/rec/calcRecord.c b/modules/database/src/std/rec/calcRecord.c index 4467c7080a..41caf35b09 100644 --- a/modules/database/src/std/rec/calcRecord.c +++ b/modules/database/src/std/rec/calcRecord.c @@ -166,6 +166,16 @@ static long get_linkNumber(int fieldIndex) { return -1; } +#define do_recursion_safe(func, plink, ...) \ +do { \ + dbScanLock((dbCommon*)prec); \ + DBLINK link = *(plink); \ + (plink)->lset = NULL; \ + func(&link, __VA_ARGS__); \ + (plink)->lset = link.lset; \ + dbScanUnlock((dbCommon*)prec); \ +} while(0) + static long get_units(DBADDR *paddr, char *units) { calcRecord *prec = (calcRecord *)paddr->precord; @@ -174,7 +184,7 @@ static long get_units(DBADDR *paddr, char *units) if(paddr->pfldDes->field_type == DBF_DOUBLE) { linkNumber = get_linkNumber(dbGetFieldIndex(paddr)); if (linkNumber >= 0) - dbGetUnits(&prec->inpa + linkNumber, units, DB_UNITS_SIZE); + do_recursion_safe(dbGetUnits, &prec->inpa + linkNumber, units, DB_UNITS_SIZE); else strncpy(units,prec->egu,DB_UNITS_SIZE); } @@ -193,10 +203,10 @@ static long get_precision(const DBADDR *paddr, long *pprecision) linkNumber = get_linkNumber(fieldIndex); if (linkNumber >= 0) { + long status; short precision; - - if (dbGetPrecision(&prec->inpa + linkNumber, &precision) == 0) - *pprecision = precision; + do_recursion_safe(status = dbGetPrecision, &prec->inpa + linkNumber, &precision); + if (status == 0) *pprecision = precision; } else recGblGetPrec(paddr, pprecision); return 0; @@ -222,11 +232,11 @@ static long get_graphic_double(DBADDR *paddr, struct dbr_grDouble *pgd) break; default: linkNumber = get_linkNumber(fieldIndex); - if (linkNumber >= 0) { - dbGetGraphicLimits(&prec->inpa + linkNumber, - &pgd->lower_disp_limit, - &pgd->upper_disp_limit); - } else + if (linkNumber >= 0) + do_recursion_safe(dbGetGraphicLimits, &prec->inpa + linkNumber, + &pgd->lower_disp_limit, + &pgd->upper_disp_limit); + else recGblGetGraphicDouble(paddr,pgd); } return 0; @@ -267,13 +277,13 @@ static long get_alarm_double(DBADDR *paddr, struct dbr_alDouble *pad) pad->upper_alarm_limit = prec->hhsv ? prec->hihi : epicsNAN; } else { linkNumber = get_linkNumber(fieldIndex); - if (linkNumber >= 0) { - dbGetAlarmLimits(&prec->inpa + linkNumber, - &pad->lower_alarm_limit, - &pad->lower_warning_limit, - &pad->upper_warning_limit, - &pad->upper_alarm_limit); - } else + if (linkNumber >= 0) + do_recursion_safe(dbGetAlarmLimits, &prec->inpa + linkNumber, + &pad->lower_alarm_limit, + &pad->lower_warning_limit, + &pad->upper_warning_limit, + &pad->upper_alarm_limit); + else recGblGetAlarmDouble(paddr, pad); } return 0; diff --git a/modules/database/src/std/rec/calcoutRecord.c b/modules/database/src/std/rec/calcoutRecord.c index 319994596d..8a474f4e81 100644 --- a/modules/database/src/std/rec/calcoutRecord.c +++ b/modules/database/src/std/rec/calcoutRecord.c @@ -413,6 +413,16 @@ static long get_linkNumber(int fieldIndex) { return -1; } +#define do_recursion_safe(func, plink, ...) \ +do { \ + dbScanLock((dbCommon*)prec); \ + DBLINK link = *(plink); \ + (plink)->lset = NULL; \ + func(&link, __VA_ARGS__); \ + (plink)->lset = link.lset; \ + dbScanUnlock((dbCommon*)prec); \ +} while(0) + static long get_units(DBADDR *paddr, char *units) { calcoutRecord *prec = (calcoutRecord *)paddr->precord; @@ -425,9 +435,9 @@ static long get_units(DBADDR *paddr, char *units) } if(paddr->pfldDes->field_type == DBF_DOUBLE) { - linkNumber = get_linkNumber(dbGetFieldIndex(paddr)); + linkNumber = get_linkNumber(fieldIndex); if (linkNumber >= 0) - dbGetUnits(&prec->inpa + linkNumber, units, DB_UNITS_SIZE); + do_recursion_safe(dbGetUnits, &prec->inpa + linkNumber, units, DB_UNITS_SIZE); else strncpy(units,prec->egu,DB_UNITS_SIZE); } @@ -451,10 +461,10 @@ static long get_precision(const DBADDR *paddr, long *pprecision) linkNumber = get_linkNumber(fieldIndex); if (linkNumber >= 0) { + long status; short precision; - - if (dbGetPrecision(&prec->inpa + linkNumber, &precision) == 0) - *pprecision = precision; + do_recursion_safe(status = dbGetPrecision, &prec->inpa + linkNumber, &precision); + if (status == 0) *pprecision = precision; } else recGblGetPrec(paddr, pprecision); return 0; @@ -484,11 +494,11 @@ static long get_graphic_double(DBADDR *paddr, struct dbr_grDouble *pgd) break; default: linkNumber = get_linkNumber(fieldIndex); - if (linkNumber >= 0) { - dbGetGraphicLimits(&prec->inpa + linkNumber, - &pgd->lower_disp_limit, - &pgd->upper_disp_limit); - } else + if (linkNumber >= 0) + do_recursion_safe(dbGetGraphicLimits, &prec->inpa + linkNumber, + &pgd->lower_disp_limit, + &pgd->upper_disp_limit); + else recGblGetGraphicDouble(paddr,pgd); } return 0; @@ -533,13 +543,13 @@ static long get_alarm_double(DBADDR *paddr, struct dbr_alDouble *pad) pad->lower_alarm_limit = prec->llsv ? prec->lolo : epicsNAN; } else { linkNumber = get_linkNumber(fieldIndex); - if (linkNumber >= 0) { - dbGetAlarmLimits(&prec->inpa + linkNumber, - &pad->lower_alarm_limit, - &pad->lower_warning_limit, - &pad->upper_warning_limit, - &pad->upper_alarm_limit); - } else + if (linkNumber >= 0) + do_recursion_safe(dbGetAlarmLimits, &prec->inpa + linkNumber, + &pad->lower_alarm_limit, + &pad->lower_warning_limit, + &pad->upper_warning_limit, + &pad->upper_alarm_limit); + else recGblGetAlarmDouble(paddr, pad); } return 0; diff --git a/modules/database/src/std/rec/seqRecord.c b/modules/database/src/std/rec/seqRecord.c index cd6212da1e..4311e3c506 100644 --- a/modules/database/src/std/rec/seqRecord.c +++ b/modules/database/src/std/rec/seqRecord.c @@ -279,39 +279,49 @@ static void processCallback(epicsCallback *arg) #define get_dol(prec, fieldOffset) \ &((linkGrp *) &prec->dly0)[fieldOffset >> 2].dol +#define do_recursion_safe(func, plink, ...) \ +do { \ + dbScanLock((dbCommon*)prec); \ + DBLINK link = *(plink); \ + (plink)->lset = NULL; \ + func(&link, __VA_ARGS__); \ + (plink)->lset = link.lset; \ + dbScanUnlock((dbCommon*)prec); \ +} while(0) + static long get_units(DBADDR *paddr, char *units) { seqRecord *prec = (seqRecord *) paddr->precord; - int fieldOffset = dbGetFieldIndex(paddr) - indexof(DLY1); + int fieldOffset = dbGetFieldIndex(paddr) - indexof(DLY0); if (fieldOffset >= 0) - switch (fieldOffset & 2) { + switch (fieldOffset & 3) { case 0: /* DLYn */ strcpy(units, "s"); break; case 2: /* DOn */ - dbGetUnits(get_dol(prec, fieldOffset), - units, DB_UNITS_SIZE); - } + do_recursion_safe(dbGetUnits, get_dol(prec, fieldOffset), + units, DB_UNITS_SIZE); + } return 0; } static long get_precision(const DBADDR *paddr, long *pprecision) { seqRecord *prec = (seqRecord *) paddr->precord; - int fieldOffset = dbGetFieldIndex(paddr) - indexof(DLY1); + long status; + int fieldOffset = dbGetFieldIndex(paddr) - indexof(DLY0); short precision; if (fieldOffset >= 0) - switch (fieldOffset & 2) { + switch (fieldOffset & 3) { case 0: /* DLYn */ *pprecision = seqDLYprecision; return 0; case 2: /* DOn */ - if (dbGetPrecision(get_dol(prec, fieldOffset), &precision) == 0) { - *pprecision = precision; - return 0; - } + do_recursion_safe(status = dbGetPrecision, get_dol(prec, fieldOffset), &precision); + if (status == 0) *pprecision = precision; + return 0; } *pprecision = prec->prec; recGblGetPrec(paddr, pprecision); @@ -321,18 +331,18 @@ static long get_precision(const DBADDR *paddr, long *pprecision) static long get_graphic_double(DBADDR *paddr, struct dbr_grDouble *pgd) { seqRecord *prec = (seqRecord *) paddr->precord; - int fieldOffset = dbGetFieldIndex(paddr) - indexof(DLY1); + int fieldOffset = dbGetFieldIndex(paddr) - indexof(DLY0); if (fieldOffset >= 0) - switch (fieldOffset & 2) { + switch (fieldOffset & 3) { case 0: /* DLYn */ pgd->lower_disp_limit = 0.0; - pgd->lower_disp_limit = 10.0; + pgd->upper_disp_limit = 10.0; return 0; case 2: /* DOn */ - dbGetGraphicLimits(get_dol(prec, fieldOffset), - &pgd->lower_disp_limit, - &pgd->upper_disp_limit); + do_recursion_safe(dbGetGraphicLimits, get_dol(prec, fieldOffset), + &pgd->lower_disp_limit, + &pgd->upper_disp_limit); return 0; } recGblGetGraphicDouble(paddr, pgd); @@ -341,9 +351,9 @@ static long get_graphic_double(DBADDR *paddr, struct dbr_grDouble *pgd) static long get_control_double(DBADDR *paddr, struct dbr_ctrlDouble *pcd) { - int fieldOffset = dbGetFieldIndex(paddr) - indexof(DLY1); + int fieldOffset = dbGetFieldIndex(paddr) - indexof(DLY0); - if (fieldOffset >= 0 && (fieldOffset & 2) == 0) { /* DLYn */ + if (fieldOffset >= 0 && (fieldOffset & 3) == 0) { /* DLYn */ pcd->lower_ctrl_limit = 0.0; pcd->upper_ctrl_limit = seqDLYlimit; } @@ -355,12 +365,12 @@ static long get_control_double(DBADDR *paddr, struct dbr_ctrlDouble *pcd) static long get_alarm_double(DBADDR *paddr, struct dbr_alDouble *pad) { seqRecord *prec = (seqRecord *) paddr->precord; - int fieldOffset = dbGetFieldIndex(paddr) - indexof(DLY1); + int fieldOffset = dbGetFieldIndex(paddr) - indexof(DLY0); - if (fieldOffset >= 0 && (fieldOffset & 2) == 2) /* DOn */ - dbGetAlarmLimits(get_dol(prec, fieldOffset), - &pad->lower_alarm_limit, &pad->lower_warning_limit, - &pad->upper_warning_limit, &pad->upper_alarm_limit); + if (fieldOffset >= 0 && (fieldOffset & 3) == 2) /* DOn */ + do_recursion_safe(dbGetAlarmLimits, get_dol(prec, fieldOffset), + &pad->lower_alarm_limit, &pad->lower_warning_limit, + &pad->upper_warning_limit, &pad->upper_alarm_limit); else recGblGetAlarmDouble(paddr, pad); return 0; diff --git a/modules/database/src/std/rec/subRecord.c b/modules/database/src/std/rec/subRecord.c index 1ef3af3dea..7fd2d5c23d 100644 --- a/modules/database/src/std/rec/subRecord.c +++ b/modules/database/src/std/rec/subRecord.c @@ -201,6 +201,16 @@ static long get_linkNumber(int fieldIndex) { return -1; } +#define do_recursion_safe(func, plink, ...) \ +do { \ + dbScanLock((dbCommon*)prec); \ + DBLINK link = *(plink); \ + (plink)->lset = NULL; \ + func(&link, __VA_ARGS__); \ + (plink)->lset = link.lset; \ + dbScanUnlock((dbCommon*)prec); \ +} while(0) + static long get_units(DBADDR *paddr, char *units) { subRecord *prec = (subRecord *)paddr->precord; @@ -209,7 +219,7 @@ static long get_units(DBADDR *paddr, char *units) if(paddr->pfldDes->field_type == DBF_DOUBLE) { linkNumber = get_linkNumber(dbGetFieldIndex(paddr)); if (linkNumber >= 0) - dbGetUnits(&prec->inpa + linkNumber, units, DB_UNITS_SIZE); + do_recursion_safe(dbGetUnits, &prec->inpa + linkNumber, units, DB_UNITS_SIZE); else strncpy(units,prec->egu,DB_UNITS_SIZE); } @@ -228,10 +238,10 @@ static long get_precision(const DBADDR *paddr, long *pprecision) linkNumber = get_linkNumber(fieldIndex); if (linkNumber >= 0) { + long status; short precision; - - if (dbGetPrecision(&prec->inpa + linkNumber, &precision) == 0) - *pprecision = precision; + do_recursion_safe(status = dbGetPrecision, &prec->inpa + linkNumber, &precision); + if (status == 0) *pprecision = precision; } else recGblGetPrec(paddr, pprecision); return 0; @@ -257,11 +267,11 @@ static long get_graphic_double(DBADDR *paddr, struct dbr_grDouble *pgd) break; default: linkNumber = get_linkNumber(fieldIndex); - if (linkNumber >= 0) { - dbGetGraphicLimits(&prec->inpa + linkNumber, - &pgd->lower_disp_limit, - &pgd->upper_disp_limit); - } else + if (linkNumber >= 0) + do_recursion_safe(dbGetGraphicLimits, &prec->inpa + linkNumber, + &pgd->lower_disp_limit, + &pgd->upper_disp_limit); + else recGblGetGraphicDouble(paddr,pgd); } return 0; @@ -302,14 +312,14 @@ static long get_alarm_double(DBADDR *paddr, struct dbr_alDouble *pad) pad->lower_alarm_limit = prec->llsv ? prec->lolo : epicsNAN; } else { linkNumber = get_linkNumber(fieldIndex); - if (linkNumber >= 0) { - dbGetAlarmLimits(&prec->inpa + linkNumber, - &pad->lower_alarm_limit, - &pad->lower_warning_limit, - &pad->upper_warning_limit, - &pad->upper_alarm_limit); - } else - recGblGetAlarmDouble(paddr, pad); + if (linkNumber >= 0) + do_recursion_safe(dbGetAlarmLimits, &prec->inpa + linkNumber, + &pad->lower_alarm_limit, + &pad->lower_warning_limit, + &pad->upper_warning_limit, + &pad->upper_alarm_limit); + else + recGblGetAlarmDouble(paddr, pad); } return 0; }