Skip to content

Commit

Permalink
netdev: Always clear struct ifreq before ioctl.
Browse files Browse the repository at this point in the history 
It's not nice to send random stack memory to a kernel over ioctl:

 Uninitialized bytes in ioctl_common_pre
   at offset 20 inside [0x7fff8899f3c0, 40)
 WARNING: MemorySanitizer: use-of-uninitialized-value
  0 0x1180b7f in af_inet_ioctl lib/socket-util-unix.c:417:15
  1 0x1180ffd in af_inet_ifreq_ioctl lib/socket-util-unix.c:428:13
  2 0x11e4fd5 in netdev_linux_set_mtu lib/netdev-linux.c:2005:13
  3 0xba250b in netdev_set_mtu lib/netdev.c:1132:30
  4 0x59a19f in update_mtu_ofproto ofproto/ofproto.c:3042:18
  5 0x596947 in update_mtu ofproto/ofproto.c:3024:5
  6 0x5976d6 in ofport_install ofproto/ofproto.c:2617:5
  7 0x572e96 in update_port ofproto/ofproto.c:2893:21
  8 0x57636b in ofproto_port_add ofproto/ofproto.c:2208:17
  9 0x4f9e0b in iface_do_create vswitchd/bridge.c:2203:13
 10 0x4f7f43 in iface_create vswitchd/bridge.c:2246:13
 11 0x4f7a63 in bridge_add_ports__ vswitchd/bridge.c:1225:21
 12 0x4d8ea4 in bridge_add_ports vswitchd/bridge.c:1241:5
 13 0x4cce3a in bridge_reconfigure vswitchd/bridge.c:952:9
 14 0x4ca156 in bridge_run vswitchd/bridge.c:3439:9
 15 0x5278e7 in main vswitchd/ovs-vswitchd.c:137:9
 16 0x7f9def in __libc_start_call_main
 17 0x7f9def in __libc_start_main@GLIBC_2.2.5
 18 0x432b14 in _start (vswitchd/ovs-vswitchd+0x432b14)

We do already initialize this structure for a few ioctls, let's
do that for all of them.

Signed-off-by: Ilya Maximets <[email protected]>
  • Loading branch information
@igsilya
igsilya committed Nov 29, 2024
1 parent 08e71f0 commit 8d835a7
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 2 deletions.
7 changes: 7 additions & 0 deletions lib/netdev-bsd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,7 @@ netdev_bsd_rxq_drain(struct netdev_rxq *rxq_)
struct ifreq ifr;
struct netdev_rxq_bsd *rxq = netdev_rxq_bsd_cast(rxq_);

