From cbc6cb49afd2d852e578baa240bf5d522ab96d46 Mon Sep 17 00:00:00 2001 From: Daniel Borkmann Date: Wed, 6 Nov 2019 10:03:43 +0100 Subject: [PATCH] link, veth: fix stack corruption from retrieving peer index For 4.20 and newer kernels VethPeerIndex() causes a stack corruption as the kernel is copying more data to golang user space than originally expected. This is due to a recent kernel commit where it extends veth driver's ethtool stats for XDP: https://git.kernel.org/torvalds/c/d397b9682c1c808344dd93b43de8750fa4d9f581 The VethPeerIndex()'s logic is utterly wrong to assume ethtool stats are never extended in the driver. Unfortunately there is no other way around in golang than to add serialize/deserialize helpers to have a dynamically sized ethtoolStats with a uint64 data array that has the size of the previous result from the ETHTOOL_GSSET_INFO query. This ensures we don't run into a buffer overflow triggered by kernel's copy_to_user() in ETHTOOL_GSTATS query (ethtool_get_stats() in kernel). Now, for the deserialize operation we really only care about the peer's ifindex which is always stored in the first uint64. Fixes: 54ad9e3a4cbb ("Two new functions: LinkSetBondSlave and VethPeerIndex") Reported-by: Jean Raby Signed-off-by: Daniel Borkmann Cc: phob0s --- ioctl_linux.go | 2 +- link_linux.go | 40 +++++++++++++++++++++++++++++++++++++--- 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/ioctl_linux.go b/ioctl_linux.go index f0b9129..4d33db5 100644 --- a/ioctl_linux.go +++ b/ioctl_linux.go @@ -59,7 +59,7 @@ type ethtoolSset struct { type ethtoolStats struct { cmd uint32 nStats uint32 - data [1]uint64 + // Followed by nStats * []uint64. } // newIocltSlaveReq returns filled IfreqSlave with proper interface names diff --git a/link_linux.go b/link_linux.go index b16a772..c161621 100644 --- a/link_linux.go +++ b/link_linux.go @@ -2919,6 +2919,28 @@ func LinkSetBondSlaveQueueId(link Link, queueId uint16) error { return pkgHandle.LinkSetBondSlaveQueueId(link, queueId) } +func vethStatsSerialize(stats ethtoolStats) ([]byte, error) { + statsSize := int(unsafe.Sizeof(stats)) + int(stats.nStats)*int(unsafe.Sizeof(uint64(0))) + b := make([]byte, 0, statsSize) + buf := bytes.NewBuffer(b) + err := binary.Write(buf, nl.NativeEndian(), stats) + return buf.Bytes()[:statsSize], err +} + +type vethEthtoolStats struct { + Cmd uint32 + NStats uint32 + Peer uint64 + // Newer kernels have XDP stats in here, but we only care + // to extract the peer ifindex here. +} + +func vethStatsDeserialize(b []byte) (vethEthtoolStats, error) { + var stats = vethEthtoolStats{} + err := binary.Read(bytes.NewReader(b), nl.NativeEndian(), &stats) + return stats, err +} + // VethPeerIndex get veth peer index. func VethPeerIndex(link *Veth) (int, error) { fd, err := getSocketUDP() @@ -2933,16 +2955,28 @@ func VethPeerIndex(link *Veth) (int, error) { return -1, fmt.Errorf("SIOCETHTOOL request for %q failed, errno=%v", link.Attrs().Name, errno) } - stats := ðtoolStats{ + stats := ethtoolStats{ cmd: ETHTOOL_GSTATS, nStats: sSet.data[0], } - ifreq.Data = uintptr(unsafe.Pointer(stats)) + + buffer, err := vethStatsSerialize(stats) + if err != nil { + return -1, err + } + + ifreq.Data = uintptr(unsafe.Pointer(&buffer[0])) _, _, errno = syscall.Syscall(syscall.SYS_IOCTL, uintptr(fd), SIOCETHTOOL, uintptr(unsafe.Pointer(ifreq))) if errno != 0 { return -1, fmt.Errorf("SIOCETHTOOL request for %q failed, errno=%v", link.Attrs().Name, errno) } - return int(stats.data[0]), nil + + vstats, err := vethStatsDeserialize(buffer) + if err != nil { + return -1, err + } + + return int(vstats.Peer), nil } func parseTuntapData(link Link, data []syscall.NetlinkRouteAttr) {