From patchwork Mon Dec 2 10:12:26 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yann Droneaud X-Patchwork-Id: 295857 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 297B92C009A for ; Mon, 2 Dec 2013 21:13:57 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753251Ab3LBKN2 (ORCPT ); Mon, 2 Dec 2013 05:13:28 -0500 Received: from smtpfb2-g21.free.fr ([212.27.42.10]:55143 "EHLO smtpfb2-g21.free.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753237Ab3LBKNW (ORCPT ); Mon, 2 Dec 2013 05:13:22 -0500 Received: from smtp5-g21.free.fr (smtp5-g21.free.fr [212.27.42.5]) by smtpfb2-g21.free.fr (Postfix) with ESMTP id 119E6CA9AE3; Mon, 2 Dec 2013 11:13:13 +0100 (CET) Received: from localhost.localdomain (unknown [37.163.28.200]) by smtp5-g21.free.fr (Postfix) with ESMTP id 9F5FFD48274; Mon, 2 Dec 2013 11:12:51 +0100 (CET) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by localhost.localdomain (8.14.7/8.14.7) with ESMTP id rB2ACgIx013868; Mon, 2 Dec 2013 11:12:43 +0100 Received: (from ydroneaud@localhost) by localhost.localdomain (8.14.7/8.14.7/Submit) id rB2ACeKQ013867; Mon, 2 Dec 2013 11:12:40 +0100 From: Yann Droneaud To: netdev@vger.kernel.org, linux-kernel@vger.kernel.org Cc: Yann Droneaud , "David S. Miller" , Al Viro Subject: [PATCH v2] net: handle error more gracefully in socketpair() Date: Mon, 2 Dec 2013 11:12:26 +0100 Message-Id: <1385979146-13825-1-git-send-email-ydroneaud@opteya.com> X-Mailer: git-send-email 1.8.4.2 In-Reply-To: <1385977019-12282-1-git-send-email-ydroneaud@opteya.com> References: <1385977019-12282-1-git-send-email-ydroneaud@opteya.com> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org socketpair() error paths can be simplified to not call heavy-weight sys_close(). This patch makes socketpair() use of error paths which do not rely on heavy-weight call to sys_lose(): it's better to try to push the file descriptor to userspace before installing the socket file to the file descriptor, so that errors are catched earlier and being easier to handle. Three distinct error paths are needed since calling fput() on file structure returned by sock_alloc_file() will implicitly call sock_release() on the associated socket structure. Cc: David S. Miller Cc: Al Viro Signed-off-by: Yann Droneaud Link: http://marc.info/?i=1385977019-12282-1-git-send-email-ydroneaud@opteya.com --- Hi, Please discard my previous patch "[PATCH] net: handle error more gracefully in socketpair()" [1] as I haven't double checked the underlying reason for the not-so-complicated error paths. This one is perhaps less appealing. But I think it's still a good thing to write the file descriptor to userspace before calling fd_install(): it's making error recovery easier. Changes from v1 [1]: - use three different error path to handle file allocated through sock_alloc_file(). Regards. [1] http://mid.gmane.org/1385977019-12282-1-git-send-email-ydroneaud@opteya.com net/socket.c | 49 +++++++++++++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/net/socket.c b/net/socket.c index 0b18693f2be6..72bf3c41f4f0 100644 --- a/net/socket.c +++ b/net/socket.c @@ -1445,48 +1445,61 @@ SYSCALL_DEFINE4(socketpair, int, family, int, type, int, protocol, err = fd1; goto out_release_both; } + fd2 = get_unused_fd_flags(flags); if (unlikely(fd2 < 0)) { err = fd2; - put_unused_fd(fd1); - goto out_release_both; + goto out_put_unused_1; } newfile1 = sock_alloc_file(sock1, flags, NULL); if (unlikely(IS_ERR(newfile1))) { err = PTR_ERR(newfile1); - put_unused_fd(fd1); - put_unused_fd(fd2); - goto out_release_both; + goto out_put_unused_both; } newfile2 = sock_alloc_file(sock2, flags, NULL); if (IS_ERR(newfile2)) { err = PTR_ERR(newfile2); - fput(newfile1); - put_unused_fd(fd1); - put_unused_fd(fd2); - sock_release(sock2); - goto out; + goto out_fput_1; } + err = put_user(fd1, &usockvec[0]); + if (err) + goto out_fput_both; + + err = put_user(fd2, &usockvec[1]); + if (err) + goto out_fput_both; + audit_fd_pair(fd1, fd2); + fd_install(fd1, newfile1); fd_install(fd2, newfile2); /* fd1 and fd2 may be already another descriptors. * Not kernel problem. */ - err = put_user(fd1, &usockvec[0]); - if (!err) - err = put_user(fd2, &usockvec[1]); - if (!err) - return 0; + return 0; - sys_close(fd2); - sys_close(fd1); - return err; +out_fput_both: + fput(newfile2); + fput(newfile1); + put_unused_fd(fd2); + put_unused_fd(fd1); + goto out; + +out_fput_1: + fput(newfile1); + put_unused_fd(fd2); + put_unused_fd(fd1); + sock_release(sock2); + goto out; +out_put_unused_both: + put_unused_fd(fd2); +out_put_unused_1: + put_unused_fd(fd1); out_release_both: sock_release(sock2); out_release_1: