From 9606690a733b67de2fd57beb399fa0fc965918b4 Mon Sep 17 00:00:00 2001 From: Mohammad Javad Pooladkhay Date: Fri, 1 Dec 2023 18:08:16 +0000 Subject: [PATCH] aya: fix tc name limit The buffer for attributes should be sized to hold at least 256 bytes, based on `CLS_BPF_NAME_LEN = 256` from the kernel: https://github.com/torvalds/linux/blob/02aee814/net/sched/cls_bpf.c#L28 Fixes: #610 aya: add netlink_clsact_qdisc_exists function public-api: add clsact_qdisc_exists integration-test: add tc_name_limit integration-test: add tc_name_limit_exceeded Signed-off-by: Mohammad Javad Pooladkhay --- aya/src/programs/tc.rs | 10 +++- aya/src/sys/netlink.rs | 52 ++++++++++++++++-- test/integration-ebpf/Cargo.toml | 8 +++ test/integration-ebpf/src/tc_name_limit.rs | 22 ++++++++ .../src/tc_name_limit_exceeded.rs | 22 ++++++++ test/integration-test/src/lib.rs | 4 ++ test/integration-test/src/tests/load.rs | 54 ++++++++++++++++++- xtask/public-api/aya.txt | 1 + 8 files changed, 167 insertions(+), 6 deletions(-) create mode 100644 test/integration-ebpf/src/tc_name_limit.rs create mode 100644 test/integration-ebpf/src/tc_name_limit_exceeded.rs diff --git a/aya/src/programs/tc.rs b/aya/src/programs/tc.rs index 92cc1699..ade238f4 100644 --- a/aya/src/programs/tc.rs +++ b/aya/src/programs/tc.rs @@ -14,8 +14,8 @@ use crate::{ }, programs::{define_link_wrapper, load_program, Link, ProgramData, ProgramError}, sys::{ - netlink_find_filter_with_name, netlink_qdisc_add_clsact, netlink_qdisc_attach, - netlink_qdisc_detach, + netlink_clsact_qdisc_exists, netlink_find_filter_with_name, netlink_qdisc_add_clsact, + netlink_qdisc_attach, netlink_qdisc_detach, }, util::{ifindex_from_ifname, tc_handler_make}, VerifierLogLevel, @@ -358,3 +358,9 @@ fn qdisc_detach_program_fast( Ok(()) } + +/// Check if the `clasct` qdisc exists on the given interface. +pub fn clsact_qdisc_exists(if_name: &str) -> Result { + let if_index = ifindex_from_ifname(if_name)?; + unsafe { netlink_clsact_qdisc_exists(if_index as i32) } +} diff --git a/aya/src/sys/netlink.rs b/aya/src/sys/netlink.rs index 96c4f01a..c58d2d49 100644 --- a/aya/src/sys/netlink.rs +++ b/aya/src/sys/netlink.rs @@ -10,8 +10,8 @@ use libc::{ getsockname, nlattr, nlmsgerr, nlmsghdr, recv, send, setsockopt, sockaddr_nl, socket, AF_NETLINK, AF_UNSPEC, ETH_P_ALL, IFF_UP, IFLA_XDP, NETLINK_EXT_ACK, NETLINK_ROUTE, NLA_ALIGNTO, NLA_F_NESTED, NLA_TYPE_MASK, NLMSG_DONE, NLMSG_ERROR, NLM_F_ACK, NLM_F_CREATE, - NLM_F_DUMP, NLM_F_ECHO, NLM_F_EXCL, NLM_F_MULTI, NLM_F_REQUEST, RTM_DELTFILTER, RTM_GETTFILTER, - RTM_NEWQDISC, RTM_NEWTFILTER, RTM_SETLINK, SOCK_RAW, SOL_NETLINK, + NLM_F_DUMP, NLM_F_ECHO, NLM_F_EXCL, NLM_F_MULTI, NLM_F_REQUEST, RTM_DELTFILTER, RTM_GETQDISC, + RTM_GETTFILTER, RTM_NEWQDISC, RTM_NEWTFILTER, RTM_SETLINK, SOCK_RAW, SOL_NETLINK, }; use thiserror::Error; @@ -80,6 +80,46 @@ pub(crate) unsafe fn netlink_set_xdp_fd( Ok(()) } +pub(crate) unsafe fn netlink_clsact_qdisc_exists(if_index: i32) -> Result { + let sock = NetlinkSocket::open()?; + + let mut req = mem::zeroed::(); + + let nlmsg_len = mem::size_of::() + mem::size_of::(); + + req.header = nlmsghdr { + nlmsg_len: nlmsg_len as u32, + nlmsg_flags: (NLM_F_REQUEST | NLM_F_ACK | NLM_F_DUMP) as u16, + nlmsg_type: RTM_GETQDISC, + nlmsg_pid: 0, + nlmsg_seq: 1, + }; + req.tc_info.tcm_family = AF_UNSPEC as u8; + + sock.send(&bytes_of(&req)[..req.header.nlmsg_len as usize])?; + + for msg in sock.recv()? { + if msg.header.nlmsg_type != RTM_NEWQDISC { + continue; + } + + let tc_msg = ptr::read_unaligned(msg.data.as_ptr() as *const tcmsg); + if tc_msg.tcm_ifindex != if_index { + continue; + } + + let attrs = parse_attrs(&msg.data[mem::size_of::()..])?; + + if let Some(opts) = attrs.get(&(TCA_KIND as u16)) { + if opts.data == b"clsact\0" { + return Ok(true); + } + } + } + + Ok(false) +} + pub(crate) unsafe fn netlink_qdisc_add_clsact(if_index: i32) -> Result<(), io::Error> { let sock = NetlinkSocket::open()?; @@ -289,7 +329,13 @@ struct Request { struct TcRequest { header: nlmsghdr, tc_info: tcmsg, - attrs: [u8; 64], + // The buffer for attributes should be sized to hold at least 256 bytes, + // based on `CLS_BPF_NAME_LEN = 256` from the kernel: + // https://github.com/torvalds/linux/blob/02aee814/net/sched/cls_bpf.c#L28 + // We currently use around ~30 bytes of attributes in addition to name. + // Rather than picking a "right sized buffer" for the payload (which is of + // varying length anyway) we use the next largest power of 2. + attrs: [u8; 512], } struct NetlinkSocket { diff --git a/test/integration-ebpf/Cargo.toml b/test/integration-ebpf/Cargo.toml index d471acf3..051be3c7 100644 --- a/test/integration-ebpf/Cargo.toml +++ b/test/integration-ebpf/Cargo.toml @@ -55,3 +55,11 @@ path = "src/xdp_sec.rs" [[bin]] name = "ring_buf" path = "src/ring_buf.rs" + +[[bin]] +name = "tc_name_limit" +path = "src/tc_name_limit.rs" + +[[bin]] +name = "tc_name_limit_exceeded" +path = "src/tc_name_limit_exceeded.rs" \ No newline at end of file diff --git a/test/integration-ebpf/src/tc_name_limit.rs b/test/integration-ebpf/src/tc_name_limit.rs new file mode 100644 index 00000000..338374ff --- /dev/null +++ b/test/integration-ebpf/src/tc_name_limit.rs @@ -0,0 +1,22 @@ +#![no_std] +#![no_main] + +use aya_bpf::{macros::classifier, programs::TcContext}; + +// A function with a 256-byte-long name (all 'a's) to be used as the name of +// the ebpf program. This name must match the name passed to userspace side +// of the program (i.e. test/integration-test/src/tests/load.rs). +// 256 is the maximum length allowed by the kernel, so this test should pass. +// https://github.com/torvalds/linux/blob/02aee814/net/sched/cls_bpf.c#L28 +#[classifier] +pub fn aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa( + _ctx: TcContext, +) -> i32 { + 0 +} + +#[cfg(not(test))] +#[panic_handler] +fn panic(_info: &core::panic::PanicInfo) -> ! { + loop {} +} diff --git a/test/integration-ebpf/src/tc_name_limit_exceeded.rs b/test/integration-ebpf/src/tc_name_limit_exceeded.rs new file mode 100644 index 00000000..6a244d6a --- /dev/null +++ b/test/integration-ebpf/src/tc_name_limit_exceeded.rs @@ -0,0 +1,22 @@ +#![no_std] +#![no_main] + +use aya_bpf::{macros::classifier, programs::TcContext}; + +// A function with a 257-byte-long name (all 'a's) to be used as the name of +// the ebpf program. This name must match the name passed to userspace side +// of the program (i.e. test/integration-test/src/tests/load.rs). +// 256 is the maximum length allowed by the kernel, so this test should fail. +// https://github.com/torvalds/linux/blob/02aee814/net/sched/cls_bpf.c#L28 +#[classifier] +pub fn aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa( + _ctx: TcContext, +) -> i32 { + 0 +} + +#[cfg(not(test))] +#[panic_handler] +fn panic(_info: &core::panic::PanicInfo) -> ! { + loop {} +} diff --git a/test/integration-test/src/lib.rs b/test/integration-test/src/lib.rs index d4708033..7c3ac219 100644 --- a/test/integration-test/src/lib.rs +++ b/test/integration-test/src/lib.rs @@ -22,6 +22,10 @@ pub const BPF_PROBE_READ: &[u8] = pub const REDIRECT: &[u8] = include_bytes_aligned!(concat!(env!("OUT_DIR"), "/redirect")); pub const XDP_SEC: &[u8] = include_bytes_aligned!(concat!(env!("OUT_DIR"), "/xdp_sec")); pub const RING_BUF: &[u8] = include_bytes_aligned!(concat!(env!("OUT_DIR"), "/ring_buf")); +pub const TC_NAME_LIMIT_TEST: &[u8] = + include_bytes_aligned!(concat!(env!("OUT_DIR"), "/tc_name_limit")); +pub const TC_NAME_LIMIT_EXCEEDED_TEST: &[u8] = + include_bytes_aligned!(concat!(env!("OUT_DIR"), "/tc_name_limit_exceeded")); #[cfg(test)] mod tests; diff --git a/test/integration-test/src/tests/load.rs b/test/integration-test/src/tests/load.rs index ee5ba713..442b325f 100644 --- a/test/integration-test/src/tests/load.rs +++ b/test/integration-test/src/tests/load.rs @@ -6,11 +6,15 @@ use std::{ time::{Duration, SystemTime}, }; +use assert_matches::assert_matches; use aya::{ maps::Array, programs::{ links::{FdLink, PinnedLink}, - loaded_links, loaded_programs, KProbe, TracePoint, UProbe, Xdp, XdpFlags, + loaded_links, loaded_programs, + tc::{clsact_qdisc_exists, qdisc_add_clsact}, + KProbe, ProgramError, SchedClassifier, TcAttachType, TcError, TracePoint, UProbe, Xdp, + XdpFlags, }, util::KernelVersion, Bpf, @@ -608,3 +612,51 @@ fn pin_lifecycle_uprobe() { // Make sure the function isn't optimized out. uprobe_function(); } + +#[test] +fn tc_name_limit() { + let clsact_exists = clsact_qdisc_exists("lo").unwrap(); + if !clsact_exists { + qdisc_add_clsact("lo").unwrap(); + } + + let mut bpf = Bpf::load(crate::TC_NAME_LIMIT_TEST).unwrap(); + + let long_name = "a".repeat(256); + + let program: &mut SchedClassifier = bpf + .program_mut(long_name.as_str()) + .unwrap() + .try_into() + .unwrap(); + program.load().unwrap(); + program.attach("lo", TcAttachType::Ingress).unwrap(); +} + +#[test] +fn tc_name_limit_exceeded() { + let clsact_exists = clsact_qdisc_exists("lo").unwrap(); + if !clsact_exists { + qdisc_add_clsact("lo").unwrap(); + } + + let mut bpf = Bpf::load(crate::TC_NAME_LIMIT_EXCEEDED_TEST).unwrap(); + + let long_name = "a".repeat(257); + + let program: &mut SchedClassifier = bpf + .program_mut(long_name.as_str()) + .unwrap() + .try_into() + .unwrap(); + program.load().unwrap(); + + assert_matches!( + program.attach("lo", TcAttachType::Ingress), + Err(ProgramError::TcError(TcError::NetlinkError { io_error })) => { + // An invalid argument error (EINVAL) with code 22 should occur. + // The invalid argument is the tc program name which is too long. + assert_eq!(io_error.raw_os_error(), Some(22)) + } + ); +} diff --git a/xtask/public-api/aya.txt b/xtask/public-api/aya.txt index 0adbf0c2..4114f56c 100644 --- a/xtask/public-api/aya.txt +++ b/xtask/public-api/aya.txt @@ -4645,6 +4645,7 @@ impl core::borrow::BorrowMut for aya::programs::tc::TcOptions where T: cor pub fn aya::programs::tc::TcOptions::borrow_mut(&mut self) -> &mut T impl core::convert::From for aya::programs::tc::TcOptions pub fn aya::programs::tc::TcOptions::from(t: T) -> T +pub fn aya::programs::tc::clsact_qdisc_exists(if_name: &str) -> core::result::Result pub fn aya::programs::tc::qdisc_add_clsact(if_name: &str) -> core::result::Result<(), std::io::error::Error> pub fn aya::programs::tc::qdisc_detach_program(if_name: &str, attach_type: aya::programs::tc::TcAttachType, name: &str) -> core::result::Result<(), std::io::error::Error> pub mod aya::programs::tp_btf