Message ID | 20230116191425.458864-1-jannh@google.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp1363497wrn; Mon, 16 Jan 2023 11:29:33 -0800 (PST) X-Google-Smtp-Source: AMrXdXsbkKPgycEYSjyQtlS81Ig9LJGDJq3yPkbfPhMHbx2EUv6XslrYWm3ITvjjeZTlnB0gkA95 X-Received: by 2002:a17:906:9495:b0:7c1:6fc:6048 with SMTP id t21-20020a170906949500b007c106fc6048mr139726ejx.24.1673897373310; Mon, 16 Jan 2023 11:29:33 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1673897373; cv=none; d=google.com; s=arc-20160816; b=sUQB/RZAydwcu6Do5VKxDTgw7eWIZVjEUEYDbaZAozlsK9vDhEwGV7OytU3HRCkv5s 7TuL19jIDtwbHYZu61ilvuPF64hpEkFzKJ7xQLe+ANfhzgB+L3KHqm5BYakOmxPsQ/Xt q9/cwTxu9Y9EKQHPCGUm2bahtXm/eYP904AznBzO3zMJ0NZH6g4XWL7yEyg3NSLkmUmx J9QsuS2auW9cReUbdY/N5Df5YyupG6bfEyQlOFkISVTFTOvS2OXtZkIhd/mXqrCUGlIg BbB8ZtMfPfSLrap6oTUEs2OvQ3w0Vh/L68uVsc5bpGJXcN3YwzoRkfPsFKIkqrUtnvk1 loHw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=3I9TKz0QCRwStrORKwRYbo4zYDMGw/8qyv/2PPzcTIw=; b=cnT1NyTg+EWeTQ24AQI8M7nrakEvaasmA4jsd1c8UeYtGL6udLwoMf2xNbqCUWuT3L Vuh4vXI2fy8Rmcnxq2mhbuj+2biaqfCF3exs1n3KGgX2tIq+MwkDaRRVH8Fe03e1k3uk dKpaSShBZ9ke7dDlt9QQfAfKGLyIBHCndYbB32xcvwSshAwIvsAxYL1YnEs2lHmjtgPE 3+xsSZXs8s88z/CGa4vP/9hcOCE8d8Sg8Yf82f+S2Szmc6744XrLq2gjmpP5zg9Ugh5/ RUyb7VAJ4DzAHhA/Jz9WgD4JRvWBjERvvgSBNB4Xteg7fHxjGLaxVMvVMxbuEzA/lUZw Qvjw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20210112 header.b=JhpTDkNc; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id hq15-20020a1709073f0f00b0086fd54a8f18si5396966ejc.512.2023.01.16.11.29.09; Mon, 16 Jan 2023 11:29:33 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20210112 header.b=JhpTDkNc; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229934AbjAPTOp (ORCPT <rfc822;stefanalexe802@gmail.com> + 99 others); Mon, 16 Jan 2023 14:14:45 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38190 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232346AbjAPTOl (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 16 Jan 2023 14:14:41 -0500 Received: from mail-wr1-x42f.google.com (mail-wr1-x42f.google.com [IPv6:2a00:1450:4864:20::42f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DEA581CAD9 for <linux-kernel@vger.kernel.org>; Mon, 16 Jan 2023 11:14:39 -0800 (PST) Received: by mail-wr1-x42f.google.com with SMTP id t5so24024599wrq.1 for <linux-kernel@vger.kernel.org>; Mon, 16 Jan 2023 11:14:39 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=3I9TKz0QCRwStrORKwRYbo4zYDMGw/8qyv/2PPzcTIw=; b=JhpTDkNcmd/K/PpIamox+UvMbgveXuheag7i9hm8ma5NnX7ZfIOvhDb1HEwnrhqXgV DMPZY4t81B1oRlC1xwXXjkqUDt3e/oxumkm8f58ziGzt6ueDgTdxVpSbxmWLa5Y/LqIP b3h5/Wepronyk03vMqkjLgLzU2Nh6Zcq+b8Ard3BfdYwkBmsxc9039CJba7v7FTcQ6ef 3swlGUN78iSzOIxCdqSboYpAB4+ZEFVbKXJU/A2J4I1H+gpE/GmWLAd7i2MSNVcGX1Jj Dp+HqtxFWRKw2OFt6pSgbCTWBBfOfZmdFLGKnK7jqQS3hYgr3ggvMSbka3FiLg4x9//h b8zg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=3I9TKz0QCRwStrORKwRYbo4zYDMGw/8qyv/2PPzcTIw=; b=SjVQWGMSMofmWazFThfklGUgbFuroZ68WsYbD54wsBy4p/gCy5nWCUcS7d2EXeG6cH xkqauoSGBv+BvTgoCRPtLwwDUDcwWl+cH+v45Uzu9ZMhHecS9T8Z9yQ+VcYhq/6aJZcC fOalkOhGi1TpmcZh9ZM1weGSt8qH3oGXDqkKLnwXJWD3/+KUZdQNSt8ZF74uh3Idcff6 c7fEHZ4H5CS3qDc3EbhzpXkle67jEUkHqDfawh7FmVa4hKSvtu5mW2CA/rEW/phT9aqi ohoM2dz0jlazamBaYzw+JlMOyw8QsxlieOqIl0mG+BWuFt90y1D87NqpbmpR1EbDZxvu 8E/w== X-Gm-Message-State: AFqh2koomlf49cdGiJW40EX5uZCHI7/xP5bSavGHExgff0oXZ7eq7Gsn HGhRgqVUl4cawFgPnT289iViFKq3jwCQnbND X-Received: by 2002:a05:6000:1d87:b0:2a1:602d:ff3 with SMTP id bk7-20020a0560001d8700b002a1602d0ff3mr1103593wrb.3.1673896478372; Mon, 16 Jan 2023 11:14:38 -0800 (PST) Received: from localhost ([2a00:79e0:9d:4:e2d9:b90f:9307:af59]) by smtp.gmail.com with ESMTPSA id u13-20020a05600c19cd00b003c6f1732f65sm42619579wmq.38.2023.01.16.11.14.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 16 Jan 2023 11:14:37 -0800 (PST) From: Jann Horn <jannh@google.com> To: Alexander Viro <viro@zeniv.linux.org.uk>, linux-fsdevel@vger.kernel.org Cc: linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org, kernel-hardening@lists.openwall.com Subject: [PATCH] fs: Use CHECK_DATA_CORRUPTION() when kernel bugs are detected Date: Mon, 16 Jan 2023 20:14:25 +0100 Message-Id: <20230116191425.458864-1-jannh@google.com> X-Mailer: git-send-email 2.39.0.314.g84b9a713c41-goog MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-17.6 required=5.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS, USER_IN_DEF_DKIM_WL,USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1755208612163494682?= X-GMAIL-MSGID: =?utf-8?q?1755208612163494682?= |
Series |
fs: Use CHECK_DATA_CORRUPTION() when kernel bugs are detected
|
|
Commit Message
Jann Horn
Jan. 16, 2023, 7:14 p.m. UTC
Currently, filp_close() and generic_shutdown_super() use printk() to log
messages when bugs are detected. This is problematic because infrastructure
like syzkaller has no idea that this message indicates a bug.
In addition, some people explicitly want their kernels to BUG() when kernel
data corruption has been detected (CONFIG_BUG_ON_DATA_CORRUPTION).
And finally, when generic_shutdown_super() detects remaining inodes on a
system without CONFIG_BUG_ON_DATA_CORRUPTION, it would be nice if later
accesses to a busy inode would at least crash somewhat cleanly rather than
walking through freed memory.
To address all three, use CHECK_DATA_CORRUPTION() when kernel bugs are
detected.
Signed-off-by: Jann Horn <jannh@google.com>
---
fs/open.c | 5 +++--
fs/super.c | 21 +++++++++++++++++----
include/linux/poison.h | 3 +++
3 files changed, 23 insertions(+), 6 deletions(-)
base-commit: 5dc4c995db9eb45f6373a956eb1f69460e69e6d4
Comments
On Mon, Jan 16, 2023 at 08:14:25PM +0100, Jann Horn wrote: > Currently, filp_close() and generic_shutdown_super() use printk() to log > messages when bugs are detected. This is problematic because infrastructure > like syzkaller has no idea that this message indicates a bug. > In addition, some people explicitly want their kernels to BUG() when kernel > data corruption has been detected (CONFIG_BUG_ON_DATA_CORRUPTION). > And finally, when generic_shutdown_super() detects remaining inodes on a > system without CONFIG_BUG_ON_DATA_CORRUPTION, it would be nice if later > accesses to a busy inode would at least crash somewhat cleanly rather than > walking through freed memory. > > To address all three, use CHECK_DATA_CORRUPTION() when kernel bugs are > detected. > > Signed-off-by: Jann Horn <jannh@google.com> > --- Looks good, Reviewed-by: Christian Brauner (Microsoft) <brauner@kernel.org>
On Mon, Jan 16, 2023 at 08:14:25PM +0100, Jann Horn wrote: > Currently, filp_close() and generic_shutdown_super() use printk() to log > messages when bugs are detected. This is problematic because infrastructure > like syzkaller has no idea that this message indicates a bug. > In addition, some people explicitly want their kernels to BUG() when kernel > data corruption has been detected (CONFIG_BUG_ON_DATA_CORRUPTION). > And finally, when generic_shutdown_super() detects remaining inodes on a > system without CONFIG_BUG_ON_DATA_CORRUPTION, it would be nice if later > accesses to a busy inode would at least crash somewhat cleanly rather than > walking through freed memory. > > To address all three, use CHECK_DATA_CORRUPTION() when kernel bugs are > detected. Seems reasonable to me. I'll carry this unless someone else speaks up. :) Reviewed-by: Kees Cook <keescook@chromium.org> -Kees > > Signed-off-by: Jann Horn <jannh@google.com> > --- > fs/open.c | 5 +++-- > fs/super.c | 21 +++++++++++++++++---- > include/linux/poison.h | 3 +++ > 3 files changed, 23 insertions(+), 6 deletions(-) > > diff --git a/fs/open.c b/fs/open.c > index 82c1a28b3308..ceb88ac0ca3b 100644 > --- a/fs/open.c > +++ b/fs/open.c > @@ -1411,8 +1411,9 @@ int filp_close(struct file *filp, fl_owner_t id) > { > int retval = 0; > > - if (!file_count(filp)) { > - printk(KERN_ERR "VFS: Close: file count is 0\n"); > + if (CHECK_DATA_CORRUPTION(file_count(filp) == 0, > + "VFS: Close: file count is 0 (f_op=%ps)", > + filp->f_op)) { > return 0; > } > > diff --git a/fs/super.c b/fs/super.c > index 12c08cb20405..cf737ec2bd05 100644 > --- a/fs/super.c > +++ b/fs/super.c > @@ -491,10 +491,23 @@ void generic_shutdown_super(struct super_block *sb) > if (sop->put_super) > sop->put_super(sb); > > - if (!list_empty(&sb->s_inodes)) { > - printk("VFS: Busy inodes after unmount of %s. " > - "Self-destruct in 5 seconds. Have a nice day...\n", > - sb->s_id); > + if (CHECK_DATA_CORRUPTION(!list_empty(&sb->s_inodes), > + "VFS: Busy inodes after unmount of %s (%s)", > + sb->s_id, sb->s_type->name)) { > + /* > + * Adding a proper bailout path here would be hard, but > + * we can at least make it more likely that a later > + * iput_final() or such crashes cleanly. > + */ > + struct inode *inode; > + > + spin_lock(&sb->s_inode_list_lock); > + list_for_each_entry(inode, &sb->s_inodes, i_sb_list) { > + inode->i_op = VFS_PTR_POISON; > + inode->i_sb = VFS_PTR_POISON; > + inode->i_mapping = VFS_PTR_POISON; > + } > + spin_unlock(&sb->s_inode_list_lock); > } > } > spin_lock(&sb_lock); > diff --git a/include/linux/poison.h b/include/linux/poison.h > index 2d3249eb0e62..0e8a1f2ceb2f 100644 > --- a/include/linux/poison.h > +++ b/include/linux/poison.h > @@ -84,4 +84,7 @@ > /********** kernel/bpf/ **********/ > #define BPF_PTR_POISON ((void *)(0xeB9FUL + POISON_POINTER_DELTA)) > > +/********** VFS **********/ > +#define VFS_PTR_POISON ((void *)(0xF5 + POISON_POINTER_DELTA)) > + > #endif > > base-commit: 5dc4c995db9eb45f6373a956eb1f69460e69e6d4 > -- > 2.39.0.314.g84b9a713c41-goog >
On Thu, Jan 26, 2023 at 08:35:49AM -0800, Kees Cook wrote: > On Mon, Jan 16, 2023 at 08:14:25PM +0100, Jann Horn wrote: > > Currently, filp_close() and generic_shutdown_super() use printk() to log > > messages when bugs are detected. This is problematic because infrastructure > > like syzkaller has no idea that this message indicates a bug. > > In addition, some people explicitly want their kernels to BUG() when kernel > > data corruption has been detected (CONFIG_BUG_ON_DATA_CORRUPTION). > > And finally, when generic_shutdown_super() detects remaining inodes on a > > system without CONFIG_BUG_ON_DATA_CORRUPTION, it would be nice if later > > accesses to a busy inode would at least crash somewhat cleanly rather than > > walking through freed memory. > > > > To address all three, use CHECK_DATA_CORRUPTION() when kernel bugs are > > detected. > > Seems reasonable to me. I'll carry this unless someone else speaks up. I've already picked this into a branch with other fs changes for coming cycle. Al, please tell me in case you end up picking this up and I'll drop it ofc. Christian
On Fri, Jan 27, 2023 at 11:58:15AM +0100, Christian Brauner wrote: > On Thu, Jan 26, 2023 at 08:35:49AM -0800, Kees Cook wrote: > > On Mon, Jan 16, 2023 at 08:14:25PM +0100, Jann Horn wrote: > > > Currently, filp_close() and generic_shutdown_super() use printk() to log > > > messages when bugs are detected. This is problematic because infrastructure > > > like syzkaller has no idea that this message indicates a bug. > > > In addition, some people explicitly want their kernels to BUG() when kernel > > > data corruption has been detected (CONFIG_BUG_ON_DATA_CORRUPTION). > > > And finally, when generic_shutdown_super() detects remaining inodes on a > > > system without CONFIG_BUG_ON_DATA_CORRUPTION, it would be nice if later > > > accesses to a busy inode would at least crash somewhat cleanly rather than > > > walking through freed memory. > > > > > > To address all three, use CHECK_DATA_CORRUPTION() when kernel bugs are > > > detected. > > > > Seems reasonable to me. I'll carry this unless someone else speaks up. > > I've already picked this into a branch with other fs changes for coming cycle. Okay, great! I'll drop it from my tree. Reviewed-by: Kees Cook <keescook@chromium.org>
diff --git a/fs/open.c b/fs/open.c index 82c1a28b3308..ceb88ac0ca3b 100644 --- a/fs/open.c +++ b/fs/open.c @@ -1411,8 +1411,9 @@ int filp_close(struct file *filp, fl_owner_t id) { int retval = 0; - if (!file_count(filp)) { - printk(KERN_ERR "VFS: Close: file count is 0\n"); + if (CHECK_DATA_CORRUPTION(file_count(filp) == 0, + "VFS: Close: file count is 0 (f_op=%ps)", + filp->f_op)) { return 0; } diff --git a/fs/super.c b/fs/super.c index 12c08cb20405..cf737ec2bd05 100644 --- a/fs/super.c +++ b/fs/super.c @@ -491,10 +491,23 @@ void generic_shutdown_super(struct super_block *sb) if (sop->put_super) sop->put_super(sb); - if (!list_empty(&sb->s_inodes)) { - printk("VFS: Busy inodes after unmount of %s. " - "Self-destruct in 5 seconds. Have a nice day...\n", - sb->s_id); + if (CHECK_DATA_CORRUPTION(!list_empty(&sb->s_inodes), + "VFS: Busy inodes after unmount of %s (%s)", + sb->s_id, sb->s_type->name)) { + /* + * Adding a proper bailout path here would be hard, but + * we can at least make it more likely that a later + * iput_final() or such crashes cleanly. + */ + struct inode *inode; + + spin_lock(&sb->s_inode_list_lock); + list_for_each_entry(inode, &sb->s_inodes, i_sb_list) { + inode->i_op = VFS_PTR_POISON; + inode->i_sb = VFS_PTR_POISON; + inode->i_mapping = VFS_PTR_POISON; + } + spin_unlock(&sb->s_inode_list_lock); } } spin_lock(&sb_lock); diff --git a/include/linux/poison.h b/include/linux/poison.h index 2d3249eb0e62..0e8a1f2ceb2f 100644 --- a/include/linux/poison.h +++ b/include/linux/poison.h @@ -84,4 +84,7 @@ /********** kernel/bpf/ **********/ #define BPF_PTR_POISON ((void *)(0xeB9FUL + POISON_POINTER_DELTA)) +/********** VFS **********/ +#define VFS_PTR_POISON ((void *)(0xF5 + POISON_POINTER_DELTA)) + #endif