From 0e5b9cdd5cf8a5c4eb29000905a30b929a41a44d Mon Sep 17 00:00:00 2001 From: Krzysztof Matula Date: Mon, 15 Apr 2019 00:25:28 +0200 Subject: [PATCH 1/6] Prevent crashing when OSD timers are configured incorrectly. --- src/main/cms/cms_menu_osd.c | 4 ++++ src/main/osd/osd.c | 14 ++++++++++---- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/main/cms/cms_menu_osd.c b/src/main/cms/cms_menu_osd.c index 9748a41996..7116664b51 100644 --- a/src/main/cms/cms_menu_osd.c +++ b/src/main/cms/cms_menu_osd.c @@ -205,6 +205,10 @@ static long menuTimersOnEnter(void) timerSource[i] = OSD_TIMER_SRC(timer); timerPrecision[i] = OSD_TIMER_PRECISION(timer); timerAlarm[i] = OSD_TIMER_ALARM(timer); + if ((int)timerSource[i] >= (int)OSD_TIMER_SRC_COUNT) + timerSource[i] = OSD_TIMER_SRC_ON; + if ((int)timerPrecision[i] >= (int)OSD_TIMER_PREC_COUNT) + timerPrecision[i] = OSD_TIMER_PREC_SECOND; } return 0; diff --git a/src/main/osd/osd.c b/src/main/osd/osd.c index cac20672d5..7802b6b6c2 100644 --- a/src/main/osd/osd.c +++ b/src/main/osd/osd.c @@ -483,13 +483,19 @@ static void osdShowStats(uint16_t endBatteryVoltage) } if (osdStatGetState(OSD_STAT_TIMER_1)) { - osdFormatTimer(buff, false, (OSD_TIMER_SRC(osdConfig()->timers[OSD_TIMER_1]) == OSD_TIMER_SRC_ON ? false : true), OSD_TIMER_1); - osdDisplayStatisticLabel(top++, osdTimerSourceNames[OSD_TIMER_SRC(osdConfig()->timers[OSD_TIMER_1])], buff); + int src = OSD_TIMER_SRC(osdConfig()->timers[OSD_TIMER_1]); + if (src >= OSD_TIMER_SRC_COUNT) + src = 0; + osdFormatTimer(buff, false, (src == OSD_TIMER_SRC_ON ? false : true), OSD_TIMER_1); + osdDisplayStatisticLabel(top++, osdTimerSourceNames[src], buff); } if (osdStatGetState(OSD_STAT_TIMER_2)) { - osdFormatTimer(buff, false, (OSD_TIMER_SRC(osdConfig()->timers[OSD_TIMER_2]) == OSD_TIMER_SRC_ON ? false : true), OSD_TIMER_2); - osdDisplayStatisticLabel(top++, osdTimerSourceNames[OSD_TIMER_SRC(osdConfig()->timers[OSD_TIMER_2])], buff); + int src = OSD_TIMER_SRC(osdConfig()->timers[OSD_TIMER_2]); + if (src >= OSD_TIMER_SRC_COUNT) + src = 0; + osdFormatTimer(buff, false, (src == OSD_TIMER_SRC_ON ? false : true), OSD_TIMER_2); + osdDisplayStatisticLabel(top++, osdTimerSourceNames[src], buff); } if (osdStatGetState(OSD_STAT_MAX_ALTITUDE)) { From a56ac05303fc555ceaa385cc9b95c6d4ecb9cf87 Mon Sep 17 00:00:00 2001 From: Krzysztof Matula Date: Mon, 15 Apr 2019 18:29:42 +0200 Subject: [PATCH 2/6] Revert "Prevent crashing when OSD timers are configured incorrectly." This reverts commit db2f3641c92a7db1aec93581592861860f292b23. --- src/main/cms/cms_menu_osd.c | 4 ---- src/main/osd/osd.c | 14 ++++---------- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/src/main/cms/cms_menu_osd.c b/src/main/cms/cms_menu_osd.c index 7116664b51..9748a41996 100644 --- a/src/main/cms/cms_menu_osd.c +++ b/src/main/cms/cms_menu_osd.c @@ -205,10 +205,6 @@ static long menuTimersOnEnter(void) timerSource[i] = OSD_TIMER_SRC(timer); timerPrecision[i] = OSD_TIMER_PRECISION(timer); timerAlarm[i] = OSD_TIMER_ALARM(timer); - if ((int)timerSource[i] >= (int)OSD_TIMER_SRC_COUNT) - timerSource[i] = OSD_TIMER_SRC_ON; - if ((int)timerPrecision[i] >= (int)OSD_TIMER_PREC_COUNT) - timerPrecision[i] = OSD_TIMER_PREC_SECOND; } return 0; diff --git a/src/main/osd/osd.c b/src/main/osd/osd.c index 7802b6b6c2..cac20672d5 100644 --- a/src/main/osd/osd.c +++ b/src/main/osd/osd.c @@ -483,19 +483,13 @@ static void osdShowStats(uint16_t endBatteryVoltage) } if (osdStatGetState(OSD_STAT_TIMER_1)) { - int src = OSD_TIMER_SRC(osdConfig()->timers[OSD_TIMER_1]); - if (src >= OSD_TIMER_SRC_COUNT) - src = 0; - osdFormatTimer(buff, false, (src == OSD_TIMER_SRC_ON ? false : true), OSD_TIMER_1); - osdDisplayStatisticLabel(top++, osdTimerSourceNames[src], buff); + osdFormatTimer(buff, false, (OSD_TIMER_SRC(osdConfig()->timers[OSD_TIMER_1]) == OSD_TIMER_SRC_ON ? false : true), OSD_TIMER_1); + osdDisplayStatisticLabel(top++, osdTimerSourceNames[OSD_TIMER_SRC(osdConfig()->timers[OSD_TIMER_1])], buff); } if (osdStatGetState(OSD_STAT_TIMER_2)) { - int src = OSD_TIMER_SRC(osdConfig()->timers[OSD_TIMER_2]); - if (src >= OSD_TIMER_SRC_COUNT) - src = 0; - osdFormatTimer(buff, false, (src == OSD_TIMER_SRC_ON ? false : true), OSD_TIMER_2); - osdDisplayStatisticLabel(top++, osdTimerSourceNames[src], buff); + osdFormatTimer(buff, false, (OSD_TIMER_SRC(osdConfig()->timers[OSD_TIMER_2]) == OSD_TIMER_SRC_ON ? false : true), OSD_TIMER_2); + osdDisplayStatisticLabel(top++, osdTimerSourceNames[OSD_TIMER_SRC(osdConfig()->timers[OSD_TIMER_2])], buff); } if (osdStatGetState(OSD_STAT_MAX_ALTITUDE)) { From e256639909cd450d4e5d312903fd9d99b90df0d2 Mon Sep 17 00:00:00 2001 From: Krzysztof Matula Date: Mon, 15 Apr 2019 18:56:41 +0200 Subject: [PATCH 3/6] OSD timers safety checks --- src/main/fc/config.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/main/fc/config.c b/src/main/fc/config.c index cf72a214e8..09af4816ae 100644 --- a/src/main/fc/config.c +++ b/src/main/fc/config.c @@ -52,6 +52,8 @@ #include "io/serial.h" #include "io/gps.h" +#include "osd/osd.h" + #include "pg/beeper.h" #include "pg/beeper_dev.h" #include "pg/rx.h" @@ -466,6 +468,21 @@ static void validateAndFixConfig(void) #if defined(TARGET_VALIDATECONFIG) targetValidateConfiguration(); #endif + +#if defined(USE_OSD) + for (int i = 0; i < OSD_TIMER_COUNT; i++) { + const uint16_t timer = osdConfig()->timers[i]; + int src = OSD_TIMER_SRC(timer); + int prc = OSD_TIMER_PRECISION(timer); + if (src >= OSD_TIMER_SRC_COUNT) { + src = 0; + } + if (prc >= OSD_TIMER_PREC_COUNT) { + prc = OSD_TIMER_PREC_COUNT; + } + osdConfigMutable()->timers[i] = OSD_TIMER(src, prc, OSD_TIMER_ALARM(timer)); + } +#endif } void validateAndFixGyroConfig(void) From 078e7fc0a5754f5b8c3c977e78de021a80e51b98 Mon Sep 17 00:00:00 2001 From: Krzysztof Matula Date: Mon, 15 Apr 2019 19:04:34 +0200 Subject: [PATCH 4/6] improved fix --- src/main/fc/config.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/fc/config.c b/src/main/fc/config.c index 09af4816ae..4ff51b3ea8 100644 --- a/src/main/fc/config.c +++ b/src/main/fc/config.c @@ -478,7 +478,7 @@ static void validateAndFixConfig(void) src = 0; } if (prc >= OSD_TIMER_PREC_COUNT) { - prc = OSD_TIMER_PREC_COUNT; + prc = 0; } osdConfigMutable()->timers[i] = OSD_TIMER(src, prc, OSD_TIMER_ALARM(timer)); } From b8b5b133dc1dbbf664827a65b96b48bd795cf6c0 Mon Sep 17 00:00:00 2001 From: Krzysztof Matula Date: Mon, 15 Apr 2019 23:24:30 +0200 Subject: [PATCH 5/6] OSD timers safety - code review changes --- src/main/fc/config.c | 13 ++++--------- src/main/osd/osd.c | 9 +++++++-- src/main/osd/osd.h | 2 ++ 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/main/fc/config.c b/src/main/fc/config.c index 4ff51b3ea8..71d85d61be 100644 --- a/src/main/fc/config.c +++ b/src/main/fc/config.c @@ -471,16 +471,11 @@ static void validateAndFixConfig(void) #if defined(USE_OSD) for (int i = 0; i < OSD_TIMER_COUNT; i++) { - const uint16_t timer = osdConfig()->timers[i]; - int src = OSD_TIMER_SRC(timer); - int prc = OSD_TIMER_PRECISION(timer); - if (src >= OSD_TIMER_SRC_COUNT) { - src = 0; + const uint16_t t = osdConfig()->timers[i]; + if (OSD_TIMER_SRC(t) >= OSD_TIMER_SRC_COUNT || + OSD_TIMER_PRECISION(t) >= OSD_TIMER_PREC_COUNT) { + osdConfigMutable()->timers[i] = osdTimerDefault[i]; } - if (prc >= OSD_TIMER_PREC_COUNT) { - prc = 0; - } - osdConfigMutable()->timers[i] = OSD_TIMER(src, prc, OSD_TIMER_ALARM(timer)); } #endif } diff --git a/src/main/osd/osd.c b/src/main/osd/osd.c index cac20672d5..eab4ea688f 100644 --- a/src/main/osd/osd.c +++ b/src/main/osd/osd.c @@ -197,6 +197,11 @@ static void osdDrawElements(timeUs_t currentTimeUs) osdDrawActiveElements(osdDisplayPort, currentTimeUs); } +const uint16_t osdTimerDefault[OSD_TIMER_COUNT] = { + OSD_TIMER(OSD_TIMER_SRC_ON, OSD_TIMER_PREC_SECOND, 10), + OSD_TIMER(OSD_TIMER_SRC_TOTAL_ARMED, OSD_TIMER_PREC_SECOND, 10) +}; + void pgResetFn_osdConfig(osdConfig_t *osdConfig) { // Position elements near centre of screen and disabled by default @@ -234,8 +239,8 @@ void pgResetFn_osdConfig(osdConfig_t *osdConfig) osdWarnSetState(i, true); } - osdConfig->timers[OSD_TIMER_1] = OSD_TIMER(OSD_TIMER_SRC_ON, OSD_TIMER_PREC_SECOND, 10); - osdConfig->timers[OSD_TIMER_2] = OSD_TIMER(OSD_TIMER_SRC_TOTAL_ARMED, OSD_TIMER_PREC_SECOND, 10); + osdConfig->timers[OSD_TIMER_1] = osdTimerDefault[OSD_TIMER_1]; + osdConfig->timers[OSD_TIMER_2] = osdTimerDefault[OSD_TIMER_2]; osdConfig->overlay_radio_mode = 2; diff --git a/src/main/osd/osd.h b/src/main/osd/osd.h index cef63acbe3..ac78718cc8 100644 --- a/src/main/osd/osd.h +++ b/src/main/osd/osd.h @@ -215,6 +215,8 @@ STATIC_ASSERT(OSD_WARNING_COUNT <= 32, osdwarnings_overflow); #define OSD_GPS_RESCUE_DISABLED_WARNING_DURATION_US 3000000 // 3 seconds +extern const uint16_t osdTimerDefault[OSD_TIMER_COUNT]; + typedef struct osdConfig_s { uint16_t item_pos[OSD_ITEM_COUNT]; From 5a737fd7782d6deb870bd0c0d5c4aa2d86c216f0 Mon Sep 17 00:00:00 2001 From: Krzysztof Matula Date: Mon, 15 Apr 2019 23:27:54 +0200 Subject: [PATCH 6/6] OSD timers safety - code review changes 2 --- src/main/fc/config.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/fc/config.c b/src/main/fc/config.c index 71d85d61be..6ce074f836 100644 --- a/src/main/fc/config.c +++ b/src/main/fc/config.c @@ -465,10 +465,6 @@ static void validateAndFixConfig(void) } #endif -#if defined(TARGET_VALIDATECONFIG) - targetValidateConfiguration(); -#endif - #if defined(USE_OSD) for (int i = 0; i < OSD_TIMER_COUNT; i++) { const uint16_t t = osdConfig()->timers[i]; @@ -478,6 +474,10 @@ static void validateAndFixConfig(void) } } #endif + +#if defined(TARGET_VALIDATECONFIG) + targetValidateConfiguration(); +#endif } void validateAndFixGyroConfig(void)