From mboxrd@z Thu Jan  1 00:00:00 1970
From: =?UTF-8?B?TWFoZXNoIEJhbmRld2FyICjgpK7gpLngpYfgpLYg4KSs4KSC4KSh4KWH4KS14KS+4KSwKQ==?=
        <maheshb@google.com>
Subject: Re: [PATCH iproute2] libnetlink: fix leak and using unused memory on error
Date: Fri, 14 Sep 2018 10:48:29 -0700
Message-ID: <CAF2d9jjPGHnzkCgT00Us2ArsYJSjXMyOBrQaUVR-Fs3Rzbaftg@mail.gmail.com>
References: <20180913193338.20233-1-stephen@networkplumber.org>
Mime-Version: 1.0
Content-Type: text/plain; charset="UTF-8"
Cc: Mahesh Bandewar <mahesh@bandewar.net>,
        linux-netdev <netdev@vger.kernel.org>
To: Stephen Hemminger <stephen@networkplumber.org>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-wm1-f67.google.com ([209.85.128.67]:52369 "EHLO
        mail-wm1-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1727790AbeINXEZ (ORCPT
        <rfc822;netdev@vger.kernel.org>); Fri, 14 Sep 2018 19:04:25 -0400
Received: by mail-wm1-f67.google.com with SMTP id y139-v6so2772155wmc.2
        for <netdev@vger.kernel.org>; Fri, 14 Sep 2018 10:48:51 -0700 (PDT)
In-Reply-To: <20180913193338.20233-1-stephen@networkplumber.org>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On Thu, Sep 13, 2018 at 12:33 PM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> If an error happens in multi-segment message (tc only)
> then report the error and stop processing further responses.
> This also fixes refering to the buffer after free.
>
> The sequence check is not necessary here because the
> response message has already been validated to be in
> the window of the sequence number of the iov.
>
> Reported-by: Mahesh Bandewar <mahesh@bandewar.net>
> Fixes: 7b2ee50c0cd5 ("hv_netvsc: common detach logic")
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Mahesh Bandewar <maheshb@google.com>
> ---
>  lib/libnetlink.c | 23 +++++++++--------------
>  1 file changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/lib/libnetlink.c b/lib/libnetlink.c
> index 928de1dd16d8..586809292594 100644
> --- a/lib/libnetlink.c
> +++ b/lib/libnetlink.c
> @@ -617,7 +617,6 @@ static int __rtnl_talk_iov(struct rtnl_handle *rtnl, struct iovec *iov,
>         msg.msg_iovlen = 1;
>         i = 0;
>         while (1) {
> -next:
>                 status = rtnl_recvmsg(rtnl->fd, &msg, &buf);
>                 ++i;
>
> @@ -660,27 +659,23 @@ next:
>
>                                 if (l < sizeof(struct nlmsgerr)) {
>                                         fprintf(stderr, "ERROR truncated\n");
> -                               } else if (!err->error) {
> +                                       free(buf);
> +                                       return -1;
> +                               }
> +
> +                               if (!err->error)
>                                         /* check messages from kernel */
>                                         nl_dump_ext_ack(h, errfn);
>
> -                                       if (answer)
> -                                               *answer = (struct nlmsghdr *)buf;
> -                                       else
> -                                               free(buf);
> -                                       if (h->nlmsg_seq == seq)
> -                                               return 0;
> -                                       else if (i < iovlen)
> -                                               goto next;
> -                                       return 0;
> -                               }
> -
>                                 if (rtnl->proto != NETLINK_SOCK_DIAG &&
>                                     show_rtnl_err)
>                                         rtnl_talk_error(h, err, errfn);
>
>                                 errno = -err->error;
> -                               free(buf);
> +                               if (answer)
> +                                       *answer = (struct nlmsghdr *)buf;
> +                               else
> +                                       free(buf);
>                                 return -i;
>                         }
>
> --
> 2.18.0
>