On 03/01/16(Sun) 14:19, Fabian Raetz wrote:
> On Sun, Jan 03, 2016 at 11:54:18AM +0100, Martin Pieuchot wrote:
> > On 30/12/15(Wed) 12:51, Fabian Raetz wrote:
> > > Hi tech@,
> > >
> > > i've found the undocumented -carpdev option in ifconfig(8) which freezes
> > > my sytem if executed.
> > >
> > > As the -carpdev option is undocumented in both ifconfig(8) and carp(4) i
> > > propose two patches to remove this functionality.
> > >
> > > The patch below will return EINVAL in SIOCSVH if carpr.carpr_carpdev is
> > > not a
> > > valid interface name.
> >
> > Did you try this diff? The SIOCGVH ioctl(2) is not always issued with
> > carpr_carpdev set, so this will break your system.
> [...]
> Or are you worried about something completly different and i'm missing
> the point?
I'm worried about the fact that your diff breaks ifconfig(8) with
carp(4) interfaces because, as I said previously, the SIOCGVH ioctl(2)
is not always issued with carpr_carpdev set :)
Here's a different approach that should prevent your NULL dereference
(untested):
Index: netinet/ip_carp.c
===================================================================
RCS file: /cvs/src/sys/netinet/ip_carp.c,v
retrieving revision 1.284
diff -u -p -r1.284 ip_carp.c
--- netinet/ip_carp.c 19 Dec 2015 11:19:35 -0000 1.284
+++ netinet/ip_carp.c 3 Jan 2016 14:51:20 -0000
@@ -1653,11 +1653,9 @@ carp_set_ifp(struct carp_softc *sc, stru
int myself = 0, error = 0;
int s;
+ KASSERT(ifp0 != sc->sc_carpdev);
KERNEL_ASSERT_LOCKED(); /* touching vhif_vrs */
- if (ifp0 == sc->sc_carpdev)
- return (0);
-
if ((ifp0->if_flags & IFF_MULTICAST) == 0)
return (EADDRNOTAVAIL);
@@ -1968,12 +1966,12 @@ carp_ioctl(struct ifnet *ifp, u_long cmd
struct carpreq carpr;
struct ifaddr *ifa = (struct ifaddr *)addr;
struct ifreq *ifr = (struct ifreq *)addr;
- struct ifnet *ifp0 = NULL;
+ struct ifnet *ifp0 = sc->sc_carpdev;
int i, error = 0;
switch (cmd) {
case SIOCSIFADDR:
- if (sc->sc_carpdev == NULL)
+ if (ifp0 == NULL)
return (EINVAL);
switch (ifa->ifa_addr->sa_family) {
@@ -2029,8 +2027,10 @@ carp_ioctl(struct ifnet *ifp, u_long cmd
sc->sc_peer.s_addr = INADDR_CARP_GROUP;
else
sc->sc_peer.s_addr = carpr.carpr_peer.s_addr;
- if ((error = carp_set_ifp(sc, ifp0)))
- return (error);
+ if (ifp0 != sc->sc_carpdev) {
+ if ((error = carp_set_ifp(sc, ifp0)))
+ return (error);
+ }
if (vhe->state != INIT && carpr.carpr_state != vhe->state) {
switch (carpr.carpr_state) {
case BACKUP:
@@ -2090,9 +2090,8 @@ carp_ioctl(struct ifnet *ifp, u_long cmd
case SIOCGVH:
memset(&carpr, 0, sizeof(carpr));
- if (sc->sc_carpdev != NULL)
- strlcpy(carpr.carpr_carpdev, sc->sc_carpdev->if_xname,
- IFNAMSIZ);
+ if (ifp0 != NULL)
+ strlcpy(carpr.carpr_carpdev, ifp0->if_xname, IFNAMSIZ);
i = 0;
KERNEL_ASSERT_LOCKED(); /* touching carp_vhosts */
SRPL_FOREACH_LOCKED(vhe, &sc->carp_vhosts, vhost_entries) {
