From patchwork Wed Mar 25 18:45:52 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boqun Feng X-Patchwork-Id: 6094201 Return-Path: X-Original-To: patchwork-linux-fsdevel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 89E7E9F399 for ; Wed, 25 Mar 2015 18:46:14 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 7BECF202C8 for ; Wed, 25 Mar 2015 18:46:13 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 95ACF2012E for ; Wed, 25 Mar 2015 18:46:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753972AbbCYSqJ (ORCPT ); Wed, 25 Mar 2015 14:46:09 -0400 Received: from mail-ob0-f176.google.com ([209.85.214.176]:35997 "EHLO mail-ob0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753103AbbCYSqH (ORCPT ); Wed, 25 Mar 2015 14:46:07 -0400 Received: by obdfc2 with SMTP id fc2so26980860obd.3; Wed, 25 Mar 2015 11:46:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id; bh=8/w5Q1oDGzvr9EoHCmeRIcbG3BVunyJGf9AToXCwLXs=; b=xOJBCljMUHTSs56jRIrVBqtE1BM/2AHaZfeVdpYs7/2Yd6CJKgAg0y1CK6TRoqBNIV MR6Kj27pqKJbQywjgpF00hobDq2Ms7MPKt/9RsZIfzGbNGl1aqs+CawJNXFeigpiw+3s QykkHY7RJNG74s1fBbx4mew+JcViYO/Z0Out3mKKawoZTx/XNW/DfNa90FIETe3vExRU b9Zvroe6pcJ/DHEQKSHhJxMi+9KKc0iWpdwcHL2lzU9/fapu3zegug1nxycYz7wsdfxg 6wx//S8dlyTxy+izYIkkqrdjTIK5IfRz6Bnd1q6a/6+qxCoy4is4oulm/iqW9PN+IljJ cNkg== X-Received: by 10.60.158.73 with SMTP id ws9mr8861819oeb.24.1427309166598; Wed, 25 Mar 2015 11:46:06 -0700 (PDT) Received: from localhost ([32.97.110.55]) by mx.google.com with ESMTPSA id p4sm2583086oif.29.2015.03.25.11.46.04 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 25 Mar 2015 11:46:05 -0700 (PDT) From: Boqun Feng To: linux-fsdevel@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Al Viro , Paul Moore , Boqun Feng Subject: [PATCH v2] vfs: avoid recopying file names in getname_flags Date: Thu, 26 Mar 2015 02:45:52 +0800 Message-Id: <1427309152-25129-1-git-send-email-boqun.feng@gmail.com> X-Mailer: git-send-email 2.3.3 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=ham 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, a file name 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 file name are already copied into kernel address space, the only reason why the recopy is needed is that "kname", which points to the string of the file name, needs to be relocated. And the recopy can be avoided if we change the memory layout of the `names_cachep` allocation. By putting `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. Of course, other functions aware of the layout of the `names_cachep` allocation, i.e., getname_kernel and putname also need to be modified to adapt to the new layout. Since we change the layout of `names_cachep` allocation, in order to figure out whether kname and the struct are separate, we now need to check whether the file name string starts at the address EMBEDDED_NAME_MAX bytes before the address of the struct. As a result, `iname`, which points the end of `struct filename`, is not needed anymore. Signed-off-by: Boqun Feng --- v1 --> v2: Rebase the patch onto the for-next branch of Al's vfs repo. fs/namei.c | 45 ++++++++++++++++++++++++++++----------------- include/linux/fs.h | 1 - 2 files changed, 28 insertions(+), 18 deletions(-) diff --git a/fs/namei.c b/fs/namei.c index 7a11ec1..6d04029 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -119,7 +119,7 @@ * PATH_MAX includes the nul terminator --RR. */ -#define EMBEDDED_NAME_MAX (PATH_MAX - offsetof(struct filename, iname)) +#define EMBEDDED_NAME_MAX (PATH_MAX - sizeof(struct filename)) struct filename * getname_flags(const char __user *filename, int flags, int *empty) @@ -132,44 +132,53 @@ 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); /* * First, try to embed the struct filename inside the names_cache * allocation */ - kname = (char *)result->iname; + result = (struct filename *)(kname + EMBEDDED_NAME_MAX); result->name = kname; len = strncpy_from_user(kname, filename, EMBEDDED_NAME_MAX); if (unlikely(len < 0)) { - __putname(result); + __putname(kname); return ERR_PTR(len); } /* * 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 (unlikely(len == EMBEDDED_NAME_MAX)) { - kname = (char *)result; - result = kzalloc(sizeof(*result), GFP_KERNEL); if (unlikely(!result)) { __putname(kname); return ERR_PTR(-ENOMEM); } result->name = kname; - len = strncpy_from_user(kname, filename, PATH_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, + PATH_MAX - EMBEDDED_NAME_MAX); + if (unlikely(len < 0)) { __putname(kname); kfree(result); return ERR_PTR(len); } + + len += EMBEDDED_NAME_MAX; if (unlikely(len == PATH_MAX)) { __putname(kname); kfree(result); @@ -204,26 +213,28 @@ 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->iname; + result = (struct filename *)(kname + EMBEDDED_NAME_MAX); + result->name = kname; } 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; result = tmp; } else { - __putname(result); + __putname(kname); return ERR_PTR(-ENAMETOOLONG); } memcpy((char *)result->name, filename, len); @@ -242,11 +253,11 @@ void putname(struct filename *name) if (--name->refcnt > 0) return; - if (name->name != name->iname) { + if (name->name != ((char *)name - EMBEDDED_NAME_MAX)) { __putname(name->name); kfree(name); } else - __putname(name); + __putname(name->name); } static int check_acl(struct inode *inode, int mask) diff --git a/include/linux/fs.h b/include/linux/fs.h index dfbd88a..dd67284 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2166,7 +2166,6 @@ struct filename { const __user char *uptr; /* original userland pointer */ struct audit_names *aname; int refcnt; - const char iname[]; }; extern long vfs_truncate(struct path *, loff_t);