git: 08992b2078f6 - main - ifconfig: Avoid issues with trying to negate unsigned values.
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Mon, 19 Jun 2023 17:38:29 UTC
The branch main has been updated by jhb:
URL: https://cgit.FreeBSD.org/src/commit/?id=08992b2078f673b2806e0be30d07f41012a650e7
commit 08992b2078f673b2806e0be30d07f41012a650e7
Author: John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2023-06-19 17:37:52 +0000
Commit: John Baldwin <jhb@FreeBSD.org>
CommitDate: 2023-06-19 17:37:52 +0000
ifconfig: Avoid issues with trying to negate unsigned values.
The if_flags and if_cap fields hold a bitmask of flags. If a flag is
the MSB of the field, then the logic in setifflags and setifcap which
uses a < 0 check does the wrong thing (it tries to clear the flag
rather than setting it). Also, trying to use -<FOO> doesn't actually
work as the result is a nop. To fix, stop overloading setifcap and
setifflags and instead add new dedicated action functions clearifcap
and clearifflags for clearing a flag. The value passed in the
argument to the command is now always the raw flag.
This was reported by a GCC warning after raising WARNS:
sbin/ifconfig/ifconfig.c:2061:33: error: integer overflow in expression '-2147483648' of type 'int' results in '-2147483648' [-Werror=overflow]
2061 | DEF_CMD("-txtlsrtlmt", -IFCAP_TXTLS_RTLMT, setifcap),
| ^
Reviewed by: emaste
Differential Revision: https://reviews.freebsd.org/D40608
---
sbin/ifconfig/ifconfig.c | 113 +++++++++++++++++++++++++++++------------------
sbin/ifconfig/ifconfig.h | 1 +
sbin/ifconfig/ifvlan.c | 10 ++---
sbin/ifconfig/ifvxlan.c | 4 +-
4 files changed, 79 insertions(+), 49 deletions(-)
diff --git a/sbin/ifconfig/ifconfig.c b/sbin/ifconfig/ifconfig.c
index 6d66677467c1..5d789795e636 100644
--- a/sbin/ifconfig/ifconfig.c
+++ b/sbin/ifconfig/ifconfig.c
@@ -1396,6 +1396,22 @@ getifflags(const char *ifname, int us, bool err_ok)
* of the ifreq structure, which may confuse other parts of ifconfig.
* Make a private copy so we can avoid that.
*/
+static void
+clearifflags(if_ctx *ctx, const char *vname, int value)
+{
+ struct ifreq my_ifr;
+ int flags;
+
+ flags = getifflags(ctx->ifname, ctx->io_s, false);
+ flags &= ~value;
+ memset(&my_ifr, 0, sizeof(my_ifr));
+ strlcpy(my_ifr.ifr_name, ctx->ifname, sizeof(my_ifr.ifr_name));
+ my_ifr.ifr_flags = flags & 0xffff;
+ my_ifr.ifr_flagshigh = flags >> 16;
+ if (ioctl(ctx->io_s, SIOCSIFFLAGS, (caddr_t)&my_ifr) < 0)
+ Perror(vname);
+}
+
static void
setifflags(if_ctx *ctx, const char *vname, int value)
{
@@ -1403,11 +1419,7 @@ setifflags(if_ctx *ctx, const char *vname, int value)
int flags;
flags = getifflags(ctx->ifname, ctx->io_s, false);
- if (value < 0) {
- value = -value;
- flags &= ~value;
- } else
- flags |= value;
+ flags |= value;
memset(&my_ifr, 0, sizeof(my_ifr));
strlcpy(my_ifr.ifr_name, ctx->ifname, sizeof(my_ifr.ifr_name));
my_ifr.ifr_flags = flags & 0xffff;
@@ -1416,6 +1428,27 @@ setifflags(if_ctx *ctx, const char *vname, int value)
Perror(vname);
}
+void
+clearifcap(if_ctx *ctx, const char *vname, int value)
+{
+ struct ifreq ifr = {};
+ int flags;
+
+ if (ioctl_ctx_ifr(ctx, SIOCGIFCAP, &ifr) < 0) {
+ Perror("ioctl (SIOCGIFCAP)");
+ exit(1);
+ }
+ flags = ifr.ifr_curcap;
+ flags &= ~value;
+ flags &= ifr.ifr_reqcap;
+ /* Check for no change in capabilities. */
+ if (ifr.ifr_curcap == flags)
+ return;
+ ifr.ifr_reqcap = flags;
+ if (ioctl_ctx(ctx, SIOCSIFCAP, &ifr) < 0)
+ Perror(vname);
+}
+
void
setifcap(if_ctx *ctx, const char *vname, int value)
{
@@ -1427,11 +1460,7 @@ setifcap(if_ctx *ctx, const char *vname, int value)
exit(1);
}
flags = ifr.ifr_curcap;
- if (value < 0) {
- value = -value;
- flags &= ~value;
- } else
- flags |= value;
+ flags |= value;
flags &= ifr.ifr_reqcap;
/* Check for no change in capabilities. */
if (ifr.ifr_curcap == flags)
@@ -1972,17 +2001,17 @@ ifmaybeload(struct ifconfig_args *args, const char *name)
static struct cmd basic_cmds[] = {
DEF_CMD("up", IFF_UP, setifflags),
- DEF_CMD("down", -IFF_UP, setifflags),
- DEF_CMD("arp", -IFF_NOARP, setifflags),
+ DEF_CMD("down", IFF_UP, clearifflags),
+ DEF_CMD("arp", IFF_NOARP, clearifflags),
DEF_CMD("-arp", IFF_NOARP, setifflags),
DEF_CMD("debug", IFF_DEBUG, setifflags),
- DEF_CMD("-debug", -IFF_DEBUG, setifflags),
+ DEF_CMD("-debug", IFF_DEBUG, clearifflags),
DEF_CMD_ARG("description", setifdescr),
DEF_CMD_ARG("descr", setifdescr),
DEF_CMD("-description", 0, unsetifdescr),
DEF_CMD("-descr", 0, unsetifdescr),
DEF_CMD("promisc", IFF_PPROMISC, setifflags),
- DEF_CMD("-promisc", -IFF_PPROMISC, setifflags),
+ DEF_CMD("-promisc", IFF_PPROMISC, clearifflags),
DEF_CMD("add", IFF_UP, notealias),
DEF_CMD("alias", IFF_UP, notealias),
DEF_CMD("-alias", -IFF_UP, notealias),
@@ -1991,7 +2020,7 @@ static struct cmd basic_cmds[] = {
#ifdef notdef
#define EN_SWABIPS 0x1000
DEF_CMD("swabips", EN_SWABIPS, setifflags),
- DEF_CMD("-swabips", -EN_SWABIPS, setifflags),
+ DEF_CMD("-swabips", EN_SWABIPS, clearifflags),
#endif
DEF_CMD_ARG("netmask", setifnetmask),
DEF_CMD_ARG("metric", setifmetric),
@@ -2004,64 +2033,64 @@ static struct cmd basic_cmds[] = {
DEF_CMD_ARG("-vnet", setifrvnet),
#endif
DEF_CMD("link0", IFF_LINK0, setifflags),
- DEF_CMD("-link0", -IFF_LINK0, setifflags),
+ DEF_CMD("-link0", IFF_LINK0, clearifflags),
DEF_CMD("link1", IFF_LINK1, setifflags),
- DEF_CMD("-link1", -IFF_LINK1, setifflags),
+ DEF_CMD("-link1", IFF_LINK1, clearifflags),
DEF_CMD("link2", IFF_LINK2, setifflags),
- DEF_CMD("-link2", -IFF_LINK2, setifflags),
+ DEF_CMD("-link2", IFF_LINK2, clearifflags),
DEF_CMD("monitor", IFF_MONITOR, setifflags),
- DEF_CMD("-monitor", -IFF_MONITOR, setifflags),
+ DEF_CMD("-monitor", IFF_MONITOR, clearifflags),
DEF_CMD("mextpg", IFCAP_MEXTPG, setifcap),
- DEF_CMD("-mextpg", -IFCAP_MEXTPG, setifcap),
+ DEF_CMD("-mextpg", IFCAP_MEXTPG, clearifcap),
DEF_CMD("staticarp", IFF_STATICARP, setifflags),
- DEF_CMD("-staticarp", -IFF_STATICARP, setifflags),
+ DEF_CMD("-staticarp", IFF_STATICARP, clearifflags),
DEF_CMD("stickyarp", IFF_STICKYARP, setifflags),
- DEF_CMD("-stickyarp", -IFF_STICKYARP, setifflags),
+ DEF_CMD("-stickyarp", IFF_STICKYARP, clearifflags),
DEF_CMD("rxcsum6", IFCAP_RXCSUM_IPV6, setifcap),
- DEF_CMD("-rxcsum6", -IFCAP_RXCSUM_IPV6, setifcap),
+ DEF_CMD("-rxcsum6", IFCAP_RXCSUM_IPV6, clearifcap),
DEF_CMD("txcsum6", IFCAP_TXCSUM_IPV6, setifcap),
- DEF_CMD("-txcsum6", -IFCAP_TXCSUM_IPV6, setifcap),
+ DEF_CMD("-txcsum6", IFCAP_TXCSUM_IPV6, clearifcap),
DEF_CMD("rxcsum", IFCAP_RXCSUM, setifcap),
- DEF_CMD("-rxcsum", -IFCAP_RXCSUM, setifcap),
+ DEF_CMD("-rxcsum", IFCAP_RXCSUM, clearifcap),
DEF_CMD("txcsum", IFCAP_TXCSUM, setifcap),
- DEF_CMD("-txcsum", -IFCAP_TXCSUM, setifcap),
+ DEF_CMD("-txcsum", IFCAP_TXCSUM, clearifcap),
DEF_CMD("netcons", IFCAP_NETCONS, setifcap),
- DEF_CMD("-netcons", -IFCAP_NETCONS, setifcap),
+ DEF_CMD("-netcons", IFCAP_NETCONS, clearifcap),
DEF_CMD_ARG("pcp", setifpcp),
DEF_CMD("-pcp", 0, disableifpcp),
DEF_CMD("polling", IFCAP_POLLING, setifcap),
- DEF_CMD("-polling", -IFCAP_POLLING, setifcap),
+ DEF_CMD("-polling", IFCAP_POLLING, clearifcap),
DEF_CMD("tso6", IFCAP_TSO6, setifcap),
- DEF_CMD("-tso6", -IFCAP_TSO6, setifcap),
+ DEF_CMD("-tso6", IFCAP_TSO6, clearifcap),
DEF_CMD("tso4", IFCAP_TSO4, setifcap),
- DEF_CMD("-tso4", -IFCAP_TSO4, setifcap),
+ DEF_CMD("-tso4", IFCAP_TSO4, clearifcap),
DEF_CMD("tso", IFCAP_TSO, setifcap),
- DEF_CMD("-tso", -IFCAP_TSO, setifcap),
+ DEF_CMD("-tso", IFCAP_TSO, clearifcap),
DEF_CMD("toe", IFCAP_TOE, setifcap),
- DEF_CMD("-toe", -IFCAP_TOE, setifcap),
+ DEF_CMD("-toe", IFCAP_TOE, clearifcap),
DEF_CMD("lro", IFCAP_LRO, setifcap),
- DEF_CMD("-lro", -IFCAP_LRO, setifcap),
+ DEF_CMD("-lro", IFCAP_LRO, clearifcap),
DEF_CMD("txtls", IFCAP_TXTLS, setifcap),
- DEF_CMD("-txtls", -IFCAP_TXTLS, setifcap),
+ DEF_CMD("-txtls", IFCAP_TXTLS, clearifcap),
DEF_CMD_SARG("rxtls", IFCAP2_RXTLS4_NAME "," IFCAP2_RXTLS6_NAME,
setifcapnv),
DEF_CMD_SARG("-rxtls", "-"IFCAP2_RXTLS4_NAME ",-" IFCAP2_RXTLS6_NAME,
setifcapnv),
DEF_CMD("wol", IFCAP_WOL, setifcap),
- DEF_CMD("-wol", -IFCAP_WOL, setifcap),
+ DEF_CMD("-wol", IFCAP_WOL, clearifcap),
DEF_CMD("wol_ucast", IFCAP_WOL_UCAST, setifcap),
- DEF_CMD("-wol_ucast", -IFCAP_WOL_UCAST, setifcap),
+ DEF_CMD("-wol_ucast", IFCAP_WOL_UCAST, clearifcap),
DEF_CMD("wol_mcast", IFCAP_WOL_MCAST, setifcap),
- DEF_CMD("-wol_mcast", -IFCAP_WOL_MCAST, setifcap),
+ DEF_CMD("-wol_mcast", IFCAP_WOL_MCAST, clearifcap),
DEF_CMD("wol_magic", IFCAP_WOL_MAGIC, setifcap),
- DEF_CMD("-wol_magic", -IFCAP_WOL_MAGIC, setifcap),
+ DEF_CMD("-wol_magic", IFCAP_WOL_MAGIC, clearifcap),
DEF_CMD("txrtlmt", IFCAP_TXRTLMT, setifcap),
- DEF_CMD("-txrtlmt", -IFCAP_TXRTLMT, setifcap),
+ DEF_CMD("-txrtlmt", IFCAP_TXRTLMT, clearifcap),
DEF_CMD("txtlsrtlmt", IFCAP_TXTLS_RTLMT, setifcap),
- DEF_CMD("-txtlsrtlmt", -IFCAP_TXTLS_RTLMT, setifcap),
+ DEF_CMD("-txtlsrtlmt", IFCAP_TXTLS_RTLMT, clearifcap),
DEF_CMD("hwrxtstmp", IFCAP_HWRXTSTMP, setifcap),
- DEF_CMD("-hwrxtstmp", -IFCAP_HWRXTSTMP, setifcap),
- DEF_CMD("normal", -IFF_LINK0, setifflags),
+ DEF_CMD("-hwrxtstmp", IFCAP_HWRXTSTMP, clearifcap),
+ DEF_CMD("normal", IFF_LINK0, clearifflags),
DEF_CMD("compress", IFF_LINK0, setifflags),
DEF_CMD("noicmp", IFF_LINK1, setifflags),
DEF_CMD_ARG("mtu", setifmtu),
diff --git a/sbin/ifconfig/ifconfig.h b/sbin/ifconfig/ifconfig.h
index e33a2c63aec1..84c1ac7eebce 100644
--- a/sbin/ifconfig/ifconfig.h
+++ b/sbin/ifconfig/ifconfig.h
@@ -255,6 +255,7 @@ extern int allmedia;
extern int exit_code;
extern char *f_inet, *f_inet6, *f_ether, *f_addr;
+void clearifcap(if_ctx *ctx, const char *, int value);
void setifcap(if_ctx *ctx, const char *, int value);
void setifcapnv(if_ctx *ctx, const char *vname, const char *arg);
diff --git a/sbin/ifconfig/ifvlan.c b/sbin/ifconfig/ifvlan.c
index b871e953c44a..adc3c4692f8b 100644
--- a/sbin/ifconfig/ifvlan.c
+++ b/sbin/ifconfig/ifvlan.c
@@ -286,15 +286,15 @@ static struct cmd vlan_cmds[] = {
/* XXX For compatibility. Should become DEF_CMD() some day. */
DEF_CMD_OPTARG("-vlandev", unsetvlandev),
DEF_CMD("vlanmtu", IFCAP_VLAN_MTU, setifcap),
- DEF_CMD("-vlanmtu", -IFCAP_VLAN_MTU, setifcap),
+ DEF_CMD("-vlanmtu", IFCAP_VLAN_MTU, clearifcap),
DEF_CMD("vlanhwtag", IFCAP_VLAN_HWTAGGING, setifcap),
- DEF_CMD("-vlanhwtag", -IFCAP_VLAN_HWTAGGING, setifcap),
+ DEF_CMD("-vlanhwtag", IFCAP_VLAN_HWTAGGING, clearifcap),
DEF_CMD("vlanhwfilter", IFCAP_VLAN_HWFILTER, setifcap),
- DEF_CMD("-vlanhwfilter", -IFCAP_VLAN_HWFILTER, setifcap),
- DEF_CMD("-vlanhwtso", -IFCAP_VLAN_HWTSO, setifcap),
+ DEF_CMD("-vlanhwfilter", IFCAP_VLAN_HWFILTER, clearifcap),
+ DEF_CMD("-vlanhwtso", IFCAP_VLAN_HWTSO, clearifcap),
DEF_CMD("vlanhwtso", IFCAP_VLAN_HWTSO, setifcap),
DEF_CMD("vlanhwcsum", IFCAP_VLAN_HWCSUM, setifcap),
- DEF_CMD("-vlanhwcsum", -IFCAP_VLAN_HWCSUM, setifcap),
+ DEF_CMD("-vlanhwcsum", IFCAP_VLAN_HWCSUM, clearifcap),
};
static struct afswtch af_vlan = {
.af_name = "af_vlan",
diff --git a/sbin/ifconfig/ifvxlan.c b/sbin/ifconfig/ifvxlan.c
index 4f54bee88b41..72e2eac35c2d 100644
--- a/sbin/ifconfig/ifvxlan.c
+++ b/sbin/ifconfig/ifvxlan.c
@@ -613,9 +613,9 @@ static struct cmd vxlan_cmds[] = {
DEF_CMD("vxlanflushall", 1, setvxlan_flush),
DEF_CMD("vxlanhwcsum", IFCAP_VXLAN_HWCSUM, setifcap),
- DEF_CMD("-vxlanhwcsum", -IFCAP_VXLAN_HWCSUM, setifcap),
+ DEF_CMD("-vxlanhwcsum", IFCAP_VXLAN_HWCSUM, clearifcap),
DEF_CMD("vxlanhwtso", IFCAP_VXLAN_HWTSO, setifcap),
- DEF_CMD("-vxlanhwtso", -IFCAP_VXLAN_HWTSO, setifcap),
+ DEF_CMD("-vxlanhwtso", IFCAP_VXLAN_HWTSO, clearifcap),
};
static struct afswtch af_vxlan = {