From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-oi1-f179.google.com (mail-oi1-f179.google.com [209.85.167.179]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5B7C8374161 for ; Mon, 12 Jan 2026 17:14:08 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.167.179 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768238051; cv=none; b=dUiQwY78oEZQlA54IGDqrsLNkmtU7i4NjS3S3NVs2tPEyEeJz5QmjgfvdKvmp5N1AgBv6S4PQRM7GB45RMCDqY1hdBZhU6pmbYVsfmm0uAruYK7/VDJoGUuzO1IQSy5Sb7gg/EPq/oV4xFuQlt3FZPhyI/Ttz8yJPF1r9ZxlkdA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768238051; c=relaxed/simple; bh=+6MD9U3/oNkwpZKjfey1D4wf+JNsO6goqtte5CPMt9I=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=RjWXKI9HXkp7fIbN9ds0oOO10JkrCt+Gz0X1V+wNzcz92BfJvyOvNSrHIOt4hA/UMh9bZ4EnJT6Xq5LVFvJAzWGInEw3v3H54Up0yZhTnCvj8xtxs5+I5o9/Fu3rd5foEQgQxaehWciDDuzp8pU2VkY0OLKMBj6RidM2vC2GyhM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=cloudflare.com; spf=pass smtp.mailfrom=cloudflare.com; dkim=pass (2048-bit key) header.d=cloudflare.com header.i=@cloudflare.com header.b=CB0HmYL1; arc=none smtp.client-ip=209.85.167.179 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=cloudflare.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=cloudflare.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=cloudflare.com header.i=@cloudflare.com header.b="CB0HmYL1" Received: by mail-oi1-f179.google.com with SMTP id 5614622812f47-45c60ecc0f8so102145b6e.3 for ; Mon, 12 Jan 2026 09:14:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cloudflare.com; s=google09082023; t=1768238048; x=1768842848; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=QyJinldTenjlTP9+ze0OVo2ZOUQ8sydojxDpmIKfmVc=; b=CB0HmYL1CSRbRXROWgXTdMCz+yLGPTbVAORnmBqVcQjpMt9W5SstZjeXQ1cLzrvxxg YBqGcDMS/sq3kToXEW6yeMtwS6E8ue9m5ljrfEJhB6Gf+ODcJT5x88Vk57B7uhLXlkwm DrRjxbWj/ZtX8YQ5kCIC7uqTDeCnRFf+cujsbrKjJ7/C9FTZmvkMK9vXKkoUW6GpHLH9 27CsPRAaDprTV2XpspGXm6B4L7gUaLR5/QCzthqReqIEgbPc63qNLosAN2hNitKrqvOh LRc5prekQmaohXmeGlQbNIH4hF9tVk3Lq5P9tyPFdmhXz55gc8mxxrLgEOan1mFznwnl Mqbw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1768238048; x=1768842848; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=QyJinldTenjlTP9+ze0OVo2ZOUQ8sydojxDpmIKfmVc=; b=YEO19v7bc1xWpFx/1UoneiNtw1XTdPOf0x+a8atCmbgyfmZPYruE/goIhpol8OBSu3 pXQg65l5hxMSPWnt7tOQuB09wf7BbhWpxr2iwWPNvxStlQM+5sHtTX6Dw3FM5oBU/mb6 XKgxP6NYiEgQ8Ge7LQOEcb7VC36nDu/2hecxtTCfBa+c8ujt7s29GcFiwVvrl7mYC96t itPek1nXjyCjj764wKB9UXOlLrOjAwYXFE8W19nQCVBzZZRnZW5w+nZJr2zmvbNR4LkX +oUqaqSiN9+PWMRMZL1Ho58zEFMuwuzX35gsom+14LHwGO1iyKPM0l7ucnM92V2cmgXS WoMQ== X-Forwarded-Encrypted: i=1; AJvYcCW+h5Lghbzds93Yq8dtuO7WG6P7I7+MAw0iLta3MjDo2Gkie11ixQJiR/Pl0QrkahhcO26sHuVo8x2vhOs=@vger.kernel.org X-Gm-Message-State: AOJu0YxDFrbviYHb0WabPYKKi2tSbt9pDd+bKDsVUyZmUIBDpSlASOgK 8+0Mbm36qEEHY3Cysgbfb4oQJHfSY9uodHva2+rgL/CUYkq3UMqBWPdXNooWPFBfBPs= X-Gm-Gg: AY/fxX6Q1Tek2O2mMfK+u9n3bv4uoTTmC7tOf518Mf+4siarSdikvXBYltmQyD/vH+8 N71/Ibn0Vt5a9tYvDbcuw60EhAKw28JQ2oeUgUKkaj/Ncn5Vm02BQoVydjV7Ss/sUQtjwK7pVuF DZAYiAABRi+h4R00nieL30KEQTmyfnv2QZiU8Xz0F7AeL7LAdOsRNuh3gFjiRADZMhNmhDLsexP jiImvwZ2Yy/jG6+FlnTwoSottc6ONcG/gOJT32+UprdN0/3+k6hR+4ApEBA4wsfkVpj5Hsn40Vj u7wxe+EPsXKPs/6Yj1VI+fKot7VmebHtF4FzKYXYSRp5NVeVpfduTob4SdBk/THroPkbgjTIpdG GwqR+YW7G34m0+VxUZ4TfldBahpNjYXMR3jlQFqBYRsQFkqR3g7aPukKN0EvHiEXe0DI= X-Google-Smtp-Source: AGHT+IG/ePwN35HG0w/1p2sUU3s0fuKkedonKLVhqDCc4g/8Nl3LR2SCuPk6C+diJOjLOmIc+3i9aQ== X-Received: by 2002:a05:6808:148a:b0:44f:e912:ea97 with SMTP id 5614622812f47-45a6bdb94b7mr9876320b6e.18.1768238047536; Mon, 12 Jan 2026 09:14:07 -0800 (PST) Received: from CMGLRV3 ([2a09:bac5:947d:1b37::2b6:f]) by smtp.gmail.com with ESMTPSA id 5614622812f47-45a5e2e903dsm8171148b6e.20.2026.01.12.09.14.05 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 12 Jan 2026 09:14:07 -0800 (PST) Date: Mon, 12 Jan 2026 11:14:04 -0600 From: Frederick Lawler To: Mimi Zohar Cc: Jeff Layton , Roberto Sassu , Dmitry Kasatkin , Eric Snowberg , Paul Moore , James Morris , "Serge E. Hallyn" , "Darrick J. Wong" , Christian Brauner , Josef Bacik , linux-kernel@vger.kernel.org, linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org, kernel-team@cloudflare.com Subject: Re: [PATCH RFC] ima: Fallback to a ctime guard without i_version updates Message-ID: References: <20251229-xfs-ima-fixup-v1-1-6a717c939f7c@cloudflare.com> <3ad9ded9b3a269908eee6c79b70dbf432e60ce8d.camel@kernel.org> <25b6d1b42ea07b058be4e4f48bb5a7c6b879b3ed.camel@kernel.org> <15eaa3613b0552cc48b55972b81882ac1e1d1150.camel@linux.ibm.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <15eaa3613b0552cc48b55972b81882ac1e1d1150.camel@linux.ibm.com> On Mon, Jan 12, 2026 at 09:02:02AM -0500, Mimi Zohar wrote: > On Tue, 2026-01-06 at 14:50 -0500, Jeff Layton wrote: > > > > > > @@ -54,11 +62,22 @@ integrity_inode_attrs_store(struct integrity_inode_attributes *attrs, > > > > > >    */ > > > > > >   static inline bool > > > > > >   integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs, > > > > > > -       const struct inode *inode) > > > > > > +       struct file *file, struct inode *inode) > > > > > >   { > > > > > > - return (inode->i_sb->s_dev != attrs->dev || > > > > > > - inode->i_ino != attrs->ino || > > > > > > - !inode_eq_iversion(inode, attrs->version)); > > > > > > + struct kstat stat; > > > > > > + > > > > > > + if (inode->i_sb->s_dev != attrs->dev || > > > > > > +     inode->i_ino != attrs->ino) > > > > > > + return true; > > > > > > + > > > > > > + if (inode_eq_iversion(inode, attrs->version)) > > > > > > + return false; > > > > > > + > > > > > > + if (!file || vfs_getattr_nosec(&file->f_path, &stat, STATX_CTIME, > > > > > > +        AT_STATX_SYNC_AS_STAT)) > > > > > > + return true; > > > > > > + > > > > > > > > > > This is rather odd. You're sampling the i_version field directly, but > > > > > if it's not equal then you go through ->getattr() to get the ctime. > > > > > > > > > > It's particularly odd since you don't know whether the i_version field > > > > > is even implemented on the fs. On filesystems where it isn't, the > > > > > i_version field generally stays at 0, so won't this never fall through > > > > > to do the vfs_getattr_nosec() call on those filesystems? > > > > > > > > > > > > > You're totally right. I didn't consider FS's caching the value at zero. > > > > > > Actually, I'm going to amend this. I think I did consider FSs without an > > > implementation. Where this is called at, it is often guarded by a > > > !IS_I_VERSION() || integrity_inode_attrs_change(). If I'm > > > understanding this correctly, the check call doesn't occur unless the inode > > > has i_version support. > > > > > > > > > It depends on what you mean by i_version support: > > > > That flag just tells the VFS that it needs to bump the i_version field > > when updating timestamps. It's not a reliable indicator of whether the > > i_version field is suitable for the purpose you want here. > > > > The problem here and the one that we ultimately fixed with multigrain > > timestamps is that XFS in particular will bump i_version on any change > > to the log. That includes atime updates due to reads. > > > > XFS still tracks the i_version the way it always has, but we've stopped > > getattr() from reporting it because it's not suitable for the purpose > > that nfsd (and IMA) need it for. > > > > > It seems to me the suggestion then is to remove the IS_I_VERSION() > > > checks guarding the call sites, grab both ctime and cookie from stat, > > > and if IS_I_VERSION() use that, otherwise cookie, and compare > > > against the cached i_version with one of those values, and then fall > > > back to ctime? > > > > > > > Not exactly. > > > > You want to call getattr() for STATX_CHANGE_COOKIE|STATX_CTIME, and > > then check the kstat->result_mask. If STATX_CHANGE_COOKIE is set, then > > use that. If it's not then use the ctime. > > > > The part I'm not sure about is whether it's actually safe to do this. > > vfs_getattr_nosec() can block in some situations. Is it ok to do this > > in any context where integrity_inode_attrs_changed() may be called?  > > Frederick, before making any changes, please describe the problem you're > actually seeing. From my limited testing, file change IS being detected. A major > change like Jeff is suggesting is not something that would or should be back > ported. Remember, Jeff's interest is remote filesystems, not necessarily with > your particular XFS concern. > > So again, what is the problem you're trying to address? It's easier if I paste a simpler version of test I've been promising for v1 to help show this (below). In 6.12 the test snippet passes, for >= 6.13 we get an audit evaluation on the each execution when there should only be 1. The struct integrity_inode_attributes.version stays at zero for XFS in the below test, as well as file systems that calls into generic_fillattr() or otherwise that doesn't set the change cookie request mask. When file systems have a mutated file, the cookie is then updated, but the compare against inode.i_version could be out of date depending on file system implementation. Thus we see since 6.13, XFS an atime change will cause another evaluation due to stale cache. I'm not expecting a backport to 6.13 as there has been a lot of changes in IMA/EVM, but I think to the 6.18 LTS is reasonable. With leaving EVM alone, it's a small diff. I have a updated patch that hopefully addresses all your concerns from other responses in this thread. I want to point out that the updated code is more EVM/IMA invariant mindful than this RFC. I'd like to submit that, and then move discussion over there if possible. Hopefully this helps, Fred _fragment.config_ CONFIG_XFS_FS=y CONFIG_OVERLAY_FS=y CONFIG_IMA=y CONFIG_IMA_WRITE_POLICY=y CONFIG_IMA_READ_POLICY=y _./test.sh_ #!/bin/bash -e IMA_POLICY="/sys/kernel/security/ima/policy" TEST_BIN="/bin/date" MNT_BASE="/tmp/ima_test_root" mkdir -p "$MNT_BASE" mount -t tmpfs tmpfs "$MNT_BASE" mkdir -p "$MNT_BASE"/{xfs_disk,upper,work,ovl} dd if=/dev/zero of="$MNT_BASE/xfs.img" bs=1M count=300 mkfs.xfs -q "$MNT_BASE/xfs.img" mount "$MNT_BASE/xfs.img" "$MNT_BASE/xfs_disk" cp "$TEST_BIN" "$MNT_BASE/xfs_disk/test_prog" mount -t overlay overlay -o \ "lowerdir=$MNT_BASE/xfs_disk,upperdir=$MNT_BASE/upper,workdir=$MNT_BASE/work" \ "$MNT_BASE/ovl" echo "audit func=BPRM_CHECK uid=$(id -u nobody)" > "$IMA_POLICY" target_prog="$MNT_BASE/ovl/test_prog" setpriv --reuid nobody "$target_prog" setpriv --reuid nobody "$target_prog" setpriv --reuid nobody "$target_prog" audit_count=$(dmesg | grep -c "file=\"$target_prog\"") if [[ "$audit_count" -eq 1 ]]; then echo "PASS: Found exactly 1 audit event." else echo "FAIL: Expected 1 audit event, but found $audit_count." exit 1 fi > > Mimi > > > > > ISTR that this was an issue at one point, but maybe isn't now that IMA > > is an LSM? >