From patchwork Fri Mar 18 14:03:42 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Austin S. Hemmelgarn" X-Patchwork-Id: 8619241 Return-Path: X-Original-To: patchwork-linux-btrfs@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 11C1AC0553 for ; Fri, 18 Mar 2016 14:04:18 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 1C41720303 for ; Fri, 18 Mar 2016 14:04:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 149DF2037F for ; Fri, 18 Mar 2016 14:04:15 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755277AbcCROEK (ORCPT ); Fri, 18 Mar 2016 10:04:10 -0400 Received: from mail-qk0-f195.google.com ([209.85.220.195]:35495 "EHLO mail-qk0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752439AbcCROEH (ORCPT ); Fri, 18 Mar 2016 10:04:07 -0400 Received: by mail-qk0-f195.google.com with SMTP id s5so3716391qkd.2 for ; Fri, 18 Mar 2016 07:04: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=FFH9RyPq1nOk/veDakG68DqHfqIa77N5xCjV7znIfjs=; b=moYIeOs8YdJ3sJpKDOXR6VAwbf+2uqVpHMJtHxWSFehn5TPdPNhw32p+lS5ZN8RNXb c149dAHB5tTWebeBnuXPf/Kyp3K5FdiY7YooHGi+2mI4IHHNySkUMc2ybLamhGavHHeO vcfDBDQ8o7dA/pzelyY2LzuVfSzF5MpbGK4FvnU4kmj5YfjOsAckTIvCOvFVo9zHzCxT g0gfZyPpTVNVesF/fAzbXF1JleoRpjqd0U+9l6UdB7QKg3apKOjYyyTR0XI4QTIv9t3g o4eIlW98WB7uuwaTKdKHuTkz5F4Z81T0srxoIM1CFd0UuX/6bnaa0W8L66Ri8YmHa61d gAXg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=FFH9RyPq1nOk/veDakG68DqHfqIa77N5xCjV7znIfjs=; b=INwxzmYukNPI7DYgziowG3fc2jXEr+mcisQBgR+8IqZ0/oUdq9z3pT9L/45n39vhCO Xzojbg95RwIaPm2n6VyKV+FAquuwRPeAvaA4NsWI4YF0WMwzVc+jnb/1cRehxOkPSBvv u3AR0jcVcqhtw7AizjRFXjW+zoGHMDgi2N1dZ1yhzgegO03LutEKFjK/5cffDp1GfXwS nB62Q/1UxfUVR6elSJU1Cre60Sf2Y281yRxUdSDW5TSvi+fNxIVc8Ta8cPiaW22iuPJQ 7wO/8uiCmiQGH1WpBBuV0oy+6sZUOyMOzob4Zq+ZTc7EsHRumfMjP5FqPK40zufYT7xQ rcew== X-Gm-Message-State: AD7BkJKaEtBtrMwq/TqVgXRbMD1OkXOZOd3VEiBNyptm1/or7uGP9gnoc4h0J3GaFIs2Ew== X-Received: by 10.55.209.148 with SMTP id o20mr22544298qkl.5.1458309845683; Fri, 18 Mar 2016 07:04:05 -0700 (PDT) Received: from wild-karde.localdomain (rrcs-70-62-41-24.central.biz.rr.com. [70.62.41.24]) by smtp.googlemail.com with ESMTPSA id x64sm5988581qhx.47.2016.03.18.07.04.04 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 18 Mar 2016 07:04:04 -0700 (PDT) Received: by wild-karde.localdomain (Postfix, from userid 1000) id AB6631A4DBD5; Fri, 18 Mar 2016 10:04:03 -0400 (EDT) From: "Austin S. Hemmelgarn" To: linux-btrfs@vger.kernel.org, dsterba@suse.cz, clm@fb.com Cc: "Austin S. Hemmelgarn" Subject: [PATCH v2] btrfs-progs: add stat check in open_ctree_fs_info Date: Fri, 18 Mar 2016 10:03:42 -0400 Message-Id: <1458309822-5550-1-git-send-email-ahferroin7@gmail.com> X-Mailer: git-send-email 2.7.4 Sender: linux-btrfs-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-btrfs@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, RP_MATCHES_RCVD, T_DKIM_INVALID, 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 Currently, open_ctree_fs_info will open whatever path you pass it and try to interpret it as a BTRFS filesystem. While this is not nessecarily dangerous (except possibly if done on a character device), it does result in some rather cryptic and non-sensical error messages when trying to run certain commands in ways they weren't intended to be run. Add a check using stat(2) to verify that the path we've been passed is in fact a regular file or a block device, or a symlink pointing to a regular file or block device. This causes the following commands to provide a helpful error message when run on a FIFO, directory, character device, or socket: * btrfs check * btrfs restore * btrfs-image * btrfs-find-root * btrfs inspect-internal dump-tree stat(2) is used instead of lstat(2), as stat(2) follows symlinks just like open(2) does, which means we check the same inode that open(2) opens, and thus don't need special handling for symlinks. Signed-off-by: Austin S. Hemmelgarn --- Changes from v1: * Updated commit message to use the new name for btrfs-debug-tree * Added a bit of clarity to the commit message to explain that stat(2) follows symlinks just like open(2) does, so we don't need special handling for symlinks.. * Added the btrfs-progs developers to the recipient list, because I somehow forgot to add them on the first version. This has been both build and runtime tested on an x86-64 system with glibc. It has been build but not runtime tested with uClibc on x86-64 and ARMv7. It has not been tested on Android or with musl, although it should work there also. There are other tools that have similarly poor error behavior when called incorrectly (btrfs rescue immediately comes to mind), but they don't use open_ctree_fs_info, so this doesn't affect them. I may do followup patches to fix those too if I have the time. open_ctree_fs_info is also used in cmds-filesystem.c, although I'm not at all sure what exactly is going on there, and btrfs filesystem appears from my testing to behave exactly the same with this change, so I don't think this will have any effect on any of the btrfs filesystem commands. Whitelisting is used instead of blacklisting because I feel it provides a more concise and more easily readable conditional, and because I think it's a lot less likely that a new file type will be added that can contain a filesystem image than it is that one will be added which can't contain a filesystem image (it's more likely that Solaris doors get ported to Linux than that we get some new file type). disk-io.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/disk-io.c b/disk-io.c index e520d80..d35153d 100644 --- a/disk-io.c +++ b/disk-io.c @@ -1310,6 +1310,13 @@ struct btrfs_fs_info *open_ctree_fs_info(const char *filename, int fp; struct btrfs_fs_info *info; int oflags = O_CREAT | O_RDWR; + struct stat sb; + + stat(filename, &sb); + if (!(((sb.st_mode & S_IFMT) == S_IFREG) || ((sb.st_mode & S_IFMT) == S_IFBLK))) { + fprintf (stderr, "%s is not a regular file or block device\n", filename); + return NULL; + } if (!(flags & OPEN_CTREE_WRITES)) oflags = O_RDONLY;