Bug 1547472

I'm guessing this was an internal error on glibc (vsnprintf). Maybe it couldn't allocate enough memory.

(gdb) select-frame 5
(gdb) p port->ifinfo
$1 = (struct team_ifinfo *) 0x0

static bool __team_port_str(struct team_port *port,
                            char **pbuf, size_t *pbufsiz)
{
        uint32_t ifindex = team_get_port_ifindex(port);
        struct team_ifinfo *ifinfo = team_get_port_ifinfo(port); <--- NULL

        return __buf_append(...
                            team_get_ifinfo_ifname(ifinfo), <--- crash

Introduced by:
commit 046fb6ba0aec8246075b18d787daec43201566fa
Author: Antti Tiainen <atiainen>
Date:   Mon Feb 6 15:41:05 2017 +0200

    libteam: resynchronize ifinfo after lost RTNLGRP_LINK notifications

team_port's ifinfo should not be null, but ifinfo's destroy is earlier than team_port, so we have to delay it, the fix could be something like:

 static void ifinfo_destroy(struct team_ifinfo *ifinfo)
 {
-       if (ifinfo->linked && ifinfo->port)
-               port_unlink(ifinfo->port);
        list_del(&ifinfo->list);
+       if (ifinfo->linked && ifinfo->port)
+               return;
        free(ifinfo);
 }

@@ -470,6 +470,9 @@ void ifinfo_unlink(struct team_ifinfo *ifinfo)
 {
        ifinfo->port = NULL;
        ifinfo->linked = false;
+
+       if (list_empty(&ifinfo->list))
+               free(ifinfo);
 }

after which, we can also do the similar thing for port and option's cleanup as the commit does for ifinfo, and remove them from elsewhere, it would be a nice improvement.
@@ -240,6 +240,13 @@ int check_call_change_handlers(struct team_handle *th,
                ifinfo_destroy_removed(th);
                ifinfo_clear_changed(th);
        }
+
+       if (call_type_mask & TEAM_PORT_CHANGE)
+               port_list_cleanup_last_state(th);
+
+       if (call_type_mask & TEAM_OPTION_CHANGE)
+               option_list_cleanup_last_state(th);
+
        th->change_handler.pending_type_mask &= ~call_type_mask;
        return err;
 }

(In reply to Xin Long from comment #3)
> (gdb) select-frame 5
> (gdb) p port->ifinfo
> $1 = (struct team_ifinfo *) 0x0
> 
> static bool __team_port_str(struct team_port *port,
>                             char **pbuf, size_t *pbufsiz)
> {
>         uint32_t ifindex = team_get_port_ifindex(port);
>         struct team_ifinfo *ifinfo = team_get_port_ifinfo(port); <--- NULL
> 
>         return __buf_append(...
>                             team_get_ifinfo_ifname(ifinfo), <--- crash

That's interesting, and odd. As it had __buf_append() in the call stack, I assumed that that had worked. I wonder how this call stack could happen then, because that expression must be evaluated before the function call.

(In reply to Marcelo Ricardo Leitner from comment #4)
> (In reply to Xin Long from comment #3)
> > (gdb) select-frame 5
> > (gdb) p port->ifinfo
> > $1 = (struct team_ifinfo *) 0x0
> > 
> > static bool __team_port_str(struct team_port *port,
> >                             char **pbuf, size_t *pbufsiz)
> > {
> >         uint32_t ifindex = team_get_port_ifindex(port);
> >         struct team_ifinfo *ifinfo = team_get_port_ifinfo(port); <--- NULL
> > 
> >         return __buf_append(...
> >                             team_get_ifinfo_ifname(ifinfo), <--- crash
> 
> That's interesting, and odd. As it had __buf_append() in the call stack, I
> assumed that that had worked. I wonder how this call stack could happen
> then, because that expression must be evaluated before the function call.
Maybe ifinfo->ifname (offset) was just a value as the parameter, team_get_ifinfo_ifname didn't yet dereference it (access the memory), until vsnprintf/strlen did that.

*** This bug has been marked as a duplicate of bug 1563155 ***