From a19458e11dfca701a058a87f0473bd199487384a Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 16 Sep 2022 15:59:41 +0200 Subject: [PATCH] bond: assert integer range in _nm_setting_bond_opt_value_as_u{8,16,32}() The bond setting does some minimal validation of the options. At least for those number typed values, it validates that the string can be interpreted as a number and is within a certain range. Add nm_assert() checks to our opt_value_u$SIZE() functions, that the requested option is validated to be in a range which is sufficiently narrow to be converted to the requested type. If that were not the case, we would need some special handling (or question whether the option should be retrieved as this type). --- src/libnm-core-impl/nm-setting-bond.c | 63 +++++++++++++++--------- src/libnm-core-impl/tests/test-setting.c | 52 +++++++++++++++++++ src/libnm-core-intern/nm-core-internal.h | 4 +- 3 files changed, 95 insertions(+), 24 deletions(-) diff --git a/src/libnm-core-impl/nm-setting-bond.c b/src/libnm-core-impl/nm-setting-bond.c index 230b2785ac..117f9ee57e 100644 --- a/src/libnm-core-impl/nm-setting-bond.c +++ b/src/libnm-core-impl/nm-setting-bond.c @@ -764,37 +764,56 @@ _nm_setting_bond_get_option_type(NMSettingBond *setting, const char *name) return option_meta->opt_type; } -guint32 -_nm_setting_bond_opt_value_as_u32(NMSettingBond *s_bond, const char *opt) +#define _opt_value_as_u64(s_bond, opt, v_max) \ + ({ \ + const OptionMeta *_meta; \ + NMSettingBond *_s_bond = (s_bond); \ + const char *_opt = (opt); \ + const guint64 _v_max = (v_max); \ + const char *_s; \ + guint64 _val; \ + \ + nm_assert(NM_IS_SETTING_BOND(_s_bond)); \ + nm_assert(_opt); \ + \ + _meta = _get_option_meta(_opt); \ + \ + nm_assert(_meta); \ + nm_assert(_meta->opt_type == NM_BOND_OPTION_TYPE_INT); \ + nm_assert(_meta->min < _meta->max); \ + nm_assert(_meta->max <= _v_max); \ + nm_assert(_meta->val); \ + \ + _s = nm_setting_bond_get_option_normalized(_s_bond, _opt); \ + if (_s) { \ + _val = _nm_utils_ascii_str_to_uint64(_s, 10, _meta->min, _meta->max, 0); \ + /* Note that _s is only a valid integer, if the profile verifies. We require + * that the caller only calls these functions on valid profile. */ \ + nm_assert(errno == 0); \ + } else { \ + _val = 0; \ + errno = EINVAL; \ + } \ + \ + _val; \ + }) + +guint8 +_nm_setting_bond_opt_value_as_u8(NMSettingBond *s_bond, const char *opt) { - nm_assert(_get_option_meta(opt)->opt_type == NM_BOND_OPTION_TYPE_INT); - return _nm_utils_ascii_str_to_uint64(nm_setting_bond_get_option_normalized(s_bond, opt), - 10, - 0, - G_MAXUINT32, - 0); + return _opt_value_as_u64(s_bond, opt, G_MAXUINT8); } guint16 _nm_setting_bond_opt_value_as_u16(NMSettingBond *s_bond, const char *opt) { - nm_assert(_get_option_meta(opt)->opt_type == NM_BOND_OPTION_TYPE_INT); - return _nm_utils_ascii_str_to_uint64(nm_setting_bond_get_option_normalized(s_bond, opt), - 10, - 0, - G_MAXUINT16, - 0); + return _opt_value_as_u64(s_bond, opt, G_MAXUINT16); } -guint8 -_nm_setting_bond_opt_value_as_u8(NMSettingBond *s_bond, const char *opt) +guint32 +_nm_setting_bond_opt_value_as_u32(NMSettingBond *s_bond, const char *opt) { - nm_assert(_get_option_meta(opt)->opt_type == NM_BOND_OPTION_TYPE_INT); - return _nm_utils_ascii_str_to_uint64(nm_setting_bond_get_option_normalized(s_bond, opt), - 10, - 0, - G_MAXUINT8, - 0); + return _opt_value_as_u64(s_bond, opt, G_MAXUINT32); } /*****************************************************************************/ diff --git a/src/libnm-core-impl/tests/test-setting.c b/src/libnm-core-impl/tests/test-setting.c index 429db52294..ed3cce969d 100644 --- a/src/libnm-core-impl/tests/test-setting.c +++ b/src/libnm-core-impl/tests/test-setting.c @@ -5068,6 +5068,56 @@ test_6lowpan_1(void) /*****************************************************************************/ +static void +test_bond_meta(void) +{ + gs_unref_object NMConnection *con = NULL; + NMSettingBond *set; + char sbuf[200]; + + create_bond_connection(&con, &set); + + g_assert_cmpstr(nm_setting_bond_get_option_normalized(set, NM_SETTING_BOND_OPTION_MODE), + ==, + "balance-rr"); + +#define _A(_nm_setting_bond_opt_value_as_xxx, set, opt, value, errsv) \ + G_STMT_START \ + { \ + g_assert_cmpint(_nm_setting_bond_opt_value_as_xxx((set), (opt)), ==, (value)); \ + g_assert_cmpint(errno, ==, (errsv)); \ + } \ + G_STMT_END + + _A(_nm_setting_bond_opt_value_as_u32, set, NM_SETTING_BOND_OPTION_MIIMON, 100, 0); + _A(_nm_setting_bond_opt_value_as_u32, set, NM_SETTING_BOND_OPTION_UPDELAY, 0, 0); + _A(_nm_setting_bond_opt_value_as_u32, set, NM_SETTING_BOND_OPTION_DOWNDELAY, 0, 0); + _A(_nm_setting_bond_opt_value_as_u32, set, NM_SETTING_BOND_OPTION_ARP_INTERVAL, 0, 0); + _A(_nm_setting_bond_opt_value_as_u32, set, NM_SETTING_BOND_OPTION_RESEND_IGMP, 1, 0); + _A(_nm_setting_bond_opt_value_as_u32, set, NM_SETTING_BOND_OPTION_MIN_LINKS, 0, 0); + _A(_nm_setting_bond_opt_value_as_u32, set, NM_SETTING_BOND_OPTION_LP_INTERVAL, 1, 0); + _A(_nm_setting_bond_opt_value_as_u32, set, NM_SETTING_BOND_OPTION_PACKETS_PER_SLAVE, 1, 0); + _A(_nm_setting_bond_opt_value_as_u32, set, NM_SETTING_BOND_OPTION_PEER_NOTIF_DELAY, 0, 0); + _A(_nm_setting_bond_opt_value_as_u16, set, NM_SETTING_BOND_OPTION_AD_ACTOR_SYS_PRIO, 0, EINVAL); + _A(_nm_setting_bond_opt_value_as_u16, set, NM_SETTING_BOND_OPTION_AD_USER_PORT_KEY, 0, EINVAL); + _A(_nm_setting_bond_opt_value_as_u8, set, NM_SETTING_BOND_OPTION_NUM_GRAT_ARP, 1, 0); + _A(_nm_setting_bond_opt_value_as_u8, set, NM_SETTING_BOND_OPTION_ALL_SLAVES_ACTIVE, 0, 0); + + nm_setting_bond_add_option(set, NM_SETTING_BOND_OPTION_ARP_INTERVAL, "5"); + _A(_nm_setting_bond_opt_value_as_u32, set, NM_SETTING_BOND_OPTION_ARP_INTERVAL, 5, 0); + + nm_setting_bond_add_option(set, + NM_SETTING_BOND_OPTION_ARP_INTERVAL, + nm_sprintf_buf(sbuf, "%d", G_MAXINT)); + _A(_nm_setting_bond_opt_value_as_u32, set, NM_SETTING_BOND_OPTION_ARP_INTERVAL, G_MAXINT, 0); + + nm_setting_bond_add_option(set, NM_SETTING_BOND_OPTION_MODE, "802.3ad"); + _A(_nm_setting_bond_opt_value_as_u16, set, NM_SETTING_BOND_OPTION_AD_ACTOR_SYS_PRIO, 65535, 0); + _A(_nm_setting_bond_opt_value_as_u16, set, NM_SETTING_BOND_OPTION_AD_USER_PORT_KEY, 0, 0); +} + +/*****************************************************************************/ + NMTST_DEFINE(); int @@ -5185,5 +5235,7 @@ main(int argc, char **argv) g_test_add_func("/libnm/test_setting_metadata", test_setting_metadata); + g_test_add_func("/libnm/test_bond_meta", test_bond_meta); + return g_test_run(); } diff --git a/src/libnm-core-intern/nm-core-internal.h b/src/libnm-core-intern/nm-core-internal.h index ee4cc25b42..ecfb2cd44f 100644 --- a/src/libnm-core-intern/nm-core-internal.h +++ b/src/libnm-core-intern/nm-core-internal.h @@ -517,9 +517,9 @@ NMConnectionMultiConnect _nm_connection_get_multi_connect(NMConnection *connecti gboolean _nm_setting_bond_option_supported(const char *option, NMBondMode mode); -guint32 _nm_setting_bond_opt_value_as_u32(NMSettingBond *s_bond, const char *opt); -guint16 _nm_setting_bond_opt_value_as_u16(NMSettingBond *s_bond, const char *opt); guint8 _nm_setting_bond_opt_value_as_u8(NMSettingBond *s_bond, const char *opt); +guint16 _nm_setting_bond_opt_value_as_u16(NMSettingBond *s_bond, const char *opt); +guint32 _nm_setting_bond_opt_value_as_u32(NMSettingBond *s_bond, const char *opt); /*****************************************************************************/