- existing API/ABI must stay unchanged. We cannot change parameter
types. Ad most we can add new variants that support 64 bit integers.
- rtnl_tc_calc_txtime64() and rtnl_tc_calc_bufsize64() are trivial.
We should not blow up the public API of libnl for such a thing.
If the users needs it, they can just reimplement it.
- getters should return an error code. Especially if the return type
does not support encoding an error there.
- don't add separate rs_rate64/rs_ceil64 field. Instead, extend the
"rs_rate" field of "struct rtnl_ratespec" to 64 bits. It's internal
API.
Rebased original pull request #214 on master. One commit ("xfrmi: add
IFLA_XFRM_* definitions to if_link.h") was thereby dropped as it
is no longer necessary. Otherwise, the two remaining patches applied
cleanly. Then, add two more patches to the branch before merging.
https://github.com/thom311/libnl/pull/214
Returning the value directly as uint32_t does not leave room for an error
code. E.g. we want to indicate to the caller whether the attribute is present
or not (-NLE_NOATTR). Currenlty, the code is quite unforgiving and will just
crash/assert against invalid arguments. In theory, we could also be more forgiving
and return a error code if the link argument is invalid.
Just be more forgiving. Also, this avoids a coverity warning:
Error: FORWARD_NULL (CWE-476): [#def1]
libnl-3.4.0/lib/route/addr.c:502: var_compare_op: Comparing "a->a_peer" to null implies that "a->a_peer" might be null.
libnl-3.4.0/lib/route/addr.c:513: var_deref_model: Passing null pointer "a->a_peer" to "nl_addr_cmp", which dereferences it.
libnl-3.4.0/lib/addr.c:587:8: deref_parm: Directly dereferencing parameter "a".
# 585| int nl_addr_cmp(const struct nl_addr *a, const struct nl_addr *b)
# 586| {
# 587|-> int d = a->a_family - b->a_family;
# 588|
# 589| if (d == 0) {
https://bugzilla.redhat.com/show_bug.cgi?id=1606988
This fixes the build with musl libc.
Additionally, several changes were made to account for changes to the
headers:
- ip_mp_alg.h was removed, since it was removed in linux commit e06e7c61
(v2.6.23), and the last use of those constants was removed in libnl
commit 535e8316.
- Uses of TCF_META_ID_SK_ROUTE_CAPS were updated to
__TCF_META_ID_SK_ROUTE_CAPS, since it was renamed in linux commit
e20e6940 (v3.1).
- Uses of IF_CARRIER_DOWN and IF_CARRIER_UP were replaced with their
values, 0 and 1, since they are not in linux/if.h (they appear to be
libnl-specific, added in libnl commit 3540e44b).
https://github.com/thom311/libnl/pull/222
These behave the same, except when used at top-level. This can't happen
since the macro body is a statement.
__func__ is standardized since C99, while __PRETTY_FUNCTION__ is a
GNU extension.
The compiler warns about string truncation:
In function ‘genl_family_add_grp’,
inlined from ‘family_clone’ at lib/genl/family.c:81:9,
inlined from ‘family_clone’ at lib/genl/family.c:66:12:
lib/genl/family.c:376:2: error: ‘strncpy’ output may be truncated copying 15 bytes from a string of length 15 [-Werror=stringop-truncation]
376 | strncpy(grp->name, name, GENL_NAMSIZ - 1);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Obvioulsy, it's a bug to use an invalid group name. But better
handle it by checking for a suitable string length.
Also use _nl_strncpy() which asserts that no truncation occurs.
The compiler warns:
In function ‘rtnl_tc_set_kind’,
inlined from ‘rtnl_tc_msg_parse’ at lib/route/tc.c:81:2:
lib/route/tc.c:532:2: error: ‘strncpy’ output may be truncated copying 31 bytes from a string of length 31 [-Werror=stringop-truncation]
532 | strncpy(tc->tc_kind, kind, sizeof(tc->tc_kind) - 1);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Now, there are two choices: either accept the truncation
or rejecting it.
While rejecting it is a change in behavior and API, I don't think that
any caller actually relied on that. That is because such "kind" name would
be invalid anyway (and rejected from kernel too).
So, tighten up the API and check for a suitable string length.
Also, use _nl_strncpy() instead of strncpy(). Note that that doesn't suppress
the warning, it merely (also) adds an _nl_assert() for something that already
shouldn't happen.
Compiler warnings:
lib/route/link/inet6.c: In function ‘inet6_dump_details’:
lib/route/link/inet6.c:383:3: error: ‘strncpy’ output may be truncated copying between 0 and 63 bytes from a string of length 63 [-Werror=stringop-truncation]
383 | strncpy(&buf[offset], buf2, strlen(buf2));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Also, passing "strlen(buf2)" as length argument to strncpy() is
certainly wrong.
The follow leaves a dangling pointer when the name argument is too long:
xfrmnl_sa_set_aead_params:
if (sa->aead)
free (sa->aead);
if ( strlen (alg_name) >= sizeof (sa->aead->alg_name)
|| (sa->aead = calloc (1, newlen)) == NULL)
return -1;
Fix that, but do more:
- ensure that we don't modify the object when the setter is going to
fail. That means, first check whether we can succeed with all the
steps that are requested, and (in case we cannot) fail without
modifing the target object.
- bonus points for making the setter self-assignment safe by reordering
the setting and freeing of the memory.
We want to build with -Wvla, because VLAs interfere with static asserts
(if the condition of a static assert is not actually static, then VLAs
make it silently pass).
Also, VLAs should be avoided because we want to be in contol how much
we allocate on the stack.
