libnm-core/utils: update hwaddr utilities

_nm_utils_hwaddr_length() did a validation of the string
and returned the length of the address. In all cases where
we were interested in that, we also either want to validate
the address, get the address in binary form, or canonicalize
the address.

We can avoid these duplicate checks, by using _nm_utils_hwaddr_aton()
which both does the parsing and returning the length.

(cherry picked from commit e5fe5a4c03)
This commit is contained in:
Thomas Haller 2016-10-13 17:51:07 +02:00
parent d242fdc319
commit 977fbf7089
4 changed files with 206 additions and 161 deletions

View file

@ -123,7 +123,10 @@ guint32 _nm_setting_get_setting_priority (NMSetting *setting);
gboolean _nm_setting_get_property (NMSetting *setting, const char *name, GValue *value);
guint _nm_utils_hwaddr_length (const char *asc);
#define NM_UTILS_HWADDR_LEN_MAX_STR (NM_UTILS_HWADDR_LEN_MAX * 3)
guint8 *_nm_utils_hwaddr_aton (const char *asc, gpointer buffer, gsize buffer_length, gsize *out_length);
const char *nm_utils_hwaddr_ntoa_buf (gconstpointer addr, gsize addr_len, gboolean upper_case, char *buf, gsize buf_len);
char *_nm_utils_bin2str (gconstpointer addr, gsize length, gboolean upper_case);

View file

