lib/utils.c: lazy initialize user_hz and psched_hz

[nickkral]

Rather than initializing user_hz and psched_hz when libnl is loaded,
defer initialization of these variables to the first time they are used.
This has several advantages:

1. Avoids an unnecessary permission denied error on /proc/net/psched,
   which can occur on systems where /proc/net isn't readable due to
   security policy.
2. Allows program code to initialize the environment variables
   PROC_NET_PSCHED and/or PROC_ROOT prior to the first libnl call, giving a
   program more flexibility about where libnl should look.
3. Trivially faster startup time (although unlikely to be significant).
4. Compiler may be able to prove that the get_psched_settings() function
   is unreachable and optimize appropriately, because the callers never
   (directly or indirectly) use this method. This could occur, for
   instance, in doing dead code elimination for programs which statically
   link libnl.

Signed-off-by: Nick Kralevich nnk@google.com

[thom311]

Hm. What problem bothers you the most there? Is this about Point 1)?

get_psched_settings() also initializes ticks_per_usec, so you would need the same call in nl_us2ticks()/nl_ticks2us().

There is a race in get_psched_settings() when two threads at the same time try to initialize. Maybe there should be a lock around reading the settings. Or do you think two threads anyway will end up reading the same value? Their environment variables might be set differently...

2 Allows program code to initialize the environment variables

This seems a bit fragile, because another user of the library in the same process could have already read the values. E.g. you might think that there are no other users, but you might use a library inadvertently that uses libnl3.

(not that other existing uses of static variables already have similar issues)

[nickkral]

Yes, the primary problem I'm trying to solve is the unnecessary read from /proc/net/psched. I'm personally not concerned about the other points.

Background: I recently submitted https://android-review.googlesource.com/318365 into the Android code base. This change incorporates libnl into Android's init binary, for a very limited purpose: Sending one type of netlink message to the kernel. It only uses 4 libnl functions: nl_socket_alloc(), nl_connect(), nl_send_simple(), and nl_socket_free(), none of which have a need to use psched_hz.

Unfortunately, merely loading the libnl library into the process triggers a read to /proc/net/psched as a side effect, even when that value is never used. This causes problems for ueventd (which shares the same binary as init, but never calls the libnl functions), because ueventd does not have SELinux permissions to access /proc/net. And nor should it: ueventd only manages device nodes, and has nothing to do with the network.

(This could also cause problems for init in the future. In Android, init starts once and then re-exec's itself after mounting various filesystems. When first stage init starts, /proc isn't even mounted. And first stage or second stage init may have /proc/net access removed in a future security policy revision.)

When an operation is disallowed by security policy, an audit event is generated by the kernel which needs to be resolved. This can be resolved by:

1. Fixing the code to not perform the unnecessary operation.
2. Fix the security policy to not generate an audit entry.
3. Allow the operation if needed by program functionality (not relevant here)

where (1) is more desirable if the operation isn't truly needed.

If the only concern here is locking for these global variables, adding a lock shouldn't be a problem. But this doesn't resolve your concerns about environment variables.

I appreciate your thoughts on how to incorporate the best fix for this issue.

[thom311]

yeah, that makes sense. Thanks for elaborating.

I merged your patch with two follow-up commits. If you dislike anything about them, please report.

Thanks!

[nickkral]

No objections to the other changes. Thank you!