From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 16E87C7EE2D for ; Sun, 21 May 2023 22:50:00 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229571AbjEUWt5 (ORCPT ); Sun, 21 May 2023 18:49:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47396 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231387AbjEUWt4 (ORCPT ); Sun, 21 May 2023 18:49:56 -0400 Received: from mail-pg1-x52c.google.com (mail-pg1-x52c.google.com [IPv6:2607:f8b0:4864:20::52c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BF3A9BA for ; Sun, 21 May 2023 15:49:54 -0700 (PDT) Received: by mail-pg1-x52c.google.com with SMTP id 41be03b00d2f7-5208be24dcbso3657459a12.1 for ; Sun, 21 May 2023 15:49:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fromorbit-com.20221208.gappssmtp.com; s=20221208; t=1684709394; x=1687301394; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=YfaiIW3biZ2nDYazMreEHIKSLRhntSglWJTWt2nlD0A=; b=IP5yZ0MMUZe32WseJQj3R2egNthpjmyvm+ciAk24LX8IEV7id8mpJCFeDWFj0Omm2f GfW41TZ5PxSnakzCNRnqNV5+26u2zf1il0w5TpmJjsm5guGJqcVuzI450S8ezK7xGm53 aJ+fCiUOOMUejIoPQNcQOFm0coGehr46kezzbLLHU6Q6EjhrP/LicNOwUetRXPpSn6i5 RPkrxmRKW1B0Yhp7ttk/tDuPOX7wOFhQJaeQTyXvqn9eQXqAZzuXWhDS+uGXNL3iUomt VQ6hKc0ZeYG9IesB6W0x2zNu/R+dTIeyCjtaPv6dKj8VfMkra6Sdh0kFoP2Mws2cdHgr mUCA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684709394; x=1687301394; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=YfaiIW3biZ2nDYazMreEHIKSLRhntSglWJTWt2nlD0A=; b=Myve/Cio4nwaCQDIqEP2f0o+fIDEwL1quaJJVnodu8WQWMOIc71rB6GXitRI204G+H 8YqLH8pEILi+l+C+24qYFc5LkHiLuhHXQE3XNKiuaZX5VCIqhNGOPPqnAmqgT0AyWuST bIg3VgroRbdr3vi3Qm2leqWglorkvKSOlH3HCVfrkriKKdC3qHdeoK0KlJ/HIy37KNZS e4FsBIN9yAkhysEsGVRi93QAMO5iynniYGYtI9kDWtdav+VisYGpDFg4bJM3RalsKqwq RIDtI724YAoTdGRvGpgQNdxHYV+29Gtxc5vJ1UiT9QCXAx5enptW1c7QFTLIGXdZ+QjQ lHPQ== X-Gm-Message-State: AC+VfDzLa3bEpHm8iEJfaF9iV1seYXFGQjz4oe/aIXVhCKbTqeh1ebD0 f7GmVpAggrJIfsJ6t9qftczmTA== X-Google-Smtp-Source: ACHHUZ72WQJhhCaeNMhFrQWZeqaQ3D4XEC9Issa3Qxfnr9+ou9y0ZTpJBeGEsgMJX5F+VThTgg1fcw== X-Received: by 2002:a17:902:c1c4:b0:1af:b681:5313 with SMTP id c4-20020a170902c1c400b001afb6815313mr820157plc.33.1684709394215; Sun, 21 May 2023 15:49:54 -0700 (PDT) Received: from dread.disaster.area (pa49-179-0-188.pa.nsw.optusnet.com.au. [49.179.0.188]) by smtp.gmail.com with ESMTPSA id ja15-20020a170902efcf00b001ac55a5e5eesm3425837plb.121.2023.05.21.15.49.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 21 May 2023 15:49:53 -0700 (PDT) Received: from dave by dread.disaster.area with local (Exim 4.96) (envelope-from ) id 1q0rs6-002J36-1t; Mon, 22 May 2023 08:49:50 +1000 Date: Mon, 22 May 2023 08:49:50 +1000 From: Dave Chinner To: Christian Brauner Cc: Mimi Zohar , Amir Goldstein , Jeff Layton , Stefan Berger , Paul Moore , linux-integrity@vger.kernel.org, miklos@szeredi.hu, linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-unionfs@vger.kernel.org, Ignaz Forster , Petr Vorel Subject: Re: [PATCH] overlayfs: Trigger file re-evaluation by IMA / EVM after writes Message-ID: References: <20230407-trasse-umgearbeitet-d580452b7a9b@brauner> <078d8c1fd6b6de59cde8aa85f8e59a056cb78614.camel@linux.ibm.com> <20230520-angenehm-orangen-80fdce6f9012@brauner> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230520-angenehm-orangen-80fdce6f9012@brauner> Precedence: bulk List-ID: X-Mailing-List: linux-unionfs@vger.kernel.org On Sat, May 20, 2023 at 11:17:35AM +0200, Christian Brauner wrote: > On Fri, May 19, 2023 at 03:42:38PM -0400, Mimi Zohar wrote: > > On Fri, 2023-04-07 at 10:31 +0200, Christian Brauner wrote: > > > So, I think we want both; we want the ovl_copyattr() and the > > > vfs_getattr_nosec() change: > > > > > > (1) overlayfs should copy up the inode version in ovl_copyattr(). That > > > is in line what we do with all other inode attributes. IOW, the > > > overlayfs inode's i_version counter should aim to mirror the > > > relevant layer's i_version counter. I wouldn't know why that > > > shouldn't be the case. Asking the other way around there doesn't > > > seem to be any use for overlayfs inodes to have an i_version that > > > isn't just mirroring the relevant layer's i_version. > > > (2) Jeff's changes for ima to make it rely on vfs_getattr_nosec(). > > > Currently, ima assumes that it will get the correct i_version from > > > an inode but that just doesn't hold for stacking filesystem. > > > > > > While (1) would likely just fix the immediate bug (2) is correct and > > > _robust_. If we change how attributes are handled vfs_*() helpers will > > > get updated and ima with it. Poking at raw inodes without using > > > appropriate helpers is much more likely to get ima into trouble. > > > > In addition to properly setting the i_version for IMA, EVM has a > > similar issue with i_generation and s_uuid. Adding them to > > ovl_copyattr() seems to resolve it. Does that make sense? > > > > diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c > > index 923d66d131c1..cd0aeb828868 100644 > > --- a/fs/overlayfs/util.c > > +++ b/fs/overlayfs/util.c > > @@ -1118,5 +1118,8 @@ void ovl_copyattr(struct inode *inode) > > inode->i_atime = realinode->i_atime; > > inode->i_mtime = realinode->i_mtime; > > inode->i_ctime = realinode->i_ctime; > > + inode->i_generation = realinode->i_generation; > > + if (inode->i_sb) > > + uuid_copy(&inode->i_sb->s_uuid, &realinode->i_sb- > > Overlayfs can consist of multiple lower layers and each of those lower > layers may have a different uuid. So everytime you trigger a > ovl_copyattr() on a different layer this patch would alter the uuid of > the overlayfs superblock. > > In addition the uuid should be set when the filesystem is mounted. > Unless the filesystem implements a dedicated ioctl() - like ext4 - to > change the uuid. IMO, that ext4 functionality is a landmine waiting to be stepped on. We should not be changing the sb->s_uuid of filesysetms dynamically. The VFS does not guarantee in any way that it is safe to change the sb->s_uuid (i.e. no locking, no change notifications, no udev events, etc). Various subsystems - both in the kernel and in userspace - use the sb->s_uuid as a canonical and/or persistent filesystem/device identifier and are unprepared to have it change while the filesystem is mounted and active. I commented on this from an XFS perspective here when it was proposed to copy this ext4 mis-feature in XFS: https://lore.kernel.org/linux-xfs/20230314062847.GQ360264@dread.disaster.area/ Further to this, I also suspect that changing uuids online will cause issues with userspace caching of fs uuids (e.g. libblkid and anything that uses it) and information that uses uuids to identify the filesystem that are set up at mount time (/dev/disk/by-uuid/ links, etc) by kernel events sent to userspace helpers... IMO, we shouldn't even be considering dynamic sb->s_uuid changes without first working through the full system impacts of having persistent userspace-visible filesystem identifiers change dynamically... -Dave. -- Dave Chinner david@fromorbit.com