svn commit: r360068 - in head/sys: kern net sys

[Ronald Klop]

Nice feature. A question below.


On Sat, 18 Apr 2020 09:50:30 +0200, Kristof Provost <kp at freebsd.org> wrote:

> Author: kp
> Date: Sat Apr 18 07:50:30 2020
> New Revision: 360068
> URL: https://svnweb.freebsd.org/changeset/base/360068
>
> Log:
>   ethersubr: Make the mac address generation more robust
>  If we create two (vnet) jails and create a bridge interface in each we  
> end up
>   with the same mac address on both bridge interfaces.
>   These very often conflicts, resulting in same mac address in both  
> jails.
>  Mitigate this problem by including the jail name in the mac address.
>  Reviewed by:	kevans, melifaro
>   MFC after:	1 week
>   Differential Revision:	https://reviews.freebsd.org/D24383
>
> Modified:
>   head/sys/kern/kern_jail.c
>   head/sys/net/if_ethersubr.c
>   head/sys/sys/jail.h
>
> Modified: head/sys/kern/kern_jail.c
> ==============================================================================
> --- head/sys/kern/kern_jail.c	Sat Apr 18 03:14:16 2020	(r360067)
> +++ head/sys/kern/kern_jail.c	Sat Apr 18 07:50:30 2020	(r360068)
> @@ -2920,6 +2920,15 @@ getcredhostid(struct ucred *cred, unsigned long  
> *hosti
>  	mtx_unlock(&cred->cr_prison->pr_mtx);
>  }
> +void
> +getjailname(struct ucred *cred, char *name, size_t len)
> +{
> +
> +	mtx_lock(&cred->cr_prison->pr_mtx);
> +	strlcpy(name, cred->cr_prison->pr_name, len);
> +	mtx_unlock(&cred->cr_prison->pr_mtx);
> +}
> +
>  #ifdef VIMAGE
>  /*
>   * Determine whether the prison represented by cred owns
>
> Modified: head/sys/net/if_ethersubr.c
> ==============================================================================
> --- head/sys/net/if_ethersubr.c	Sat Apr 18 03:14:16 2020	(r360067)
> +++ head/sys/net/if_ethersubr.c	Sat Apr 18 07:50:30 2020	(r360068)
> @@ -1419,27 +1419,39 @@ ether_8021q_frame(struct mbuf **mp, struct ifnet  
> *ife,
> /*
>   * Allocate an address from the FreeBSD Foundation OUI.  This uses a
> - * cryptographic hash function on the containing jail's UUID and the  
> interface
> - * name to attempt to provide a unique but stable address.   
> Pseudo-interfaces
> - * which require a MAC address should use this function to allocate
> - * non-locally-administered addresses.
> + * cryptographic hash function on the containing jail's name, UUID and  
> the
> + * interface name to attempt to provide a unique but stable address.
> + * Pseudo-interfaces which require a MAC address should use this  
> function to
> + * allocate non-locally-administered addresses.
>   */
>  void
>  ether_gen_addr(struct ifnet *ifp, struct ether_addr *hwaddr)
>  {
> -#define	ETHER_GEN_ADDR_BUFSIZ	HOSTUUIDLEN + IFNAMSIZ + 2
>  	SHA1_CTX ctx;
> -	char buf[ETHER_GEN_ADDR_BUFSIZ];
> +	char *buf;
>  	char uuid[HOSTUUIDLEN + 1];
>  	uint64_t addr;
>  	int i, sz;
>  	char digest[SHA1_RESULTLEN];
> +	char jailname[MAXHOSTNAMELEN];
> 	getcredhostuuid(curthread->td_ucred, uuid, sizeof(uuid));
> -	sz = snprintf(buf, ETHER_GEN_ADDR_BUFSIZ, "%s-%s", uuid,  
> ifp->if_xname);
> +	/* If each (vnet) jail would also have a unique hostuuid this would not
> +	 * be necessary. */
> +	getjailname(curthread->td_ucred, jailname, sizeof(jailname));
> +	sz = asprintf(&buf, M_TEMP, "%s-%s-%s", uuid, if_name(ifp),
> +	    jailname);
> +	if (sz < 0) {
> +		/* Fall back to a random mac address. */


I was wondering if it would be valuable to give this fall back something  
like:

            printf("%s: unable to create fixed mac address; using random  
mac address", if_name(ifp));

This will only be printed in rare circumstances. But in that case will  
provide valuable information.

Regards,

Ronald.


> +		arc4rand(hwaddr, sizeof(*hwaddr), 0);
> +		hwaddr->octet[0] = 0x02;
> +		return;
> +	}
> +
>  	SHA1Init(&ctx);
>  	SHA1Update(&ctx, buf, sz);
>  	SHA1Final(digest, &ctx);
> +	free(buf, M_TEMP);
> 	addr = ((digest[0] << 16) | (digest[1] << 8) | digest[2]) &
>  	    OUI_FREEBSD_GENERATED_MASK;
>
> Modified: head/sys/sys/jail.h
> ==============================================================================
> --- head/sys/sys/jail.h	Sat Apr 18 03:14:16 2020	(r360067)
> +++ head/sys/sys/jail.h	Sat Apr 18 07:50:30 2020	(r360068)
> @@ -382,6 +382,7 @@ void getcredhostname(struct ucred *, char *, size_t);
>  void getcreddomainname(struct ucred *, char *, size_t);
>  void getcredhostuuid(struct ucred *, char *, size_t);
>  void getcredhostid(struct ucred *, unsigned long *);
> +void getjailname(struct ucred *cred, char *name, size_t len);
>  void prison0_init(void);
>  int prison_allow(struct ucred *, unsigned);
>  int prison_check(struct ucred *cred1, struct ucred *cred2);
> _______________________________________________
> svn-src-all at freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/svn-src-all
> To unsubscribe, send any mail to "svn-src-all-unsubscribe at freebsd.org"