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
This commit is contained in:
John Baldwin 2023-06-19 10:37:52 -07:00
parent e60316d1ea
commit 08992b2078
4 changed files with 79 additions and 49 deletions

View file

@ -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),

View file

@ -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);

View file

@ -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",

View file

@ -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 = {