netdev - Re: [PATCH iproute2] libnetlink: fix leak and using unused memory on error

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAF2d9jjPGHnzkCgT00Us2ArsYJSjXMyOBrQaUVR-Fs3Rzbaftg@mail.gmail.com>
Date:   Fri, 14 Sep 2018 10:48:29 -0700
From:   Mahesh Bandewar (महेश बंडेवार) 
        <maheshb@...gle.com>
To:     Stephen Hemminger <stephen@...workplumber.org>
Cc:     Mahesh Bandewar <mahesh@...dewar.net>,
        linux-netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH iproute2] libnetlink: fix leak and using unused memory on error

On Thu, Sep 13, 2018 at 12:33 PM, Stephen Hemminger
<stephen@...workplumber.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@...dewar.net>
> Fixes: 7b2ee50c0cd5 ("hv_netvsc: common detach logic")
> Signed-off-by: Stephen Hemminger <stephen@...workplumber.org>
Acked-by: Mahesh Bandewar <maheshb@...gle.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
>