[PATCH nft] mnl: do not cache sender buffer size

[PATCH nft] mnl: do not cache sender buffer size

[Pablo Neira Ayuso]

SO_SNDBUF never fails, this socket option just provides a hint to the
kernel.  SO_SNDBUFFORCE sets the buffer size to zero if the value goes
over INT_MAX. Userspace is caching the buffer hint that sends to the
kernel, so it might leave userspace out of sync if the kernel ignores
the hint. Do not make assumptions, fetch the sender buffer size from the
kernel via getsockopt().

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 src/mnl.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/mnl.c b/src/mnl.c
index 57ff89f50e23..19631e33dc9d 100644
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -218,24 +218,24 @@ void mnl_err_list_free(struct mnl_err *err)
 	xfree(err);
 }
 
-static int nlbuffsiz;
-
 static void mnl_set_sndbuffer(const struct mnl_socket *nl,
 			      struct nftnl_batch *batch)
 {
+	int sndnlbuffsiz = 0;
 	int newbuffsiz;
+	socklen_t len;
 
-	if (nftnl_batch_iovec_len(batch) * BATCH_PAGE_SIZE <= nlbuffsiz)
-		return;
+	getsockopt(mnl_socket_get_fd(nl), SOL_SOCKET, SO_SNDBUF,
+		   &sndnlbuffsiz, &len);
 
 	newbuffsiz = nftnl_batch_iovec_len(batch) * BATCH_PAGE_SIZE;
+	if (newbuffsiz <= sndnlbuffsiz)
+		return;
 
 	/* Rise sender buffer length to avoid hitting -EMSGSIZE */
 	if (setsockopt(mnl_socket_get_fd(nl), SOL_SOCKET, SO_SNDBUFFORCE,
 		       &newbuffsiz, sizeof(socklen_t)) < 0)
 		return;
-
-	nlbuffsiz = newbuffsiz;
 }
 
 static unsigned int nlsndbufsiz;
-- 
2.11.0

Re: [PATCH nft] mnl: do not cache sender buffer size

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> SO_SNDBUF never fails, this socket option just provides a hint to the
> kernel.  SO_SNDBUFFORCE sets the buffer size to zero if the value goes
> over INT_MAX. Userspace is caching the buffer hint that sends to the
> kernel, so it might leave userspace out of sync if the kernel ignores
> the hint. Do not make assumptions, fetch the sender buffer size from the
> kernel via getsockopt().
> 
> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> ---
>  src/mnl.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/src/mnl.c b/src/mnl.c
> index 57ff89f50e23..19631e33dc9d 100644
> --- a/src/mnl.c
> +++ b/src/mnl.c
> @@ -218,24 +218,24 @@ void mnl_err_list_free(struct mnl_err *err)
>  	xfree(err);
>  }
>  
> -static int nlbuffsiz;
> -
>  static void mnl_set_sndbuffer(const struct mnl_socket *nl,
>  			      struct nftnl_batch *batch)
>  {
> +	int sndnlbuffsiz = 0;
>  	int newbuffsiz;
> +	socklen_t len;

IIRC this needs to be

	len = sizeof(sndnlbuffsiz);