Re: IPv6 DHCP-PD/SLAAC - no /64 route in routing table

[Martin Pieuchot]

On 01/11/15(Sun) 17:41, Yury Shefer wrote:
> Hi all,
> 
> I'm having trouble with enabling IPv6 routing on my 5.8 gateway.
> 
> (Internet)[DHCPv6+PD](em0-GW-axe0)[SLAAC/rtadvd]
> 
> My box is connected to Comcast, I'm getting IPv6 address assignment over
> DHCPv6 (wide dhcp6c) on WAN interface(em0) together with prefix delegation
> and assigning this prefix to axe0 (internal interface).
> 
> On axe0 i'm running rtadvd. My clients are getting IPv6 addresses properly
> assigned and i'm able to ping link-local address of my gw. but if I check
> ipv6 neighbors on my gw I see that there are no global addresses except
> permanent (GW-owned addresses).
>
> [...] 
>
> Any suggestions/ideas?

I'd interested to know if your problem is fixed in -current.

Re: support more nd options for tcpdump print-icmp6.c

[Martin Pieuchot]

On 02/11/15(Mon) 16:58, Stuart Henderson wrote:
> This diff adds support for some more ND options to tcpdump: printing
> v6 nameserver addresses from RDNSS options, and basic support to print
> the option name (rather than just "unknown opt_type=XX") for DNSSL
> and RFC4191 route information (no full printer for these yet - for
> DNSSL the domain names are sent with DNS-encoding so it could do with
> ns_nprint() being exposed).
> 
> Any comments/OKs?

I was about the do it!  ok with me, thanks.

