Re: [PATCH 1/3] net: Clean up SCM_CREDENTIALS code

[ebiederm@xmission.com (Eric W. Biederman)]

Andy Lutomirski <luto@amacapital.net> writes:

> I was curious whether the uids, gids, and pids passed around worked
> correctly in the presence of multiple namespaces.  I gave up trying
> to figure it out: there are two copies of the pid (one of which has
> type u32, which is odd), a struct cred * (!), and a separate kuid
> and kgid.  IOW, all of the relevant data is stored twice, and it's
> unclear which copy is used when.
>
> I also wondered what prevented a SO_CREDENTIALS message from being
> recieved when the credentials weren't filled out.  Answer: not very
> much (and there have been serious security bugs here in the past).
>
> So just rewrite the thing to store a pid_t relative to the init pid
> ns, a kuid, and a kgid, and to explicitly track whether the data is
> filled out.
>
> I haven't played with the secid code.  I have no idea whether it has
> similar problems.
>
> I haven't benchmarked this, but it should be a respectable speedup
> in the cases where the credentials are in use.

The basic principle of no longer passing the struct cred we can
certainly do.

I am less convinced about the struct pid, but arguably that is the
proper approach.

A patch that proclaims that you didn't understand what the code was
doing but you changed it anyway, suggests there are subtle bugs
in there that you overlooked.

Certainly killing NETLINK_CB(sbk).ssk is a bug.

I do think there is a lot of good stuff in here and if you break this up
into smaller patches simpler patches, and keep an eye on the speed of
sending things messages without credentials.  I am pretty certain you
can cook up something that is mergable.

Eric