Making security hooks optional

The Linux Security Module effort ran into a bit of a snag this week as its developers tried to get another set of hooks merged into the 2.5 mainline. The result was a "back to the drawing board" experience which is likely to improve the quality of the LSM Patch overall.

The LSM team posted a set of hooks for networking operations for inclusion. There has been concern about the performance impact of the networking hooks since last June's Kernel Summit, so the LSM developers have put quite a bit of effort into minimizing any potential slowdowns. The current patches, it is said, have no measurable impact in 100MB/s networking, and a 1-2% slowdown with gigibit networks.

That is a small impact, but it was still too much for the networking hackers. Those folks have put a great deal of effort into creating the fastest networking on the planet, and they are not much interested in patches which slow things down. They take particular exception to just how these hooks are implemented. Consider one piece from the network hooks patches:

        if (skb) {
                security_ops->skb_recv_datagram(skb, sk, flags);
                return skb;
        }

The LSM patch, of course, adds the security_ops line.

The problem here is that the security hook is always called. If no particular security module has been loaded, then a dummy hook is called. So, even in the case where no security policy is being implemented (the usual case for most systems into the foreseeable future), a long-distance, indirect call is being made, with the usual effects on cache and TLB performance. The impact may be small, but it is still too much for the networking developers.

The solution, as posted by Greg Kroah-Hartman, is to move the hook invocation into a separate (inline) function. So the code fragment above would change to something like:

        if (skb) {
                security_skb_recv_datagram(skb, sk, flags);
                return skb;
        }

where security_skb_recv_datagram() would look like:

	static inline void void security_skb_recv_datagram(...) 
	{
		security_ops->skb_recv_datagram(...);
	}

This approach may not seem all that different. But now it is easy to introduce a CONFIG_SECURITY configuration option that makes all of the security hook invocations disappear entirely. Thus, for people who know that they will not load security modules (and for distributors who choose not to support security modules), the overhead of the module hooks vanishes entirely. With this change in place, the networking team is happier.

This change will also help address a couple of other problems that Rusty Russell (fresh back from his honeymoon) has pointed out. There is current a (small) race condition with module removal; it is possible that a security module could be removed from memory while other threads are still executing within the module's code. Fixing this problem will require the addition of some sort of reference counting, or the use of the recently-merged read-copy-update mechanism. It may also be desirable to control the environment in which security hooks run; for example, it could be decided that security hooks should run with preemption disabled. Both problems are more easily solved if the invocation of the hooks is wrapped within another function.

---

(Log in to post comments)