tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Adding sc_pid to SCM_CREDS



On 3/30/2016 10:10 AM, Roy Marples wrote:
On 30/03/2016 15:01, Eric Haszlakiewicz wrote:
On 3/29/2016 12:57 PM, Roy Marples wrote:
Attached is a patch which adds sc_pid to struct sockcred and correctly
fills it out. The old sockcred structure is available as osockcred in
compat/sys/socket.h and guarded by COMPAT_70.

Everything seems to work ok with old binaries, but would appreciate some
extra eyes on the compat code or any suggestion how it could be done
better.

Roy
--- sys/sys/socket.h    13 Oct 2015 21:28:34 -0000    1.118
+++ sys/sys/socket.h    29 Mar 2016 16:10:49 -0000
@@ -349,6 +349,7 @@
   * Socket credentials.
   */
  struct sockcred {
+    pid_t    sc_pid;            /* process id */

Would it be too much of a hack to keep backwards compat by putting
sc_pid *after* sc_groups, and simply returning a slightly larger
structure than old callers would use?  i.e. realsizeof(new sockcred) ==
realsizeof(old sockcred) + sizeof(pid_t)
There would be no declared sc_pid field (unless we define a struct
sockcred_tail), but doing so would eliminate 90% of the changes.
I'm not sure we can do that because sc_groups can be variable length.

Sure you can, you just need an extra step to get at sc_pid.  e.g.:
struct sockcred_tail {
   pid_t sc_pid;
};
struct sockcred {
...existing fields...
   gid_t sc_groups[1]; /* variable length */
   /* struct sockcred_tail sc_tail */
};
#define sockcred_gettail(sc) (struct sockcred_tail *)(((char *)(sc)) + SOCKCREDSIZE((sc)->sc_ngroups))

And accessing it would be:
  struct sockcred *sc;
  ...user code to get sc...
  struct sockcred_tail *sc_tail = sockcred_gettail(sc);
  pid_t thepid = sc_tail->sc_pid;

and obviously the code that allocates a struct sockcred would need to account for the struct sockcred_tail after sc_groups.

Is there existing code for other OS's that expects a sc_pid field to be directly present in struct sockcred? If not, then something like the above seems like it could be useful.

--- sys/sys/unpcb.h    24 Apr 2008 11:38:39 -0000    1.17
+++ sys/sys/unpcb.h    29 Mar 2016 16:10:49 -0000
@@ -97,11 +97,12 @@
   * in with data for the listening process.  This is set up in
unp_bind() when
   * it fills in unp_connid for later consumption by unp_connect().
   */
-#define    UNP_WANTCRED    0x0001        /* credentials wanted */
+#define    UNP_OWANTCRED    0x0001        /* credentials wanted */
The interface is SCM_CREDS, and constants like this don't get exposed
outside the kernel, right?  If so, it seems like it would be cleaner to
keep UNP_WANTCRED, etc... w/ the same value and have UNP_OWANTCRED be
the new one.
How would we know if we want the old struct for old binaries or the new
struct for new binaries? Happy to hear other ways of doing it.
If the structs need to be different, SCM_CREDS and SCM_OCREDS would still have different values, but if UNP_WANTCRED, etc.. are only used within the kernel and aren't exposed as part of the API, there's no need to change *those* values. Just have SCM_OCREDS map to the new kernel-internal defines, with new values.

Eric


Home | Main Index | Thread Index | Old Index