Received: by 2002:a25:d7c1:0:0:0:0:0 with SMTP id o184csp2694409ybg; Mon, 28 Oct 2019 00:23:05 -0700 (PDT) X-Google-Smtp-Source: APXvYqxOoXKPtLvnirTe1Oh1xue4W1lOReK0eDh85H8275qhml8rj+YBwXdFiSS5U09ermYcjP7S X-Received: by 2002:a05:6402:1ad0:: with SMTP id ba16mr9083415edb.107.1572247385509; Mon, 28 Oct 2019 00:23:05 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1572247385; cv=none; d=google.com; s=arc-20160816; b=ZHuNLx00f/rAguWmF4dyMy96PgEoiMdhMqa4q1zQc4j5/PP2TUqgVFf1mAjv4g5GVX mmvMAPiN6snR1i3N8b8xkAh/hcj/C8cnZAjLCeKTg4+Q6YF5INCJcnR4wrIodewuBWFi waaa08+RcU7Qf3oCv7cxNa44RS60WQsLXGKTBxmQkl3yqfL7FxZzGj2PPlu8LQpyS+cE iUDeiGGjITuAxB3kwWYdYLB5ZOG8Sb54D7CwTXnJvoat0mbHl5edz7hl0NJaU4GXWwDF vvIrTE2Xcx7OJfiJGQH5iKxavyHtWD8mftQBEviws14NWbiiOhR599dZwMbD1MMPS+O7 TCDQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:subject:cc:to :from:dkim-signature; bh=/gw7sowOUxewoPYzmEoK8L4mB9c98fZGRWQq89GGQ5I=; b=NxlPeWXMak/tlNJDYYqj8BPly7X3n1wRbimsdjJlLbvpkX1CGo/i5HRjjXWpdufSLM cT3E9Uq8o7EOMkbmdDDUXEtJVS3Iuh500FuvmURyf5QeGe1McIznJPU9UIjf4oYplV7A 4SY4bDIoiQj8XhK6NApsLPz3CJTQxLzA6Gtlz4rNPd0Pcp5w5e0LNRq77Xz8NPpKqvcN l8rsyBbL2kw+MYaLSIW+XWOx+4NAeW/1mQ4UvEcWgtM1pRU1rQZSzhNA08b1BS+SkFDX 23X46PDv3+rOQ6tb9xOKeiIA2srtZQfOfo+qb4oEMOtefVyZsFNekGSWpmmJy2SErIoa bEpw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b="aDdzFDU/"; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id k5si534877ejj.356.2019.10.28.00.22.42; Mon, 28 Oct 2019 00:23:05 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=default header.b="aDdzFDU/"; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1731523AbfJ0VTm (ORCPT + 99 others); Sun, 27 Oct 2019 17:19:42 -0400 Received: from mail.kernel.org ([198.145.29.99]:40014 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1731509AbfJ0VTj (ORCPT ); Sun, 27 Oct 2019 17:19:39 -0400 Received: from localhost (100.50.158.77.rev.sfr.net [77.158.50.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 68724214E0; Sun, 27 Oct 2019 21:19:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1572211177; bh=pdGeY4BdZh9wxfXX9Yk77xJOQObU4tbUQX9BnmzykKk=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=aDdzFDU/paDv3GeGwTV311Vx6p3AVTmZn9PBW9YY+0jNZvAsWnaffNwRu//TD1v+0 nEA2eWa4/izpLFEHhXdFr/3KVCzlpCljWybLsk2qdwy8K70FRN8+9ZEi2ppLXekzuO w6jKFGULCdlNVIUDCKUm3ua8j/y6gr2vtInKx2kg= From: Greg Kroah-Hartman To: linux-kernel@vger.kernel.org Cc: Greg Kroah-Hartman , stable@vger.kernel.org, Alexander Viro , Jann Horn , "Eric W. Biederman" , Linus Torvalds , Sasha Levin Subject: [PATCH 5.3 059/197] Make filldir[64]() verify the directory entry filename is valid Date: Sun, 27 Oct 2019 21:59:37 +0100 Message-Id: <20191027203354.857292722@linuxfoundation.org> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191027203351.684916567@linuxfoundation.org> References: <20191027203351.684916567@linuxfoundation.org> User-Agent: quilt/0.66 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org From: Linus Torvalds [ Upstream commit 8a23eb804ca4f2be909e372cf5a9e7b30ae476cd ] This has been discussed several times, and now filesystem people are talking about doing it individually at the filesystem layer, so head that off at the pass and just do it in getdents{64}(). This is partially based on a patch by Jann Horn, but checks for NUL bytes as well, and somewhat simplified. There's also commentary about how it might be better if invalid names due to filesystem corruption don't cause an immediate failure, but only an error at the end of the readdir(), so that people can still see the filenames that are ok. There's also been discussion about just how much POSIX strictly speaking requires this since it's about filesystem corruption. It's really more "protect user space from bad behavior" as pointed out by Jann. But since Eric Biederman looked up the POSIX wording, here it is for context: "From readdir: The readdir() function shall return a pointer to a structure representing the directory entry at the current position in the directory stream specified by the argument dirp, and position the directory stream at the next entry. It shall return a null pointer upon reaching the end of the directory stream. The structure dirent defined in the header describes a directory entry. From definitions: 3.129 Directory Entry (or Link) An object that associates a filename with a file. Several directory entries can associate names with the same file. ... 3.169 Filename A name consisting of 1 to {NAME_MAX} bytes used to name a file. The characters composing the name may be selected from the set of all character values excluding the slash character and the null byte. The filenames dot and dot-dot have special meaning. A filename is sometimes referred to as a 'pathname component'." Note that I didn't bother adding the checks to any legacy interfaces that nobody uses. Also note that if this ends up being noticeable as a performance regression, we can fix that to do a much more optimized model that checks for both NUL and '/' at the same time one word at a time. We haven't really tended to optimize 'memchr()', and it only checks for one pattern at a time anyway, and we really _should_ check for NUL too (but see the comment about "soft errors" in the code about why it currently only checks for '/') See the CONFIG_DCACHE_WORD_ACCESS case of hash_name() for how the name lookup code looks for pathname terminating characters in parallel. Link: https://lore.kernel.org/lkml/20190118161440.220134-2-jannh@google.com/ Cc: Alexander Viro Cc: Jann Horn Cc: Eric W. Biederman Signed-off-by: Linus Torvalds Signed-off-by: Sasha Levin --- fs/readdir.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/fs/readdir.c b/fs/readdir.c index 579c8ea894ae3..19bea591c3f1d 100644 --- a/fs/readdir.c +++ b/fs/readdir.c @@ -118,6 +118,40 @@ out: } EXPORT_SYMBOL(iterate_dir); +/* + * POSIX says that a dirent name cannot contain NULL or a '/'. + * + * It's not 100% clear what we should really do in this case. + * The filesystem is clearly corrupted, but returning a hard + * error means that you now don't see any of the other names + * either, so that isn't a perfect alternative. + * + * And if you return an error, what error do you use? Several + * filesystems seem to have decided on EUCLEAN being the error + * code for EFSCORRUPTED, and that may be the error to use. Or + * just EIO, which is perhaps more obvious to users. + * + * In order to see the other file names in the directory, the + * caller might want to make this a "soft" error: skip the + * entry, and return the error at the end instead. + * + * Note that this should likely do a "memchr(name, 0, len)" + * check too, since that would be filesystem corruption as + * well. However, that case can't actually confuse user space, + * which has to do a strlen() on the name anyway to find the + * filename length, and the above "soft error" worry means + * that it's probably better left alone until we have that + * issue clarified. + */ +static int verify_dirent_name(const char *name, int len) +{ + if (WARN_ON_ONCE(!len)) + return -EIO; + if (WARN_ON_ONCE(memchr(name, '/', len))) + return -EIO; + return 0; +} + /* * Traditional linux readdir() handling.. * @@ -227,6 +261,9 @@ static int filldir(struct dir_context *ctx, const char *name, int namlen, int reclen = ALIGN(offsetof(struct linux_dirent, d_name) + namlen + 2, sizeof(long)); + buf->error = verify_dirent_name(name, namlen); + if (unlikely(buf->error)) + return buf->error; buf->error = -EINVAL; /* only used if we fail.. */ if (reclen > buf->count) return -EINVAL; @@ -316,6 +353,9 @@ static int filldir64(struct dir_context *ctx, const char *name, int namlen, int reclen = ALIGN(offsetof(struct linux_dirent64, d_name) + namlen + 1, sizeof(u64)); + buf->error = verify_dirent_name(name, namlen); + if (unlikely(buf->error)) + return buf->error; buf->error = -EINVAL; /* only used if we fail.. */ if (reclen > buf->count) return -EINVAL; -- 2.20.1