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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Wed, 22 Oct 2014 13:36:11 +0200 From: Hannes Frederic Sowa <hannes@...essinduktion.org> To: Erik Kline <ek@...gle.com> Cc: netdev <netdev@...r.kernel.org>, Ben Hutchings <ben@...adent.org.uk>, Lorenzo Colitti <lorenzo@...gle.com> Subject: Re: [RFC PATCH v2 net-next] net: ipv6: Add a sysctl to make optimistic addresses useful candidates Hi, On Mi, 2014-10-22 at 20:15 +0900, Erik Kline wrote: > >> >> case IPV6_SADDR_RULE_PREFIX: > >> >> /* Rule 8: Use longest matching prefix */ > >> >> ret = ipv6_addr_diff(&score->ifa->addr, dst->addr); > >> >> @@ -3222,8 +3240,13 @@ static void addrconf_dad_begin(struct inet6_ifaddr *ifp) > >> >> * Optimistic nodes can start receiving > >> >> * Frames right away > >> >> */ > >> >> - if (ifp->flags & IFA_F_OPTIMISTIC) > >> >> + if (ifp->flags & IFA_F_OPTIMISTIC) { > >> >> ip6_ins_rt(ifp->rt); > >> >> + /* Because optimistic nodes can receive frames, notify > >> >> + * listeners. If DAD fails, RTM_DELADDR is sent. > >> >> + */ > >> >> + ipv6_ifa_notify(RTM_NEWADDR, ifp); > >> >> + } > >> > > >> > I wonder if we can now delete the ipv6_ifa_notify(RTM_NEWADDR, ifp) in > >> > addrconf_dad_completed. > >> > >> I don't know what everyone's general preference would be, but mine > >> would be to err on the side of notifying on state changes. It seems > >> harmless to me to keep it in, and something in userspace might want to > >> know if/when DAD completes. > > > > Userspace expects to communicate with an address which gets announced > > via RTM_NEWADDR, so I would make the RTM_NEWADDR notifications > > conditional on use_optimistic flag in both, the completed and the > > dad_begin function. This sounds like the best option to me, no? > > That's easy enough to do in addrconf_dad_begin(). Unfortunately, by > the time we get to addrconf_dad_completed() the IFA_F_OPTIMISTIC flag > has already been cleared. > > I have a version that attempts to take its best guess as to whether an > RTM_NEWADDR _should_ have already been sent--something like: > > #ifdef CONFIG_IPV6_OPTIMISTIC_DAD > // We probably already sent a notification in addrconf_dad_begin(). > if (!ifp->idev->cnf.optimistic_dad || !ifp->idev->cnf.use_optimistic) > #endif > ipv6_ifa_notify(RTM_NEWADDR, ifp); > > but that doesn't seem that clean to me (apart from the ifdef-y > messiness of it). Do you think this "best guess" approach is > reasonable? I would definitely ack a patch which removes the macros and the config option CONFIG_IPV6_OPTIMISTIC_DAD completely. You also can split the ifdefed part into a small helper function: static inline bool ipv6_use_optimistic_addr(struct inet6_dev *idev) { #ifdef CONFIG_IPV6_OPTIMISTIC_DAD return idev->cnf.optimistic_dad && idev->cnf.use_optimistic; #else return false; #endif } ... and then in both locations update the RTM_NEWADDR like this: addrconf_dad_begin: if (ipv6_use_optimisitic_addr(idev)) notify... and the dad_completed with a negated check. I think this is the easiest option. > With just the "use_optimistic" check in addrconf_dad_begin(), > userspace can still communicate with addresses it gets via > RTM_NEWADDR, it will just get /two/ such notifications: one when it > can probably use it and one when it definitely can. > > Thoughts? In the near future we also must introduce specific DAD events needed for RFC7217 addresses (to count DAD progress and safe the information per prefix in user space). I would prefer to keep the logic for RTM_NEWADDR that the kernel will definitely allow the use of the address but RTM_NEWADDR should be handled idempotent by user space. Thanks, Hannes -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists