Re: Adding sc_pid to SCM_CREDS

[Roy Marples <roy%marples.name@localhost>]

Hi Eric

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.

> Assuming we're going with all of the compat changes:
> @@ -596,8 +597,9 @@
>  #define    SCM_RIGHTS    0x01        /* access rights (array of int) */
>  #if defined(_NETBSD_SOURCE)
>  /*             0x02           timestamp (struct timeval50) */
> -#define    SCM_CREDS    0x04        /* credentials (struct sockcred) */
> +/*            0x04           credentials (struct osockcred) */
> How about including "SCM_OCREDS" in this comment, to make it easier to
> grep for?

Happy to do that, but was just following the style of the timestamp
change made earlier.
You can find SCM_OCREDS in sys/compat/sys/socket.h alongside SCM_OTIMESTAMP.

> --- 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.

Roy