lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <effba957-26ce-4b04-841a-40c863e6931f@gmail.com> Date: Mon, 16 Oct 2023 17:00:39 -0700 From: Kui-Feng Lee <sinquersw@...il.com> To: Kuniyuki Iwashima <kuniyu@...zon.com>, "David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, David Ahern <dsahern@...nel.org>, Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>, Song Liu <song@...nel.org>, Yonghong Song <yonghong.song@...ux.dev>, John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...gle.com>, Hao Luo <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>, Mykola Lysenko <mykolal@...com> Cc: Kuniyuki Iwashima <kuni1840@...il.com>, bpf@...r.kernel.org, netdev@...r.kernel.org Subject: Re: [PATCH v1 bpf-next 03/11] tcp: Clean up goto labels in cookie_v[46]_check(). 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@...zon.com> > --- > 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. > diff --git a/net/ipv6/syncookies.c b/net/ipv6/syncookies.c > index cbee2df8a006..b8ef6efbb60e 100644 > --- a/net/ipv6/syncookies.c > +++ b/net/ipv6/syncookies.c > @@ -171,11 +171,10 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb) > if (!cookie_timestamp_decode(net, &tcp_opt)) > goto out; > > - ret = NULL; > req = cookie_tcp_reqsk_alloc(&tcp6_request_sock_ops, > &tcp_request_sock_ipv6_ops, sk, skb); > if (!req) > - goto out; > + goto out_drop; > > ireq = inet_rsk(req); > treq = tcp_rsk(req); > @@ -263,5 +262,6 @@ struct sock *cookie_v6_check(struct sock *sk, struct sk_buff *skb) > return ret; > out_free: > reqsk_free(req); > +out_drop: > return NULL; > } Same here!
Powered by blists - more mailing lists