lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <33467f55-4bbf-4078-af21-d91c6aab82ee@lunn.ch> Date: Sat, 21 Oct 2023 00:18:53 +0200 From: Andrew Lunn <andrew@...n.ch> To: Daniel Borkmann <daniel@...earbox.net> Cc: bpf@...r.kernel.org, netdev@...r.kernel.org, martin.lau@...ux.dev, razor@...ckwall.org, ast@...nel.org, andrii@...nel.org, john.fastabend@...il.com, sdf@...gle.com, toke@...nel.org Subject: Re: [PATCH bpf-next v2 1/7] netkit, bpf: Add bpf programmable net device > +static void netkit_get_drvinfo(struct net_device *dev, > + struct ethtool_drvinfo *info) > +{ > + strscpy(info->driver, DRV_NAME, sizeof(info->driver)); > + strscpy(info->version, "n/a", sizeof(info->version)); If you don't put anything in version, the core will put in the git hash of the kernel. Its more useful than "n/a". > + ether_setup(dev); > + dev->min_mtu = ETH_MIN_MTU; ether_setup() sets min_mtu to ETH_MIN_MTU. > +static int netkit_new_link(struct net *src_net, struct net_device *dev, > + struct nlattr *tb[], struct nlattr *data[], > + struct netlink_ext_ack *extack) > +{ ... > + err = register_netdevice(peer); > + put_net(net); > + if (err < 0) > + goto err_register_peer; > + > + netif_carrier_off(peer); > + > + err = rtnl_configure_link(peer, ifmp, 0, NULL); > + if (err < 0) > + goto err_configure_peer; Seeing code after calling register_netdevice() often means bugs. The interface is live, and in use before the function even returns. The kernel can try to get an IP address, mount an NFS root etc. This might be safe, because you have two linked interfaces here, and the other one is not yet registered. Maybe some comment about this would be good, or can the rtnl_configure_link() be done earlier? > + > + if (mode == NETKIT_L2) > + eth_hw_addr_random(dev); > + if (tb[IFLA_IFNAME]) > + nla_strscpy(dev->name, tb[IFLA_IFNAME], IFNAMSIZ); > + else > + snprintf(dev->name, IFNAMSIZ, "m%%d"); > + > + err = register_netdevice(dev); > + if (err < 0) > + goto err_configure_peer; We have the same here, but now we have both peers registers, the kernel could of configured both up in order to find its NFS root etc. Is it safe to have packets flowing at this point? Before the remaining configuration happens? > + > + netif_carrier_off(dev); > + > + nk = netdev_priv(dev); > + nk->primary = true; > + nk->policy = default_prim; > + nk->mode = mode; > + if (nk->mode == NETKIT_L2) > + dev_change_flags(dev, dev->flags & ~IFF_NOARP, NULL); > + bpf_mprog_bundle_init(&nk->bundle); > + RCU_INIT_POINTER(nk->active, NULL); > + rcu_assign_pointer(nk->peer, peer); > + > + nk = netdev_priv(peer); > + nk->primary = false; > + nk->policy = default_peer; > + nk->mode = mode; > + if (nk->mode == NETKIT_L2) > + dev_change_flags(peer, peer->flags & ~IFF_NOARP, NULL); > + bpf_mprog_bundle_init(&nk->bundle); > + RCU_INIT_POINTER(nk->active, NULL); > + rcu_assign_pointer(nk->peer, dev); > + return 0; > +err_configure_peer: > + unregister_netdevice(peer); > + return err; > +err_register_peer: > + free_netdev(peer); > + return err; > +} Andrew
Powered by blists - more mailing lists