NetBSD-Bugs archive

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

Re: kern/56533: fs/ffs/t_miscquota:default_deny_user_big fails randomly



The following reply was made to PR kern/56533; it has been noted by GNATS.

From: Andreas Gustafsson <gson%gson.org@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: 
Subject: Re: kern/56533: fs/ffs/t_miscquota:default_deny_user_big fails randomly
Date: Sat, 4 Dec 2021 23:34:12 +0200

 I tracked this down to a bug in librumpuser.
 
 When rumpuser_sp_init() sets up a unix domain socket for the rump
 service, it calls bind() using a socket address that is malloced in
 unix_parse().  The size of the allocation is sufficient for the actual
 pathname in the sun_path field and its terminating NUL, but does not
 include the unused remainder of sun_path.  Nonetheless, bind() is
 called with a namelen argument of sizeof(struct sockaddr_un), which
 does include the unused part of sun_path.  In rare circumstances, the
 allocation may happen to be immediately followed by an unmapped page,
 and then bind() will return EFAULT causing the test to fail.
 
 The commit of src/sys/net/if.c 1.492 probably just changed the pattern
 of allocations such that those rare circumstances happen in this
 particular test case on this particular port.
 
 I intend to fix this by making unix_parse() always allocate a
 full-sized struct sockaddr_un on all host systems rather than just
 some of them, which not only fixes the bug but also reduces the amount
 of system dependent code:
 
 Index: sp_common.c
 ===================================================================
 RCS file: /cvsroot/src/lib/librumpuser/sp_common.c,v
 retrieving revision 1.42
 diff -u -r1.42 sp_common.c
 --- sp_common.c	13 Jun 2020 16:51:59 -0000	1.42
 +++ sp_common.c	4 Dec 2021 21:20:16 -0000
 @@ -670,12 +670,10 @@
  		}
  	}
  	strcat(s_un.sun_path, addr);
 -#if defined(__linux__) || defined(__sun__) || defined(__CYGWIN__)
 -	slen = sizeof(s_un);
 -#else
 +#if !(defined(__linux__) || defined(__sun__) || defined(__CYGWIN__))
  	s_un.sun_len = SUN_LEN(&s_un);
 -	slen = s_un.sun_len+1; /* get the 0 too */
  #endif
 +	slen = sizeof(s_un);
  
  	if (savepath && *parsedurl == '\0') {
  		snprintf(parsedurl, sizeof(parsedurl),
 -- 
 Andreas Gustafsson, gson%gson.org@localhost
 


Home | Main Index | Thread Index | Old Index