On Fri, Jun 28, 2024 at 01:19:49PM -0400, Chuck Lever wrote:
> On Fri, Jun 28, 2024 at 06:31:23PM +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@xxxxxxxxx>
> > Signed-off-by: Lex Siegel <usiegl00@xxxxxxxxx>
> > Signed-off-by: Daniel Borkmann <daniel@xxxxxxxxxxxxx>
> > Link: https://github.com/cilium/cilium/issues/33395
> > Link: https://lore.kernel.org/bpf/171374175513.12877.8993642908082014881@xxxxxxxxxxxxxxxxxxxxx
> > ---
> > [ Fixes tags are set to the orig connect commit so that stable team
> > can pick this up. ]
> >
> > v1 -> v2:
> > - Plain resend, adding more sunrpc folks to Cc
> >
> > 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.
> > --
> > 2.21.0
> >
>
> Hi Daniel -
>
> I know this is not documented in MAINTAINERS, but changes to
> net/sunrpc/xprtsock.c go to Anna Schumaker and Trond Myklebust,
> cc: linux-nfs@vger.
Would it be possible to update MAINTAINERS accordingly?