> sample rtadvd.conf to include some data that will be handled by this:
> 
> default:\
> :rdnss="2001:4860:4860::,2001:4860:4860::8844":\
> :dnssl="openbsd.org"
> vlan2:tc=default
> vlan3:tc=default
> 
> 
> Index: print-icmp6.c
> ===
> RCS file: /cvs/src/usr.sbin/tcpdump/print-icmp6.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 print-icmp6.c
> --- print-icmp6.c 16 Jan 2015 06:40:21 -  1.15
> +++ print-icmp6.c 2 Nov 2015 16:53:44 -
> @@ -511,8 +511,9 @@ icmp6_opt_print(register const u_char *b
>   register const struct nd_opt_prefix_info *opp;
>   register const struct icmp6_opts_redirect *opr;
>   register const struct nd_opt_mtu *opm;
> + register const struct nd_opt_rdnss *oprd;
>   register const u_char *ep;
> - int opts_len;
> + int i, opts_len;
>  #if 0
>   register const struct ip6_hdr *ip;
>   register const char *str;
> @@ -623,6 +624,34 @@ icmp6_opt_print(register const u_char *b
>   if (opm->nd_opt_mtu_len != 1)
>   printf("!");
>   printf(")");
> + icmp6_opt_print((const u_char *)op + (op->nd_opt_len << 3),
> + resid - (op->nd_opt_len << 3));
> + break;
> + case ND_OPT_ROUTE_INFO:
> + printf("(route-info: opt_len=%d)", op->nd_opt_len);
> + icmp6_opt_print((const u_char *)op + (op->nd_opt_len << 3),
> + resid - (op->nd_opt_len << 3));
> + break;
> + case ND_OPT_RDNSS:
> + oprd = (const struct nd_opt_rdnss *)op;
> + printf("(rdnss: ");
> + TCHECK(oprd->nd_opt_rdnss_lifetime);
> + printf("lifetime=%us",
> + (u_int32_t)ntohl(oprd->nd_opt_rdnss_lifetime));
> + if (oprd->nd_opt_rdnss_len < 3) {
> + printf("!");
> + } else for (i = 0; i < ((oprd->nd_opt_rdnss_len - 1) / 2); i++) 
> {
> + struct in6_addr *addr = (struct in6_addr *)(oprd + 1) + 
> i;
> + TCHECK2(*addr, sizeof(struct in6_addr));
> + printf(", addr=%s", ip6addr_string(addr));
> + }
> + printf(")");
> + icmp6_opt_print((const u_char *)op + (op->nd_opt_len << 3),
> + resid - (op->nd_opt_len << 3));
> + break;
> + case ND_OPT_DNSSL:
> + printf("(dnssl: opt_len=%d)", op->nd_opt_len);
> + /* XXX */
>   icmp6_opt_print((const u_char *)op + (op->nd_opt_len << 3),
>   resid - (op->nd_opt_len << 3));
>   break;
> 

Unsigned char cast for ctype func in uniq(1)

[Michael McConville]

ok?


Index: usr.bin/uniq/uniq.c
===
RCS file: /cvs/src/usr.bin/uniq/uniq.c,v
retrieving revision 1.22
diff -u -p -r1.22 uniq.c
--- usr.bin/uniq/uniq.c 9 Oct 2015 01:37:09 -   1.22
+++ usr.bin/uniq/uniq.c 2 Nov 2015 17:17:13 -
@@ -214,7 +214,7 @@ obsolete(char *argv[])
return;
} else if (ap[1] == '-')
return;
-   if (!isdigit(ap[1]))
+   if (!isdigit((unsigned char)ap[1]))
continue;
/*
 * Digit signifies an old-style option.  Malloc space for dash,

crontab: use setegid() instead of swap functions

[Todd C. Miller]

Using setegid() directly makes the code easier to read.
Some of these calls will be removed in a later diff.

 - todd

Index: crontab.c
===
RCS file: /cvs/src/usr.sbin/cron/crontab.c,v
retrieving revision 1.78
diff -u -p -u -r1.78 crontab.c
--- crontab.c   31 Oct 2015 12:13:01 -  1.78
+++ crontab.c   2 Nov 2015 18:31:52 -
@@ -30,7 +30,8 @@ enum opt_t{ opt_unknown, opt_list, opt_
 static char*getoptargs = "u:ler";
 
 static pid_t   Pid;
-static gid_t   save_egid;
+static gid_t   crontab_gid;
+static gid_t   user_gid;
 static charUser[MAX_UNAME], RealUser[MAX_UNAME];
 static charFilename[MAX_FNAME], TempFilename[MAX_FNAME];
 static FILE*NewCrontab;
@@ -47,17 +48,6 @@ static   voidlist_cmd(void),
die(int);
 static int replace_cmd(void);
 
-static int swap_gids(void)
-{
-   save_egid = getegid();
-   return (setegid(getgid()));
-}
-
-static int swap_gids_back(void)
-{
-   return (setegid(save_egid));
-}
-
 static void
 usage(const char *msg)
 {
@@ -78,6 +68,8 @@ main(int argc, char *argv[])
int exitstatus;
 
Pid = getpid();
+   user_gid = getgid();
+   crontab_gid = getegid();
ProgramName = argv[0];
 
if (pledge("stdio rpath wpath cpath fattr getpw unix flock id proc 
exec",
@@ -208,16 +200,16 @@ parse_args(int argc, char *argv[])
 * the race.
 */
 
-   if (swap_gids() < 0) {
-   perror("swapping gids");
+   if (setegid(user_gid) < 0) {
+   perror("setegid(user_gid)");
exit(EXIT_FAILURE);
}
if (!(NewCrontab = fopen(Filename, "r"))) {
perror(Filename);
exit(EXIT_FAILURE);
}
-   if (swap_gids_back() < 0) {
-   perror("swapping gids back");
+   if (setegid(crontab_gid) < 0) {
+   perror("setegid(crontab_gid)");
exit(EXIT_FAILURE);
}
}
@@ -322,13 +314,13 @@ edit_cmd(void)
fprintf(stderr, "path too long\n");
goto fatal;
}
-   if (swap_gids() < 0) {
-   perror("swapping gids");
+   if (setegid(user_gid) < 0) {
+   perror("setegid(user_gid)");
exit(EXIT_FAILURE);
}
t = mkstemp(Filename);
-   if (swap_gids_back() < 0) {
-   perror("swapping gids back");
+   if (setegid(crontab_gid) < 0) {
+   perror("setegid(crontab_gid)");
exit(EXIT_FAILURE);
}
if (t == -1) {
@@ -355,13 +347,13 @@ edit_cmd(void)
fprintf(stderr, "%s: error while writing new crontab to %s\n",
ProgramName, Filename);
  fatal:
-   if (swap_gids() < 0) {
-   perror("swapping gids");
+   if (setegid(user_gid) < 0) {
+   perror("setegid(user_gid)");
exit(EXIT_FAILURE);
}
unlink(Filename);
-   if (swap_gids_back() < 0) {
-   perror("swapping gids back");
+   if (setegid(crontab_gid) < 0) {
+   perror("setegid(crontab_gid)");
exit(EXIT_FAILURE);
}
exit(EXIT_FAILURE);
@@ -384,8 +376,8 @@ edit_cmd(void)
goto fatal;
}
if (timespeccmp([1], _mtim, ==)) {
-   if (swap_gids() < 0) {
-   perror("swapping gids");
+   if (setegid(user_gid) < 0) {
+   perror("setegid(user_gid)");
exit(EXIT_FAILURE);
}
if (lstat(Filename, ) == 0 &&
@@ -393,8 +385,8 @@ edit_cmd(void)
fprintf(stderr, "%s: crontab temp file moved, editor "
   "may create backup files improperly\n", ProgramName);
}
-   if (swap_gids_back() < 0) {
-   perror("swapping gids back");
+   if (setegid(crontab_gid) < 0) {
+   perror("setegid(crontab_gid)");
exit(EXIT_FAILURE);
}
fprintf(stderr, "%s: no changes made to crontab\n",
@@ -437,13 +429,13 @@ edit_cmd(void)
goto fatal;
}
  remove:
-   if (swap_gids() < 0) {
-   perror("swapping gids");
+   if (setegid(user_gid) < 0) {
+   perror("setegid(user_gid)");
exit(EXIT_FAILURE);
}

Re: Drop register keyword from less(1)

[Todd C. Miller]

On Mon, 02 Nov 2015 09:16:07 +, Nicholas Marriott wrote:

> I looked briefly at this and it wouldn't be that hard. However, while it
> would be fantastic to clean up all the crap from less, it isn't clear if
> Garrett D'Amore is going to be keeping his fork up to date - if he
> doesn't then we are then left with a much harder job to merge later
> changes from the original less. Unless we are happy to fork and maintain
> less ourselves without an upstream.

Would it really be so bad to consider less/more feature complete
and only apply bug fixes?  The only place I expect large changes
would be in the UTF-8 support.

 - todd

Re: Drop register keyword from less(1)

[Nicholas Marriott]

On Mon, Nov 02, 2015 at 09:32:46AM -0700, Todd C. Miller wrote:
> On Mon, 02 Nov 2015 09:16:07 +, Nicholas Marriott wrote:
> 
> > I looked briefly at this and it wouldn't be that hard. However, while it
> > would be fantastic to clean up all the crap from less, it isn't clear if
> > Garrett D'Amore is going to be keeping his fork up to date - if he
> > doesn't then we are then left with a much harder job to merge later
> > changes from the original less. Unless we are happy to fork and maintain
> > less ourselves without an upstream.
> 
> Would it really be so bad to consider less/more feature complete
> and only apply bug fixes?  The only place I expect large changes
> would be in the UTF-8 support.

I suppose not, I don't think there has been much of interest for a while.

cvs(1) simplification

[Michael McConville]

Don't bother mallocing a statically-sized 1,024-byte chunk of mem, for
simplicity and speed.

ok?


Index: usr.bin/cvs/server.c
===
RCS file: /cvs/src/usr.bin/cvs/server.c,v
retrieving revision 1.102
diff -u -p -r1.102 server.c
--- usr.bin/cvs/server.c16 Jan 2015 06:40:07 -  1.102
+++ usr.bin/cvs/server.c2 Nov 2015 17:17:13 -
@@ -323,7 +323,7 @@ void
 cvs_server_directory(char *data)
 {
CVSENTRIES *entlist;
-   char *dir, *repo, *parent, *entry, *dirn, *p;
+   char *dir, *repo, *parent, entry[CVS_ENT_MAXLINELEN], *dirn, *p;
 
if (current_cvsroot == NULL)
fatal("No Root specified for Directory");
@@ -357,13 +357,11 @@ cvs_server_directory(char *data)
fatal("cvs_server_directory: %s", strerror(errno));
 
if (strcmp(parent, ".")) {
-   entry = xmalloc(CVS_ENT_MAXLINELEN);
cvs_ent_line_str(dirn, NULL, NULL, NULL, NULL, 1, 0,
entry, CVS_ENT_MAXLINELEN);
 
entlist = cvs_ent_open(parent);
cvs_ent_add(entlist, entry);
-   xfree(entry);
}
 
if (server_currentdir != NULL)

LibreSSL MIPS64 build with GCC5

[Ruslan Babayev]

This fixes the portable LibreSSL build on Linux with GCC5 for MIPS64.

Index: lib/libssl/src/crypto/bn/bn_lcl.h
===
RCS file: /cvs/src/lib/libssl/src/crypto/bn/bn_lcl.h,v
retrieving revision 1.21
diff -r1.21 bn_lcl.h
262c262
< #   if __GNUC__>=4 && __GNUC_MINOR__>=4 /* "h" constraint is no more since 4.4 */
---
> #   if __GNUC__>=4 && __GNUC_MINOR__>=4 || __GNUC__>=5 /* "h" constraint is no more since 4.4 */

[patch] tcpdump print-tcp printf format tweaks

[Kevin Reay]

Change printf format to print unsigned values. Minor spacing change of
casts to match file/style(9).

Attempted to match printf formating of unsigned 32bits to rest of
file.
Index: print-tcp.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/print-tcp.c,v
retrieving revision 1.33
diff -u -p -r1.33 print-tcp.c
--- print-tcp.c 20 Aug 2015 22:39:29 -  1.33
+++ print-tcp.c 3 Nov 2015 07:17:39 -
@@ -221,7 +221,7 @@ tcp_print(register const u_char *bp, reg
 
ch = '\0';
if (length < sizeof(*tp)) {
-   (void)printf("truncated-tcp %d", length);
+   (void)printf("truncated-tcp %lu", (long)length);
return;
}
 
@@ -476,15 +476,15 @@ tcp_print(register const u_char *bp, reg
 
length -= hlen;
if (vflag > 1 || length > 0 || flags & (TH_SYN | TH_FIN | TH_RST))
-   (void)printf(" %lu:%lu(%d)", (long) seq, (long) (seq + length),
-   length);
+   (void)printf(" %lu:%lu(%lu)", (long)seq, (long)(seq + length),
+   (long)length);
if (flags & TH_ACK)
-   (void)printf(" ack %u", ack);
+   (void)printf(" ack %lu", (long)ack);
 
-   (void)printf(" win %d", win);
+   (void)printf(" win %u", win);
 
if (flags & TH_URG)
-   (void)printf(" urg %d", urp);
+   (void)printf(" urg %u", urp);
/*
 * Handle any options.
 */

replace ppp(4) sc_npqueue+sc_npqtail with an mbuf_list

[David Gwynne]

while im shining this turd...

ok?

Index: if_ppp.c
===
RCS file: /cvs/src/sys/net/if_ppp.c,v
retrieving revision 1.92
diff -u -p -r1.92 if_ppp.c
--- if_ppp.c2 Nov 2015 23:39:20 -   1.92
+++ if_ppp.c3 Nov 2015 06:28:46 -
@@ -293,8 +293,7 @@ pppalloc(pid_t pid)
 #endif /* PPP_COMPRESS */
 for (i = 0; i < NUM_NP; ++i)
sc->sc_npmode[i] = NPMODE_ERROR;
-sc->sc_npqueue = NULL;
-sc->sc_npqtail = >sc_npqueue;
+ml_init(>sc_npqueue);
 sc->sc_last_sent = sc->sc_last_recv = time_second;
 
 return sc;
@@ -319,10 +318,7 @@ pppdealloc(struct ppp_softc *sc)
ppp_pkt_free(pkt);
 while ((m = mq_dequeue(>sc_inq)) != NULL)
m_freem(m);
-while ((m = sc->sc_npqueue) != NULL) {
-   sc->sc_npqueue = m->m_nextpkt;
-   m_freem(m);
-}
+ml_purge(>sc_npqueue);
 m_freem(sc->sc_togo);
 sc->sc_togo = NULL;
 
@@ -769,9 +765,7 @@ pppoutput(struct ifnet *ifp, struct mbuf
 s = splsoftnet();
 if (mode == NPMODE_QUEUE) {
/* XXX we should limit the number of packets on this queue */
-   *sc->sc_npqtail = m0;
-   m0->m_nextpkt = NULL;
-   sc->sc_npqtail = >m_nextpkt;
+ml_enqueue(>sc_npqueue, m0);
 } else {
IFQ_ENQUEUE(>sc_if.if_snd, m0, error);
if (error) {
@@ -801,13 +797,14 @@ bad:
 static void
 ppp_requeue(struct ppp_softc *sc)
 {
-struct mbuf *m, **mpp;
+struct mbuf_list ml = MBUF_LIST_INITIALIZER();
+struct mbuf *m;
 enum NPmode mode;
 int error;
 
 splsoftassert(IPL_SOFTNET);
 
-for (mpp = >sc_npqueue; (m = *mpp) != NULL; ) {
+while ((m = ml_dequeue(>sc_npqueue)) != NULL) {
switch (PPP_PROTOCOL(mtod(m, u_char *))) {
case PPP_IP:
mode = sc->sc_npmode[NP_IP];
@@ -818,11 +815,6 @@ ppp_requeue(struct ppp_softc *sc)
 
switch (mode) {
case NPMODE_PASS:
-   /*
-* This packet can now go on one of the queues to be sent.
-*/
-   *mpp = m->m_nextpkt;
-   m->m_nextpkt = NULL;
IFQ_ENQUEUE(>sc_if.if_snd, m, error);
if (error) {
sc->sc_if.if_oerrors++;
@@ -832,16 +824,15 @@ ppp_requeue(struct ppp_softc *sc)
 
case NPMODE_DROP:
case NPMODE_ERROR:
-   *mpp = m->m_nextpkt;
m_freem(m);
break;
 
case NPMODE_QUEUE:
-   mpp = >m_nextpkt;
+   ml_enqueue(, m);
break;
}
 }
-sc->sc_npqtail = mpp;
+sc->sc_npqueue = ml;
 }
 
 /*
Index: if_pppvar.h
===
RCS file: /cvs/src/sys/net/if_pppvar.h,v
retrieving revision 1.17
diff -u -p -r1.17 if_pppvar.h
--- if_pppvar.h 3 Jun 2015 00:50:09 -   1.17
+++ if_pppvar.h 3 Nov 2015 06:28:46 -
@@ -111,8 +111,7 @@ struct ppp_softc {
struct  mbuf_queue sc_inq;  /* queue of input packets for daemon */
struct  ifqueue sc_fastq;   /* interactive output packet q */
struct  mbuf *sc_togo;  /* output packet ready to go */
-   struct  mbuf *sc_npqueue;   /* output packets not to be sent yet */
-   struct  mbuf **sc_npqtail;  /* ptr to last next ptr in npqueue */
+   struct  mbuf_list sc_npqueue;   /* output packets not to be sent yet */
struct  pppstat sc_stats;   /* count of bytes/pkts sent/rcvd */
caddr_t sc_bpf; /* hook for BPF */
enumNPmode sc_npmode[NUM_NP]; /* what to do with each NP */

em(4) watchdog timeouts

[Mark Kettenis]

Can those that are experiencing watchdog timeouts check if the diff
below gets rid of them?


Index: if_em.h
===
RCS file: /home/cvs/src/sys/dev/pci/if_em.h,v
retrieving revision 1.58
diff -u -p -r1.58 if_em.h
--- if_em.h 30 Sep 2015 11:25:08 -  1.58
+++ if_em.h 2 Nov 2015 19:07:55 -
@@ -190,7 +190,7 @@ typedef int boolean_t;
  * Thise parameter controls the minimum number of available transmit
  * descriptors needed before we attempt transmission of a packet.
  */
-#define EM_TX_OP_THRESHOLD (sc->num_tx_desc / 32)
+#define EM_TX_OP_THRESHOLD (EM_MAX_SCATTER + 6)
 
 /*
  * This parameter controls whether or not autonegotiation is enabled.
@@ -271,7 +271,7 @@ typedef int boolean_t;
 #define EM_MCLBYTESEM_RXBUFFER_2048
 #endif
 
-#define EM_MAX_SCATTER 64
+#define EM_MAX_SCATTER 32
 #define EM_TSO_SIZE65535
 
 struct em_buffer {

RFC2292 remnants in ip6(4)

[Jérémie Courrèges-Anglas]

Hi,

Ted recently removed the backwards compat for RFC2292 options, here's
a small nit about documentation: IPV6_PKTOPTIONS is deprecated by
RFC3542, and it doesn't appear anymore in /usr/src, except in this
manpage.

ok?

Index: share/man/man4/ip6.4
===
RCS file: /cvs/src/share/man/man4/ip6.4,v
retrieving revision 1.34
diff -u -p -r1.34 ip6.4
--- share/man/man4/ip6.425 Oct 2015 14:43:07 -  1.34
+++ share/man/man4/ip6.42 Nov 2015 19:29:27 -
@@ -331,35 +331,6 @@ routine and family of routines may be us
 This option requires superuser privileges.
 Turning this option on will result in this socket getting cmsg data of
 type IPV6_RTHDR.
-.It Dv IPV6_PKTOPTIONS Fa "struct cmsghdr *"
-Get or set all header options and extension headers at one time on the
-last packet sent or received on the socket.
-All options must fit within the size of an mbuf (see
-.Xr mbuf 9 ) .
-Options are specified as a series of
-.Vt cmsghdr
-structures followed by corresponding values.
-.Va cmsg_level
-is set to
-.Dv IPPROTO_IPV6 ,
-.Va cmsg_type
-to one of the other values in this list, and trailing data to the option
-value.
-When setting options, if the length
-.Va optlen
-to
-.Xr setsockopt 2
-is zero, all header options will be reset to their default values.
-Otherwise, the length should specify the size the series of control
-messages consumes.
-.Pp
-Instead of using
-.Xr sendmsg 2
-to specify option values, the ancillary data used in these calls that
-correspond to the desired header options may be directly specified as
-the control message in the series of control messages provided as the
-argument to
-.Xr setsockopt 2 .
 .It Dv IPV6_CHECKSUM Fa "int *"
 Get or set the byte offset into a packet where the 16-bit checksum is
 located.
@@ -461,15 +432,10 @@ This implementation determines the value
 options specified by using ancillary data (i.e.,
 .Xr sendmsg 2 )
 are considered first,
-options specified by using
-.Dv IPV6_PKTOPTIONS
-to set
-.Dq sticky
-options are considered second,
 options specified by using the individual, basic, and direct socket
 options (e.g.,
 .Dv IPV6_UNICAST_HOPS )
-are considered third,
+are considered second,
 and options specified in the socket address supplied to
 .Xr sendto 2
 are the last choice.


-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: crontab: use setegid() instead of swap functions

[Jérémie Courrèges-Anglas]

"Todd C. Miller"  writes:

> Using setegid() directly makes the code easier to read.
> Some of these calls will be removed in a later diff.

looks fine, ok jca@

[...]

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: crontab: use setegid() instead of swap functions

[Nicholas Marriott]

Looks good to me, ok nicm


On Mon, Nov 02, 2015 at 11:35:21AM -0700, Todd C. Miller wrote:
> Using setegid() directly makes the code easier to read.
> Some of these calls will be removed in a later diff.
> 
>  - todd
> 
> Index: crontab.c
> ===
> RCS file: /cvs/src/usr.sbin/cron/crontab.c,v
> retrieving revision 1.78
> diff -u -p -u -r1.78 crontab.c
> --- crontab.c 31 Oct 2015 12:13:01 -  1.78
> +++ crontab.c 2 Nov 2015 18:31:52 -
> @@ -30,7 +30,8 @@ enum opt_t  { opt_unknown, opt_list, opt_
>  static char  *getoptargs = "u:ler";
>  
>  static   pid_t   Pid;
> -static   gid_t   save_egid;
> +static   gid_t   crontab_gid;
> +static   gid_t   user_gid;
>  static   charUser[MAX_UNAME], RealUser[MAX_UNAME];
>  static   charFilename[MAX_FNAME], TempFilename[MAX_FNAME];
>  static   FILE*NewCrontab;
> @@ -47,17 +48,6 @@ static voidlist_cmd(void),
>   die(int);
>  static   int replace_cmd(void);
>  
> -static int swap_gids(void)
> -{
> - save_egid = getegid();
> - return (setegid(getgid()));
> -}
> -
> -static int swap_gids_back(void)
> -{
> - return (setegid(save_egid));
> -}
> -
>  static void
>  usage(const char *msg)
>  {
> @@ -78,6 +68,8 @@ main(int argc, char *argv[])
>   int exitstatus;
>  
>   Pid = getpid();
> + user_gid = getgid();
> + crontab_gid = getegid();
>   ProgramName = argv[0];
>  
>   if (pledge("stdio rpath wpath cpath fattr getpw unix flock id proc 
> exec",
> @@ -208,16 +200,16 @@ parse_args(int argc, char *argv[])
>* the race.
>*/
>  
> - if (swap_gids() < 0) {
> - perror("swapping gids");
> + if (setegid(user_gid) < 0) {
> + perror("setegid(user_gid)");
>   exit(EXIT_FAILURE);
>   }
>   if (!(NewCrontab = fopen(Filename, "r"))) {
>   perror(Filename);
>   exit(EXIT_FAILURE);
>   }
> - if (swap_gids_back() < 0) {
> - perror("swapping gids back");
> + if (setegid(crontab_gid) < 0) {
> + perror("setegid(crontab_gid)");
>   exit(EXIT_FAILURE);
>   }
>   }
> @@ -322,13 +314,13 @@ edit_cmd(void)
>   fprintf(stderr, "path too long\n");
>   goto fatal;
>   }
> - if (swap_gids() < 0) {
> - perror("swapping gids");
> + if (setegid(user_gid) < 0) {
> + perror("setegid(user_gid)");
>   exit(EXIT_FAILURE);
>   }
>   t = mkstemp(Filename);
> - if (swap_gids_back() < 0) {
> - perror("swapping gids back");
> + if (setegid(crontab_gid) < 0) {
> + perror("setegid(crontab_gid)");
>   exit(EXIT_FAILURE);
>   }
>   if (t == -1) {
> @@ -355,13 +347,13 @@ edit_cmd(void)
>   fprintf(stderr, "%s: error while writing new crontab to %s\n",
>   ProgramName, Filename);
>   fatal:
> - if (swap_gids() < 0) {
> - perror("swapping gids");
> + if (setegid(user_gid) < 0) {
> + perror("setegid(user_gid)");
>   exit(EXIT_FAILURE);
>   }
>   unlink(Filename);
> - if (swap_gids_back() < 0) {
> - perror("swapping gids back");
> + if (setegid(crontab_gid) < 0) {
> + perror("setegid(crontab_gid)");
>   exit(EXIT_FAILURE);
>   }
>   exit(EXIT_FAILURE);
> @@ -384,8 +376,8 @@ edit_cmd(void)
>   goto fatal;
>   }
>   if (timespeccmp([1], _mtim, ==)) {
> - if (swap_gids() < 0) {
> - perror("swapping gids");
> + if (setegid(user_gid) < 0) {
> + perror("setegid(user_gid)");
>   exit(EXIT_FAILURE);
>   }
>   if (lstat(Filename, ) == 0 &&
> @@ -393,8 +385,8 @@ edit_cmd(void)
>   fprintf(stderr, "%s: crontab temp file moved, editor "
>  "may create backup files improperly\n", ProgramName);
>   }
> - if (swap_gids_back() < 0) {
> - perror("swapping gids back");
> + if (setegid(crontab_gid) < 0) {
> + perror("setegid(crontab_gid)");
>   exit(EXIT_FAILURE);
>   }
>   fprintf(stderr, "%s: no changes made to crontab\n",
> @@ -437,13 +429,13 @@ edit_cmd(void)
>   goto fatal;
>   

Re: whois(1) -I (whois.iana.org)

[Jérémie Courrèges-Anglas]

Stuart Henderson  writes:

> This seems quite a useful database now that there are 500+ TLDs,
> OK to add a flag to use it more easily from whois(1)?

That's indeed nicer. :)

ok jca@

[...]

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: cvs(1) simplification

[Nicholas Marriott]

Sure, but this idiom is all over the place in opencvs, are you going to
change the rest?


On Mon, Nov 02, 2015 at 12:31:14PM -0500, Michael McConville wrote:
> Don't bother mallocing a statically-sized 1,024-byte chunk of mem, for
> simplicity and speed.
> 
> ok?
> 
> 
> Index: usr.bin/cvs/server.c
> ===
> RCS file: /cvs/src/usr.bin/cvs/server.c,v
> retrieving revision 1.102
> diff -u -p -r1.102 server.c
> --- usr.bin/cvs/server.c  16 Jan 2015 06:40:07 -  1.102
> +++ usr.bin/cvs/server.c  2 Nov 2015 17:17:13 -
> @@ -323,7 +323,7 @@ void
>  cvs_server_directory(char *data)
>  {
>   CVSENTRIES *entlist;
> - char *dir, *repo, *parent, *entry, *dirn, *p;
> + char *dir, *repo, *parent, entry[CVS_ENT_MAXLINELEN], *dirn, *p;
>  
>   if (current_cvsroot == NULL)
>   fatal("No Root specified for Directory");
> @@ -357,13 +357,11 @@ cvs_server_directory(char *data)
>   fatal("cvs_server_directory: %s", strerror(errno));
>  
>   if (strcmp(parent, ".")) {
> - entry = xmalloc(CVS_ENT_MAXLINELEN);
>   cvs_ent_line_str(dirn, NULL, NULL, NULL, NULL, 1, 0,
>   entry, CVS_ENT_MAXLINELEN);
>  
>   entlist = cvs_ent_open(parent);
>   cvs_ent_add(entlist, entry);
> - xfree(entry);
>   }
>  
>   if (server_currentdir != NULL)
> 

Re: Unsigned char cast for ctype func in uniq(1)

[Jérémie Courrèges-Anglas]

Michael McConville  writes:

> ok?

Sure.

>
> Index: usr.bin/uniq/uniq.c
> ===
> RCS file: /cvs/src/usr.bin/uniq/uniq.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 uniq.c
> --- usr.bin/uniq/uniq.c   9 Oct 2015 01:37:09 -   1.22
> +++ usr.bin/uniq/uniq.c   2 Nov 2015 17:17:13 -
> @@ -214,7 +214,7 @@ obsolete(char *argv[])
>   return;
>   } else if (ap[1] == '-')
>   return;
> - if (!isdigit(ap[1]))
> + if (!isdigit((unsigned char)ap[1]))
>   continue;
>   /*
>* Digit signifies an old-style option.  Malloc space for dash,
>


-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: LibreSSL MIPS64 build with GCC5

[Michael McConville]

Ruslan Babayev wrote:
> This fixes the portable LibreSSL build on Linux with GCC5 for MIPS64.

Is __GNUC_PREREQ__ from /usr/include/sys/cdefs.h in other OSs? If not,
you could probably just add it to the LibreSSL portability headers. It's
probably the easiest and most readable solution if these sorts of
preproc conditions exist in more than one place.

> Index: lib/libssl/src/crypto/bn/bn_lcl.h
> ===
> RCS file: /cvs/src/lib/libssl/src/crypto/bn/bn_lcl.h,v
> retrieving revision 1.21
> diff -r1.21 bn_lcl.h
> 262c262
> < #   if __GNUC__>=4 && __GNUC_MINOR__>=4 /* "h" constraint is no more since 
> 4.4 */
> ---
> > #   if __GNUC__>=4 && __GNUC_MINOR__>=4 || __GNUC__>=5 /* "h" constraint is 
> > no more since 4.4 */

ml_purge for netinet/if_ether.c

[David Gwynne]

ml_purge returns how many mbufs they freed, so we can decrement the
la_hold_total with that instead of inside an ml_dequeue/m_freem
loop.

ok?

Index: netinet/if_ether.c
===
RCS file: /cvs/src/sys/netinet/if_ether.c,v
retrieving revision 1.182
diff -u -p -r1.182 if_ether.c
--- netinet/if_ether.c  2 Nov 2015 15:05:23 -   1.182
+++ netinet/if_ether.c  3 Nov 2015 06:15:03 -
@@ -141,7 +141,6 @@ arp_rtrequest(struct ifnet *ifp, int req
struct sockaddr *gate = rt->rt_gateway;
struct llinfo_arp *la = (struct llinfo_arp *)rt->rt_llinfo;
struct ifaddr *ifa;
-   struct mbuf *m;
 
if (!arpinit_done) {
static struct timeout arptimer_to;
@@ -243,10 +242,7 @@ arp_rtrequest(struct ifnet *ifp, int req
LIST_REMOVE(la, la_list);
rt->rt_llinfo = 0;
rt->rt_flags &= ~RTF_LLINFO;
-   while ((m = ml_dequeue(>la_ml)) != NULL) {
-   la_hold_total--;
-   m_freem(m);
-   }
+   la_hold_total -= ml_purge(>la_ml);
pool_put(_pool, la);
}
 }
@@ -395,10 +391,7 @@ arpresolve(struct ifnet *ifp, struct rte
ml_enqueue(>la_ml, m);
la_hold_total++;
} else {
-   while ((mh = ml_dequeue(>la_ml)) != NULL) {
-   la_hold_total--;
-   m_freem(mh);
-   }
+   la_hold_total -= ml_purge(>la_ml);
m_freem(m);
}
 
@@ -431,10 +424,7 @@ arpresolve(struct ifnet *ifp, struct rte
rt->rt_flags |= RTF_REJECT;
rt->rt_expire += arpt_down;
la->la_asked = 0;
-   while ((mh = ml_dequeue(>la_ml)) != NULL) {
-   la_hold_total--;
-   m_freem(mh);
-   }
+   la_hold_total -= ml_purge(>la_ml);
}
}
}

[patch] hostapd iapp.h frame-type name array typo

[Kevin Reay]

Add a missing delimiter to the IEEE80211_IAPP_FRAME_TYPE_NAME array.

The missing comma would cause the tcpdump IAPP printer to segfault
when an i_command value of 15 was processed (as the array only
contained 15 elements).

The array definition doesn't appear to be used anywhere else in the
tree.
Index: iapp.h
===
RCS file: /cvs/src/usr.sbin/hostapd/iapp.h,v
retrieving revision 1.3
diff -u -p -r1.3 iapp.h
--- iapp.h  10 Mar 2006 18:10:16 -  1.3
+++ iapp.h  3 Nov 2015 05:58:10 -
@@ -58,7 +58,7 @@ enum ieee80211_iapp_frame_type {
"reserved#10",  \
"reserved#11",  \
"hostapd radiotap", \
-   "hostapd pcap"  \
+   "hostapd pcap", \
"reserved#14",  \
"reserved#15",  \
 }

support more nd options for tcpdump print-icmp6.c

[Stuart Henderson]

This diff adds support for some more ND options to tcpdump: printing
v6 nameserver addresses from RDNSS options, and basic support to print
the option name (rather than just "unknown opt_type=XX") for DNSSL
and RFC4191 route information (no full printer for these yet - for
DNSSL the domain names are sent with DNS-encoding so it could do with
ns_nprint() being exposed).

Any comments/OKs?

sample rtadvd.conf to include some data that will be handled by this:

default:\
:rdnss="2001:4860:4860::,2001:4860:4860::8844":\
:dnssl="openbsd.org"
vlan2:tc=default
vlan3:tc=default


Index: print-icmp6.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/print-icmp6.c,v
retrieving revision 1.15
diff -u -p -r1.15 print-icmp6.c
--- print-icmp6.c   16 Jan 2015 06:40:21 -  1.15
+++ print-icmp6.c   2 Nov 2015 16:53:44 -
@@ -511,8 +511,9 @@ icmp6_opt_print(register const u_char *b
register const struct nd_opt_prefix_info *opp;
register const struct icmp6_opts_redirect *opr;
register const struct nd_opt_mtu *opm;
+   register const struct nd_opt_rdnss *oprd;
register const u_char *ep;
-   int opts_len;
+   int i, opts_len;
 #if 0
register const struct ip6_hdr *ip;
register const char *str;
@@ -623,6 +624,34 @@ icmp6_opt_print(register const u_char *b
if (opm->nd_opt_mtu_len != 1)
printf("!");
printf(")");
+   icmp6_opt_print((const u_char *)op + (op->nd_opt_len << 3),
+   resid - (op->nd_opt_len << 3));
+   break;
+   case ND_OPT_ROUTE_INFO:
+   printf("(route-info: opt_len=%d)", op->nd_opt_len);
+   icmp6_opt_print((const u_char *)op + (op->nd_opt_len << 3),
+   resid - (op->nd_opt_len << 3));
+   break;
+   case ND_OPT_RDNSS:
+   oprd = (const struct nd_opt_rdnss *)op;
+   printf("(rdnss: ");
+   TCHECK(oprd->nd_opt_rdnss_lifetime);
+   printf("lifetime=%us",
+   (u_int32_t)ntohl(oprd->nd_opt_rdnss_lifetime));
+   if (oprd->nd_opt_rdnss_len < 3) {
+   printf("!");
+   } else for (i = 0; i < ((oprd->nd_opt_rdnss_len - 1) / 2); i++) 
{
+   struct in6_addr *addr = (struct in6_addr *)(oprd + 1) + 
i;
+   TCHECK2(*addr, sizeof(struct in6_addr));
+   printf(", addr=%s", ip6addr_string(addr));
+   }
+   printf(")");
+   icmp6_opt_print((const u_char *)op + (op->nd_opt_len << 3),
+   resid - (op->nd_opt_len << 3));
+   break;
+   case ND_OPT_DNSSL:
+   printf("(dnssl: opt_len=%d)", op->nd_opt_len);
+   /* XXX */
icmp6_opt_print((const u_char *)op + (op->nd_opt_len << 3),
resid - (op->nd_opt_len << 3));
break;

Unsigned char cast for ctype in whois(1)

[Michael McConville]

ok?


Index: usr.bin/whois/whois.c
===
RCS file: /cvs/src/usr.bin/whois/whois.c,v
retrieving revision 1.50
diff -u -p -r1.50 whois.c
--- usr.bin/whois/whois.c   9 Oct 2015 01:37:09 -   1.50
+++ usr.bin/whois/whois.c   2 Nov 2015 17:05:06 -
@@ -302,7 +302,7 @@ choose_server(const char *name, const ch
return (MNICHOST);
else
return (NICHOST);
-   } else if (isdigit(*(++qhead)))
+   } else if (isdigit((unsigned char)*(++qhead)))
return (ANICHOST);
len = strlen(qhead) + sizeof(QNICHOST_TAIL);
if ((nserver = realloc(server, len)) == NULL)

Re: [PATCH] rcs: buf_free/rcsnum_free

[Todd C. Miller]

On Mon, 02 Nov 2015 08:59:19 +, Nicholas Marriott wrote:

> Any other oks for this?

OK millert@

 - todd

Re: Drop register keyword from less(1)

[Michael McConville]

Todd C. Miller wrote:
> On Mon, 02 Nov 2015 09:16:07 +, Nicholas Marriott wrote:
> > I looked briefly at this and it wouldn't be that hard. However,
> > while it would be fantastic to clean up all the crap from less, it
> > isn't clear if Garrett D'Amore is going to be keeping his fork up to
> > date - if he doesn't then we are then left with a much harder job to
> > merge later changes from the original less. Unless we are happy to
> > fork and maintain less ourselves without an upstream.
> 
> Would it really be so bad to consider less/more feature complete and
> only apply bug fixes?  The only place I expect large changes would be
> in the UTF-8 support.

It would be nice to be able to clean it up some. I often muse on the
fact that less is one of the first binaries to touch untrusted data on
my machines, and that it's known to be messy code.

at: remove privs.h

[Todd C. Miller]

at(1) tries to run as little code as possible with privileges.  This
creates a false sense of security since if there is an overflow an
attacker can easily change the effective gid anyway.

The only place we really need to drop the setgid crontab is when
reading a file with the -f flag.

 - todd

Index: usr.bin/at/at.c
===
RCS file: /cvs/src/usr.bin/at/at.c,v
retrieving revision 1.66
diff -u -p -u -r1.66 at.c
--- usr.bin/at/at.c 28 Oct 2015 20:17:31 -  1.66
+++ usr.bin/at/at.c 2 Nov 2015 21:16:09 -
@@ -35,7 +35,6 @@
 
 #include "cron.h"
 #include "at.h"
-#include "privs.h"
 #include 
 
 #define ALARMC 10  /* Number of seconds to wait for timeout */
@@ -56,6 +55,9 @@ char vflag = 0;   /* show completed but 
 char force = 0;/* suppress errors (atrm) */
 char interactive = 0;  /* interactive mode (atrm) */
 static int send_mail = 0;  /* whether we are sending mail */
+uid_t user_uid;/* user's real uid */
+gid_t user_gid;/* user's real gid */
+gid_t spool_gid;   /* gid for writing to at spool */
 
 static void sigc(int);
 static void alarmc(int);
@@ -77,11 +79,8 @@ static __dead void
 panic(const char *a)
 {
(void)fprintf(stderr, "%s: %s\n", ProgramName, a);
-   if (fcreated) {
-   PRIV_START;
+   if (fcreated)
unlink(atfile);
-   PRIV_END;
-   }
 
exit(EXIT_FAILURE);
 }
@@ -93,11 +92,8 @@ static __dead void
 panic2(const char *a, const char *b)
 {
(void)fprintf(stderr, "%s: %s%s\n", ProgramName, a, b);
-   if (fcreated) {
-   PRIV_START;
+   if (fcreated)
unlink(atfile);
-   PRIV_END;
-   }
 
exit(EXIT_FAILURE);
 }
@@ -110,11 +106,8 @@ perr(const char *a)
 {
if (!force)
perror(a);
-   if (fcreated) {
-   PRIV_START;
+   if (fcreated)
unlink(atfile);
-   PRIV_END;
-   }
 
exit(EXIT_FAILURE);
 }
@@ -135,11 +128,8 @@ static void
 sigc(int signo)
 {
/* If the user presses ^C, remove the spool file and exit. */
-   if (fcreated) {
-   PRIV_START;
+   if (fcreated)
(void)unlink(atfile);
-   PRIV_END;
-   }
 
_exit(EXIT_FAILURE);
 }
@@ -204,8 +194,6 @@ writefile(const char *cwd, time_t runtim
act.sa_flags = 0;
sigaction(SIGINT, , NULL);
 
-   PRIV_START;
-
if ((lockdes = open(AT_DIR, O_RDONLY, 0)) < 0)
perr("Cannot open jobs dir");
 
@@ -241,11 +229,9 @@ writefile(const char *cwd, time_t runtim
if ((fd2 = dup(fdes)) < 0)
perr("Error in dup() of job file");
 
-   if (fchown(fd2, real_uid, real_gid) != 0)
+   if (fchown(fd2, user_uid, user_gid) != 0)
perr("Cannot give away file");
 
-   PRIV_END;
-
/*
 * We've successfully created the file; let's set the flag so it
 * gets removed in case of an interrupt or error.
@@ -268,7 +254,7 @@ writefile(const char *cwd, time_t runtim
 
if ((mailname == NULL) || (mailname[0] == '\0') ||
(strlen(mailname) > MAX_UNAME) || (getpwnam(mailname) == NULL)) {
-   pass_entry = getpwuid(real_uid);
+   pass_entry = getpwuid(user_uid);
if (pass_entry != NULL)
mailname = pass_entry->pw_name;
}
@@ -279,7 +265,7 @@ writefile(const char *cwd, time_t runtim
 * that, /bin/sh.
 */
if ((shell = getenv("SHELL")) == NULL || *shell == '\0') {
-   pass_entry = getpwuid(real_uid);
+   pass_entry = getpwuid(user_uid);
if (pass_entry != NULL && *pass_entry->pw_shell != '\0')
shell = pass_entry->pw_shell;
else
@@ -287,7 +273,7 @@ writefile(const char *cwd, time_t runtim
}
 
(void)fprintf(fp, "#!/bin/sh\n# atrun uid=%lu gid=%lu\n# mail %*s %d\n",
-   (unsigned long)real_uid, (unsigned long)real_gid,
+   (unsigned long)user_uid, (unsigned long)user_gid,
MAX_UNAME, mailname, send_mail);
 
/* Write out the umask at the time of invocation */
@@ -394,9 +380,7 @@ writefile(const char *cwd, time_t runtim
(void)close(fd2);
 
/* Poke cron so it knows to reload the at spool. */
-   PRIV_START;
poke_daemon(AT_DIR, RELOAD_AT);
-   PRIV_END;
 
runtime = *localtime();
strftime(timestr, TIMESIZE, "%a %b %e %T %Y", );
@@ -485,7 +469,7 @@ list_jobs(int argc, char **argv, int cou
for (i = 0; i < argc; i++) {
if ((pw = getpwnam(argv[i])) == NULL)
panic2(argv[i], ": invalid user name");
-   if (pw->pw_uid != real_uid && real_uid != 0)
+  

Re: Drop register keyword from less(1)

[Ted Unangst]

Todd C. Miller wrote:
> On Mon, 02 Nov 2015 09:16:07 +, Nicholas Marriott wrote:
> 
> > I looked briefly at this and it wouldn't be that hard. However, while it
> > would be fantastic to clean up all the crap from less, it isn't clear if
> > Garrett D'Amore is going to be keeping his fork up to date - if he
> > doesn't then we are then left with a much harder job to merge later
> > changes from the original less. Unless we are happy to fork and maintain
> > less ourselves without an upstream.
> 
> Would it really be so bad to consider less/more feature complete
> and only apply bug fixes?  The only place I expect large changes
> would be in the UTF-8 support.

I would say less has reached the point in its lifecycle where it can only get
bigger and worse, so there's some merit to putting it on a diet.

Re: cvs(1) simplification

[Nicholas Marriott]

Hi

Sure, so what are you saying... we shouldn't change this?

Or we should change it? If so, why not also change, for example,
cvs_add_entry, cvs_remove_local and update_clear_conflict, which have
very similar bits of code?




On Mon, Nov 02, 2015 at 03:38:24PM -0500, Michael McConville wrote:
> Nicholas Marriott wrote:
> > Sure, but this idiom is all over the place in opencvs, are you going to
> > change the rest?
> 
> As Theo mentioned recently, there's an inherent tradeoff here. Stack
> allocation is faster at runtime and easier to write. However, we miss
> out on malloc's memory sanitization. So, maybe we should only change it
> in performance-relevant areas where we're very confident in the safety
> of the program logic.
> 
> > On Mon, Nov 02, 2015 at 12:31:14PM -0500, Michael McConville wrote:
> > > Don't bother mallocing a statically-sized 1,024-byte chunk of mem, for
> > > simplicity and speed.
> > > 
> > > ok?
> > > 
> > > 
> > > Index: usr.bin/cvs/server.c
> > > ===
> > > RCS file: /cvs/src/usr.bin/cvs/server.c,v
> > > retrieving revision 1.102
> > > diff -u -p -r1.102 server.c
> > > --- usr.bin/cvs/server.c  16 Jan 2015 06:40:07 -  1.102
> > > +++ usr.bin/cvs/server.c  2 Nov 2015 17:17:13 -
> > > @@ -323,7 +323,7 @@ void
> > >  cvs_server_directory(char *data)
> > >  {
> > >   CVSENTRIES *entlist;
> > > - char *dir, *repo, *parent, *entry, *dirn, *p;
> > > + char *dir, *repo, *parent, entry[CVS_ENT_MAXLINELEN], *dirn, *p;
> > >  
> > >   if (current_cvsroot == NULL)
> > >   fatal("No Root specified for Directory");
> > > @@ -357,13 +357,11 @@ cvs_server_directory(char *data)
> > >   fatal("cvs_server_directory: %s", strerror(errno));
> > >  
> > >   if (strcmp(parent, ".")) {
> > > - entry = xmalloc(CVS_ENT_MAXLINELEN);
> > >   cvs_ent_line_str(dirn, NULL, NULL, NULL, NULL, 1, 0,
> > >   entry, CVS_ENT_MAXLINELEN);
> > >  
> > >   entlist = cvs_ent_open(parent);
> > >   cvs_ent_add(entlist, entry);
> > > - xfree(entry);
> > >   }
> > >  
> > >   if (server_currentdir != NULL)
> > > 

Re: whois(1) -I (whois.iana.org)

[Giovanni Bechis]

Stuart Henderson  ha scritto:
>This seems quite a useful database now that there are 500+ TLDs,
>OK to add a flag to use it more easily from whois(1)?
>
>Index: whois.1
>===
>RCS file: /cvs/src/usr.bin/whois/whois.1,v
>retrieving revision 1.33
>diff -u -p -r1.33 whois.1
>--- whois.19 Apr 2015 19:29:53 -   1.33
>+++ whois.12 Nov 2015 18:12:34 -
>@@ -38,7 +38,7 @@
> .Nd Internet domain name and network number directory service
> .Sh SYNOPSIS
> .Nm whois
>-.Op Fl AadgilmPQRr
>+.Op Fl AadgIilmPQRr
> .Oo
> .Fl c Ar country-code | Fl h Ar host
> .Oc
>@@ -156,6 +156,11 @@ or
> to the
> .Tn NIC
> handle in the query.)
>+.It Fl I
>+Use the Internet Assigned Numbers Authority
>+.Pq Tn whois.iana.org
>+root zone database.
>+It contains information about top-level domains.
> .It Fl l
> Use the Latin American and Caribbean IP address Regional Registry
> .Pq Tn LACNIC
>Index: whois.c
>===
>RCS file: /cvs/src/usr.bin/whois/whois.c,v
>retrieving revision 1.51
>diff -u -p -r1.51 whois.c
>--- whois.c2 Nov 2015 17:16:35 -   1.51
>+++ whois.c2 Nov 2015 18:12:34 -
>@@ -58,6 +58,7 @@
> #define   AFNICHOST   "whois.afrinic.net"
> #define BNICHOST  "whois.registro.br"
> #define   PDBHOST "whois.peeringdb.com"
>+#define   IANAHOST"whois.iana.org"
> #define   QNICHOST_TAIL   ".whois-servers.net"
> 
> #define   WHOIS_PORT  "whois"
>@@ -82,7 +83,7 @@ main(int argc, char *argv[])
> 
>   country = host = NULL;
>   flags = rval = 0;
>-  while ((ch = getopt(argc, argv, "aAc:dgh:ilmp:PqQrR")) != -1)
>+  while ((ch = getopt(argc, argv, "aAc:dgh:iIlmp:PqQrR")) != -1)
>   switch (ch) {
>   case 'a':
>   host = ANICHOST;
>@@ -105,6 +106,9 @@ main(int argc, char *argv[])
>   case 'i':
>   host = INICHOST;
>   break;
>+  case 'I':
>+  host = IANAHOST;
>+  break;
>   case 'l':
>   host = LNICHOST;
>   break;
>@@ -343,7 +347,7 @@ usage(void)
>   extern char *__progname;
> 
>   fprintf(stderr,
>-  "usage: %s [-AadgilmPQRr] [-c country-code | -h host] "
>+  "usage: %s [-AadgIilmPQRr] [-c country-code | -h host] "
>   "[-p port] name ...\n", __progname);
>   exit(1);
> }

Sure, ok.
  Cheers
Giovanni

Re: ftp(1): pledge smaller subset in SMALL version

[Jérémie Courrèges-Anglas]

Frederic Nowak  writes:

> Hi there,

Hi,

> at the moment ftp pledges "proc exec" in its SMALL version, but not
> otherwise. This seems wrong, because the SMALL version does not support
> interactive mode (which needs "proc exec" for e.g. the page command),
> while the !SMALL version does.
> The patch below switches the two pledges, so that the SMALL version
> pledges a smaller subset.

I can confirm that it is possible to have ftp(1) killed by pledge in the
non-SMALL build.

The diff looks correct and I doubt that there's a risk for it to
introduce further regressions.  However there's a popen() call in
recvrequest() that could trigger pledge protection.  If anyone wants to
make sense of that code, be my guest...

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: Drop register keyword from less(1)

[Nicholas Marriott]

On Mon, Nov 02, 2015 at 11:48:07AM -0500, Michael McConville wrote:
> Todd C. Miller wrote:
> > On Mon, 02 Nov 2015 09:16:07 +, Nicholas Marriott wrote:
> > > I looked briefly at this and it wouldn't be that hard. However,
> > > while it would be fantastic to clean up all the crap from less, it
> > > isn't clear if Garrett D'Amore is going to be keeping his fork up to
> > > date - if he doesn't then we are then left with a much harder job to
> > > merge later changes from the original less. Unless we are happy to
> > > fork and maintain less ourselves without an upstream.
> > 
> > Would it really be so bad to consider less/more feature complete and
> > only apply bug fixes?  The only place I expect large changes would be
> > in the UTF-8 support.
> 
> It would be nice to be able to clean it up some. I often muse on the
> fact that less is one of the first binaries to touch untrusted data on
> my machines, and that it's known to be messy code.

I think we should move to the Illumos fork, it looks good. I've got it
building easily enough, will take a look at porting our changes at some
point.

crontab: remove useless gid swapping

[Todd C. Miller]

The only place we really need to drop setgid is when opening an
arbitrary file, e.g. "crontab /foo/bar", which prevents the user
from viewing other people's crontab files.  Now that TMPDIR support
is gone things are a bit simpler...

 - todd

Index: crontab.c
===
RCS file: /cvs/src/usr.sbin/cron/crontab.c,v
retrieving revision 1.79
diff -u -p -u -r1.79 crontab.c
--- crontab.c   2 Nov 2015 20:09:02 -   1.79
+++ crontab.c   2 Nov 2015 20:21:48 -
@@ -314,15 +314,7 @@ edit_cmd(void)
fprintf(stderr, "path too long\n");
goto fatal;
}
-   if (setegid(user_gid) < 0) {
-   perror("setegid(user_gid)");
-   exit(EXIT_FAILURE);
-   }
t = mkstemp(Filename);
-   if (setegid(crontab_gid) < 0) {
-   perror("setegid(crontab_gid)");
-   exit(EXIT_FAILURE);
-   }
if (t == -1) {
perror(Filename);
goto fatal;
@@ -347,15 +339,7 @@ edit_cmd(void)
fprintf(stderr, "%s: error while writing new crontab to %s\n",
ProgramName, Filename);
  fatal:
-   if (setegid(user_gid) < 0) {
-   perror("setegid(user_gid)");
-   exit(EXIT_FAILURE);
-   }
unlink(Filename);
-   if (setegid(crontab_gid) < 0) {
-   perror("setegid(crontab_gid)");
-   exit(EXIT_FAILURE);
-   }
exit(EXIT_FAILURE);
}
 
@@ -376,19 +360,11 @@ edit_cmd(void)
goto fatal;
}
if (timespeccmp([1], _mtim, ==)) {
-   if (setegid(user_gid) < 0) {
-   perror("setegid(user_gid)");
-   exit(EXIT_FAILURE);
-   }
if (lstat(Filename, ) == 0 &&
statbuf.st_ino != xstatbuf.st_ino) {
fprintf(stderr, "%s: crontab temp file moved, editor "
   "may create backup files improperly\n", ProgramName);
}
-   if (setegid(crontab_gid) < 0) {
-   perror("setegid(crontab_gid)");
-   exit(EXIT_FAILURE);
-   }
fprintf(stderr, "%s: no changes made to crontab\n",
ProgramName);
goto remove;
@@ -429,15 +405,7 @@ edit_cmd(void)
goto fatal;
}
  remove:
-   if (setegid(user_gid) < 0) {
-   perror("setegid(user_gid)");
-   exit(EXIT_FAILURE);
-   }
unlink(Filename);
-   if (setegid(crontab_gid) < 0) {
-   perror("setegid(crontab_gid)");
-   exit(EXIT_FAILURE);
-   }
  done:
log_it(RealUser, Pid, "END EDIT", User);
 }

Re: em(4) watchdog timeouts

[Gregor Best]

On Mon, Nov 02, 2015 at 08:11:30PM +0100, Mark Kettenis wrote:
> Can those that are experiencing watchdog timeouts check if the diff
> below gets rid of them?
> [...]

Looks good so far. I've run a few light tests and the usual load that
caused the timeouts before, haven't seen any yet.

For the record, this is with

em0 at pci1 dev 0 function 0 "Intel 82583V" rev 0x00: msi, address 
00:03:2d:20:cf:84
em1 at pci2 dev 0 function 0 "Intel 82583V" rev 0x00: msi, address 
00:03:2d:20:cf:85
em2 at pci3 dev 0 function 0 "Intel 82583V" rev 0x00: msi, address 
00:03:2d:20:cf:86
em3 at pci4 dev 0 function 0 "Intel 82583V" rev 0x00: msi, address 
00:03:2d:20:cf:87

on i386 (GENERIC.MP).

-- 
Gregor

Re: Drop register keyword from less(1)

[Todd C. Miller]

On Mon, 02 Nov 2015 21:13:31 +, Nicholas Marriott wrote:

> I think we should move to the Illumos fork, it looks good. I've got it
> building easily enough, will take a look at porting our changes at some
> point.

Awesome.

 - todd

Re: should pledge(2) allow raise(3) and abort(3)?

[Ted Unangst]

Theo Buehler wrote:
> While playing with Daniel Micay's malloc patches, I ran into a lot of
> pledge aborts since pledge("stdio") disallows raise(3) and abort(3).
> That's because raise sends the to 'pid + THREAD_PID_OFFSET' instead
> of the pid itself.  The first sentence of the comment and the logic is
> taken from kern_sig.c.

I think this may be too tight. Intra-process signalling should be allowed, not
just killing self. I think the test should be

if (pid == 0 || pid == p->p_pid || pid > THREAD_PID_OFFSET)

There are checks in kern_sig.c that any thread signal must be in the same
process.

> 
> Index: /sys/kern/kern_pledge.c
> ===
> RCS file: /var/cvs/src/sys/kern/kern_pledge.c,v
> retrieving revision 1.97
> diff -u -p -r1.97 kern_pledge.c
> --- /sys/kern/kern_pledge.c   1 Nov 2015 19:03:33 -   1.97
> +++ /sys/kern/kern_pledge.c   2 Nov 2015 10:24:35 -
> @@ -1355,7 +1355,13 @@ pledge_kill(struct proc *p, pid_t pid)
>   return 0;
>   if (p->p_p->ps_pledge & PLEDGE_PROC)
>   return 0;
> - if (pid == 0 || pid == p->p_pid)
> + /*
> +  * If the target pid is > THREAD_PID_OFFSET then this
> +  * must be a kill of another thread in the same process.
> +  * This allows raise(3) and abort(3).
> +  */
> + if (pid == 0 || p->p_pid == (pid > THREAD_PID_OFFSET ?
> + pid - THREAD_PID_OFFSET : pid))
>   return 0;
>   return pledge_fail(p, EPERM, PLEDGE_PROC);
>  }
> 

Re: [PATCH] pledging dhclient

[Jérémie Courrèges-Anglas]

Loganaden Velvindron  writes:

> Hi guys,

Hi,

> I've been playing with pledge in base. Here's a small patch for dhclient.
> It's still a WiP.
>
> I can kill -HUP dhclient, and so far no issues.
>
> I would like it to pledge before however, so that write operations (write_*)
> that take their input from the network are further tightened down. One
> of the vulnerabilities in ISC dhcp was a stack overflow due to unchecked
> condititions when writing to files.
>
> I was thinking about pledging the privchild proces. Or that might be 
> overkill ?
>
> fork_privchld(int fd, int fd2) is calling dispatch_imsg() which contains the
> write operations to files.
>
>
> Feedback welcomed:

Sadly with this patch dhclient doesn't survive a suspend/resume or
a cable unplugging.  Having this solved first would be nice, before
thinking about further tightening.

Nit below,

> Index: dhclient.c
> ===
> RCS file: /cvs/src/sbin/dhclient/dhclient.c,v
> retrieving revision 1.365
> diff -u -p -r1.365 dhclient.c
> --- dhclient.c26 Oct 2015 16:32:33 -  1.365
> +++ dhclient.c2 Nov 2015 07:11:15 -
> @@ -64,6 +64,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  char *path_dhclient_conf = _PATH_DHCLIENT_CONF;
>  char *path_dhclient_db = NULL;
> @@ -595,6 +596,10 @@ main(int argc, char *argv[])
>   endpwent();
>  
>   setproctitle("%s", ifi->name);
> +
> + if (pledge("stdio dns route inet proc", NULL) == -1)
> + error("pledge");

Please use error("pledge: %s", strerror(errno))

> +
>   time(>startup_time);
>  
>   if (ifi->linkstat) {
>


-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: cvs(1) simplification

[Michael McConville]

Nicholas Marriott wrote:
> Sure, but this idiom is all over the place in opencvs, are you going to
> change the rest?

As Theo mentioned recently, there's an inherent tradeoff here. Stack
allocation is faster at runtime and easier to write. However, we miss
out on malloc's memory sanitization. So, maybe we should only change it
in performance-relevant areas where we're very confident in the safety
of the program logic.

> On Mon, Nov 02, 2015 at 12:31:14PM -0500, Michael McConville wrote:
> > Don't bother mallocing a statically-sized 1,024-byte chunk of mem, for
> > simplicity and speed.
> > 
> > ok?
> > 
> > 
> > Index: usr.bin/cvs/server.c
> > ===
> > RCS file: /cvs/src/usr.bin/cvs/server.c,v
> > retrieving revision 1.102
> > diff -u -p -r1.102 server.c
> > --- usr.bin/cvs/server.c16 Jan 2015 06:40:07 -  1.102
> > +++ usr.bin/cvs/server.c2 Nov 2015 17:17:13 -
> > @@ -323,7 +323,7 @@ void
> >  cvs_server_directory(char *data)
> >  {
> > CVSENTRIES *entlist;
> > -   char *dir, *repo, *parent, *entry, *dirn, *p;
> > +   char *dir, *repo, *parent, entry[CVS_ENT_MAXLINELEN], *dirn, *p;
> >  
> > if (current_cvsroot == NULL)
> > fatal("No Root specified for Directory");
> > @@ -357,13 +357,11 @@ cvs_server_directory(char *data)
> > fatal("cvs_server_directory: %s", strerror(errno));
> >  
> > if (strcmp(parent, ".")) {
> > -   entry = xmalloc(CVS_ENT_MAXLINELEN);
> > cvs_ent_line_str(dirn, NULL, NULL, NULL, NULL, 1, 0,
> > entry, CVS_ENT_MAXLINELEN);
> >  
> > entlist = cvs_ent_open(parent);
> > cvs_ent_add(entlist, entry);
> > -   xfree(entry);
> > }
> >  
> > if (server_currentdir != NULL)
> > 

Re: Drop register keyword from less(1)

[Nicholas Marriott]

On Sun, Nov 01, 2015 at 06:22:53PM -0700, Todd C. Miller wrote:
> If we are going to diverge from upstream less, a better starting
> point would be https://github.com/gdamore/less-fork
> 
> See also http://garrett.damore.org/2014_09_01_archive.html
> 
> If you decide to tackle that you'll also want to diff our less
> against the stock version to make sure we don't lose any local
> changes, of which there were a few.

I looked briefly at this and it wouldn't be that hard. However, while it
would be fantastic to clean up all the crap from less, it isn't clear if
Garrett D'Amore is going to be keeping his fork up to date - if he
doesn't then we are then left with a much harder job to merge later
changes from the original less. Unless we are happy to fork and maintain
less ourselves without an upstream.

IIRC our local changes are not huge and it is pretty easy to see them in
the diff (strlcpy, SMALL, less_is_more, a few other bits).

Re: IPv6 DHCP-PD/SLAAC - no /64 route in routing table

[Stuart Henderson]

On 2015/11/01 17:41, Yury Shefer wrote:
> I'm having trouble with enabling IPv6 routing on my 5.8 gateway.
> 
> (Internet)[DHCPv6+PD](em0-GW-axe0)[SLAAC/rtadvd]
> 
> My box is connected to Comcast, I'm getting IPv6 address assignment over
> DHCPv6 (wide dhcp6c) on WAN interface(em0) together with prefix delegation
> and assigning this prefix to axe0 (internal interface).
> 
> On axe0 i'm running rtadvd. My clients are getting IPv6 addresses properly
> assigned and i'm able to ping link-local address of my gw. but if I check
> ipv6 neighbors on my gw I see that there are no global addresses except
> permanent (GW-owned addresses).

This (static v6 addresses on a system using SLAAC) is broken in 5.8.

The following diff backported from -current may fix it, but is untested
(may not even compile, I don't have a handy 5.8 system and config(8) has
changed).

Index: sys/net/if_spppsubr.c
===
RCS file: /cvs/src/sys/net/if_spppsubr.c,v
retrieving revision 1.136
diff -u -p -r1.136 if_spppsubr.c
--- sys/net/if_spppsubr.c   18 Jul 2015 15:51:16 -  1.136
+++ sys/net/if_spppsubr.c   2 Nov 2015 09:20:03 -
@@ -4790,9 +4790,6 @@ sppp_set_ip6_addr(struct sppp *sp, const
 */
ifra->ifra_prefixmask.sin6_family = AF_UNSPEC;
 
-   /* DAD is redundant after an IPv6CP exchange. */
-   ifra->ifra_flags |= IN6_IFF_NODAD;
-
task_add(systq, >ipv6cp.set_addr_task);
 }
 
Index: sys/netinet6/in6.c
===
RCS file: /cvs/src/sys/netinet6/in6.c,v
retrieving revision 1.161
diff -u -p -r1.161 in6.c
--- sys/netinet6/in6.c  18 Jul 2015 15:05:32 -  1.161
+++ sys/netinet6/in6.c  2 Nov 2015 09:20:03 -
@@ -462,7 +462,6 @@ in6_control(struct socket *so, u_long cm
 
case SIOCAIFADDR_IN6:
{
-   struct nd_prefix *pr;
int plen, error = 0;
 
/* reject read-only flags */
@@ -492,39 +491,24 @@ in6_control(struct socket *so, u_long cm
break;
}
 
+   /* Perform DAD, if needed. */
+   if (ia6->ia6_flags & IN6_IFF_TENTATIVE)
+   nd6_dad_start(>ia_ifa);
+
plen = in6_mask2len(>ifra_prefixmask.sin6_addr, NULL);
if (plen == 128) {
dohooks(ifp->if_addrhooks, 0);
break;  /* we don't need to install a host route. */
}
 
-   /*
-* then, make the prefix on-link on the interface.
-* XXX: we'd rather create the prefix before the address, but
-* we need at least one address to install the corresponding
-* interface route, so we configure the address first.
-*/
-   pr = nd6_prefix_add(ifp, >ifra_addr,
-   >ifra_prefixmask, >ifra_lifetime,
-   ((ifra->ifra_flags & IN6_IFF_AUTOCONF) != 0));
-   if (pr == NULL) {
-   log(LOG_ERR, "cannot add prefix\n");
-   return (EINVAL); /* XXX panic here? */
-   }
-
-   /* relate the address to the prefix */
-   if (ia6->ia6_ndpr == NULL) {
-   ia6->ia6_ndpr = pr;
-   pr->ndpr_refcnt++;
-   }
-
s = splsoftnet();
-   /*
-* this might affect the status of autoconfigured addresses,
-* that is, this address might make other addresses detached.
-*/
-   pfxlist_onlink_check();
-
+   error = rt_ifa_add(>ia_ifa,
+   RTF_UP|RTF_CLONING|RTF_CONNECTED, ia6->ia_ifa.ifa_addr);
+   if (error) {
+   in6_purgeaddr(>ia_ifa);
+   splx(s);
+   return (error);
+   }
dohooks(ifp->if_addrhooks, 0);
splx(s);
break;
@@ -946,17 +930,6 @@ in6_update_ifa(struct ifnet *ifp, struct
LIST_INSERT_HEAD(>ia6_memberships, imm, i6mm_chain);
}
 
-   /*
-* Perform DAD, if needed.
-* XXX It may be of use, if we can administratively
-* disable DAD.
-*/
-   if (hostIsNew && in6if_do_dad(ifp) &&
-   (ifra->ifra_flags & IN6_IFF_NODAD) == 0)
-   {
-   nd6_dad_start(>ia_ifa, NULL);
-   }
-
return (error);
 
   unlink:
@@ -1022,24 +995,19 @@ in6_purgeaddr(struct ifaddr *ifa)
 void
 in6_unlink_ifa(struct in6_ifaddr *ia6, struct ifnet *ifp)
 {
+   struct ifaddr *ifa = >ia_ifa;
+
splsoftassert(IPL_SOFTNET);
 
-   ifa_del(ifp, >ia_ifa);
+   ifa_del(ifp, ifa);
 
TAILQ_REMOVE(_ifaddr, ia6, ia_list);
 
/* Release the reference to the base prefix. */
if (ia6->ia6_ndpr == NULL) {
-   char 

[PATCH] pledging dhclient

[Loganaden Velvindron]

Hi guys,

I've been playing with pledge in base. Here's a small patch for dhclient.
It's still a WiP.

I can kill -HUP dhclient, and so far no issues.

I would like it to pledge before however, so that write operations (write_*)
that take their input from the network are further tightened down. One
of the vulnerabilities in ISC dhcp was a stack overflow due to unchecked
condititions when writing to files.

I was thinking about pledging the privchild proces. Or that might be 
overkill ?

fork_privchld(int fd, int fd2) is calling dispatch_imsg() which contains the
write operations to files.


Feedback welcomed:

Index: dhclient.c
===
RCS file: /cvs/src/sbin/dhclient/dhclient.c,v
retrieving revision 1.365
diff -u -p -r1.365 dhclient.c
--- dhclient.c  26 Oct 2015 16:32:33 -  1.365
+++ dhclient.c  2 Nov 2015 07:11:15 -
@@ -64,6 +64,7 @@
 #include 
 #include 
 #include 
+#include 
 
 char *path_dhclient_conf = _PATH_DHCLIENT_CONF;
 char *path_dhclient_db = NULL;
@@ -595,6 +596,10 @@ main(int argc, char *argv[])
endpwent();
 
setproctitle("%s", ifi->name);
+
+   if (pledge("stdio dns route inet proc", NULL) == -1)
+   error("pledge");
+
time(>startup_time);
 
if (ifi->linkstat) {

Re: [PATCH] rcs: buf_free/rcsnum_free

[Nicholas Marriott]

Any other oks for this?



On Sun, Nov 01, 2015 at 08:58:27AM +0800, Michael W. Bombardieri wrote:
> Thanks again for checking this.
> Correcting ci.c ...
> 
> On Fri, Oct 30, 2015 at 03:53:58PM +, Nicholas Marriott wrote:
> > Sorry, the one I pointed out in ci.c is wrong:
> > 
> > >   rcs_close(pb.file);
> > > - if (rev_str != NULL)
> > > - rcsnum_free(pb.newrev);
> > > + rcsnum_free(pb.newrev);
> > >   pb.newrev = NULL;
> > 
> > pb.newrev can be changed by checkin_init or checkin_update, in which
> > case it shouldn't be freed, so this check needs to stay the same.
> > 
> > The rest is ok nicm
> 
> 
> Index: buf.c
> ===
> RCS file: /cvs/src/usr.bin/rcs/buf.c,v
> retrieving revision 1.25
> diff -u -p -u -r1.25 buf.c
> --- buf.c 13 Jun 2015 20:15:21 -  1.25
> +++ buf.c 1 Nov 2015 01:21:13 -
> @@ -138,6 +138,8 @@ out:
>  void
>  buf_free(BUF *b)
>  {
> + if (b == NULL)
> + return;
>   free(b->cb_buf);
>   free(b);
>  }
> Index: ci.c
> ===
> RCS file: /cvs/src/usr.bin/rcs/ci.c,v
> retrieving revision 1.222
> diff -u -p -u -r1.222 ci.c
> --- ci.c  5 Sep 2015 09:38:23 -   1.222
> +++ ci.c  1 Nov 2015 01:21:13 -
> @@ -369,12 +369,9 @@ checkin_diff_file(struct checkin_params 
>  
>   return (b3);
>  out:
> - if (b1 != NULL)
> - buf_free(b1);
> - if (b2 != NULL)
> - buf_free(b2);
> - if (b3 != NULL)
> - buf_free(b3);
> + buf_free(b1);
> + buf_free(b2);
> + buf_free(b3);
>   free(path1);
>   free(path2);
>  
> Index: diff3.c
> ===
> RCS file: /cvs/src/usr.bin/rcs/diff3.c,v
> retrieving revision 1.37
> diff -u -p -u -r1.37 diff3.c
> --- diff3.c   5 Sep 2015 09:47:08 -   1.37
> +++ diff3.c   1 Nov 2015 01:21:14 -
> @@ -234,14 +234,10 @@ merge_diff3(char **av, int flags)
>   warnx("warning: overlaps or other problems during merge");
>  
>  out:
> - if (b2 != NULL)
> - buf_free(b2);
> - if (b3 != NULL)
> - buf_free(b3);
> - if (d1 != NULL)
> - buf_free(d1);
> - if (d2 != NULL)
> - buf_free(d2);
> + buf_free(b2);
> + buf_free(b3);
> + buf_free(d1);
> + buf_free(d2);
>  
>   (void)unlink(path1);
>   (void)unlink(path2);
> @@ -354,14 +350,10 @@ rcs_diff3(RCSFILE *rf, char *workfile, R
>   warnx("warning: overlaps or other problems during merge");
>  
>  out:
> - if (b2 != NULL)
> - buf_free(b2);
> - if (b3 != NULL)
> - buf_free(b3);
> - if (d1 != NULL)
> - buf_free(d1);
> - if (d2 != NULL)
> - buf_free(d2);
> + buf_free(b2);
> + buf_free(b3);
> + buf_free(d1);
> + buf_free(d2);
>  
>   (void)unlink(path1);
>   (void)unlink(path2);
> Index: ident.c
> ===
> RCS file: /cvs/src/usr.bin/rcs/ident.c,v
> retrieving revision 1.30
> diff -u -p -u -r1.30 ident.c
> --- ident.c   2 Oct 2014 06:23:15 -   1.30
> +++ ident.c   1 Nov 2015 01:21:14 -
> @@ -156,8 +156,7 @@ ident_line(FILE *fp)
>  
>   found++;
>  out:
> - if (bp != NULL)
> - buf_free(bp);
> + buf_free(bp);
>  }
>  
>  __dead void
> Index: rcs.c
> ===
> RCS file: /cvs/src/usr.bin/rcs/rcs.c,v
> retrieving revision 1.83
> diff -u -p -u -r1.83 rcs.c
> --- rcs.c 13 Jun 2015 20:15:21 -  1.83
> +++ rcs.c 1 Nov 2015 01:21:14 -
> @@ -177,10 +177,8 @@ rcs_close(RCSFILE *rfp)
>   free(rlp);
>   }
>  
> - if (rfp->rf_head != NULL)
> - rcsnum_free(rfp->rf_head);
> - if (rfp->rf_branch != NULL)
> - rcsnum_free(rfp->rf_branch);
> + rcsnum_free(rfp->rf_head);
> + rcsnum_free(rfp->rf_branch);
>  
>   if (rfp->rf_file != NULL)
>   fclose(rfp->rf_file);
> @@ -1406,10 +1404,8 @@ rcs_freedelta(struct rcs_delta *rdp)
>  {
>   struct rcs_branch *rb;
>  
> - if (rdp->rd_num != NULL)
> - rcsnum_free(rdp->rd_num);
> - if (rdp->rd_next != NULL)
> - rcsnum_free(rdp->rd_next);
> + rcsnum_free(rdp->rd_num);
> + rcsnum_free(rdp->rd_next);
>  
>   free(rdp->rd_author);
>   free(rdp->rd_locker);
> Index: rcsclean.c
> ===
> RCS file: /cvs/src/usr.bin/rcs/rcsclean.c,v
> retrieving revision 1.54
> diff -u -p -u -r1.54 rcsclean.c
> --- rcsclean.c16 Jan 2015 06:40:11 -  1.54
> +++ rcsclean.c1 Nov 2015 01:21:14 -
> @@ -209,10 +209,8 @@ rcsclean_file(char *fname, const char *r
>   rcs_set_mtime(file, 

Re: Drop register keyword from less(1)

[Alexandr Shadchin]

On Mon, Nov 2, 2015 at 2:16 PM, Nicholas Marriott <
nicholas.marri...@gmail.com> wrote:

> On Sun, Nov 01, 2015 at 06:22:53PM -0700, Todd C. Miller wrote:
> > If we are going to diverge from upstream less, a better starting
> > point would be https://github.com/gdamore/less-fork
> >
> > See also http://garrett.damore.org/2014_09_01_archive.html
> >
> > If you decide to tackle that you'll also want to diff our less
> > against the stock version to make sure we don't lose any local
> > changes, of which there were a few.
>
> I looked briefly at this and it wouldn't be that hard. However, while it
> would be fantastic to clean up all the crap from less, it isn't clear if
> Garrett D'Amore is going to be keeping his fork up to date - if he
> doesn't then we are then left with a much harder job to merge later
> changes from the original less. Unless we are happy to fork and maintain
> less ourselves without an upstream.
>
> IIRC our local changes are not huge and it is pretty easy to see them in
> the diff (strlcpy, SMALL, less_is_more, a few other bits).
>
>
Our patches https://github.com/shadchin/less/commits/openbsd for less-471
(man from -current, without merge changes less-471)

-- 
Alexandr Shadchin

Re: cvs(1) simplification

[Tobias Stoeckmann]

I wouldn't call this definition readable:

void
cvs_ent_line_str(const char *name, char *rev, char *tstamp, char *opts,
char *sticky, int isdir, int isremoved, char *buf, size_t len)

So what about changing it to return allocated memory by itself,
which would mean changing the internals from xsnprintf() to xasprintf().

Removes the need for two arguments. And the code does no error handling
currently, anyway.

char *
cvs_ent_line_str(const char *name, char *rev, char *tstamp, char *opts,
char *sticky, int isdir, int isremoved)

I can tell that rev can be VERY large... Wait, am I remembering these
(Open)CVS internals all of a sudden again? :P

Re: Drop register keyword from less(1)

[Theo de Raadt]

> I think we should move to the Illumos fork, it looks good. I've got it
> building easily enough, will take a look at porting our changes at some
> point.

I was the first one to bring it up with Todd about 4 weeks ago.
I am a big fan of this, becuase it will allow us to pledge^Wrefactor
it also.

Re: enhanced use-after-free detection for malloc v2

[Daniel Micay]

On 02/11/15 06:40 AM, Theo Buehler wrote:
> Sorry for this rather long mail:
> 
> I have three small comments on the patch itself
> (starting 80 lines below).
> 
> For those who want to try both new features, I attached a patch against
> -current that merges the three parts of Daniel's diff (plus the trivial
> two of the nits below) at the very end of this mail.
> 
> In addition to Daniel's test program, here's a small program that
> in 1000 test runs on amd64 always triggered an abort at the second
> free().
> 
> 
> $ cat heap.c
> #include 
> #include 
> #include 
> #include 
> 
> int
> main(void)
> {
>   char *p;
> 
>   p = malloc(1);
>   if (p == NULL)
>   err(1, "malloc");
> 
>   p = strcpy(p, "1234567");
>   printf("%s\n", p);
>   free(p);
> 
>   p = malloc(1);
>   p = strcpy(p, "12345678");
>   printf("%s\n", p);
>   free(p);
> 
>   return 0;
> }
> $ cc -o heap heap.c
> /tmp//ccHH1E1h.o: In function `main': heap.c:(.text+0x6b): warning: warning: 
> strcpy() is almost always
> misused, please use strlcpy()
> $ ./heap
> 1234567
> 12345678
> heap(5992) in free(): error: chunk canary corrupted 0x1f4619f0e9c0
> Abort trap (core dumped)
> $
> 
> 
> The performance impact of MALLOC_OPTIONS='CSV' is very noticeable,
> but 'CV' is negligible and 'CJV' makes the system feel a bit less
> snappy on my machine, but the actual effect seems almost negligible.
> 
> Here's a rough 'benchmark':
> 
> Building the base system on my i7 Thinkpad T420 with 4G RAM,
> with the various malloc options each with a freshly booted system.
> 
> defaults: 31m04
> CV:   33m16
> CJV:  33m46
> CSV:  53m32
> 
> Unfortunately, I have yet to hit an actual bug with this, one that isn't
> triggered by simple test programs.  Casual browsing with firefox, even
> watching movies with mplayer is possible with MALLOC_OPTIONS='CSV'.

It would help quite a bit to bump up the size of the quarantine. The
default 16 large one doesn't keep around the allocations for very long.

A more expensive version of the feature could also check for either zero
or junk data before allocating anything. Haven't decided how to expose
that via the option though.

Most bugs triggering in fairly normal situations with Firefox, etc. have
probably already been found by Valgrind though. The strength of features
like this in malloc is that they can enabled all the time with little
performance loss, so eventually they'll find new bugs (which has been
the case for us on Android, but perhaps that's mostly because many apps
were never tested at all with ASAN/Valgrind).

It may also be viable as a security feature in some situations, not just
a way of finding bugs. Depends on how far away the UAF is from the free
call since one other free is all that's needed to lose reliable
detection. It might work better with a FIFO ring buffer rather than the
current fully randomized array (perhaps a mix? dunno).

> On Sun, Nov 01, 2015 at 02:56:11PM -0500, Daniel Micay wrote:
>> @@ -560,6 +563,12 @@ omalloc_init(struct dir_info **dp)
>>  case 'J':
>>  mopts.malloc_junk = 2;
>>  break;
>> +case 'v':
>> +mopts.malloc_validate = 0;
>> +break;
>> +case 'V':
>> +mopts.malloc_validate = 1;
>> +break;
>>  case 'n':
>>  case 'N':
>>  break;
> 
> I'd keep the alphabetical order in the switch.
> 
>> @@ -608,6 +617,9 @@ omalloc_init(struct dir_info **dp)
>>  }
>>  }
>>  
>> +if (!mopts.malloc_junk)
>> +mopts.malloc_validate = 0;
>> +
> 
> Wouldn't it make sense to just let 'V' imply 'J' (and 'j' imply 'v') as
> is already done with 'F' and 'U', respectively?

The V option doesn't benefit from J since it's only able to check for
use-after-frees in small allocations. I don't think there's a good
reason to tie them together.

A side note is that it would be nice if junk freeing was split up into
the security feature (full junk fill on free) and the debugging feature
(full junk fill on init). I ended up doing that in CopperheadOS because
it makes J significantly cheaper without giving up the nice security
properties gained by sanitizing everything on free. Might be too late to
change the meaning in OpenBSD though.

>> @@ -1190,6 +1208,35 @@ malloc(size_t size)
>>  /*DEF_STRONG(malloc);*/
>>  
>>  static void
>> +validate_junk(void *p) {
>> +struct region_info *r;
>> +struct dir_info *pool = getpool();
>> +size_t byte, sz;
>> +if (p == NULL)
>> +return;
>> +r = find(pool, p);
>> +if (r == NULL)
>> +wrterror("bogus pointer in validate_junk", p);
>> +REALSIZE(sz, r);
>> +for (byte = 0; byte < sz; byte++) {
>> +if (((char *)p)[byte] != 

Re: cvs(1) simplification

[Theo de Raadt]

> I can tell that rev can be VERY large... Wait, am I remembering these
> (Open)CVS internals all of a sudden again? :P

Some of us have memories wired for guilt.

Re: pledge idea

[Peter J. Philipp]

On Thu, Oct 29, 2015 at 06:39:58PM +0100, Peter J. Philipp wrote:
> Hi Reyk,
> 
> deraadt already told me there was a patch for this already.  Yes it
> would be more cycles for stdio I see that.
> 
> Thanks for your effort in making me see this.
> 
> -peter
> 
> > $ time obj/sleep 0.01 
> > 0m00.01s real 0m00.00s user 0m00.01s system
> >
> > Are you trying to solve an issue?

Hi,

I just want to revisit this because I couldn't believe it.  I turned on
system accounting and rebooted my test box.  Here is what I found the following
programs were run this many times:

23 sh
10 ntpd
 9 smtpd
 7 domainname
 6 id
 6 getty
 6 getcap
 6 basename

I'm gonna stop here.  Some of these programs were not pledged yet in my sources
(-current from last week).  

So I did the tedious work of adding up the cycles of pledge/strcmp on sh binary
and then gave the bsearch idea a guessed average of 6 rounds per lookup.  What 
I came up with was that pledge/strcmp has 120 cycles where bsearch had 60 
cycles.  Multiplied by 23 times would give pledge/strcmp 2760 cycles and 
bsearch 1380 cycles.  /bin/sh is probably always going to be the most used in
any system, well it is at startup.

Another comparison was for getty:
pledge/strcmp 420 cycles
bsearch = 216 cycles

getcap had stdio and rpath which is 3 cycles, here it wins against bsearch
which has 12 cycles.

I do understand that most little programs such as basename have only stdio
in pledge and thus will save cycles but when you average it all out by 
occurences there might be a case for using bsearch?

I know it was just halloween but I'm still creeped out by this.

-peter

Re: enhanced use-after-free detection for malloc v2

[Theo Buehler]

Sorry for this rather long mail:

I have three small comments on the patch itself
(starting 80 lines below).

For those who want to try both new features, I attached a patch against
-current that merges the three parts of Daniel's diff (plus the trivial
two of the nits below) at the very end of this mail.

In addition to Daniel's test program, here's a small program that
in 1000 test runs on amd64 always triggered an abort at the second
free().


$ cat heap.c
#include 
#include 
#include 
#include 

int
main(void)
{
char *p;

p = malloc(1);
if (p == NULL)
err(1, "malloc");

p = strcpy(p, "1234567");
printf("%s\n", p);
free(p);

p = malloc(1);
p = strcpy(p, "12345678");
printf("%s\n", p);
free(p);

return 0;
}
$ cc -o heap heap.c
/tmp//ccHH1E1h.o: In function `main': heap.c:(.text+0x6b): warning: warning: 
strcpy() is almost always
misused, please use strlcpy()
$ ./heap
1234567
12345678
heap(5992) in free(): error: chunk canary corrupted 0x1f4619f0e9c0
Abort trap (core dumped)
$


The performance impact of MALLOC_OPTIONS='CSV' is very noticeable,
but 'CV' is negligible and 'CJV' makes the system feel a bit less
snappy on my machine, but the actual effect seems almost negligible.

Here's a rough 'benchmark':

Building the base system on my i7 Thinkpad T420 with 4G RAM,
with the various malloc options each with a freshly booted system.

defaults:   31m04
CV: 33m16
CJV:33m46
CSV:53m32

Unfortunately, I have yet to hit an actual bug with this, one that isn't
triggered by simple test programs.  Casual browsing with firefox, even
watching movies with mplayer is possible with MALLOC_OPTIONS='CSV'.


On Sun, Nov 01, 2015 at 02:56:11PM -0500, Daniel Micay wrote:
> @@ -560,6 +563,12 @@ omalloc_init(struct dir_info **dp)
>   case 'J':
>   mopts.malloc_junk = 2;
>   break;
> + case 'v':
> + mopts.malloc_validate = 0;
> + break;
> + case 'V':
> + mopts.malloc_validate = 1;
> + break;
>   case 'n':
>   case 'N':
>   break;

I'd keep the alphabetical order in the switch.

> @@ -608,6 +617,9 @@ omalloc_init(struct dir_info **dp)
>   }
>   }
>  
> + if (!mopts.malloc_junk)
> + mopts.malloc_validate = 0;
> +

Wouldn't it make sense to just let 'V' imply 'J' (and 'j' imply 'v') as
is already done with 'F' and 'U', respectively?

> @@ -1190,6 +1208,35 @@ malloc(size_t size)
>  /*DEF_STRONG(malloc);*/
>  
>  static void
> +validate_junk(void *p) {
> + struct region_info *r;
> + struct dir_info *pool = getpool();
> + size_t byte, sz;
> + if (p == NULL)
> + return;
> + r = find(pool, p);
> + if (r == NULL)
> + wrterror("bogus pointer in validate_junk", p);
> + REALSIZE(sz, r);
> + for (byte = 0; byte < sz; byte++) {
> + if (((char *)p)[byte] != SOME_FREEJUNK) {

This cast should be an (unsigned char *):
we have SOME_FREEJUNK == 0xdf, so '((char *)p)[byte] != SOME_FREEJUNK'
will always be true (this is pointed out when compiling with both clang
and gcc on amd64).

Index: stdlib/malloc.c
===
RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.176
diff -u -p -r1.176 malloc.c
--- stdlib/malloc.c 13 Sep 2015 20:29:23 -  1.176
+++ stdlib/malloc.c 2 Nov 2015 09:52:32 -
@@ -182,15 +182,18 @@ struct malloc_readonly {
int malloc_freeunmap;   /* mprotect free pages PROT_NONE? */
int malloc_hint;/* call madvice on free pages?  */
int malloc_junk;/* junk fill? */
+   int malloc_validate;/* validate junk */
int malloc_move;/* move allocations to end of page? */
int malloc_realloc; /* always realloc? */
int malloc_xmalloc; /* xmalloc behaviour? */
+   size_t  malloc_canaries;/* use canaries after chunks? */
size_t  malloc_guard;   /* use guard pages after allocations? */
u_int   malloc_cache;   /* free pages we cache */
 #ifdef MALLOC_STATS
int malloc_stats;   /* dump statistics at end */
 #endif
u_int32_t malloc_canary;/* Matched against ones in malloc_pool 
*/
+   uintptr_t malloc_chunk_canary;
 };
 
 /* This object is mapped PROT_READ after initialisation to prevent tampering */
@@ -218,6 +221,8 @@ static void malloc_exit(void);
 #define CALLER NULL
 #endif
 
+static void validate_delayed_chunks(void);
+
 /* low bits of r->p determine size: 0 means >= page size and p->size holding
  *  

clean up ppp(4) "fastq"

[David Gwynne]

ppp(4) had code to prioritise IP packets with the low delay type of service set.

maintaining this complicates refactoring the interface send queue,
which im doing on the way to mpsafety for that side of the stack.
id argue the functionality can be better implemented by using queues
in pf.

i can't test this so id appreciate a test report. or an ok.

Index: if_ppp.c
===
RCS file: /cvs/src/sys/net/if_ppp.c,v
retrieving revision 1.91
diff -u -p -r1.91 if_ppp.c
--- if_ppp.c25 Oct 2015 11:58:11 -  1.91
+++ if_ppp.c2 Nov 2015 11:34:05 -
@@ -163,7 +163,6 @@ struct mbuf *   ppp_pkt_mbuf(struct ppp_pk
  * We steal two bits in the mbuf m_flags, to mark high-priority packets
  * for output, and received packets following lost/corrupted packets.
  */
-#define M_HIGHPRI  M_PROTO1/* output packet for sc_fastq */
 #define M_ERRMARK  M_LINK0 /* steal a bit in mbuf m_flags */
 
 
@@ -226,7 +225,6 @@ ppp_clone_create(struct if_clone *ifc, i
 sc->sc_if.if_rtrequest = p2p_rtrequest;
 IFQ_SET_MAXLEN(>sc_if.if_snd, IFQ_MAXLEN);
 mq_init(>sc_inq, IFQ_MAXLEN, IPL_NET);
-IFQ_SET_MAXLEN(>sc_fastq, IFQ_MAXLEN);
 ppp_pkt_list_init(>sc_rawq, IFQ_MAXLEN);
 IFQ_SET_READY(>sc_if.if_snd);
 if_attach(>sc_if);
@@ -319,14 +317,7 @@ pppdealloc(struct ppp_softc *sc)
 sc->sc_xfer = 0;
 while ((pkt = ppp_pkt_dequeue(>sc_rawq)) != NULL)
ppp_pkt_free(pkt);
-while ((m = mq_dequeue(>sc_inq)) != NULL)
-   m_freem(m);
-for (;;) {
-   IF_DEQUEUE(>sc_fastq, m);
-   if (m == NULL)
-   break;
-   m_freem(m);
-}
+mq_purge(>sc_inq);
 while ((m = sc->sc_npqueue) != NULL) {
sc->sc_npqueue = m->m_nextpkt;
m_freem(m);
@@ -657,8 +648,6 @@ pppoutput(struct ifnet *ifp, struct mbuf
 int protocol, address, control;
 u_char *cp;
 int s, error;
-struct ip *ip;
-struct ifqueue *ifq;
 enum NPmode mode;
 int len;
 
@@ -679,21 +668,12 @@ pppoutput(struct ifnet *ifp, struct mbuf
 /*
  * Compute PPP header.
  */
-m0->m_flags &= ~M_HIGHPRI;
 switch (dst->sa_family) {
 case AF_INET:
address = PPP_ALLSTATIONS;
control = PPP_UI;
protocol = PPP_IP;
mode = sc->sc_npmode[NP_IP];
-
-   /*
-* If this packet has the "low delay" bit set in the IP header,
-* put it on the fastq instead.
-*/
-   ip = mtod(m0, struct ip *);
-   if (ip->ip_tos & IPTOS_LOWDELAY)
-   m0->m_flags |= M_HIGHPRI;
break;
 case AF_UNSPEC:
address = PPP_ADDRESS(dst->sa_data);
@@ -792,19 +772,7 @@ pppoutput(struct ifnet *ifp, struct mbuf
m0->m_nextpkt = NULL;
sc->sc_npqtail = >m_nextpkt;
 } else {
-   if (m0->m_flags & M_HIGHPRI) {
-   ifq = >sc_fastq;
-   if (IF_QFULL(ifq) && dst->sa_family != AF_UNSPEC) {
-   IF_DROP(ifq);
-   m_freem(m0);
-   error = ENOBUFS;
-   }
-   else {
-   IF_ENQUEUE(ifq, m0);
-   error = 0;
-   }
-   } else
-   IFQ_ENQUEUE(>sc_if.if_snd, m0, error);
+   IFQ_ENQUEUE(>sc_if.if_snd, m0, error);
if (error) {
splx(s);
sc->sc_if.if_oerrors++;
@@ -833,7 +801,6 @@ static void
 ppp_requeue(struct ppp_softc *sc)
 {
 struct mbuf *m, **mpp;
-struct ifqueue *ifq;
 enum NPmode mode;
 int error;
 
@@ -855,19 +822,7 @@ ppp_requeue(struct ppp_softc *sc)
 */
*mpp = m->m_nextpkt;
m->m_nextpkt = NULL;
-   if (m->m_flags & M_HIGHPRI) {
-   ifq = >sc_fastq;
-   if (IF_QFULL(ifq)) {
-   IF_DROP(ifq);
-   m_freem(m);
-   error = ENOBUFS;
-   }
-   else {
-   IF_ENQUEUE(ifq, m);
-   error = 0;
-   }
-   } else
-   IFQ_ENQUEUE(>sc_if.if_snd, m, error);
+   IFQ_ENQUEUE(>sc_if.if_snd, m, error);
if (error) {
sc->sc_if.if_oerrors++;
sc->sc_stats.ppp_oerrors++;
@@ -919,9 +874,7 @@ ppp_dequeue(struct ppp_softc *sc)
  * Grab a packet to send: first try the fast queue, then the
  * normal queue.
  */
-IF_DEQUEUE(>sc_fastq, m);
-if (m == NULL)
-   IFQ_DEQUEUE(>sc_if.if_snd, m);
+IFQ_DEQUEUE(>sc_if.if_snd, m);
 if (m == NULL)
   return NULL;
 
@@ -1048,8 +1001,7 @@ pppintr(void)
 
 LIST_FOREACH(sc, _softc_list, sc_list) {
if (!(sc->sc_flags & SC_TBUSY)
-   && (!IFQ_IS_EMPTY(>sc_if.if_snd) ||
-   !IFQ_IS_EMPTY(>sc_fastq))) {
+   && (!IFQ_IS_EMPTY(>sc_if.if_snd))) {
s = splnet();
sc->sc_flags |= SC_TBUSY;
splx(s);

should pledge(2) allow raise(3) and abort(3)?

[Theo Buehler]

While playing with Daniel Micay's malloc patches, I ran into a lot of
pledge aborts since pledge("stdio") disallows raise(3) and abort(3).
That's because raise sends the to 'pid + THREAD_PID_OFFSET' instead
of the pid itself.  The first sentence of the comment and the logic is
taken from kern_sig.c.

Index: /sys/kern/kern_pledge.c
===
RCS file: /var/cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.97
diff -u -p -r1.97 kern_pledge.c
--- /sys/kern/kern_pledge.c 1 Nov 2015 19:03:33 -   1.97
+++ /sys/kern/kern_pledge.c 2 Nov 2015 10:24:35 -
@@ -1355,7 +1355,13 @@ pledge_kill(struct proc *p, pid_t pid)
return 0;
if (p->p_p->ps_pledge & PLEDGE_PROC)
return 0;
-   if (pid == 0 || pid == p->p_pid)
+   /*
+* If the target pid is > THREAD_PID_OFFSET then this
+* must be a kill of another thread in the same process.
+* This allows raise(3) and abort(3).
+*/
+   if (pid == 0 || p->p_pid == (pid > THREAD_PID_OFFSET ?
+   pid - THREAD_PID_OFFSET : pid))
return 0;
return pledge_fail(p, EPERM, PLEDGE_PROC);
 }

Re: snmpd loses ARP table information

[Mike Belopuhov]

On Mon, Nov 02, 2015 at 13:36 +0100, Gerhard Roth wrote:
> Hi,
> 
> snmpd pernanently loses its ARP table information:
> 
>   # snmpctl walk 127.0.0.1 oid ipNetToMediaPhysAddress
>   ipNetToMediaPhysAddress.2.192.168.16.1="xx:xx:xx:xx:xx:xx"
>   ipNetToMediaPhysAddress.2.192.168.16.126="xx:xx:xx:xx:xx:xx"
>   ipNetToMediaPhysAddress.2.192.168.19.132="xx:xx:xx:xx:xx:xx"
> 
> Query the whole tree and this information is gone:
> 
>   # snmpctl walk 127.0.0.1 oid iso.org>/dev/null
>   # snmpctl walk 127.0.0.1 oid ipNetToMediaPhysAddress
>   0=6
> 
> The reason is that several OIDs in mib.c call kr_updateif()
> and this function deletes the kif_node and the restores it
> via fetchifs(). But then the ARP information held by the
> old kif_node is gone.
> 
> There is no need to delete the kif_node prior to calling
> fetchifs() since it only calls rtmsg_process() which is
> also called during normal processing of messages from the
> routing socket.
> 
> And while here, also handle RTM_DESYNC messages.
> 
> Gerhard
>

I'm surprised this didn't show up before. Your diff looks
good to me.  FWIW, OK mikeb

> 
> Index: usr.sbin/snmpd/kroute.c
> ===
> RCS file: /cvs/src/usr.sbin/snmpd/kroute.c,v
> retrieving revision 1.31
> diff -u -p -u -p -r1.31 kroute.c
> --- usr.sbin/snmpd/kroute.c   18 Jul 2015 00:27:32 -  1.31
> +++ usr.sbin/snmpd/kroute.c   2 Nov 2015 11:41:51 -
> @@ -365,12 +365,6 @@ kr_iflastchange(void)
>  int
>  kr_updateif(u_int if_index)
>  {
> - struct kif_node *kn;
> -
> - if ((kn = kif_find(if_index)) != NULL)
> - kif_remove(kn);
> -
> - /* Do not update the interface address list */
>   return (fetchifs(if_index));
>  }
>  
> @@ -1380,6 +1374,12 @@ rtmsg_process(char *buf, int len)
>   break;
>   case RTM_IFANNOUNCE:
>   if_announce(next);
> + break;
> + case RTM_DESYNC:
> + kr_shutdown();
> + if (fetchifs(0) == -1)
> + fatalx("rtmsg_process: fetchifs");
> + ktable_init();
>   break;
>   default:
>   /* ignore for now */
> 

Re: em(4) watchdog timeouts

[Sonic]

On Mon, Nov 2, 2015 at 2:11 PM, Mark Kettenis  wrote:
> Can those that are experiencing watchdog timeouts check if the diff
> below gets rid of them?

Sorry to report that the diff does not solve the timeout problem here.

All was working fine with the if_em* versions from 2015/09/29 (I
downgraded to this version per Stuarts post on 10-14):
"try backing out the last commits to
if_em.c and if_em.h ("cd /sys/dev/pci; cvs up -D 2015/09/29 if_em*") to
see if it makes a difference."

However, that version no longer compiles with -current and the
watchdog timeouts are back (even with the diff).

$ dmesg
OpenBSD 5.8-current (GENERIC.MP) #12: Mon Nov  2 20:29:08 EST 2015
   r...@stargate.grizzly.bear:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 4277665792 (4079MB)
avail mem = 4143894528 (3951MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.6 @ 0x9f000 (19 entries)
bios0: vendor American Megatrends Inc. version "1.2b" date 07/19/13
bios0: Supermicro X7SPA-HF
acpi0 at bios0: rev 2
acpi0: sleep states S0 S1 S4 S5
acpi0: tables DSDT FACP APIC MCFG OEMB HPET EINJ BERT ERST HEST
acpi0: wakeup devices P0P1(S4) PS2K(S4) PS2M(S4) USB0(S4) USB1(S4)
USB2(S4) USB5(S4) EUSB(S4) USB3(S4) USB4(S4) USB6(S4) USBE(S4) P0P4(S
4) P0P5(S4) P0P6(S4) P0P7(S4) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Atom(TM) CPU D525 @ 1.80GHz, 1800.24 MHz
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,DTES64
,MWAIT,DS-CPL,TM2,SSSE3,CX16,xTPR,PDCM,MOVBE,NXE,LONG,LAHF,PERF,SENSOR
cpu0: 512KB 64b/line 8-way L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
cpu0: apic clock running at 199MHz
cpu0: mwait min=64, max=64, C-substates=0.1, IBE
cpu1 at mainbus0: apid 2 (application processor)
cpu1: Intel(R) Atom(TM) CPU D525 @ 1.80GHz, 1800.00 MHz
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,DTES64
,MWAIT,DS-CPL,TM2,SSSE3,CX16,xTPR,PDCM,MOVBE,NXE,LONG,LAHF,PERF,SENSOR
cpu1: 512KB 64b/line 8-way L2 cache
cpu1: smt 0, core 1, package 0
ioapic0 at mainbus0: apid 3 pa 0xfec0, version 20, 24 pins
ioapic0: misconfigured as apic 1, remapped to apid 3
acpimcfg0 at acpi0 addr 0xe000, bus 0-255
acpihpet0 at acpi0: 14318179 Hz
acpiprt0 at acpi0: bus 0 (PCI0)
acpiprt1 at acpi0: bus 4 (P0P1)
acpiprt2 at acpi0: bus 1 (P0P4)
acpiprt3 at acpi0: bus 2 (P0P8)
acpiprt4 at acpi0: bus 3 (P0P9)
acpicpu0 at acpi0: C1(@1 halt!)
acpicpu1 at acpi0: C1(@1 halt!)
acpibtn0 at acpi0: SLPB
acpibtn1 at acpi0: PWRB
ipmi at mainbus0 not configured
pci0 at mainbus0 bus 0
pchb0 at pci0 dev 0 function 0 "Intel Pineview DMI" rev 0x02
uhci0 at pci0 dev 26 function 0 "Intel 82801I USB" rev 0x02: apic 3 int 16
uhci1 at pci0 dev 26 function 1 "Intel 82801I USB" rev 0x02: apic 3 int 21
uhci2 at pci0 dev 26 function 2 "Intel 82801I USB" rev 0x02: apic 3 int 19
ehci0 at pci0 dev 26 function 7 "Intel 82801I USB" rev 0x02: apic 3 int 18
usb0 at ehci0: USB revision 2.0
uhub0 at usb0 "Intel EHCI root hub" rev 2.00/1.00 addr 1
ppb0 at pci0 dev 28 function 0 "Intel 82801I PCIE" rev 0x02: msi
pci1 at ppb0 bus 1
ppb1 at pci0 dev 28 function 4 "Intel 82801I PCIE" rev 0x02: msi
pci2 at ppb1 bus 2
em0 at pci2 dev 0 function 0 "Intel 82574L" rev 0x00: msi, address
00:25:90:92:d4:f8
ppb2 at pci0 dev 28 function 5 "Intel 82801I PCIE" rev 0x02: msi
pci3 at ppb2 bus 3
em1 at pci3 dev 0 function 0 "Intel 82574L" rev 0x00: msi, address
00:25:90:92:d4:f9
uhci3 at pci0 dev 29 function 0 "Intel 82801I USB" rev 0x02: apic 3 int 23
uhci4 at pci0 dev 29 function 1 "Intel 82801I USB" rev 0x02: apic 3 int 19
uhci5 at pci0 dev 29 function 2 "Intel 82801I USB" rev 0x02: apic 3 int 18
ehci1 at pci0 dev 29 function 7 "Intel 82801I USB" rev 0x02: apic 3 int 23
usb1 at ehci1: USB revision 2.0
uhub1 at usb1 "Intel EHCI root hub" rev 2.00/1.00 addr 1
ppb3 at pci0 dev 30 function 0 "Intel 82801BA Hub-to-PCI" rev 0x92
pci4 at ppb3 bus 4
vga1 at pci4 dev 4 function 0 "Matrox MGA G200eW" rev 0x0a
wsdisplay0 at vga1 mux 1: console (80x25, vt100 emulation)
wsdisplay0: screen 1-5 added (80x25, vt100 emulation)
pcib0 at pci0 dev 31 function 0 "Intel 82801IR LPC" rev 0x02
ahci0 at pci0 dev 31 function 2 "Intel 82801I AHCI" rev 0x02: msi, AHCI 1.2
ahci0: port 0: 3.0Gb/s
scsibus1 at ahci0: 32 targets
sd0 at scsibus1 targ 0 lun 0:  SCSI3
0/direct fixed naa.5001517959323666
sd0: 76319MB, 512 bytes/sector, 156301488 sectors, thin
ichiic0 at pci0 dev 31 function 3 "Intel 82801I SMBus" rev 0x02: apic 3 int 18
iic0 at ichiic0
lm1 at iic0 addr 0x2d: W83627DHG
spdmem0 at iic0 addr 0x50: 2GB DDR3 SDRAM PC3-10600 SO-DIMM
spdmem1 at iic0 addr 0x51: 2GB DDR3 SDRAM PC3-10600 SO-DIMM
usb2 at uhci0: USB revision 

snmpd loses ARP table information

[Gerhard Roth]

Hi,

snmpd pernanently loses its ARP table information:

# snmpctl walk 127.0.0.1 oid ipNetToMediaPhysAddress
ipNetToMediaPhysAddress.2.192.168.16.1="xx:xx:xx:xx:xx:xx"
ipNetToMediaPhysAddress.2.192.168.16.126="xx:xx:xx:xx:xx:xx"
ipNetToMediaPhysAddress.2.192.168.19.132="xx:xx:xx:xx:xx:xx"

Query the whole tree and this information is gone:

# snmpctl walk 127.0.0.1 oid iso.org>/dev/null
# snmpctl walk 127.0.0.1 oid ipNetToMediaPhysAddress
0=6

The reason is that several OIDs in mib.c call kr_updateif()
and this function deletes the kif_node and the restores it
via fetchifs(). But then the ARP information held by the
old kif_node is gone.

There is no need to delete the kif_node prior to calling
fetchifs() since it only calls rtmsg_process() which is
also called during normal processing of messages from the
routing socket.

And while here, also handle RTM_DESYNC messages.

Gerhard


Index: usr.sbin/snmpd/kroute.c
===
RCS file: /cvs/src/usr.sbin/snmpd/kroute.c,v
retrieving revision 1.31
diff -u -p -u -p -r1.31 kroute.c
--- usr.sbin/snmpd/kroute.c 18 Jul 2015 00:27:32 -  1.31
+++ usr.sbin/snmpd/kroute.c 2 Nov 2015 11:41:51 -
@@ -365,12 +365,6 @@ kr_iflastchange(void)
 int
 kr_updateif(u_int if_index)
 {
-   struct kif_node *kn;
-
-   if ((kn = kif_find(if_index)) != NULL)
-   kif_remove(kn);
-
-   /* Do not update the interface address list */
return (fetchifs(if_index));
 }
 
@@ -1380,6 +1374,12 @@ rtmsg_process(char *buf, int len)
break;
case RTM_IFANNOUNCE:
if_announce(next);
+   break;
+   case RTM_DESYNC:
+   kr_shutdown();
+   if (fetchifs(0) == -1)
+   fatalx("rtmsg_process: fetchifs");
+   ktable_init();
break;
default:
/* ignore for now */

tidy up pledge_ioctl

[Ted Unangst]

The last argument is always a file, so we can type it instead of using void.
Also, as a safety belt, leave vp null if the file type isn't vnode.


Index: kern/kern_pledge.c
===
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.100
diff -u -p -r1.100 kern_pledge.c
--- kern/kern_pledge.c  2 Nov 2015 17:53:00 -   1.100
+++ kern/kern_pledge.c  3 Nov 2015 03:30:31 -
@@ -1062,9 +1062,8 @@ pledge_sendit(struct proc *p, const void
 }
 
 int
-pledge_ioctl(struct proc *p, long com, void *v)
+pledge_ioctl(struct proc *p, long com, struct file *fp)
 {
-   struct file *fp = v;
struct vnode *vp = NULL;
 
if ((p->p_p->ps_flags & PS_PLEDGE) == 0)
@@ -1082,7 +1081,8 @@ pledge_ioctl(struct proc *p, long com, v
}
 
/* fp != NULL was already checked */
-   vp = (struct vnode *)fp->f_data;
+   if (fp->f_type == DTYPE_VNODE)
+   vp = (struct vnode *)fp->f_data;
 
/*
 * Further sets of ioctl become available, but are checked a
Index: sys/pledge.h
===
RCS file: /cvs/src/sys/sys/pledge.h,v
retrieving revision 1.17
diff -u -p -r1.17 pledge.h
--- sys/pledge.h2 Nov 2015 16:31:55 -   1.17
+++ sys/pledge.h3 Nov 2015 03:29:07 -
@@ -107,7 +107,7 @@ int pledge_adjtime(struct proc *p, const
 intpledge_sendit(struct proc *p, const void *to);
 intpledge_sockopt(struct proc *p, int set, int level, int optname);
 intpledge_socket(struct proc *p, int dns);
-intpledge_ioctl(struct proc *p, long com, void *);
+intpledge_ioctl(struct proc *p, long com, struct file *);
 intpledge_flock(struct proc *p);
 intpledge_fcntl(struct proc *p, int cmd);
 intpledge_swapctl(struct proc *p);