Re: [PATCH 0/2] ima: policy search speedup

[Kasatkin, Dmitry]

On Wed, Dec 12, 2012 at 1:02 AM, Eric Paris <eparis@xxxxxxxxxxxxxx> wrote:
> Linus made it clear he likes per-inode.  Do a test with per-inode
> S_NOIMA.  See how that compares to your 100,000 vs 10,000 lookups.
> Then we can discuss with facts if per sb is worth it or not.  We still
> don't actually know if 100,000 lookups vs 10,000 lookups really
> matters, but at least we'll have some facts...
>

Hello,

I have done few tests.
Ratio between lookups is different, but I do not really remember what
exactly it was.
Probably I did measurement with directory integrity protection...

First test is done using upstream IMA.

IMA-appraisal
[   59.554072] counter1 - before (inode->i_flags & S_NOIMA): 74586
[   59.554628] counter2 - after (inode->i_flags & S_NOIMA): 74558
[   59.555024] counter3 - after (inode->i_sb->s_feature_flags & SF_NOIMA): 52895

You can see 3 counters after 55 seconds uptime.
Counters count entries to the policy search function.
First counter counts every entry.
Second counter counts if control passes after inode flag S_NOIMA.
Third counts if control passes after SB flag SF_NOIMA.

As you can see, inode flag does not make a difference - just 28
entries differences.

But for SB flag, difference is over 22000.

Directory integrity protection is not really relevant to this discussion,
because it is under work now,  but with directory integrity protection
ratio is very different.

IMA-appraisal with directories
[   59.999077] counter1 - before (inode->i_flags & S_NOIMA): 198761
[   59.999078] counter2 - after (inode->i_flags & S_NOIMA): 198728
[   59.999079] counter3 - after (inode->i_sb->s_feature_flags & SF_NOIMA): 50266

Again, inode flags does not make a difference.
SB flag gives ~150 000 less searches.

Basically what I want to show is that inode flag has no benefit.
We need SB flag for that.

Regarding referencing i_sb, IMA policy search might do it for every rule, like

	if ((rule->flags & IMA_FSMAGIC)
	    && rule->fsmagic != inode->i_sb->s_magic)
		return false;


So 20k for 55 seconds can be multiplied roughly by the number of rules.

In fact earlier check for (inode->i_sb->s_feature_flags & SF_NOIMA)
only decreases
the total number of referencing.


- Dmitry


> On Tue, Dec 11, 2012 at 5:57 PM, Kasatkin, Dmitry
> <dmitry.kasatkin@xxxxxxxxx> wrote:
>> On Tue, Dec 11, 2012 at 10:08 PM, Eric Paris <eparis@xxxxxxxxxxxxxx> wrote:
>>> S_PRIVATE is totally unacceptable as it has a meaning across all LSMs,
>>> not just IMA.
>>>
>>> S_NOSEC means 'this is not setuid or setgid and we don't need to do
>>> those checks on modify'
>>>
>>> You are going to need to use a S_NOIMA.
>>>
>>> Of Dmitry's 90,000 fewer policy lookups using the per sb flag, how
>>> many of them are the same inode over and over again which would be
>>> circumvented using S_NOIMA per inode flag?
>>>
>>
>> IIRC those were only newly instantiated inodes.
>>
>> For new inodes, S_NOIMA would not be set and used at that point.
>> IMA must do the policy search and then would set the S_NOIMA.
>>
>> For subsequent calls, S_NOIMA makes sense.
>>
>> If we would have the SB flag like MS_NOIMA, then we could replicate sb
>> flag to inode S_NOIMA flag,
>> in similar way as inode_has_no_xattr() does:
>>
>> static inline void inode_has_no_xattr(struct inode *inode)
>> {
>>         if (!is_sxid(inode->i_mode) && (inode->i_sb->s_flags & MS_NOSEC))
>>                 inode->i_flags |= S_NOSEC;
>> }
>>
>> Then later there is no need to dereference inode->i_sb...
>>
>> Can we reach conclusion about it?
>> Will we have SB flag?
>> if yes, then where, s_flags, or in some other field like s_feature_flags?
>> if not, then we can stop this discussion...
>>
>> - Dmitry
>>
>>>
>>>
>>> On Tue, Dec 11, 2012 at 2:48 PM, Mimi Zohar <zohar@xxxxxxxxxxxxxxxxxx> wrote:
>>>> On Tue, 2012-12-11 at 11:10 -0800, Linus Torvalds wrote:
>>>>
>>>>> Anyway, the whole "you can do it at file granularity" isn't the bulk
>>>>> of my argument (the "we already have the field that makes sense" is).
>>>>> But my point is that per-inode is not only the logically more
>>>>> straightforward place to do it, it's also the much more flexible place
>>>>> to do it. Because it *allows* for things like that.
>>>>
>>>> Ok. To summarize, S_IMA indicates that there is a rule and that the iint
>>>> was allocated.  To differentiate between 'haven't looked/don't know' and
>>>> 'definitely not', we need another bit.  For this, you're suggesting
>>>> using IS_PRIVATE()?  Hopefully, I misunderstood.
>>>>
>>>> thanks,
>>>>
>>>> Mimi
>>>>
>>>>
>>>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/