mirror/openvpn3
0
0
Fork 0
mirror of https://github.com/OpenVPN/openvpn3.git synced 2024-09-19 19:52:15 +02:00
Code

SITNL: revert change of sitnl_send return type, return int

Browse Source 
This was changed in commit
ae663c573a ("Using new numeric
conversion tools") to avoid some conversion warnings. But
after understanding the workings of the function better, the
change turns out to have been wrong. Instead the function was
changed to use different intermediate variables for different
purposes.

This change ripples through the whole Netlink/SITNL interface.

Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
This commit is contained in:
Frank Lichtenheld 2023-10-30 14:25:40 +01:00 committed by David Sommerseth
parent 3b945e62e6
commit 22ba196429
No known key found for this signature in database
GPG Key ID: 86CF944C9671FDF2
2 changed files with 59 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

98
openvpn/tun/linux/client/sitnl.hpp
View File

@ -228,10 +228,10 @@ class SITNL
* @param arg_cb data argument for cb, can be NULL * @param arg_cb data argument for cb, can be NULL
* @return If any error occurs, negative errno. If cb is set, return value of last cb call, if not then value of error attribute in NLMSG_ERROR. * @return If any error occurs, negative errno. If cb is set, return value of last cb call, if not then value of error attribute in NLMSG_ERROR.
*/ */
static ssize_t static int
sitnl_send(struct nlmsghdr *payload, pid_t peer, unsigned int groups, sitnl_parse_reply_cb cb, void *arg_cb) sitnl_send(struct nlmsghdr *payload, pid_t peer, unsigned int groups, sitnl_parse_reply_cb cb, void *arg_cb)
{ {
ssize_t ret = 0; int ret = 0;
const size_t buf_len = 16 * 1024; const size_t buf_len = 16 * 1024;
struct sockaddr_nl nladdr = { struct sockaddr_nl nladdr = {
.nl_family = AF_NETLINK, .nl_family = AF_NETLINK,
@ -575,7 +575,7 @@ class SITNL
* @param [out] best_iface network interface on which gw was found * @param [out] best_iface network interface on which gw was found
* @return * @return
*/ */
static ssize_t static int
sitnl_route_best_gw(const std::string &iface_to_ignore, sitnl_route_best_gw(const std::string &iface_to_ignore,
const IP::Route &route, const IP::Route &route,
IP::Addr &best_gw, IP::Addr &best_gw,
@ -590,7 +590,7 @@ class SITNL
res.metric = -1; res.metric = -1;
res.prefix_len = -1; res.prefix_len = -1;
ssize_t ret = -EINVAL; int ret = -EINVAL;
if (!is_safe_conversion<unsigned char>(route.addr.family())) if (!is_safe_conversion<unsigned char>(route.addr.family()))
return -EINVAL; return -EINVAL;
@ -706,7 +706,7 @@ class SITNL
* @param [out] address/netmask of interface * @param [out] address/netmask of interface
* @return success if 0, error if < 0 * @return success if 0, error if < 0
*/ */
static ssize_t static int
sitnl_iface_addr(const int ifindex, sitnl_iface_addr(const int ifindex,
const int family, const int family,
IP::Route &route) IP::Route &route)
@ -726,7 +726,7 @@ class SITNL
req.n.nlmsg_flags |= NLM_F_DUMP; req.n.nlmsg_flags |= NLM_F_DUMP;
const auto ret = sitnl_send(&req.n, 0, 0, sitnl_iface_addr_save, &res); const auto ret = sitnl_send(&req.n, 0, 0, sitnl_iface_addr_save, &res);
if (ret >= 0 && res.route.defined()) if (ret == 0 && res.route.defined())
{ {
/* save result in output variables */ /* save result in output variables */
route = res.route; route = res.route;
@ -742,7 +742,7 @@ class SITNL
return ret; return ret;
} }
static ssize_t static int
sitnl_addr_set(const unsigned short cmd, sitnl_addr_set(const unsigned short cmd,
const unsigned short flags, const unsigned short flags,
const std::string &iface, const std::string &iface,
@ -752,7 +752,7 @@ class SITNL
const IP::Addr &broadcast) const IP::Addr &broadcast)
{ {
struct sitnl_addr_req req = {}; struct sitnl_addr_req req = {};
ssize_t ret = -EINVAL; int ret = -EINVAL;
if (iface.empty()) if (iface.empty())
{ {
@ -821,11 +821,12 @@ class SITNL
ret = 0; ret = 0;
} }
/* for SITNL_ADDATTR */
err: err:
return ret; return -EINVAL;
} }
static ssize_t static int
sitnl_addr_ptp_add(const std::string &iface, sitnl_addr_ptp_add(const std::string &iface,
const IP::Addr &local, const IP::Addr &local,
const IP::Addr &remote) const IP::Addr &remote)
@ -839,7 +840,7 @@ class SITNL
IP::Addr::from_zero(local.version())); IP::Addr::from_zero(local.version()));
} }
static ssize_t static int
sitnl_addr_ptp_del(const std::string &iface, const IP::Addr &local) sitnl_addr_ptp_del(const std::string &iface, const IP::Addr &local)
{ {
return sitnl_addr_set(RTM_DELADDR, return sitnl_addr_set(RTM_DELADDR,
@ -851,7 +852,7 @@ class SITNL
IP::Addr::from_zero(local.version())); IP::Addr::from_zero(local.version()));
} }
static ssize_t static int
sitnl_route_set(const unsigned short cmd, sitnl_route_set(const unsigned short cmd,
const unsigned short flags, const unsigned short flags,
const std::string &iface, const std::string &iface,
@ -864,7 +865,7 @@ class SITNL
const unsigned char type) const unsigned char type)
{ {
struct sitnl_route_req req = {}; struct sitnl_route_req req = {};
ssize_t ret = -1; int ret = -1;
req.n.nlmsg_len = NLMSG_LENGTH(sizeof(req.r)); req.n.nlmsg_len = NLMSG_LENGTH(sizeof(req.r));
req.n.nlmsg_type = cmd; req.n.nlmsg_type = cmd;
@ -930,11 +931,12 @@ class SITNL
ret = 0; ret = 0;
} }
/* for SITNL_ADDATTR */
err: err:
return ret; return -1;
} }
static ssize_t static int
sitnl_addr_add(const std::string &iface, sitnl_addr_add(const std::string &iface,
const IP::Addr &addr, const IP::Addr &addr,
unsigned char prefixlen, unsigned char prefixlen,
@ -949,7 +951,7 @@ class SITNL
broadcast); broadcast);
} }
static ssize_t static int
sitnl_addr_del(const std::string &iface, const IP::Addr &addr, unsigned char prefixlen) sitnl_addr_del(const std::string &iface, const IP::Addr &addr, unsigned char prefixlen)
{ {
return sitnl_addr_set(RTM_DELADDR, return sitnl_addr_set(RTM_DELADDR,
@ -961,7 +963,7 @@ class SITNL
IP::Addr::from_zero(addr.version())); IP::Addr::from_zero(addr.version()));
} }
static ssize_t static int
sitnl_route_add(const IP::Route &route, sitnl_route_add(const IP::Route &route,
const IP::Addr &gw, const IP::Addr &gw,
const std::string &iface, const std::string &iface,
@ -980,7 +982,7 @@ class SITNL
RTN_UNICAST); RTN_UNICAST);
} }
static ssize_t static int
sitnl_route_del(const IP::Route &route, sitnl_route_del(const IP::Route &route,
const IP::Addr &gw, const IP::Addr &gw,
const std::string &iface, const std::string &iface,
@ -1000,22 +1002,21 @@ class SITNL
} }
public: public:
static ssize_t static int
net_route_best_gw(const IP::Route6 &route, net_route_best_gw(const IP::Route6 &route,
IPv6::Addr &best_gw6, IPv6::Addr &best_gw6,
std::string &best_iface, std::string &best_iface,
const std::string &iface_to_ignore = "") const std::string &iface_to_ignore = "")
{ {
IP::Addr best_gw; IP::Addr best_gw;
ssize_t ret;
OPENVPN_LOG(__func__ << " query IPv6: " << route); OPENVPN_LOG(__func__ << " query IPv6: " << route);
ret = sitnl_route_best_gw(iface_to_ignore, const auto ret = sitnl_route_best_gw(iface_to_ignore,
IP::Route(IP::Addr::from_ipv6(route.addr), route.prefix_len), IP::Route(IP::Addr::from_ipv6(route.addr), route.prefix_len),
best_gw, best_gw,
best_iface); best_iface);
if (ret >= 0) if (ret == 0)
{ {
best_gw6 = best_gw.to_ipv6(); best_gw6 = best_gw.to_ipv6();
} }
@ -1023,22 +1024,21 @@ class SITNL
return ret; return ret;
} }
static ssize_t static int
net_route_best_gw(const IP::Route4 &route, net_route_best_gw(const IP::Route4 &route,
IPv4::Addr &best_gw4, IPv4::Addr &best_gw4,
std::string &best_iface, std::string &best_iface,
const std::string &iface_to_ignore = "") const std::string &iface_to_ignore = "")
{ {
IP::Addr best_gw; IP::Addr best_gw;
ssize_t ret;
OPENVPN_LOG(__func__ << " query IPv4: " << route); OPENVPN_LOG(__func__ << " query IPv4: " << route);
ret = sitnl_route_best_gw(iface_to_ignore, const auto ret = sitnl_route_best_gw(iface_to_ignore,
IP::Route(IP::Addr::from_ipv4(route.addr), route.prefix_len), IP::Route(IP::Addr::from_ipv4(route.addr), route.prefix_len),
best_gw, best_gw,
best_iface); best_iface);
if (ret >= 0) if (ret == 0)
{ {
best_gw4 = best_gw.to_ipv4(); best_gw4 = best_gw.to_ipv4();
} }
@ -1075,12 +1075,11 @@ class SITNL
* @param type interface link type (for example "ovpn-dco") * @param type interface link type (for example "ovpn-dco")
* @return int 0 on success, negative error code on error * @return int 0 on success, negative error code on error
*/ */
static ssize_t static int
net_iface_new(const std::string &iface, const std::string &type) net_iface_new(const std::string &iface, const std::string &type)
{ {
struct sitnl_link_req req = {}; struct sitnl_link_req req = {};
struct rtattr *tail = NULL; struct rtattr *tail = NULL;
ssize_t ret = -1;
if (iface.empty()) if (iface.empty())
{ {
@ -1109,12 +1108,13 @@ class SITNL
OPENVPN_LOG(__func__ << ": add " << iface << " type " << type); OPENVPN_LOG(__func__ << ": add " << iface << " type " << type);
ret = sitnl_send(&req.n, 0, 0, NULL, NULL); return sitnl_send(&req.n, 0, 0, NULL, NULL);
/* for SITNL_ADDATTR */
err: err:
return ret; return -1;
} }
static ssize_t static int
net_iface_del(const std::string &iface) net_iface_del(const std::string &iface)
{ {
struct sitnl_link_req req = {}; struct sitnl_link_req req = {};
@ -1146,7 +1146,7 @@ class SITNL
return sitnl_send(&req.n, 0, 0, NULL, NULL); return sitnl_send(&req.n, 0, 0, NULL, NULL);
} }
static ssize_t static int
net_iface_up(std::string &iface, bool up) net_iface_up(std::string &iface, bool up)
{ {
struct sitnl_link_req req = {}; struct sitnl_link_req req = {};
@ -1187,7 +1187,7 @@ class SITNL
return sitnl_send(&req.n, 0, 0, NULL, NULL); return sitnl_send(&req.n, 0, 0, NULL, NULL);
} }
static ssize_t static int
net_iface_mtu_set(std::string &iface, uint32_t mtu) net_iface_mtu_set(std::string &iface, uint32_t mtu)
{ {
struct sitnl_link_req req = {}; struct sitnl_link_req req = {};
@ -1221,7 +1221,7 @@ class SITNL
return sitnl_send(&req.n, 0, 0, NULL, NULL); return sitnl_send(&req.n, 0, 0, NULL, NULL);
} }
static ssize_t static int
net_addr_add(const std::string &iface, net_addr_add(const std::string &iface,
const IPv4::Addr &addr, const IPv4::Addr &addr,
const unsigned char prefixlen, const unsigned char prefixlen,
@ -1237,7 +1237,7 @@ class SITNL
IP::Addr::from_ipv4(broadcast)); IP::Addr::from_ipv4(broadcast));
} }
static ssize_t static int
net_addr_add(const std::string &iface, net_addr_add(const std::string &iface,
const IPv6::Addr &addr, const IPv6::Addr &addr,
const unsigned char prefixlen) const unsigned char prefixlen)
@ -1251,7 +1251,7 @@ class SITNL
IP::Addr::from_zero(IP::Addr::V6)); IP::Addr::from_zero(IP::Addr::V6));
} }
static ssize_t static int
net_addr_del(const std::string &iface, net_addr_del(const std::string &iface,
const IPv4::Addr &addr, const IPv4::Addr &addr,
const unsigned char prefixlen) const unsigned char prefixlen)
@ -1264,7 +1264,7 @@ class SITNL
prefixlen); prefixlen);
} }
static ssize_t static int
net_addr_del(const std::string &iface, net_addr_del(const std::string &iface,
const IPv6::Addr &addr, const IPv6::Addr &addr,
const unsigned char prefixlen) const unsigned char prefixlen)
@ -1277,7 +1277,7 @@ class SITNL
prefixlen); prefixlen);
} }
static ssize_t static int
net_addr_ptp_add(const std::string &iface, net_addr_ptp_add(const std::string &iface,
const IPv4::Addr &local, const IPv4::Addr &local,
const IPv4::Addr &remote) const IPv4::Addr &remote)
@ -1291,7 +1291,7 @@ class SITNL
IP::Addr::from_ipv4(remote)); IP::Addr::from_ipv4(remote));
} }
static ssize_t static int
net_addr_ptp_del(const std::string &iface, net_addr_ptp_del(const std::string &iface,
const IPv4::Addr &local, const IPv4::Addr &local,
const IPv4::Addr &remote) const IPv4::Addr &remote)
@ -1303,7 +1303,7 @@ class SITNL
IP::Addr::from_ipv4(local)); IP::Addr::from_ipv4(local));
} }
static ssize_t static int
net_route_add(const IP::Route4 &route, net_route_add(const IP::Route4 &route,
const IPv4::Addr &gw, const IPv4::Addr &gw,
const std::string &iface, const std::string &iface,
@ -1323,7 +1323,7 @@ class SITNL
metric); metric);
} }
static ssize_t static int
net_route_add(const IP::Route6 &route, net_route_add(const IP::Route6 &route,
const IPv6::Addr &gw, const IPv6::Addr &gw,
const std::string &iface, const std::string &iface,
@ -1343,7 +1343,7 @@ class SITNL
metric); metric);
} }
static ssize_t static int
net_route_del(const IP::Route4 &route, net_route_del(const IP::Route4 &route,
const IPv4::Addr &gw, const IPv4::Addr &gw,
const std::string &iface, const std::string &iface,
@ -1360,7 +1360,7 @@ class SITNL
metric); metric);
} }
static ssize_t static int
net_route_del(const IP::Route6 &route, net_route_del(const IP::Route6 &route,
const IPv6::Addr &gw, const IPv6::Addr &gw,
const std::string &iface, const std::string &iface,

27
openvpn/tun/linux/client/tunnetlink.hpp
View File

@ -68,15 +68,13 @@ struct NetlinkLinkSet : public Action
virtual void execute(std::ostream &os) override virtual void execute(std::ostream &os) override
{ {
ssize_t ret;
if (dev.empty()) if (dev.empty())
{ {
os << "Error: can't call NetlinkLinkSet with no interface" << std::endl; os << "Error: can't call NetlinkLinkSet with no interface" << std::endl;
return; return;
} }
ret = SITNL::net_iface_mtu_set(dev, mtu); auto ret = SITNL::net_iface_mtu_set(dev, mtu);
if (ret) if (ret)
{ {
os << "Error while executing NetlinkLinkSet " << dev << " mtu " << mtu os << "Error while executing NetlinkLinkSet " << dev << " mtu " << mtu
@ -137,14 +135,13 @@ struct NetlinkAddr4 : public Action
virtual void execute(std::ostream &os) override virtual void execute(std::ostream &os) override
{ {
ssize_t ret;
if (dev.empty()) if (dev.empty())
{ {
os << "Error: can't call NetlinkAddr4 with no interface" << std::endl; os << "Error: can't call NetlinkAddr4 with no interface" << std::endl;
return; return;
} }
int ret;
if (add) if (add)
{ {
ret = SITNL::net_addr_add(dev, addr, prefixlen, broadcast); ret = SITNL::net_addr_add(dev, addr, prefixlen, broadcast);
@ -208,14 +205,13 @@ struct NetlinkAddr6 : public Action
virtual void execute(std::ostream &os) override virtual void execute(std::ostream &os) override
{ {
ssize_t ret;
if (dev.empty()) if (dev.empty())
{ {
os << "Error: can't call NetlinkAddr6 with no interface" << std::endl; os << "Error: can't call NetlinkAddr6 with no interface" << std::endl;
return; return;
} }
int ret;
if (add) if (add)
{ {
ret = SITNL::net_addr_add(dev, addr, prefixlen); ret = SITNL::net_addr_add(dev, addr, prefixlen);
@ -277,14 +273,13 @@ struct NetlinkAddr4PtP : public Action
virtual void execute(std::ostream &os) override virtual void execute(std::ostream &os) override
{ {
ssize_t ret;
if (dev.empty()) if (dev.empty())
{ {
os << "Error: can't call NetlinkAddr4PtP with no interface" << std::endl; os << "Error: can't call NetlinkAddr4PtP with no interface" << std::endl;
return; return;
} }
int ret;
if (add) if (add)
{ {
ret = SITNL::net_addr_ptp_add(dev, local, remote); ret = SITNL::net_addr_ptp_add(dev, local, remote);
@ -344,14 +339,13 @@ struct NetlinkRoute4 : public Action
virtual void execute(std::ostream &os) override virtual void execute(std::ostream &os) override
{ {
ssize_t ret;
if (dev.empty()) if (dev.empty())
{ {
os << "Error: can't call NetlinkRoute4 with no interface" << std::endl; os << "Error: can't call NetlinkRoute4 with no interface" << std::endl;
return; return;
} }
int ret;
if (add) if (add)
{ {
ret = SITNL::net_route_add(route, gw, dev, 0, 0); ret = SITNL::net_route_add(route, gw, dev, 0, 0);
@ -413,14 +407,13 @@ struct NetlinkRoute6 : public Action
virtual void execute(std::ostream &os) override virtual void execute(std::ostream &os) override
{ {
ssize_t ret;
if (dev.empty()) if (dev.empty())
{ {
os << "Error: can't call NetlinkRoute6 with no interface" << std::endl; os << "Error: can't call NetlinkRoute6 with no interface" << std::endl;
return; return;
} }
int ret;
if (add) if (add)
{ {
ret = SITNL::net_route_add(route, gw, dev, 0, 0); ret = SITNL::net_route_add(route, gw, dev, 0, 0);
@ -467,9 +460,9 @@ enum
* @param type interface link type (such as "ovpn-dco") * @param type interface link type (such as "ovpn-dco")
* @return int 0 on success, negative error code on error * @return int 0 on success, negative error code on error
*/ */
inline ssize_t iface_new(std::ostringstream &os, const std::string &dev, const std::string &type) inline int iface_new(std::ostringstream &os, const std::string &dev, const std::string &type)
{ {
ssize_t ret = -1; int ret = -1;
if (dev.empty()) if (dev.empty())
{ {
@ -492,9 +485,9 @@ inline ssize_t iface_new(std::ostringstream &os, const std::string &dev, const s
return ret; return ret;
} }
inline ssize_t iface_del(std::ostringstream &os, const std::string &dev) inline int iface_del(std::ostringstream &os, const std::string &dev)
{ {
ssize_t ret = -1; int ret = -1;
if (dev.empty()) if (dev.empty())
{ {