From patchwork Wed Dec 14 19:35:39 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Marshall X-Patchwork-Id: 9474553 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 78BE06021C for ; Wed, 14 Dec 2016 19:35:50 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 650652866E for ; Wed, 14 Dec 2016 19:35:50 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 599402870D; Wed, 14 Dec 2016 19:35:50 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B74F22866E for ; Wed, 14 Dec 2016 19:35:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753996AbcLNTfm (ORCPT ); Wed, 14 Dec 2016 14:35:42 -0500 Received: from mail-wm0-f48.google.com ([74.125.82.48]:37474 "EHLO mail-wm0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752075AbcLNTfm (ORCPT ); Wed, 14 Dec 2016 14:35:42 -0500 Received: by mail-wm0-f48.google.com with SMTP id t79so10779412wmt.0 for ; Wed, 14 Dec 2016 11:35:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=omnibond-com.20150623.gappssmtp.com; s=20150623; h=mime-version:from:date:message-id:subject:to; bh=ec0J2cEXeftIeOxvZpcmvKKV8B4Ajzwkfr28myT0KRw=; b=Lhmku6GHacZ/hGKpqPAzVxOBdJxKnhjn8uciSDMVnlp85dLuobOEFdP3Y4l4iQk8Pf qXX6aqlxT9av3aix0uNzcdX4xHk1g8BlxMESVat6wx9l6OEvzBXDNUnfcu1ZUY3EdVBl JpTdg7+Sl7UHbw7HexBAeczzm/O9kEsGGOGT43jDiZINXIoED/huVxkO42I5WMKl0Qb5 Y759/qe/zszjg8KIhMW4pCn+Fn/EOE60GoEzTb11cvq/Y/8dm3aztgOiILTxOg4+egjH dswTpVHi/EvT5m1oU7IcytVqcUzrDXhu+irwnkJz9tZm8oEEI4GcQMJGvqcqrDGe13rT ekHA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:from:date:message-id:subject:to; bh=ec0J2cEXeftIeOxvZpcmvKKV8B4Ajzwkfr28myT0KRw=; b=lfvByd7g5oJ7w+SetBBcwcRyiCd8o3HfYToxxUuXQ0TMKSMP+EITRT4cQjlMgaHQ81 km+6bEe59HWLpsthOUH5rMv82PsBgQSH4ZpQC10zeNaW/Hx84cH5cQVW91Yn6YWG7C/P jD3AjWqOFdKepcBC5hiXV5200DYzGjpp+haRiwBh0UEsOayWrmgajnGG7jPwloYnNhFG FQIiTdjL0hFib4gwFzXRdqazOgYBVl4+igRDrow/6smqvLO+Nkd7KKV4rBkAZvULriaA 9CHgWzUXl8Pue1ZHpGnw2kjNV0DkThXwrh0YFx6akuGsrVw+BjQtqiEVlhm3jhWlrEso pBHw== X-Gm-Message-State: AKaTC01kfMJPmFjMPI3rwparhvvK0NWpo8kI9xzwBTQtWDTxI/ahKq3xSxHkzlEwna4F1ad2WPYE5JQMgKaMEA== X-Received: by 10.25.162.198 with SMTP id l189mr34019527lfe.50.1481744140093; Wed, 14 Dec 2016 11:35:40 -0800 (PST) MIME-Version: 1.0 Received: by 10.25.131.150 with HTTP; Wed, 14 Dec 2016 11:35:39 -0800 (PST) From: Mike Marshall Date: Wed, 14 Dec 2016 14:35:39 -0500 Message-ID: Subject: [RFC] racy use of ->d_name To: linux-fsdevel , Mike Marshall Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP In a recent thread I read on fs-devel, Al Viro said: AV> the use of ->d_name *is* racy, and while it might be AV> tolerable for occasional debugging, for anything in heavier use it's AV> asking for trouble. Naturally we use d_name in Orangefs some, so I looked at some other code for inspiration on how to protect its usage. I'm guessing this example is how I should protect simple usages like this: the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/fs/orangefs/dcache.c b/fs/orangefs/dcache.c index 5355efb..05321f4 100644 --- a/fs/orangefs/dcache.c +++ b/fs/orangefs/dcache.c @@ -30,9 +30,11 @@ static int orangefs_revalidate_lookup(struct dentry *dentry) new_op->upcall.req.lookup.sym_follow = ORANGEFS_LOOKUP_LINK_NO_FOLLOW; new_op->upcall.req.lookup.parent_refn = parent->refn; + spin_lock(&dentry->d_lock); strncpy(new_op->upcall.req.lookup.d_name, dentry->d_name.name, ORANGEFS_NAME_MAX); + spin_unlock(&dentry->d_lock); gossip_debug(GOSSIP_DCACHE_DEBUG, "%s:%s:%d interrupt flag [%d]\n", It seems that in debug statements, icky looking things like file->f_path.dentry->d_name.name get replaced with "file" and "%pD". I have a place where I use file->f_path.dentry->d_name.name in a strcmp, and the way I "fixed" it seems verbose and hackerly... what should I do instead? diff --git a/fs/orangefs/orangefs-debugfs.c b/fs/orangefs/orangefs-debugfs.c index b5dbc9c..bb1e8a8 100644 --- a/fs/orangefs/orangefs-debugfs.c +++ b/fs/orangefs/orangefs-debugfs.c @@ -432,6 +432,8 @@ static ssize_t orangefs_debug_write(struct file *file, int rc = -EFAULT; size_t silly = 0; char *debug_string; + char *debug_file_name; + int debug_file_buf_len = strlen(ORANGEFS_KMOD_DEBUG_FILE) + 10; struct orangefs_kernel_op_s *new_op = NULL; struct client_debug_mask c_mask = { NULL, 0, 0 }; @@ -452,6 +454,11 @@ static ssize_t orangefs_debug_write(struct file *file, if (!buf) goto out; + debug_file_name = + kzalloc(debug_file_buf_len, GFP_KERNEL); + if (!debug_file_name) + goto out; + if (copy_from_user(buf, ubuf, count - 1)) { gossip_debug(GOSSIP_DEBUGFS_DEBUG, "%s: copy_from_user failed!\n", @@ -468,8 +475,20 @@ static ssize_t orangefs_debug_write(struct file *file, * A service operation is required to set a new client-side * debug mask. */ - if (!strcmp(file->f_path.dentry->d_name.name, - ORANGEFS_KMOD_DEBUG_FILE)) { + + /* + * We need to look and see whether they're setting the keyword + * string in the kernel debug file, or the client debug file. + * Fetching the filename out of "file" with "%pD" is better + * than fetching it out of file->f_path.dentry->d_name.name. + * + * debug_file_buf_len is longer than it needs to be in case + * some weirdo or malicious file that matches "the right thing + * and then some" gets passed into here somehow. + */ + snprintf(debug_file_name, debug_file_buf_len, "%pD", file); + + if (!strcmp(debug_file_name, ORANGEFS_KMOD_DEBUG_FILE)) { debug_string_to_mask(buf, &orangefs_gossip_debug_mask, 0); debug_mask_to_string(&orangefs_gossip_debug_mask, 0); debug_string = kernel_debug_string; @@ -536,6 +555,7 @@ static ssize_t orangefs_debug_write(struct file *file, "orangefs_debug_write: rc: %d\n", rc); kfree(buf); + kfree(debug_file_name); return rc; } -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in