Make sure rtnl_handle_newlink behave like
ioctl and set the caller's crednetial during calls to ifc_create_ifp_nl and
ifc_modify_ifp_nl.
Details
Tested in if_vlan, and my new geneve module
which is based on netlink.
To test via if_vlan:
ifconfig epair0 create up jail -c persist name=nlvnet1 mount.devfs devfs_ruleset=5 vnet vnet.interface=epair0b path=/ ifconfig -j nlvnet1 epair0b up ifconfig vlan0 create inet6 2001:db8::1 vlan 10 vlandev epair0a ifconfig -j nlvnet1 vlan0 create inet6 2001:db8::2 vlandev epair0b vlan 10 ping -c1 2001:db8::2 jexec -l nlvnet1 ping -c1 2001:db8::1
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 69125 Build 66008: arc lint + arc unit
Event Timeline
During clone creation of my new Geneve interface module, I found that when I use ether_gen_addr it generates the same MAC address for interfaces with the same name in different jails.
However, it worked correctly when I'm not using create_nl (netlink). I found that netlink does not set the curvnet the way ioctl does.
Looks correct! Thanks. What did happen before? This command failed?
ifconfig -j nlvnet1 vlan0 create inet6 2001:db8::2 vlandev epair0b vlan 10
No, However, it is the only interface that uses the if_clone_addreq_v2. So it's the only test that I can do to make sure existing implementation doesn't break.
For the geneve, I'm using this code below:
static int
geneve_clone_create_nl(struct if_clone *ifc, char *name, size_t len,
struct ifc_data_nl *ifd)
{
struct nl_parsed_link *lattrs = ifd->lattrs;
struct nl_pstate *npt = ifd->npt;
struct nl_parsed_geneve attrs = {};
int error;
if ((lattrs->ifla_idata == NULL) ||
(!nl_has_attr(ifd->bm, IFLA_LINKINFO))) {
nlmsg_report_err_msg(npt, "geneve protocol is required");
return (ENOTSUP);
}
error = nl_parse_nested(lattrs->ifla_idata, &geneve_create_parser, npt, &attrs);
if (error != 0)
return (error);
if (geneve_check_proto(attrs.ifla_proto)) {
nlmsg_report_err_msg(npt, "Unsupported ethertype: 0x%04X", attrs.ifla_proto);
return (ENOTSUP);
}
struct geneve_params gnvp = {
.ifla_proto = attrs.ifla_proto
};
struct ifc_data ifd_new = {
.flags = IFC_F_SYSSPACE,
.unit = ifd->unit,
.params = &gnvp
};
return (geneve_clone_create(ifc, name, len, &ifd_new, &ifd->ifp));
}And when I set the curvnet by using npt->nlp->nl_socket->so_vnet everything works properly.
CURVNET_SET(npt->nlp->nl_socket->so_vnet); error = geneve_clone_create(ifc, name, len, &ifd_new, &ifd->ifp); CURVNET_RESTORE();
The code that mostly sensitive to vnet was ether_gen_addr inside the geneve_clone_create and it was producing same MAC address between vnets.
That's how I found about it.
So the test I wrote is for the only existing implementation (if_vlan), which is not much depends on vnet for cloning.
The geneve interface I wrote is almost ready and it has some other netlink changes inside it.
I will submit it soon. but it depends on this change.
After writing some tests with kyua I found a problem to my code.
CURVNET_SET sets the curthread->td_vnet. However, ether_gen_addr use curthread->td_ucred->cr_prison->pr_vnet which is different.
It seems like nl_taskqueue_handler already sets curthread->td_vnet.
In summary, TD_TO_VNET value is not same as curvnet, and ether_gen_addr use curthread->td_ucred.
I wonder how can I change the td_ucred to the actual one.
Maybe I should use OSD(9)... I have to figure things out.
The problem with netlink_socket not having the correct prison credential is that it’s run as an async/taskqueue function in a separate thread, therefore it doesn't have the same credentials as the caller thread. When we call the ioctl it runs as a synchronous thread and therefore doesn’t change its thread and its credentials. However, when we call netlink it runs in a separate thread named netlink_socket.
Is there a way to ensure the new netlink_socket thread uses the caller’s credentials? we have the correct credentials in nl_socket.
It feels wrong, but I have to ask: is it correct to set curthread->td_ucred to nlp->nl_socket->so_cred using crdup or crcopy? we could reverse it afterward, similar to what saved_vnet does, to avoid affecting other operations.
To be clear, my goal is to make netlink behave just like ifioctl in passing credential to not delegating credential management to other modules.
@glebius
This is tested and works. However, I'm not sure if it's clean or need a lock due to changing references in a async thread
without this patch:
# kyua test -k /usr/tests/sys/net/Kyuafile if_geneve if_geneve:ether_ipv4 -> Dec 8 15:53:29 ftsr1 kernel: geneve1: DAD detected duplicate IPv6 address fe80:d::5a9c:fcff:fe10:3953: NS in/out/loopback=1/1/0, NA in=0 Dec 8 15:53:29 ftsr1 kernel: geneve1: DAD complete for fe80:d::5a9c:fcff:fe10:3953 - duplicate found Dec 8 15:53:29 ftsr1 kernel: geneve1: manual intervention required Dec 8 15:53:29 ftsr1 kernel: geneve1: possible hardware address duplication detected, disable IPv6 failed: atf-check failed; see the output of the test for details [1.162s] if_geneve:ether_ipv6 -> Dec 8 15:53:32 ftsr1 kernel: geneve1: DAD detected duplicate IPv6 address fe80:d::5a9c:fcff:fe10:3953: NS in/out/loopback=1/1/0, NA in=0 Dec 8 15:53:32 ftsr1 kernel: geneve1: DAD complete for fe80:d::5a9c:fcff:fe10:3953 - duplicate found Dec 8 15:53:32 ftsr1 kernel: geneve1: manual intervention required Dec 8 15:53:32 ftsr1 kernel: geneve1: possible hardware address duplication detected, disable IPv6 failed: atf-check failed; see the output of the test for details [1.172s] if_geneve:inherit_ipv4 -> passed [0.129s] if_geneve:inherit_ipv6 -> passed [0.166s]
after this patch, mac addresses generated by ether_gen_addr is unique:
# kyua test -k /usr/tests/sys/net/Kyuafile if_geneve if_geneve:ether_ipv4 -> passed [0.142s] if_geneve:ether_ipv6 -> passed [0.159s] if_geneve:inherit_ipv4 -> passed [0.149s] if_geneve:inherit_ipv6 -> passed [0.165s]