memset(&ifr, 0, sizeof ifr);
strcpy(ifr.ifr_name, netdev_get_kernel_name(netdev_rxq_get_netdev(rxq_)));
if (ioctl(rxq->fd, BIOCFLUSH, &ifr) == -1) {
VLOG_DBG_RL(&rl, "%s: ioctl(BIOCFLUSH) failed: %s",
Expand Down Expand Up @@ -828,6 +829,7 @@ netdev_bsd_get_mtu(const struct netdev *netdev_, int *mtup)
if (!(netdev->cache_valid & VALID_MTU)) {
struct ifreq ifr;

memset(&ifr, 0, sizeof ifr);
error = af_inet_ifreq_ioctl(netdev_get_kernel_name(netdev_), &ifr,
SIOCGIFMTU, "SIOCGIFMTU");
if (!error) {
Expand Down Expand Up @@ -1440,6 +1442,8 @@ do_set_addr(struct netdev *netdev,
struct in_addr addr)
{
struct ifreq ifr;

memset(&ifr, 0, sizeof ifr);
make_in4_sockaddr(&ifr.ifr_addr, addr);
return af_inet_ifreq_ioctl(netdev_get_kernel_name(netdev), &ifr, ioctl_nr,
ioctl_name);
Expand Down Expand Up @@ -1547,6 +1551,7 @@ destroy_tap(int fd, const char *name)
struct ifreq ifr;

close(fd);
memset(&ifr, 0, sizeof ifr);
strcpy(ifr.ifr_name, name);
/* XXX What to do if this call fails? */
af_inet_ioctl(SIOCIFDESTROY, &ifr);
Expand All @@ -1558,6 +1563,7 @@ get_flags(const struct netdev *netdev, int *flags)
struct ifreq ifr;
int error;

memset(&ifr, 0, sizeof ifr);
error = af_inet_ifreq_ioctl(netdev_get_kernel_name(netdev), &ifr,
SIOCGIFFLAGS, "SIOCGIFFLAGS");

Expand All @@ -1571,6 +1577,7 @@ set_flags(const char *name, int flags)
{
struct ifreq ifr;

memset(&ifr, 0, sizeof ifr);
ifr_set_flags(&ifr, flags);

return af_inet_ifreq_ioctl(name, &ifr, SIOCSIFFLAGS, "SIOCSIFFLAGS");
Expand Down
17 changes: 15 additions & 2 deletions lib/netdev-linux.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1028,6 +1028,8 @@ netdev_linux_construct_tap(struct netdev *netdev_)
ovsthread_once_done(&once);
}

memset(&ifr, 0, sizeof ifr);

ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
if (tap_supports_vnet_hdr) {
ifr.ifr_flags |= IFF_VNET_HDR;
Expand Down Expand Up @@ -1582,8 +1584,11 @@ netdev_linux_rxq_drain(struct netdev_rxq *rxq_)
struct netdev_rxq_linux *rx = netdev_rxq_linux_cast(rxq_);
if (rx->is_tap) {
struct ifreq ifr;
int error = af_inet_ifreq_ioctl(netdev_rxq_get_name(rxq_), &ifr,
SIOCGIFTXQLEN, "SIOCGIFTXQLEN");
int error;

memset(&ifr, 0, sizeof ifr);
error = af_inet_ifreq_ioctl(netdev_rxq_get_name(rxq_), &ifr,
SIOCGIFTXQLEN, "SIOCGIFTXQLEN");
if (error) {
return error;
}
Expand Down Expand Up @@ -1939,6 +1944,7 @@ netdev_linux_get_mtu__(struct netdev_linux *netdev, int *mtup)
/* Fall back to ioctl if netlink fails */
struct ifreq ifr;

memset(&ifr, 0, sizeof ifr);
netdev->netdev_mtu_error = af_inet_ifreq_ioctl(
netdev_get_name(&netdev->up), &ifr, SIOCGIFMTU, "SIOCGIFMTU");
netdev->mtu = ifr.ifr_mtu;
Expand Down Expand Up @@ -2001,7 +2007,10 @@ netdev_linux_set_mtu(struct netdev *netdev_, int mtu)
}
netdev->cache_valid &= ~VALID_MTU;
}

memset(&ifr, 0, sizeof ifr);
ifr.ifr_mtu = mtu;

error = af_inet_ifreq_ioctl(netdev_get_name(netdev_), &ifr,
SIOCSIFMTU, "SIOCSIFMTU");
if (!error || error == ENODEV) {
Expand Down Expand Up @@ -3570,6 +3579,7 @@ do_set_addr(struct netdev *netdev,
{
struct ifreq ifr;

memset(&ifr, 0, sizeof ifr);
make_in4_sockaddr(&ifr.ifr_addr, addr);
return af_inet_ifreq_ioctl(netdev_get_name(netdev), &ifr, ioctl_nr,
ioctl_name);
Expand Down Expand Up @@ -6767,6 +6777,7 @@ get_flags(const struct netdev *dev, unsigned int *flags)
struct ifreq ifr;
int error;

memset(&ifr, 0, sizeof ifr);
*flags = 0;
error = af_inet_ifreq_ioctl(dev->name, &ifr, SIOCGIFFLAGS, "SIOCGIFFLAGS");
if (!error) {
Expand All @@ -6780,6 +6791,7 @@ set_flags(const char *name, unsigned int flags)
{
struct ifreq ifr;

memset(&ifr, 0, sizeof ifr);
ifr.ifr_flags = flags;
return af_inet_ifreq_ioctl(name, &ifr, SIOCSIFFLAGS, "SIOCSIFFLAGS");
}
Expand All @@ -6790,6 +6802,7 @@ linux_get_ifindex(const char *netdev_name)
struct ifreq ifr;
int error;

memset(&ifr, 0, sizeof ifr);
ovs_strzcpy(ifr.ifr_name, netdev_name, sizeof ifr.ifr_name);
COVERAGE_INC(netdev_get_ifindex);

Expand Down

0 comments on commit 8d835a7

Please sign in to comment.