Received-SPF: pass (google.com: domain of linux-kernel+bounces-50098-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <4ce0e20d-ed14-490d-9446-a6cfbd532bca@linux.ibm.com>
Date: Fri, 2 Feb 2024 11:06:15 -0500
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 4/5] evm: Use the real inode's metadata to calculate
 metadata hash
Content-Language: en-US
To: Amir Goldstein <amir73il@gmail.com>
Cc: linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org,
        linux-unionfs@vger.kernel.org, linux-kernel@vger.kernel.org,
        paul@paul-moore.com, jmorris@namei.org, serge@hallyn.com,
        zohar@linux.ibm.com, roberto.sassu@huawei.com, miklos@szeredi.hu
References: <20240130214620.3155380-1-stefanb@linux.ibm.com>
 <20240130214620.3155380-5-stefanb@linux.ibm.com>
 <38230b4c-54ae-45ed-a6fb-34e63501e5b1@linux.ibm.com>
 <CAOQ4uxiYARZBSgzb4_W-RKvB1XLSF3GUBqeLw2kH+eVeZ_8ARQ@mail.gmail.com>
 <c018b014-9ba8-4395-86dc-b61346ab20a8@linux.ibm.com>
 <CAOQ4uxi6Te8izWpXROthknRaXrVA9jho5nbc+mkuQDrcTLY44Q@mail.gmail.com>
 <CAOQ4uxigdNeE+2nfr4VxS9piQf5hez=ryT0a-jzW+tW0BT-zuw@mail.gmail.com>
 <492ea12a-d79d-47da-9bbe-a7f33051bd3f@linux.ibm.com>
 <CAOQ4uxgiO1RbsmqOu4F4Foy-MBPecnEXO7BvgDGz-Lzb1Eysog@mail.gmail.com>
 <4c584bfb-d282-4584-bb20-18c26b1033c0@linux.ibm.com>
 <CAOQ4uxjftr7GGx6tuW_yB_MTaVB57m6p_d=UHhN3Z23YVXY0QQ@mail.gmail.com>
 <11abffea-15c5-4d13-9d0f-edbc54b09bf3@linux.ibm.com>
 <CAOQ4uxjZ6p9+H54G0LNTUnU56WRaoLtWOUj2nOaKJ4JvBGqLVg@mail.gmail.com>
 <427ce381-73fa-48f9-8e18-77e23813b918@linux.ibm.com>
 <CAOQ4uxggqa7j0NS1MN3KSvF_qG1FMVmFxacEYSTx+LuvuosJ5g@mail.gmail.com>
From: Stefan Berger <stefanb@linux.ibm.com>
In-Reply-To: <CAOQ4uxggqa7j0NS1MN3KSvF_qG1FMVmFxacEYSTx+LuvuosJ5g@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0


On 2/2/24 10:51, Amir Goldstein wrote:
> On Fri, Feb 2, 2024 at 4:59 PM Stefan Berger <stefanb@linux.ibm.com> wrote:
>>
>>
>>
>> On 2/2/24 04:24, Amir Goldstein wrote:
>>> On Thu, Feb 1, 2024 at 10:35 PM Stefan Berger <stefanb@linux.ibm.com> wrote:
>>
>>>
>>>>
>>>> and your suggested change to this patch :
>>>>
>>>> -       struct inode *inode = d_real_inode(dentry);
>>>> +       struct inode *inode = d_inode(d_real(dentry, false));;
>>>>
>>>
>>> In the new version I change the API to use an enum instead of bool, e.g.:
>>>
>>>          struct inode *inode = d_inode(d_real(dentry, D_REAL_METADATA));
>>
>> Thanks. I will use it.
>>
>>>
>>> This catches in build time and in run time, callers that were not converted
>>> to the new API.
>>>
>>>> The test cases are now passing with and without metacopy enabled. Yay!
>>>
>>> Too soon to be happy.
>>> I guess you are missing a test for the following case:
>>> 1. file was meta copied up (change is detected)
>>> 2. the lower file that contains the data is being changed (change is
>>> not detected)
>>
>> Right. Though it seems there's something wrong with overlayfs as well
>> after appending a byte to the file on the lower.
>>
>> -rwxr-xr-x    1 0        0               25 Feb  2 14:55
>> /ext4.mount/lower/test_rsa_portable2
>> -rwxr-xr-x    1 0        0               24 Feb  2 14:55
>> /ext4.mount/overlay/test_rsa_portable2
>> bb16aa5350bcc8863da1a873c846fec9281842d9
>> /ext4.mount/lower/test_rsa_portable2
>> bb16aa5350bcc8863da1a873c846fec9281842d9
>> /ext4.mount/overlay/test_rsa_portable2
>>
>> We have a hash collision on a file with 24 bytes and the underlying one
>> with 25 byte. (-;  :-)
> 
> https://docs.kernel.org/filesystems/overlayfs.html#changes-to-underlying-filesystems
> 
> If you modify the lower file underneath overlayfs, you get no
> guarantee from overlayfs about expected results.
> 
> This makes your work more challenging.
The odd thing is my updated test case '2' seems to indicate that 
everything already works as expected with CONFIG_OVERLAY_FS_METACOPY=y. 
After causing copy-up of metadata changes to the file content on the 
lower layer still cause permission error to file execution on the 
overlay layer and after restoring the file content on the lower the file 
on the overlay again runs as expected. The file content change + copy-up 
of file content also has completely decoupled the lower file from the 
file on the overlay and changes to the file on the lower cause no more 
file execution rejections on the overlay.

  Stefan
> 
> Thanks,
> Amir.