From patchwork Wed Feb 25 12:31:08 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boqun Feng X-Patchwork-Id: 5879521 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id AA87BBF440 for ; Wed, 25 Feb 2015 12:31:37 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id BEEC120373 for ; Wed, 25 Feb 2015 12:31:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id AF4772034F for ; Wed, 25 Feb 2015 12:31:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753231AbbBYMbU (ORCPT ); Wed, 25 Feb 2015 07:31:20 -0500 Received: from mail-pd0-f181.google.com ([209.85.192.181]:34301 "EHLO mail-pd0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753160AbbBYMbU (ORCPT ); Wed, 25 Feb 2015 07:31:20 -0500 Received: by pdjg10 with SMTP id g10so4658559pdj.1; Wed, 25 Feb 2015 04:31:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id; bh=8YUr8+/sGA5LlI+znEmbUOCnWYlQErmkaJqxc9hc7ok=; b=lwmWan8cRACgyTG/H+MoNzbHTzYq3gM0dKXMpUS5mBd2yJkXhQrZwwnpbmWcSy/JiG LnyRBNDDAjIumqGCCPKBP0MloghC9N2ciGgrhlYkMMu+hZUqlpmlP7SZwy3GIxf3J3QA AynRzxaWPaxbjYdTnl3e/Gs5R+sSW+T0RAAM+YnzhdqQ0BSll7H+IssrtnUznSETbqPd Gan4k/0SAhrTjQvJLK97FLJpz0kMD8i1atPsAcSEdXDRC0bPXs0XkdBiWaykgoadgwe0 GzfgekqNh2yRP5kRGvMusthXL7tqBq82d183bQb6xumJMmGsLV1f9Gda97kklA+URW10 k9qA== X-Received: by 10.70.124.138 with SMTP id mi10mr5096627pdb.23.1424867479579; Wed, 25 Feb 2015 04:31:19 -0800 (PST) Received: from localhost ([162.243.141.196]) by mx.google.com with ESMTPSA id z1sm13957817pdp.52.2015.02.25.04.31.16 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 25 Feb 2015 04:31:18 -0800 (PST) From: Boqun Feng To: linux-fsdevel@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Boqun Feng , Al Viro , Paul Moore Subject: [PATCH] vfs: avoid recopying filename in getname_flags Date: Wed, 25 Feb 2015 20:31:08 +0800 Message-Id: <1424867468-17346-1-git-send-email-boqun.feng@gmail.com> X-Mailer: git-send-email 2.3.0 Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Spam-Status: No, score=-6.8 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, T_DKIM_INVALID, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP In the current implementation of getname_flags, filename in the user-space will be recopied if it takes more space that EMBEDDED_NAME_MAX, however, at this moment, EMBEDDED_NAME_MAX bytes of the filename are already copied into kernel space, the only reason why the recopy is needed is that "kname" needs to be relocated. And the recopy can be avoided if we change the memory layout of the "names_cache" allocation. By putting the struct "filename" at the tail of the allocation instead of the head, relocation of kname is avoided. Once putting the struct at the tail, each byte in the user space will be copied only one time, so the recopy is avoided and code is more clear. Of course, other functions aware of the layout of the names_cache allocation, i.e., getname_kernel and putname also need to be modified to adapt to the new layout. This patch is based on v4.0-rc1. Cc: Al Viro Cc: Paul Moore Signed-off-by: Boqun Feng --- fs/namei.c | 50 +++++++++++++++++++++++++++++++------------------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index c83145a..3be372b 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -133,21 +133,20 @@ getname_flags(const char __user *filename, int flags, int *empty) if (result) return result; - result = __getname(); - if (unlikely(!result)) + kname = __getname(); + if (unlikely(!kname)) return ERR_PTR(-ENOMEM); - result->refcnt = 1; /* * First, try to embed the struct filename inside the names_cache * allocation */ - kname = (char *)result + sizeof(*result); + result = (struct filename *)(kname + EMBEDDED_NAME_MAX); result->name = kname; result->separate = false; + result->refcnt = 1; max = EMBEDDED_NAME_MAX; -recopy: len = strncpy_from_user(kname, filename, max); if (unlikely(len < 0)) { err = ERR_PTR(len); @@ -157,23 +156,34 @@ recopy: /* * Uh-oh. We have a name that's approaching PATH_MAX. Allocate a * separate struct filename so we can dedicate the entire - * names_cache allocation for the pathname, and re-do the copy from + * names_cache allocation for the pathname, and continue the copy from * userland. */ - if (len == EMBEDDED_NAME_MAX && max == EMBEDDED_NAME_MAX) { - kname = (char *)result; - + if (len == EMBEDDED_NAME_MAX) { result = kzalloc(sizeof(*result), GFP_KERNEL); if (!result) { err = ERR_PTR(-ENOMEM); - result = (struct filename *)kname; + result = (struct filename *)(kname + EMBEDDED_NAME_MAX); goto error; } result->name = kname; result->separate = true; result->refcnt = 1; - max = PATH_MAX; - goto recopy; + max = PATH_MAX - EMBEDDED_NAME_MAX; + /* we can't simply add the number of rest chars we copy to len, + * since strncpy_from_user may return negative to indicate + * something is wrong, so we do the addition later, after + * strncpy_from_user succeeds, and we know we already copy + * EMBEDDED_NAME_MAX chars. + */ + len = strncpy_from_user(kname + EMBEDDED_NAME_MAX, + filename + EMBEDDED_NAME_MAX, max); + + if (unlikely(len < 0)) { + err = ERR_PTR(len); + goto error; + } + len += EMBEDDED_NAME_MAX; } /* The empty path is special. */ @@ -209,28 +219,30 @@ struct filename * getname_kernel(const char * filename) { struct filename *result; + char *kname; int len = strlen(filename) + 1; - result = __getname(); - if (unlikely(!result)) + kname = __getname(); + if (unlikely(!kname)) return ERR_PTR(-ENOMEM); if (len <= EMBEDDED_NAME_MAX) { - result->name = (char *)(result) + sizeof(*result); + result = (struct filename *)(kname + EMBEDDED_NAME_MAX); + result->name = kname; result->separate = false; } else if (len <= PATH_MAX) { struct filename *tmp; tmp = kmalloc(sizeof(*tmp), GFP_KERNEL); if (unlikely(!tmp)) { - __putname(result); + __putname(kname); return ERR_PTR(-ENOMEM); } - tmp->name = (char *)result; + tmp->name = kname; tmp->separate = true; result = tmp; } else { - __putname(result); + __putname(kname); return ERR_PTR(-ENAMETOOLONG); } memcpy((char *)result->name, filename, len); @@ -253,7 +265,7 @@ void putname(struct filename *name) __putname(name->name); kfree(name); } else - __putname(name); + __putname(name->name); } static int check_acl(struct inode *inode, int mask)