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: 54ad9e3a4c ("Two new functions: LinkSetBondSlave and VethPeerIndex")
Reported-by: Jean Raby <jean@raby.sh>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: phob0s <git@phob0s.pl>
This commit is contained in:
Daniel Borkmann 2019-11-06 10:03:43 +01:00 committed by Vish (Ishaya) Abrams
parent b9fd9670a1
commit cbc6cb49af
2 changed files with 38 additions and 4 deletions

View File

@ -59,7 +59,7 @@ type ethtoolSset struct {
type ethtoolStats struct { type ethtoolStats struct {
cmd uint32 cmd uint32
nStats uint32 nStats uint32
data [1]uint64 // Followed by nStats * []uint64.
} }
// newIocltSlaveReq returns filled IfreqSlave with proper interface names // newIocltSlaveReq returns filled IfreqSlave with proper interface names

View File

@ -2919,6 +2919,28 @@ func LinkSetBondSlaveQueueId(link Link, queueId uint16) error {
return pkgHandle.LinkSetBondSlaveQueueId(link, queueId) 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. // VethPeerIndex get veth peer index.
func VethPeerIndex(link *Veth) (int, error) { func VethPeerIndex(link *Veth) (int, error) {
fd, err := getSocketUDP() 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) return -1, fmt.Errorf("SIOCETHTOOL request for %q failed, errno=%v", link.Attrs().Name, errno)
} }
stats := &ethtoolStats{ stats := ethtoolStats{
cmd: ETHTOOL_GSTATS, cmd: ETHTOOL_GSTATS,
nStats: sSet.data[0], 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))) _, _, errno = syscall.Syscall(syscall.SYS_IOCTL, uintptr(fd), SIOCETHTOOL, uintptr(unsafe.Pointer(ifreq)))
if errno != 0 { if errno != 0 {
return -1, fmt.Errorf("SIOCETHTOOL request for %q failed, errno=%v", link.Attrs().Name, errno) 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) { func parseTuntapData(link Link, data []syscall.NetlinkRouteAttr) {