All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH iproute2] libnetlink: fix leak and using unused memory on error
@ 2018-09-13 19:33 Stephen Hemminger
  2018-09-14 17:48 ` Mahesh Bandewar (महेश बंडेवार)
  2018-09-17 15:37 ` Stephen Hemminger
  0 siblings, 2 replies; 4+ messages in thread
From: Stephen Hemminger @ 2018-09-13 19:33 UTC (permalink / raw)
  To: Mahesh Bandewar; +Cc: netdev, Stephen Hemminger

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>
---
 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

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH iproute2] libnetlink: fix leak and using unused memory on error
  2018-09-13 19:33 [PATCH iproute2] libnetlink: fix leak and using unused memory on error Stephen Hemminger
@ 2018-09-14 17:48 ` Mahesh Bandewar (महेश बंडेवार)
  2018-09-17  3:54   ` David Ahern
  2018-09-17 15:37 ` Stephen Hemminger
  1 sibling, 1 reply; 4+ messages in thread
From: Mahesh Bandewar (महेश बंडेवार) @ 2018-09-14 17:48 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Mahesh Bandewar, linux-netdev

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
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH iproute2] libnetlink: fix leak and using unused memory on error
  2018-09-14 17:48 ` Mahesh Bandewar (महेश बंडेवार)
@ 2018-09-17  3:54   ` David Ahern
  0 siblings, 0 replies; 4+ messages in thread
From: David Ahern @ 2018-09-17  3:54 UTC (permalink / raw)
  To: Mahesh Bandewar (महेश
	बंडेवार),
	Stephen Hemminger
  Cc: Mahesh Bandewar, linux-netdev

On 9/14/18 10:48 AM, Mahesh Bandewar (महेश बंडेवार) wrote:
> 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")

$ git show 7b2ee50c0cd5
fatal: ambiguous argument '7b2ee50c0cd5': unknown revision or path not
in the working tree.

but we knew that since iproute2 does not have hv_netsvc code. Copy and
paste error?

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH iproute2] libnetlink: fix leak and using unused memory on error
  2018-09-13 19:33 [PATCH iproute2] libnetlink: fix leak and using unused memory on error Stephen Hemminger
  2018-09-14 17:48 ` Mahesh Bandewar (महेश बंडेवार)
@ 2018-09-17 15:37 ` Stephen Hemminger
  1 sibling, 0 replies; 4+ messages in thread
From: Stephen Hemminger @ 2018-09-17 15:37 UTC (permalink / raw)
  To: Mahesh Bandewar; +Cc: netdev

On Thu, 13 Sep 2018 12:33:38 -0700
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")
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Bad habit of working on two projects as once.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-09-17 21:05 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-13 19:33 [PATCH iproute2] libnetlink: fix leak and using unused memory on error Stephen Hemminger
2018-09-14 17:48 ` Mahesh Bandewar (महेश बंडेवार)
2018-09-17  3:54   ` David Ahern
2018-09-17 15:37 ` Stephen Hemminger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.