[PATCH v2 15/18] fs/kernel_file_read: Add "offset" arg for partial reads

[Scott Branden]

Hi Kees,

On 2020-07-24 11:39 a.m., Kees Cook wrote:
> On Fri, Jul 24, 2020 at 11:23:37AM -0700, Kees Cook wrote:
>> On Thu, Jul 23, 2020 at 10:41:07PM -0700, Scott Branden wrote:
>>>
>>> On 2020-07-23 12:15 p.m., Kees Cook wrote:
>>>> On Wed, Jul 22, 2020 at 03:29:26PM -0700, Scott Branden wrote:
>>>>> These changes don't pass the kernel-selftest for partial reads I added
>>>>> (which are at the end of this patch v2 series).
>>>> Oh, interesting. Is there any feedback in dmesg? I wonder if I have the
>>>> LSMs configured differently than you?
>>> I have no LSMs configured that I know of.
>>> Yes, there is failure in dmesg which is how I determined to add my
>>> workaround.
>>> Without workaround, dmesg log attached after booting and running
>>> fw_run_tests.h
>>>>> See change below added for temp workaround for issue.
>>>>>> [...]
>>>>>> +
>>>>>> +	whole_file = (offset == 0 && i_size <= buf_size);
>>>>> A hack to get this passing I added which probably breaks some security?
>>>>> if (whole_file) {
>>>>>> +	ret = security_kernel_read_file(file, id, whole_file);
>>>>>> +	if (ret)
>>>>>> +		goto out;
>>>>>> +
>>>>> }
>>>> This would imply I did something wrong in the LSM hook refactoring (i.e.
>>>> some LSM is rejecting the !whole_file case, but if the entire call to
>>>> the hooks are skipped, it's okay).
>>>>
>>>> What does this return on your test system:
>>>>
>>>> 	echo $(cat /sys/kernel/security/lsm)
>>> ima kernel configs are enabled but I don't enable security policies
>>> on the kernel command line.
>>>
>>> echo $(cat /sys/kernel/security/lsm)
>>> cat: /sys/kernel/security/lsm: No such file or directory
>> Oh, er... CONFIG_SECURITYFS is missing?
>>
>> Can you send me your .config?
> Ah, nevermind, I found my config mistake. I thought I had the right
> setting, but I'd missed CONFIG_IMA_APPRAISE=y. With that enabled, the
> firmware tests _correctly_ fail, since IMA can't appraise partial reads.
>
> So, this doesn't look like a bug to me.
>
Now I'm confused.  The original patch series I made with IMA additions 
under Mimi's direction
passed the kernel selftests with partial read.  And 
request_partial_firmware_into_buf therefore worked.
Your changes don't work with CONFIG_IMA_APPRAISE=y on?  Is there a way 
to make IMA ignore this file to make things work then?
Seems like another change is needed for IMA to ignore partial reads if 
it can't appraise them?