[PATCH] Avoid capable() call in UNIX buffer check unless necessary

[PATCH] Avoid capable() call in UNIX buffer check unless necessary

[Dan Smith]

As pointed out by Serge, we shouldn't call capable() unless we know we'll
need to exercise the ability.

Signed-off-by: Dan Smith <danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Cc: serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org
---
 net/unix/checkpoint.c |   11 ++++++++++-
 1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/net/unix/checkpoint.c b/net/unix/checkpoint.c
index 23040ce..55dfac1 100644
--- a/net/unix/checkpoint.c
+++ b/net/unix/checkpoint.c
@@ -278,9 +278,18 @@ static int sock_read_buffer_sendmsg(struct ckpt_ctx *ctx,
 		unix_sk(sk)->peer->sk_shutdown &= ~SHUTDOWN_MASK;
 	}
 
-	/* Make sure there's room in the send buffer */
+	/* Make sure there's room in the send buffer: Worst case, we
+	 * give them the benefit of the doubt and set the buffer limit
+	 * to the system default.  This should cover the case where
+	 * the user set the limit low after loading up the buffer.
+	 *
+	 * However, if there isn't room in the buffer and the system
+	 * default won't accommodate them either, then increase the
+	 * limit as needed, only if they have CAP_NET_ADMIN.
+	 */
 	sndbuf = sk->sk_sndbuf;
 	if (((sk->sk_sndbuf - atomic_read(&sk->sk_wmem_alloc)) < h->lin_len) &&
+	    (h->lin_len > sysctl_wmem_max) &&
 	    capable(CAP_NET_ADMIN))
 		sk->sk_sndbuf += h->lin_len;
 	else
-- 
1.6.2.5

Re: [PATCH] Avoid capable() call in UNIX buffer check unless necessary

[Serge E. Hallyn]

Quoting Dan Smith (danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org):
> As pointed out by Serge, we shouldn't call capable() unless we know we'll
> need to exercise the ability.
> 
> Signed-off-by: Dan Smith <danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> Cc: serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org

thanks

Acked-by: Serge Hallyn <serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

> ---
>  net/unix/checkpoint.c |   11 ++++++++++-
>  1 files changed, 10 insertions(+), 1 deletions(-)
> 
> diff --git a/net/unix/checkpoint.c b/net/unix/checkpoint.c
> index 23040ce..55dfac1 100644
> --- a/net/unix/checkpoint.c
> +++ b/net/unix/checkpoint.c
> @@ -278,9 +278,18 @@ static int sock_read_buffer_sendmsg(struct ckpt_ctx *ctx,
>  		unix_sk(sk)->peer->sk_shutdown &= ~SHUTDOWN_MASK;
>  	}
> 
> -	/* Make sure there's room in the send buffer */
> +	/* Make sure there's room in the send buffer: Worst case, we
> +	 * give them the benefit of the doubt and set the buffer limit
> +	 * to the system default.  This should cover the case where
> +	 * the user set the limit low after loading up the buffer.
> +	 *
> +	 * However, if there isn't room in the buffer and the system
> +	 * default won't accommodate them either, then increase the
> +	 * limit as needed, only if they have CAP_NET_ADMIN.
> +	 */
>  	sndbuf = sk->sk_sndbuf;
>  	if (((sk->sk_sndbuf - atomic_read(&sk->sk_wmem_alloc)) < h->lin_len) &&
> +	    (h->lin_len > sysctl_wmem_max) &&
>  	    capable(CAP_NET_ADMIN))
>  		sk->sk_sndbuf += h->lin_len;
>  	else
> -- 
> 1.6.2.5

Re: [PATCH] Avoid capable() call in UNIX buffer check unless necessary

[Oren Laadan]

Queued for v19-rc2, thanks.

Oren.

Dan Smith wrote:
> As pointed out by Serge, we shouldn't call capable() unless we know we'll
> need to exercise the ability.
> 
> Signed-off-by: Dan Smith <danms-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
> Cc: serue-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org
> ---
>  net/unix/checkpoint.c |   11 ++++++++++-
>  1 files changed, 10 insertions(+), 1 deletions(-)
> 
> diff --git a/net/unix/checkpoint.c b/net/unix/checkpoint.c
> index 23040ce..55dfac1 100644
> --- a/net/unix/checkpoint.c
> +++ b/net/unix/checkpoint.c
> @@ -278,9 +278,18 @@ static int sock_read_buffer_sendmsg(struct ckpt_ctx *ctx,
>  		unix_sk(sk)->peer->sk_shutdown &= ~SHUTDOWN_MASK;
>  	}
>  
> -	/* Make sure there's room in the send buffer */
> +	/* Make sure there's room in the send buffer: Worst case, we
> +	 * give them the benefit of the doubt and set the buffer limit
> +	 * to the system default.  This should cover the case where
> +	 * the user set the limit low after loading up the buffer.
> +	 *
> +	 * However, if there isn't room in the buffer and the system
> +	 * default won't accommodate them either, then increase the
> +	 * limit as needed, only if they have CAP_NET_ADMIN.
> +	 */
>  	sndbuf = sk->sk_sndbuf;
>  	if (((sk->sk_sndbuf - atomic_read(&sk->sk_wmem_alloc)) < h->lin_len) &&
> +	    (h->lin_len > sysctl_wmem_max) &&
>  	    capable(CAP_NET_ADMIN))
>  		sk->sk_sndbuf += h->lin_len;
>  	else