netdev - Re: [PATCH 1/3] net: Clean up SCM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 20 Mar 2013 23:54:05 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Andy Lutomirski <luto@...capital.net>
Cc:	netdev@...r.kernel.org, containers@...ts.linux-foundation.org
Subject: Re: [PATCH 1/3] net: Clean up SCM_CREDENTIALS code

Andy Lutomirski <luto@...capital.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
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html