netdev - Re: [PATCH bpf-next] libbpf: Use dynamically allocated buffer when receiving netlink messages

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20220211211221.wc5ouk32krtlxhlr@apollo.legion>
Date:   Sat, 12 Feb 2022 02:42:21 +0530
From:   Kumar Kartikeya Dwivedi <memxor@...il.com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>
Cc:     Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        Zhiqian Guan <zhguan@...hat.com>, netdev@...r.kernel.org,
        bpf@...r.kernel.org
Subject: Re: [PATCH bpf-next] libbpf: Use dynamically allocated buffer when
 receiving netlink messages

On Sat, Feb 12, 2022 at 01:21:00AM IST, Toke Høiland-Jørgensen wrote:
> When receiving netlink messages, libbpf was using a statically allocated
> stack buffer of 4k bytes. This happened to work fine on systems with a 4k
> page size, but on systems with larger page sizes it can lead to truncated
> messages. The user-visible impact of this was that libbpf would insist no
> XDP program was attached to some interfaces because that bit of the netlink
> message got chopped off.
>
> Fix this by switching to a dynamically allocated buffer; we borrow the
> approach from iproute2 of using recvmsg() with MSG_PEEK|MSG_TRUNC to get
> the actual size of the pending message before receiving it, adjusting the
> buffer as necessary. While we're at it, also add retries on interrupted
> system calls around the recvmsg() call.
>
> Reported-by: Zhiqian Guan <zhguan@...hat.com>
> Fixes: 8bbb77b7c7a2 ("libbpf: Add various netlink helpers")
> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>
> ---

Thanks for the fix!

Acked-by: Kumar Kartikeya Dwivedi <memxor@...il.com>

>  tools/lib/bpf/netlink.c | 55 ++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 52 insertions(+), 3 deletions(-)
>
> diff --git a/tools/lib/bpf/netlink.c b/tools/lib/bpf/netlink.c
> index c39c37f99d5c..9a6e95206bf0 100644
> --- a/tools/lib/bpf/netlink.c
> +++ b/tools/lib/bpf/netlink.c
> @@ -87,22 +87,70 @@ enum {
>  	NL_DONE,
>  };
>
> +static int __libbpf_netlink_recvmsg(int sock, struct msghdr *mhdr, int flags)
> +{
> +	int len;
> +
> +	do {
> +		len = recvmsg(sock, mhdr, flags);
> +	} while (len < 0 && (errno == EINTR || errno == EAGAIN));
> +
> +	if (len < 0)
> +		return -errno;
> +	return len;
> +}
> +
> +static int libbpf_netlink_recvmsg(int sock, struct msghdr *mhdr, char **buf)
> +{
> +	struct iovec *iov = mhdr->msg_iov;
> +	void *nbuf;
> +	int len;
> +
> +	len = __libbpf_netlink_recvmsg(sock, mhdr, MSG_PEEK | MSG_TRUNC);
> +	if (len < 0)
> +		return len;
> +
> +	if (len < 4096)
> +		len = 4096;
> +
> +	if (len > iov->iov_len) {
> +		nbuf = realloc(iov->iov_base, len);
> +		if (!nbuf) {
> +			free(iov->iov_base);
> +			return -ENOMEM;
> +		}
> +		iov->iov_base = nbuf;
> +		iov->iov_len = len;
> +	}
> +
> +	len = __libbpf_netlink_recvmsg(sock, mhdr, 0);
> +	if (len > 0)
> +		*buf = iov->iov_base;
> +	return len;
> +}
> +
>  static int libbpf_netlink_recv(int sock, __u32 nl_pid, int seq,
>  			       __dump_nlmsg_t _fn, libbpf_dump_nlmsg_t fn,
>  			       void *cookie)
>  {
> +	struct iovec iov = {};
> +	struct msghdr mhdr = {
> +		.msg_iov = &iov,
> +		.msg_iovlen = 1,
> +	};
>  	bool multipart = true;
>  	struct nlmsgerr *err;
>  	struct nlmsghdr *nh;
> -	char buf[4096];
>  	int len, ret;
> +	char *buf;
> +
>
>  	while (multipart) {
>  start:
>  		multipart = false;
> -		len = recv(sock, buf, sizeof(buf), 0);
> +		len = libbpf_netlink_recvmsg(sock, &mhdr, &buf);
>  		if (len < 0) {
> -			ret = -errno;
> +			ret = len;
>  			goto done;
>  		}
>
> @@ -151,6 +199,7 @@ static int libbpf_netlink_recv(int sock, __u32 nl_pid, int seq,
>  	}
>  	ret = 0;
>  done:
> +	free(iov.iov_base);
>  	return ret;
>  }
>
> --
> 2.35.1
>

--
Kartikeya