On Sat, Dec 17, 2016 at 02:33:37AM +0100, Alexander Bluhm wrote:
> Hi,
>
> If rt_ifa_addlocal() in in_ifinit() fails, the address has been
> added to the interface address list, but the local route is missing.
> This inconsistency can result in a panic later.
>
> panic: kernel diagnostic assertion "ifa == rt->rt_ifa" failed: file
> "/crypt/home/bluhm/openbsd/cvs/src/sys/netinet/if_ether.c", line 206
>
> I have seen this crash in production on a 5.9 system and could
> reproduce it on -current by inducing an error in rt_ifa_addlocal().
>
> So in case of an error, remove the interface address to get a
> consistent state again.
>
> ok?
OK.
The little dance with an aditional error variable is a bit confusing.
It would be nice to avoid the extra variable. But off-hand I could not
find a simpler diff which keeps the semantics of re-adding the original
address if the ioctl errors. So your approach seems fine.
>
> bluhm
>
> Index: netinet/in.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/in.c,v
> retrieving revision 1.130
> diff -u -p -u -p -r1.130 in.c
> --- netinet/in.c 5 Dec 2016 15:31:43 -0000 1.130
> +++ netinet/in.c 17 Dec 2016 01:27:30 -0000
> @@ -603,7 +603,7 @@ in_ifinit(struct ifnet *ifp, struct in_i
> {
> u_int32_t i = sin->sin_addr.s_addr;
> struct sockaddr_in oldaddr;
> - int error = 0;
> + int error = 0, rterror;
>
> splsoftassert(IPL_SOFTNET);
>
> @@ -633,10 +633,16 @@ in_ifinit(struct ifnet *ifp, struct in_i
> * error occured, put back the original address.
> */
> ifa_add(ifp, &ia->ia_ifa);
> - rt_ifa_addlocal(&ia->ia_ifa);
> + rterror = rt_ifa_addlocal(&ia->ia_ifa);
>
> if (error)
> goto out;
> + if (rterror) {
> + if (!newaddr)
> + ifa_del(ifp, &ia->ia_ifa);
> + error = rterror;
> + goto out;
> + }
>
> if (ia->ia_netmask == 0) {
> if (IN_CLASSA(i))
>
