Received: by 2002:a05:6a10:c604:0:0:0:0 with SMTP id y4csp4111049pxt; Tue, 10 Aug 2021 20:43:12 -0700 (PDT) X-Google-Smtp-Source: ABdhPJywr7EW7HOUUya4y3b2MrUcWm7piSfQMa94/vFuuamI13w2+vBLC+zKTbv+UKv1OnYkbV78 X-Received: by 2002:a05:6402:26d1:: with SMTP id x17mr9192539edd.126.1628653391899; Tue, 10 Aug 2021 20:43:11 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1628653391; cv=none; d=google.com; s=arc-20160816; b=aryENwSWi49c+KXxTiy8xUH3rWzrZctQ1ZRBfFHuryNV7qaZ13C1Bjii0KEYl5j73k /ctsitdeH0Oe/yCdvqCZHPNdZA6u+aH4gxrnVk37Vt3koN6cWPYLeN0Ysy2hytd9k69j RToQbHMCjkETNylvSlLjaf2Wvh8TsLQen4Pgen5waHu1fuFkhLZYvfCuGfDzts4x54P4 HcVosSSoIwY2oF2IN0byWXwgDkuCxN0oo9g7KWwJ2ukHk+QCOeMKVmQ1b/HpsTiFrOeK VnBrmzSztkM/RpQ6UotqK/HecydWwEXgOCjRt/93/g4NIs3Q8hlHYF4BLkFuUhZrwXM9 eWUw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:in-reply-to :mime-version:user-agent:date:message-id:from:references:cc:to :subject; bh=U0EFmaeDnnTv2OCH8uB163xfyk2UpYaheq2vS9vxIBE=; b=gk2ouReuzIX/dihoLeUw5PQ67fdMcaKSiucUA+PrYgpWkqZvVYYhmUCWyg6gvZoD54 y8ERstrFbUOTyJzaLwZfoiZVrOu9QMgVitjMJMkrxAbqrtBYo2QAL0KpAQySU2WJ/+Kh eC3MAPiYMGCsn5gYC/NZPnVtfPoV/Czu9qTcShqOHQAAOyrcNIN+Y4jNL520mOKy70c3 LPwoTtSsqmmoK+/7BFNWsMlFuL5ojIqSE4hXXifVRu5BPXifVxePb/A/0aEyGlw2tu2e PEfjGyYuAlOvlkG9WMZfFkEjPCpHq3f4iI1lxBRUHKrGQB1bSgxWsqrZ9l/NAg1oh2Ze kFsw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=alibaba.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id p26si23027869edu.78.2021.08.10.20.42.47; Tue, 10 Aug 2021 20:43:11 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=alibaba.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233217AbhHKDlv (ORCPT + 99 others); Tue, 10 Aug 2021 23:41:51 -0400 Received: from out4436.biz.mail.alibaba.com ([47.88.44.36]:27303 "EHLO out4436.biz.mail.alibaba.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232795AbhHKDlv (ORCPT ); Tue, 10 Aug 2021 23:41:51 -0400 X-Alimail-AntiSpam: AC=PASS;BC=-1|-1;BR=01201311R701e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=e01e04394;MF=wenyang@linux.alibaba.com;NM=1;PH=DS;RN=7;SR=0;TI=SMTPD_---0Uied3FN_1628653275; Received: from IT-C02W23QPG8WN.local(mailfrom:wenyang@linux.alibaba.com fp:SMTPD_---0Uied3FN_1628653275) by smtp.aliyun-inc.com(127.0.0.1); Wed, 11 Aug 2021 11:41:16 +0800 Subject: Re: [PATCH] ipv4: return early for possible invalid uaddr To: Jakub Kicinski Cc: davem@davemloft.net, David Ahern , Hideaki YOSHIFUJI , Baoyou Xie , netdev@vger.kernel.org, linux-kernel@vger.kernel.org References: <20210807171938.38501-1-wenyang@linux.alibaba.com> <20210809153251.4c51c3cd@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> From: Wen Yang Message-ID: <72dd5ee4-d7f3-576c-c7b9-3f8f4980faf3@linux.alibaba.com> Date: Wed, 11 Aug 2021 11:41:15 +0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <20210809153251.4c51c3cd@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com> Content-Type: text/plain; charset=gbk; format=flowed Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org ?? 2021/8/10 ????6:32, Jakub Kicinski ะด??: > On Sun, 8 Aug 2021 01:19:38 +0800 Wen Yang wrote: >> The inet_dgram_connect() first calls inet_autobind() to select an >> ephemeral port, then checks uaddr in udp_pre_connect() or >> __ip4_datagram_connect(), but the port is not released until the socket >> is closed. >> >> We should return early for invalid uaddr to improve performance and >> simplify the code a bit, > > The performance improvement would be if the benchmark is calling > connect with invalid arguments? That seems like an extremely rare > scenario in real life. > Thanks for your comments. On the one hand, it is the performance impact, but we also found that it may cause DoS: simulate a scenario where udp connect is frequently performed (illegal addrlen, and the socket is not closed), the local ports will be exhausted quickly. >> and also switch from a mix of tabs and spaces to just tabs. > > Please never mix unrelated whitespace cleanup into patches making real > code changes. > OK. >> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c >> index 5464818..97b6fc4 100644 >> --- a/net/ipv4/af_inet.c >> +++ b/net/ipv4/af_inet.c >> @@ -569,6 +569,11 @@ int inet_dgram_connect(struct socket *sock, struct sockaddr *uaddr, >> if (uaddr->sa_family == AF_UNSPEC) >> return sk->sk_prot->disconnect(sk, flags); >> >> + if (uaddr->sa_family != AF_INET) >> + return -EAFNOSUPPORT; > > And what about IPv6 which also calls this function? > Sorry that currently only ipv4 has been modified, we will continue to improve, and the v2 patch will be submitted later, thank you. -- Best wishes?? Wen >> + if (addr_len < sizeof(struct sockaddr_in)) >> + return -EINVAL; >> + >> if (BPF_CGROUP_PRE_CONNECT_ENABLED(sk)) { >> err = sk->sk_prot->pre_connect(sk, uaddr, addr_len); >> if (err)