@ -2965,17 +2965,64 @@ nm_utils_wifi_strength_bars (guint8 strength)
gsize
nm_utils_hwaddr_len (int type)
{
g_return_val_if_fail (type == ARPHRD_ETHER || type == ARPHRD_INFINIBAND, 0);
if (type == ARPHRD_ETHER)
return ETH_ALEN;
else if (type == ARPHRD_INFINIBAND)
return INFINIBAND_ALEN;
g_assert_not_reached ();
g_return_val_if_reached (0);
}
#define HEXVAL(c) ((c) <= '9' ? (c) - '0' : ((c) & 0x4F) - 'A' + 10)
static guint8 *
hwaddr_aton (const char *asc, guint8 *buffer, gsize buffer_length, gsize *out_len)
{
const char *in = asc;
guint8 *out = buffer;
guint8 delimiter = '\0';
nm_assert (asc);
nm_assert (buffer);
nm_assert (buffer_length);
nm_assert (out_len);
while (TRUE) {
const guint8 d1 = in[0];
guint8 d2;
if (!g_ascii_isxdigit (d1))
return NULL;
#define HEXVAL(c) ((c) <= '9' ? (c) - '0' : ((c) & 0x4F) - ('A' - 10))
/* If there's no leading zero (ie "aa:b:cc") then fake it */
d2 = in[1];
if (d2 && g_ascii_isxdigit (d2)) {
*out++ = (HEXVAL (d1) << 4) + HEXVAL (d2);
d2 = in[2];
in += 3;
} else {
/* Fake leading zero */
*out++ = HEXVAL (d1);
in += 2;
}
if (!d2)
break;
if (--buffer_length == 0)
return NULL;
if (d2 != delimiter) {
if ( delimiter == '\0'
&& (d2 == ':' || d2 == '-'))
delimiter = d2;
else
return NULL;
}
}
*out_len = out - buffer;
return buffer;
}
/**
* nm_utils_hwaddr_atoba:
@ -2992,18 +3039,52 @@ GByteArray *
nm_utils_hwaddr_atoba (const char *asc, gsize length)
{
GByteArray *ba;
gsize l;
g_return_val_if_fail (asc != NULL, NULL);
g_return_val_if_fail (asc, NULL);
g_return_val_if_fail (length > 0 && length <= NM_UTILS_HWADDR_LEN_MAX, NULL);
ba = g_byte_array_sized_new (length);
g_byte_array_set_size (ba, length);
if (!nm_utils_hwaddr_aton (asc, ba->data, length)) {
g_byte_array_unref (ba);
return NULL;
}
if (!hwaddr_aton (asc, ba->data, length, &l))
goto fail;
if (length != l)
goto fail;
return ba;
fail:
g_byte_array_unref (ba);
return NULL;
}
/**
* _nm_utils_hwaddr_aton:
* @asc: the ASCII representation of a hardware address
* @buffer: buffer to store the result into. Must have
* at least a size of @buffer_length.
* @buffer_length: the length of the input buffer @buffer.
* The result must fit into that buffer, otherwise
* the function fails and returns %NULL.
* @out_length: the output length in case of success.
*
* Parses @asc and converts it to binary form in @buffer.
* Bytes in @asc can be sepatared by colons (:), or hyphens (-), but not mixed.
*
* It is like nm_utils_hwaddr_aton(), but contrary to that it
* can parse addresses of any length. That is, you don't need
* to know the length before-hand.
*
* Return value: @buffer, or %NULL if @asc couldn't be parsed.
*/
guint8 *
_nm_utils_hwaddr_aton (const char *asc, gpointer buffer, gsize buffer_length, gsize *out_length)
{
g_return_val_if_fail (asc, NULL);
g_return_val_if_fail (buffer, NULL);
g_return_val_if_fail (buffer_length > 0, NULL);
g_return_val_if_fail (out_length, NULL);
return hwaddr_aton (asc, buffer, buffer_length, out_length);
}
/**
@ -3022,72 +3103,55 @@ nm_utils_hwaddr_atoba (const char *asc, gsize length)
guint8 *
nm_utils_hwaddr_aton (const char *asc, gpointer buffer, gsize length)
{
const char *in = asc;
guint8 *out = (guint8 *)buffer;
char delimiter = '\0';
gsize l;
g_return_val_if_fail (asc != NULL, NULL);
g_return_val_if_fail (buffer != NULL, NULL);
g_return_val_if_fail (asc, NULL);
g_return_val_if_fail (buffer, NULL);
g_return_val_if_fail (length > 0 && length <= NM_UTILS_HWADDR_LEN_MAX, NULL);
while (length && *in) {
guint8 d1 = in[0], d2 = in[1];
if (!hwaddr_aton (asc, buffer, length, &l))
return NULL;
if (length != l)
return NULL;
return buffer;
}
if (!g_ascii_isxdigit (d1))
return NULL;
static void
_bin2str_buf (gconstpointer addr, gsize length, gboolean upper_case, char *out)
{
const guint8 *in = addr;
const char *LOOKUP = upper_case ? "0123456789ABCDEF" : "0123456789abcdef";
/* If there's no leading zero (ie "aa:b:cc") then fake it */
if (d2 && g_ascii_isxdigit (d2)) {
*out++ = (HEXVAL (d1) << 4) + HEXVAL (d2);
in += 2;
} else {
/* Fake leading zero */
*out++ = (HEXVAL ('0') << 4) + HEXVAL (d1);
in += 1;
}
nm_assert (addr);
nm_assert (out);
nm_assert (length > 0);
/* @out must contain at least @length*3 bytes */
for (;;) {
const guint8 v = *in++;
*out++ = LOOKUP[v >> 4];
*out++ = LOOKUP[v & 0x0F];
length--;
if (*in) {
if (delimiter == '\0') {
if (*in == ':' || *in == '-')
delimiter = *in;
else
return NULL;
} else {
if (*in != delimiter)
return NULL;
}
in++;
}
if (!length)
break;
*out++ = ':';
}
if (length == 0 && !*in)
return buffer;
else
return NULL;
*out = 0;
}
static char *
_bin2str (gconstpointer addr, gsize length, gboolean upper_case)
{
const guint8 *in = addr;
char *out, *result;
const char *LOOKUP = upper_case ? "0123456789ABCDEF" : "0123456789abcdef";
char *result;
g_return_val_if_fail (addr != NULL, g_strdup (""));
g_return_val_if_fail (length > 0, g_strdup (""));
nm_assert (addr);
nm_assert (length > 0);
result = out = g_malloc (length * 3);
while (length--) {
guint8 v = *in++;
*out++ = LOOKUP[v >> 4];
*out++ = LOOKUP[v & 0x0F];
if (length)
*out++ = ':';
}
*out = 0;
result = g_malloc (length * 3);
_bin2str_buf (addr, length, upper_case, result);
return result;
}
@ -3103,9 +3167,25 @@ _bin2str (gconstpointer addr, gsize length, gboolean upper_case)
char *
nm_utils_hwaddr_ntoa (gconstpointer addr, gsize length)
{
g_return_val_if_fail (addr, g_strdup (""));
g_return_val_if_fail (length > 0, g_strdup (""));
return _bin2str (addr, length, TRUE);
}
const char *
nm_utils_hwaddr_ntoa_buf (gconstpointer addr, gsize addr_len, gboolean upper_case, char *buf, gsize buf_len)
{
g_return_val_if_fail (addr, NULL);
g_return_val_if_fail (addr_len > 0, NULL);
g_return_val_if_fail (buf, NULL);
if (buf_len < addr_len * 3)
g_return_val_if_reached (NULL);
_bin2str_buf (addr, addr_len, TRUE, buf);
return buf;
}
/**
* _nm_utils_bin2str:
* @addr: (type guint8) (array length=length): a binary hardware address
@ -3119,51 +3199,12 @@ nm_utils_hwaddr_ntoa (gconstpointer addr, gsize length)
char *
_nm_utils_bin2str (gconstpointer addr, gsize length, gboolean upper_case)
{
g_return_val_if_fail (addr, g_strdup (""));
g_return_val_if_fail (length > 0, g_strdup (""));
return _bin2str (addr, length, upper_case);
}
static int
hwaddr_binary_len (const char *asc)
{
int octets = 1;
if (!*asc)
return 0;
for (; *asc; asc++) {
if (*asc == ':' || *asc == '-')
octets++;
}
return octets;
}
/**
* _nm_utils_hwaddr_length:
* @asc: the ASCII representation of the hardware address
*
* Validates that @asc is a valid representation of a hardware
* address up to (including) %NM_UTILS_HWADDR_LEN_MAX bytes.
*
* Returns: binary length of the hardware address @asc or
* 0 on error.
*/
guint
_nm_utils_hwaddr_length (const char *asc)
{
int l;
if (!asc)
return 0;
l = hwaddr_binary_len (asc);
if (l <= 0 || l > NM_UTILS_HWADDR_LEN_MAX)
return 0;
if (!nm_utils_hwaddr_valid (asc, l))
return 0;
return l;
}
/**
* nm_utils_hwaddr_valid:
* @asc: the ASCII representation of a hardware address
@ -3180,17 +3221,18 @@ gboolean
nm_utils_hwaddr_valid (const char *asc, gssize length)
{
guint8 buf[NM_UTILS_HWADDR_LEN_MAX];
gsize l;
g_return_val_if_fail (asc != NULL, FALSE);
g_return_val_if_fail (length == -1 || (length > 0 && length <= NM_UTILS_HWADDR_LEN_MAX), FALSE);
if (length == -1) {
length = hwaddr_binary_len (asc);
if (length == 0 || length > NM_UTILS_HWADDR_LEN_MAX)
if (length > 0 && length <= NM_UTILS_HWADDR_LEN_MAX) {
if (!hwaddr_aton (asc, buf, length, &l))
return FALSE;
}
return nm_utils_hwaddr_aton (asc, buf, length) != NULL;
return length == l;
} else if (length == -1) {
return !!hwaddr_aton (asc, buf, sizeof (buf), &l);
} else
g_return_val_if_reached (FALSE);
}
/**
@ -3210,20 +3252,23 @@ char *
nm_utils_hwaddr_canonical (const char *asc, gssize length)
{
guint8 buf[NM_UTILS_HWADDR_LEN_MAX];
gsize l;
g_return_val_if_fail (asc != NULL, NULL);
g_return_val_if_fail (asc, NULL);
g_return_val_if_fail (length == -1 || (length > 0 && length <= NM_UTILS_HWADDR_LEN_MAX), NULL);
if (length == -1) {
length = hwaddr_binary_len (asc);
if (length == 0 || length > NM_UTILS_HWADDR_LEN_MAX)
if (length > 0 && length <= NM_UTILS_HWADDR_LEN_MAX) {
if (!hwaddr_aton (asc, buf, length, &l))
return NULL;
}
if (l != length)
return NULL;
} else if (length == -1) {
if (!hwaddr_aton (asc, buf, NM_UTILS_HWADDR_LEN_MAX, &l))
return NULL;
} else
g_return_val_if_reached (NULL);
if (nm_utils_hwaddr_aton (asc, buf, length) == NULL)
return NULL;
return nm_utils_hwaddr_ntoa (buf, length);
return nm_utils_hwaddr_ntoa (buf, l);
}
/* This is used to possibly canonicalize values passed to MAC address property
@ -3287,17 +3332,17 @@ nm_utils_hwaddr_matches (gconstpointer hwaddr1,
gssize hwaddr2_len)
{
guint8 buf1[NM_UTILS_HWADDR_LEN_MAX], buf2[NM_UTILS_HWADDR_LEN_MAX];
gsize l;
if (hwaddr1_len == -1) {
g_return_val_if_fail (hwaddr1 != NULL, FALSE);
hwaddr1_len = hwaddr_binary_len (hwaddr1);
if (hwaddr1_len == 0 || hwaddr1_len > NM_UTILS_HWADDR_LEN_MAX)
if (!hwaddr_aton (hwaddr1, buf1, sizeof (buf1), &l)) {
g_return_val_if_fail ((hwaddr2_len == -1 && hwaddr2) || (hwaddr2_len > 0 && hwaddr2_len <= NM_UTILS_HWADDR_LEN_MAX), FALSE);
return FALSE;
if (!nm_utils_hwaddr_aton (hwaddr1, buf1, hwaddr1_len))
return FALSE;
}
hwaddr1 = buf1;
hwaddr1_len = l;
} else {
g_return_val_if_fail (hwaddr1_len > 0 && hwaddr1_len <= NM_UTILS_HWADDR_LEN_MAX, FALSE);
@ -3310,23 +3355,24 @@ nm_utils_hwaddr_matches (gconstpointer hwaddr1,
if (hwaddr2_len == -1) {
g_return_val_if_fail (hwaddr2 != NULL, FALSE);
if (!nm_utils_hwaddr_aton (hwaddr2, buf2, hwaddr1_len))
if (!hwaddr_aton (hwaddr2, buf2, sizeof (buf2), &l))
return FALSE;
if (l != hwaddr1_len)
return FALSE;
hwaddr2 = buf2;
hwaddr2_len = hwaddr1_len;
} else {
g_return_val_if_fail (hwaddr2_len > 0 && hwaddr2_len <= NM_UTILS_HWADDR_LEN_MAX, FALSE);
if (hwaddr2_len != hwaddr1_len)
return FALSE;
if (!hwaddr2) {
memset (buf2, 0, hwaddr2_len);
hwaddr2 = buf2;
}
}
if (hwaddr1_len != hwaddr2_len)
return FALSE;
if (hwaddr1_len == INFINIBAND_ALEN) {
hwaddr1 = (guint8 *)hwaddr1 + INFINIBAND_ALEN - 8;
hwaddr2 = (guint8 *)hwaddr2 + INFINIBAND_ALEN - 8;
@ -3342,16 +3388,11 @@ static GVariant *
_nm_utils_hwaddr_to_dbus_impl (const char *str)
{
guint8 buf[NM_UTILS_HWADDR_LEN_MAX];
int len;
gsize len;
if (!str)
return NULL;
len = _nm_utils_hwaddr_length (str);
if (len == 0)
return NULL;
if (!nm_utils_hwaddr_aton (str, buf, len))
if (!hwaddr_aton (str, buf, sizeof (buf), &len))
return NULL;
return g_variant_new_fixed_array (G_VARIANT_TYPE_BYTE, buf, len, 1);

View file

@ -11718,25 +11718,25 @@ nm_device_hw_addr_is_explict (NMDevice *self)
}
static gboolean
_hw_addr_matches (NMDevice *self, const char *addr)
_hw_addr_matches (NMDevice *self, const guint8 *addr, gsize addr_len)
{
const char *cur_addr;
cur_addr = nm_device_get_hw_address (self);
return cur_addr && nm_utils_hwaddr_matches (cur_addr, -1, addr, -1);
return cur_addr && nm_utils_hwaddr_matches (addr, addr_len, cur_addr, -1);
}
static gboolean
_hw_addr_set (NMDevice *self,
const char *addr,
const char *operation,
const char *detail)
const char *const addr,
const char *const operation,
const char *const detail)
{
NMDevicePrivate *priv;
gboolean success = FALSE;
NMPlatformError plerr;
guint8 addr_bytes[NM_UTILS_HWADDR_LEN_MAX];
guint hw_addr_len;
gsize addr_len;
gboolean was_up;
nm_assert (NM_IS_DEVICE (self));
@ -11745,18 +11745,20 @@ _hw_addr_set (NMDevice *self,
priv = NM_DEVICE_GET_PRIVATE (self);
if (!_nm_utils_hwaddr_aton (addr, addr_bytes, sizeof (addr_bytes), &addr_len))
g_return_val_if_reached (FALSE);
/* Do nothing if current MAC is same */
if (_hw_addr_matches (self, addr)) {
if (_hw_addr_matches (self, addr_bytes, addr_len)) {
_LOGT (LOGD_DEVICE, "set-hw-addr: no MAC address change needed (%s)", addr);
return TRUE;
}
hw_addr_len = priv->hw_addr_len;
if (!hw_addr_len)
hw_addr_len = _nm_utils_hwaddr_length (addr);
if ( !hw_addr_len
|| !nm_utils_hwaddr_aton (addr, addr_bytes, hw_addr_len))
g_return_val_if_reached (FALSE);
if (priv->hw_addr_len != addr_len) {
if (priv->hw_addr_len)
g_return_val_if_reached (FALSE);
priv->hw_addr_len = addr_len;
}
_LOGT (LOGD_DEVICE, "set-hw-addr: setting MAC address to '%s' (%s, %s)...", addr, operation, detail);
@ -11766,12 +11768,12 @@ _hw_addr_set (NMDevice *self,
nm_device_take_down (self, FALSE);
}
plerr = nm_platform_link_set_address (NM_PLATFORM_GET, nm_device_get_ip_ifindex (self), addr_bytes, hw_addr_len);
plerr = nm_platform_link_set_address (NM_PLATFORM_GET, nm_device_get_ip_ifindex (self), addr_bytes, addr_len);
success = (plerr == NM_PLATFORM_ERROR_SUCCESS);
if (success) {
/* MAC address succesfully changed; update the current MAC to match */
nm_device_update_hw_address (self);
if (_hw_addr_matches (self, addr)) {
if (_hw_addr_matches (self, addr_bytes, addr_len)) {
_LOGI (LOGD_DEVICE, "set-hw-addr: %s MAC address to %s (%s)",
operation, addr, detail);
} else {
@ -11801,7 +11803,7 @@ _hw_addr_set (NMDevice *self,
goto handle_fail;
if (!nm_device_update_hw_address (self))
goto handle_wait;
if (!_hw_addr_matches (self, addr))
if (!_hw_addr_matches (self, addr_bytes, addr_len))
goto handle_fail;
break;
@ -11928,9 +11930,8 @@ nm_device_hw_addr_set_cloned (NMDevice *self, NMConnection *connection, gboolean
addr = hw_addr_generated;
} else {
/* this must be a valid address. Otherwise, we shouldn't come here. */
if (_nm_utils_hwaddr_length (addr) <= 0) {
if (!nm_utils_hwaddr_valid (addr, -1))
g_return_val_if_reached (FALSE);
}
priv->hw_addr_type = HW_ADDR_TYPE_EXPLICIT;
}
@ -12127,13 +12128,16 @@ constructor (GType type,
}
if (priv->hw_addr_perm) {
priv->hw_addr_len = _nm_utils_hwaddr_length (priv->hw_addr_perm);
if (!priv->hw_addr_len) {
guint8 buf[NM_UTILS_HWADDR_LEN_MAX];
gsize l;
if (!_nm_utils_hwaddr_aton (priv->hw_addr_perm, buf, sizeof (buf), &l)) {
g_clear_pointer (&priv->hw_addr_perm, g_free);
g_return_val_if_reached (object);
}
priv->hw_addr = g_strdup (priv->hw_addr_perm);
priv->hw_addr_len = l;
priv->hw_addr = nm_utils_hwaddr_ntoa (buf, l);
_LOGT (LOGD_DEVICE, "hw-addr: has permanent hw-address '%s'", priv->hw_addr_perm);
}

View file

@ -1272,7 +1272,7 @@ nm_match_spec_hwaddr (const GSList *specs, const char *hwaddr)
{
const GSList *iter;
NMMatchSpecMatchType match = NM_MATCH_SPEC_NO_MATCH;
guint hwaddr_len = 0;
gsize hwaddr_len = 0;
guint8 hwaddr_bin[NM_UTILS_HWADDR_LEN_MAX];
nm_assert (nm_utils_hwaddr_valid (hwaddr, -1));
@ -1297,11 +1297,8 @@ nm_match_spec_hwaddr (const GSList *specs, const char *hwaddr)
continue;
if (G_UNLIKELY (hwaddr_len == 0)) {
hwaddr_len = _nm_utils_hwaddr_length (hwaddr);
if (!hwaddr_len)
if (!_nm_utils_hwaddr_aton (hwaddr, hwaddr_bin, sizeof (hwaddr_bin), &hwaddr_len))
g_return_val_if_reached (NM_MATCH_SPEC_NO_MATCH);
if (!nm_utils_hwaddr_aton (hwaddr, hwaddr_bin, hwaddr_len))
nm_assert_not_reached ();
}
if (nm_utils_hwaddr_matches (spec_str, -1, hwaddr_bin, hwaddr_len)) {