From patchwork Fri Nov 6 06:32:04 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Biggers X-Patchwork-Id: 7566561 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 5DD3C9F71A for ; Fri, 6 Nov 2015 06:32:12 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 5AC3D2071E for ; Fri, 6 Nov 2015 06:32:11 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 412DD20715 for ; Fri, 6 Nov 2015 06:32:10 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757292AbbKFGcI (ORCPT ); Fri, 6 Nov 2015 01:32:08 -0500 Received: from mail-ig0-f181.google.com ([209.85.213.181]:38181 "EHLO mail-ig0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754566AbbKFGcH (ORCPT ); Fri, 6 Nov 2015 01:32:07 -0500 Received: by igbxm8 with SMTP id xm8so3214089igb.1; Thu, 05 Nov 2015 22:32:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=JS+Km050ewmy+BpK2fQBEcikqeUoleTDJ0WTaz/wKtU=; b=RZx1+I3NYNjca9Zcbwbd+Nhlr/O0+A4gvg4vfDng6UNNQ9A+ahFpnI2UQ7Pc+5QlE/ V6cliCWYcsrVnWC8YS6M5cc6lfVGcTIm4ibSIsTaluBvDvj3e4KfmFSvSqSwAEG/dz2P HowRCFMd1hX4X2bph3l8hDrKINPE1NMx0Inlt0k9RApV4h0pMF6qB1OTiDm+2TgAQK9Q V43HHDA+VhoPxva0OUN4PBp+9yFo6pKYeDst18K3sMc+7PoUqz94ix1hPVks9aZWbr5w rZ1Dk2opfy48TjZ7waRNDWQYnk6/nhXUGoZt8XmJDdgRdd/kzEGFnnuSGeEkh20fuPHt sdyA== X-Received: by 10.50.62.104 with SMTP id x8mr7045381igr.7.1446791526481; Thu, 05 Nov 2015 22:32:06 -0800 (PST) Received: from zzz (c-75-72-182-251.hsd1.mn.comcast.net. [75.72.182.251]) by smtp.gmail.com with ESMTPSA id o9sm579940igh.5.2015.11.05.22.32.05 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 05 Nov 2015 22:32:06 -0800 (PST) Date: Fri, 6 Nov 2015 00:32:04 -0600 From: Eric Biggers To: Linus Torvalds Cc: linux-fsdevel , Linux Kernel Mailing List , Al Viro , Eric Dumazet Subject: Re: [PATCH] vfs: clear remainder of 'full_fds_bits' in dup_fd() Message-ID: <20151106063204.GA865@zzz> References: <1446781368-13766-1-git-send-email-ebiggers3@gmail.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) 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 Here's the revised patch: vfs: clear remainder of 'full_fds_bits' in dup_fd() This fixes a bug from commit f3f86e33dc3d ("vfs: Fix pathological performance case for __alloc_fd()"). v2: refactor to share fd bitmap copying code Signed-off-by: Eric Biggers --- fs/file.c | 64 +++++++++++++++++++++++++++++++-------------------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/fs/file.c b/fs/file.c index 6f6eb2b..a6ff7c3 100644 --- a/fs/file.c +++ b/fs/file.c @@ -60,8 +60,31 @@ static void free_fdtable_rcu(struct rcu_head *rcu) #define BITBIT_SIZE(nr) (BITBIT_NR(nr) * sizeof(long)) /* - * Expand the fdset in the files_struct. Called with the files spinlock - * held for write. + * Copy 'count' fd bits from the old table to the new table and clear the extra + * space if any. This does not copy the file pointers. Called with the files + * spinlock held for write. + */ +static void copy_fd_bitmaps(struct fdtable *nfdt, struct fdtable *ofdt, + unsigned int count) +{ + unsigned int cpy, set; + + cpy = count / BITS_PER_BYTE; + set = (nfdt->max_fds - count) / BITS_PER_BYTE; + memcpy(nfdt->open_fds, ofdt->open_fds, cpy); + memset((char *)nfdt->open_fds + cpy, 0, set); + memcpy(nfdt->close_on_exec, ofdt->close_on_exec, cpy); + memset((char *)nfdt->close_on_exec + cpy, 0, set); + + cpy = BITBIT_SIZE(count); + set = BITBIT_SIZE(nfdt->max_fds) - cpy; + memcpy(nfdt->full_fds_bits, ofdt->full_fds_bits, cpy); + memset((char *)nfdt->full_fds_bits + cpy, 0, set); +} + +/* + * Copy all file descriptors from the old table to the new, expanded table and + * clear the extra space. Called with the files spinlock held for write. */ static void copy_fdtable(struct fdtable *nfdt, struct fdtable *ofdt) { @@ -72,19 +95,9 @@ static void copy_fdtable(struct fdtable *nfdt, struct fdtable *ofdt) cpy = ofdt->max_fds * sizeof(struct file *); set = (nfdt->max_fds - ofdt->max_fds) * sizeof(struct file *); memcpy(nfdt->fd, ofdt->fd, cpy); - memset((char *)(nfdt->fd) + cpy, 0, set); + memset((char *)nfdt->fd + cpy, 0, set); - cpy = ofdt->max_fds / BITS_PER_BYTE; - set = (nfdt->max_fds - ofdt->max_fds) / BITS_PER_BYTE; - memcpy(nfdt->open_fds, ofdt->open_fds, cpy); - memset((char *)(nfdt->open_fds) + cpy, 0, set); - memcpy(nfdt->close_on_exec, ofdt->close_on_exec, cpy); - memset((char *)(nfdt->close_on_exec) + cpy, 0, set); - - cpy = BITBIT_SIZE(ofdt->max_fds); - set = BITBIT_SIZE(nfdt->max_fds) - cpy; - memcpy(nfdt->full_fds_bits, ofdt->full_fds_bits, cpy); - memset(cpy+(char *)nfdt->full_fds_bits, 0, set); + copy_fd_bitmaps(nfdt, ofdt, ofdt->max_fds); } static struct fdtable * alloc_fdtable(unsigned int nr) @@ -276,7 +289,7 @@ struct files_struct *dup_fd(struct files_struct *oldf, int *errorp) { struct files_struct *newf; struct file **old_fds, **new_fds; - int open_files, size, i; + int open_files, i; struct fdtable *old_fdt, *new_fdt; *errorp = -ENOMEM; @@ -333,13 +346,11 @@ struct files_struct *dup_fd(struct files_struct *oldf, int *errorp) open_files = count_open_files(old_fdt); } + copy_fd_bitmaps(new_fdt, old_fdt, open_files); + old_fds = old_fdt->fd; new_fds = new_fdt->fd; - memcpy(new_fdt->open_fds, old_fdt->open_fds, open_files / 8); - memcpy(new_fdt->close_on_exec, old_fdt->close_on_exec, open_files / 8); - memcpy(new_fdt->full_fds_bits, old_fdt->full_fds_bits, BITBIT_SIZE(open_files)); - for (i = open_files; i != 0; i--) { struct file *f = *old_fds++; if (f) { @@ -357,19 +368,8 @@ struct files_struct *dup_fd(struct files_struct *oldf, int *errorp) } spin_unlock(&oldf->file_lock); - /* compute the remainder to be cleared */ - size = (new_fdt->max_fds - open_files) * sizeof(struct file *); - - /* This is long word aligned thus could use a optimized version */ - memset(new_fds, 0, size); - - if (new_fdt->max_fds > open_files) { - int left = (new_fdt->max_fds - open_files) / 8; - int start = open_files / BITS_PER_LONG; - - memset(&new_fdt->open_fds[start], 0, left); - memset(&new_fdt->close_on_exec[start], 0, left); - } + /* clear the remainder */ + memset(new_fds, 0, (new_fdt->max_fds - open_files) * sizeof(struct file *)); rcu_assign_pointer(newf->fdt, new_fdt);