Re: [PATCH v1 bpf-next 03/11] tcp: Clean up goto labels in cookie_v[46]_check().

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Kui-Feng Lee <sinquersw@xxxxxxxxx>
Date: Mon, 16 Oct 2023 17:00:39 -0700
> On 10/13/23 15:04, Kuniyuki Iwashima wrote:
> > We will add a SOCK_OPS hook to validate SYN Cookie.
> > 
> > We invoke the hook after allocating reqsk.  In case it fails,
> > we will respond with RST instead of just dropping the ACK.
> > 
> > Then, there would be more duplicated error handling patterns.
> > To avoid that, let's clean up goto labels.
> > 
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@xxxxxxxxxx>
> > ---
> >   net/ipv4/syncookies.c | 22 +++++++++++-----------
> >   net/ipv6/syncookies.c |  4 ++--
> >   2 files changed, 13 insertions(+), 13 deletions(-)
> > 
> > diff --git a/net/ipv4/syncookies.c b/net/ipv4/syncookies.c
> > index 64280cf42667..b0cf6f4d66d8 100644
> > --- a/net/ipv4/syncookies.c
> > +++ b/net/ipv4/syncookies.c
> > @@ -369,11 +369,10 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
> >   	if (!cookie_timestamp_decode(net, &tcp_opt))
> >   		goto out;
> >   
> > -	ret = NULL;
> >   	req = cookie_tcp_reqsk_alloc(&tcp_request_sock_ops,
> >   				     &tcp_request_sock_ipv4_ops, sk, skb);
> >   	if (!req)
> > -		goto out;
> > +		goto out_drop;
> >   
> >   	ireq = inet_rsk(req);
> >   	treq = tcp_rsk(req);
> > @@ -405,10 +404,8 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
> >   	 */
> >   	RCU_INIT_POINTER(ireq->ireq_opt, tcp_v4_save_options(net, skb));
> >   
> > -	if (security_inet_conn_request(sk, skb, req)) {
> > -		reqsk_free(req);
> > -		goto out;
> > -	}
> > +	if (security_inet_conn_request(sk, skb, req))
> > +		goto out_free;
> >   
> >   	req->num_retrans = 0;
> >   
> > @@ -425,10 +422,8 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
> >   			   ireq->ir_loc_addr, th->source, th->dest, sk->sk_uid);
> >   	security_req_classify_flow(req, flowi4_to_flowi_common(&fl4));
> >   	rt = ip_route_output_key(net, &fl4);
> > -	if (IS_ERR(rt)) {
> > -		reqsk_free(req);
> > -		goto out;
> > -	}
> > +	if (IS_ERR(rt))
> > +		goto out_free;
> >   
> >   	/* Try to redo what tcp_v4_send_synack did. */
> >   	req->rsk_window_clamp = tp->window_clamp ? :dst_metric(&rt->dst, RTAX_WINDOW);
> > @@ -452,5 +447,10 @@ struct sock *cookie_v4_check(struct sock *sk, struct sk_buff *skb)
> >   	 */
> >   	if (ret)
> >   		inet_sk(ret)->cork.fl.u.ip4 = fl4;
> > -out:	return ret;
> > +out:
> > +	return ret;
> > +out_free:
> > +	reqsk_free(req);
> > +out_drop:
> > +	return NULL;
> >   }
> 
> Looks like you don't use out_free and out_drop at all
> in the patch 5 & 6. Are these changes still necessary?
> Especially, the line 'goto out_drop' can be 'return NULL' concisely.

I think it's hard to follow a function where goto and return
are mixed, so I cleaned up the labels while at it.




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux