Skip to content

Commit cbc6cb4

Browse files
borkmannvishvananda
authored andcommitted
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: 54ad9e3 ("Two new functions: LinkSetBondSlave and VethPeerIndex") Reported-by: Jean Raby <[email protected]> Signed-off-by: Daniel Borkmann <[email protected]> Cc: phob0s <[email protected]>
1 parent b9fd967 commit cbc6cb4

File tree

2 files changed

+38
-4
lines changed

2 files changed

+38
-4
lines changed

ioctl_linux.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ type ethtoolSset struct {
5959
type ethtoolStats struct {
6060
cmd uint32
6161
nStats uint32
62-
data [1]uint64
62+
// Followed by nStats * []uint64.
6363
}
6464

6565
// newIocltSlaveReq returns filled IfreqSlave with proper interface names

link_linux.go

+37-3
Original file line numberDiff line numberDiff line change
@@ -2919,6 +2919,28 @@ func LinkSetBondSlaveQueueId(link Link, queueId uint16) error {
29192919
return pkgHandle.LinkSetBondSlaveQueueId(link, queueId)
29202920
}
29212921

2922+
func vethStatsSerialize(stats ethtoolStats) ([]byte, error) {
2923+
statsSize := int(unsafe.Sizeof(stats)) + int(stats.nStats)*int(unsafe.Sizeof(uint64(0)))
2924+
b := make([]byte, 0, statsSize)
2925+
buf := bytes.NewBuffer(b)
2926+
err := binary.Write(buf, nl.NativeEndian(), stats)
2927+
return buf.Bytes()[:statsSize], err
2928+
}
2929+
2930+
type vethEthtoolStats struct {
2931+
Cmd uint32
2932+
NStats uint32
2933+
Peer uint64
2934+
// Newer kernels have XDP stats in here, but we only care
2935+
// to extract the peer ifindex here.
2936+
}
2937+
2938+
func vethStatsDeserialize(b []byte) (vethEthtoolStats, error) {
2939+
var stats = vethEthtoolStats{}
2940+
err := binary.Read(bytes.NewReader(b), nl.NativeEndian(), &stats)
2941+
return stats, err
2942+
}
2943+
29222944
// VethPeerIndex get veth peer index.
29232945
func VethPeerIndex(link *Veth) (int, error) {
29242946
fd, err := getSocketUDP()
@@ -2933,16 +2955,28 @@ func VethPeerIndex(link *Veth) (int, error) {
29332955
return -1, fmt.Errorf("SIOCETHTOOL request for %q failed, errno=%v", link.Attrs().Name, errno)
29342956
}
29352957

2936-
stats := &ethtoolStats{
2958+
stats := ethtoolStats{
29372959
cmd: ETHTOOL_GSTATS,
29382960
nStats: sSet.data[0],
29392961
}
2940-
ifreq.Data = uintptr(unsafe.Pointer(stats))
2962+
2963+
buffer, err := vethStatsSerialize(stats)
2964+
if err != nil {
2965+
return -1, err
2966+
}
2967+
2968+
ifreq.Data = uintptr(unsafe.Pointer(&buffer[0]))
29412969
_, _, errno = syscall.Syscall(syscall.SYS_IOCTL, uintptr(fd), SIOCETHTOOL, uintptr(unsafe.Pointer(ifreq)))
29422970
if errno != 0 {
29432971
return -1, fmt.Errorf("SIOCETHTOOL request for %q failed, errno=%v", link.Attrs().Name, errno)
29442972
}
2945-
return int(stats.data[0]), nil
2973+
2974+
vstats, err := vethStatsDeserialize(buffer)
2975+
if err != nil {
2976+
return -1, err
2977+
}
2978+
2979+
return int(vstats.Peer), nil
29462980
}
29472981

29482982
func parseTuntapData(link Link, data []syscall.NetlinkRouteAttr) {

0 commit comments

Comments
 (0)