From patchwork Tue Aug 21 21:33:52 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "J. Bruce Fields" X-Patchwork-Id: 1358161 Return-Path: X-Original-To: patchwork-linux-nfs@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id 5BDA8DFB34 for ; Tue, 21 Aug 2012 21:33:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758427Ab2HUVdx (ORCPT ); Tue, 21 Aug 2012 17:33:53 -0400 Received: from fieldses.org ([174.143.236.118]:33988 "EHLO fieldses.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757430Ab2HUVdx (ORCPT ); Tue, 21 Aug 2012 17:33:53 -0400 Received: from bfields by fieldses.org with local (Exim 4.72) (envelope-from ) id 1T3w5A-0004xZ-7o; Tue, 21 Aug 2012 17:33:52 -0400 Date: Tue, 21 Aug 2012 17:33:52 -0400 From: "J. Bruce Fields" To: Chuck Lever Cc: "J. Bruce Fields" , linux-nfs@vger.kernel.org Subject: Re: [PATCH 08/14] svcrpc: ignore unknown address type in udp receive Message-ID: <20120821213352.GE18637@fieldses.org> References: <1345582652-18476-1-git-send-email-bfields@redhat.com> <1345582652-18476-9-git-send-email-bfields@redhat.com> <05C6926C-B688-4A71-AA1C-26F251BBD635@oracle.com> <20120821212431.GC18637@fieldses.org> <822DBA73-7B4C-43CC-B096-9D8B4A219390@oracle.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <822DBA73-7B4C-43CC-B096-9D8B4A219390@oracle.com> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-nfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-nfs@vger.kernel.org On Tue, Aug 21, 2012 at 05:29:16PM -0400, Chuck Lever wrote: > > On Aug 21, 2012, at 5:24 PM, J. Bruce Fields wrote: > > > On Tue, Aug 21, 2012 at 05:02:14PM -0400, Chuck Lever wrote: > >> > >> On Aug 21, 2012, at 4:57 PM, J. Bruce Fields wrote: > >> > >>> From: "J. Bruce Fields" > >>> > >>> How would this happen? > >> > >> If an unsupported address family is used in the rqstp. > > > > Right, but that's impossible, isn't it? > > The point is to catch this case when someone adds support for new address families. It won't happen in current code. > > >>> In any case, it appears this would be returned all the way up to the > >>> caller of svc_recv(), and it's obvious that none of them are equipped to > >>> handle it, and not clear what they would want to do with it anyway. > >>> Let's just drop this and return -EAGAIN. > >> > >> EAGAIN is incorrect; the correct error code is EAFNOSUPPORT. If callers are not prepared for this error return, perhaps BUG_ON() would be more appropriate here. > > > > Yeah. Actually on a quick check this is the only caller that even > > checks for this case. So probably the check should go in svc_addr_len. > > Maybe we should be nice and make it a warning. > > I normally find BUG pretty harsh, but in this case it's true a software error, and as you say, should be "impossible": thus it should be a BUG. The code should stop before the zero length is used. Eh, OK, I guess I can live with that. commit 751f877b10f8ce0d12b40d2a102f3b42b26dc08d Author: J. Bruce Fields Date: Tue Aug 21 17:22:11 2012 -0400 svcrpc: don't bother checking bad svc_addr_len result None of the callers should see an unsupported address family (only one of them even bothers to check for that case), so just check for the buggy case in svc_addr_len and don't bother elsewhere. Signed-off-by: J. Bruce Fields Acked-by: Chuck Lever --- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/include/linux/sunrpc/svc_xprt.h b/include/linux/sunrpc/svc_xprt.h index b3f64b1..4f3a6ab 100644 --- a/include/linux/sunrpc/svc_xprt.h +++ b/include/linux/sunrpc/svc_xprt.h @@ -166,8 +166,7 @@ static inline size_t svc_addr_len(const struct sockaddr *sa) case AF_INET6: return sizeof(struct sockaddr_in6); } - - return 0; + BUG(); } static inline unsigned short svc_xprt_local_port(const struct svc_xprt *xprt) diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c index 998aa8c..13b005c 100644 --- a/net/sunrpc/svcsock.c +++ b/net/sunrpc/svcsock.c @@ -601,8 +601,6 @@ static int svc_udp_recvfrom(struct svc_rqst *rqstp) return -EAGAIN; } len = svc_addr_len(svc_addr(rqstp)); - if (len == 0) - return -EAFNOSUPPORT; rqstp->rq_addrlen = len; if (skb->tstamp.tv64 == 0) { skb->tstamp = ktime_get_real();