Re: [PATCH net v3 resend] net, sunrpc: Remap EPERM in case of connection failure in xs_tcp_setup_socket

[Paolo Abeni <pabeni@redhat.com>]

On Thu, 2024-07-04 at 08:41 +0200, Daniel Borkmann wrote:
> When using a BPF program on kernel_connect(), the call can return -EPERM. This
> causes xs_tcp_setup_socket() to loop forever, filling up the syslog and causing
> the kernel to potentially freeze up.
> 
> Neil suggested:
> 
>   This will propagate -EPERM up into other layers which might not be ready
>   to handle it. It might be safer to map EPERM to an error we would be more
>   likely to expect from the network system - such as ECONNREFUSED or ENETDOWN.
> 
> ECONNREFUSED as error seems reasonable. For programs setting a different error
> can be out of reach (see handling in 4fbac77d2d09) in particular on kernels
> which do not have f10d05966196 ("bpf: Make BPF_PROG_RUN_ARRAY return -err
> instead of allow boolean"), thus given that it is better to simply remap for
> consistent behavior. UDP does handle EPERM in xs_udp_send_request().
> 
> Fixes: d74bad4e74ee ("bpf: Hooks for sys_connect")
> Fixes: 4fbac77d2d09 ("bpf: Hooks for sys_bind")
> Co-developed-by: Lex Siegel <usiegl00@gmail.com>
> Signed-off-by: Lex Siegel <usiegl00@gmail.com>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Cc: Neil Brown <neilb@suse.de>
> Cc: Trond Myklebust <trondmy@kernel.org>
> Cc: Anna Schumaker <anna@kernel.org>
> Link: https://github.com/cilium/cilium/issues/33395
> Link: https://lore.kernel.org/bpf/171374175513.12877.8993642908082014881@noble.neil.brown.name
> ---
>  [ Fixes tags are set to the orig connect commit so that stable team
>    can pick this up.
> 
>    Resend as it turns out that patchwork did not pick up the earlier
>    resends likely due to the message id being the same. ]
> 
>  v1 -> v2 -> v3:
>    - Plain resend, adding correct sunrpc folks to Cc
>      https://lore.kernel.org/bpf/Zn7wtStV+iafWRXj@tissot.1015granger.net/
> 
>  net/sunrpc/xprtsock.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index dfc353eea8ed..0e1691316f42 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -2441,6 +2441,13 @@ static void xs_tcp_setup_socket(struct work_struct *work)
>  		transport->srcport = 0;
>  		status = -EAGAIN;
>  		break;
> +	case -EPERM:
> +		/* Happens, for instance, if a BPF program is preventing
> +		 * the connect. Remap the error so upper layers can better
> +		 * deal with it.
> +		 */
> +		status = -ECONNREFUSED;
> +		fallthrough;
>  	case -EINVAL:
>  		/* Happens, for instance, if the user specified a link
>  		 * local IPv6 address without a scope-id.

The patch looks sane to me. @Trond, @Anna, are you ok for this to go
directly into the net tree?

Thanks!

Paolo