Re: relayd/ctl alternative control socket

[Maxim Bourmistrov]

But what about people not running relay on rdomain, but rather just want to run 
separate 
instances of relayd ?

> 28 nov. 2017 kl. 16:06 skrev Sebastian benoit 
> :
> 
> Hi,
> 
> your diff looks good, but i would rather do it the way bgpd/bgpctl do it:
> 
> there the default is  /var/run/bgpd.sock. where  is the 
> routing domain bgpctl is running in.  To administer bgpd(8) in a different 
> routing domain, run bgpctl in said routing domain.
> 
> i.e. it detects the rdomain at startup, bgpctl does the same.
> 
> Can you do that in relayd? It was commited there in sometime in summer.
> 
> /Benno
> 
> 
> On 11/28/17 11:54, Kapetanakis Giannis wrote:
>> Hi,
>> On June I've posted a patch about using alternative control socket for 
>> relayd and relayctl.
>> There was a comment from David Gwynne which was evaluated.
>> Is it OK to get this is in order to be able to control multiple relayd 
>> daemons on different rdomains?
>> thanks
>> Giannis
>> Index: config.c
>> ===
>> RCS file: /cvs/src/usr.sbin/relayd/config.c,v
>> retrieving revision 1.35
>> diff -u -p -r1.35 config.c
>> --- config.c 27 Nov 2017 23:21:16 -  1.35
>> +++ config.c 28 Nov 2017 10:43:37 -
>> @@ -44,6 +44,7 @@ config_init(struct relayd *env)
>>  env->sc_conf.interval.tv_usec = 0;
>>  env->sc_conf.prefork_relay = RELAY_NUMPROC;
>>  env->sc_conf.statinterval.tv_sec = RELAY_STATINTERVAL;
>> +env->sc_ps->ps_csock.cs_name = RELAYD_SOCKET;
>>  }
>>  ps->ps_what[PROC_PARENT] = CONFIG_ALL;
>> Index: parse.y
>> ===
>> RCS file: /cvs/src/usr.sbin/relayd/parse.y,v
>> retrieving revision 1.220
>> diff -u -p -r1.220 parse.y
>> --- parse.y  27 Nov 2017 23:21:16 -  1.220
>> +++ parse.y  28 Nov 2017 10:43:38 -
>> @@ -418,6 +418,9 @@ main : INTERVAL NUMBER   {
>>  AGENTX_SOCKET,
>>  sizeof(conf->sc_conf.snmp_path));
>>  }
>> +| SOCKET STRING {
>> +conf->sc_ps->ps_csock.cs_name = $2;
>> +}
>>  ;
>>trap  : /* nothing */ { $$ = 0; }
>> Index: relayd.c
>> ===
>> RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
>> retrieving revision 1.170
>> diff -u -p -r1.170 relayd.c
>> --- relayd.c 27 Nov 2017 21:06:26 -  1.170
>> +++ relayd.c 28 Nov 2017 10:43:38 -
>> @@ -199,9 +199,6 @@ main(int argc, char *argv[])
>>  if ((ps->ps_pw =  getpwnam(RELAYD_USER)) == NULL)
>>  errx(1, "unknown user %s", RELAYD_USER);
>>  -   /* Configure the control socket */
>> -ps->ps_csock.cs_name = RELAYD_SOCKET;
>> -
>>  log_init(debug, LOG_DAEMON);
>>  log_setverbose(verbose);
>>  Index: relayd.conf.5
>> ===
>> RCS file: /cvs/src/usr.sbin/relayd/relayd.conf.5,v
>> retrieving revision 1.180
>> diff -u -p -r1.180 relayd.conf.5
>> --- relayd.conf.527 Nov 2017 23:21:16 -  1.180
>> +++ relayd.conf.528 Nov 2017 10:43:38 -
>> @@ -163,6 +163,12 @@ will be used.
>>  See
>>  .Xr snmpd.conf 5
>>  for more information about SNMP configuration.
>> +.It Ic socket Qo Ar path Qc
>> +Create a control socket at
>> +.Ar path .
>> +By default
>> +.Pa /var/run/relayd.sock
>> +is created and no other sockets are created.
>>  .It Ic timeout Ar number
>>  Set the global timeout in milliseconds for checks.
>>  This can be overridden by the timeout value in the table definitions.
>> Index: relayctl.8
>> ===
>> RCS file: /cvs/src/usr.sbin/relayctl/relayctl.8,v
>> retrieving revision 1.32
>> diff -u -p -r1.32 relayctl.8
>> --- relayctl.8   28 Nov 2015 01:22:44 -  1.32
>> +++ relayctl.8   28 Nov 2017 10:43:22 -
>> @@ -23,6 +23,7 @@
>>  .Nd control the relay daemon
>>  .Sh SYNOPSIS
>>  .Nm
>> +.Op Fl s Ar socket
>>  .Ar command
>>  .Op Ar argument ...
>>  .Sh DESCRIPTION
>> @@ -31,6 +32,17 @@ The
>>  program controls the
>>  .Xr relayd 8
>>  daemon.
>> +.Pp
>> +The following options are available:
>> +.Bl -tag -width Ds
>> +.It Fl s Ar socket
>> +Use
>> +.Ar socket
>> +instead of the default
>> +.Pa /var/run/relayd.sock
>> +to communicate with
>> +.Xr relayd 8 .
>> +.El
>>  .Pp
>>  The following commands are available:
>>  .Bl -tag -width Ds
>> Index: relayctl.c
>> ===
>> RCS file: /cvs/src/usr.sbin/relayctl/relayctl.c,v
>> retrieving revision 1.57
>> diff -u -p -r1.57 relayctl.c
>> --- relayctl.c   3 Sep 2016 14:44:21 -   1.57
>> +++ relayctl.c   28 Nov 2017 10:43:22 -
>> @@ -88,7 +88,8 @@ usage(void)
>>  {
>>  extern char *__progname;
>>  -   fprintf(stderr, 

Re: [patch] upon install of new operating system version, do not set root password to empty string

[Otto Moerbeek]

On Tue, Nov 28, 2017 at 06:59:06PM -0500, Ian Sutton wrote:

> This is a highly theoretical and experimental mitigation which stops the
> root password on newly upgraded/installed systems from being an empty
> string. The thinking is that by not shipping an operating system with a
> known root password, certain classes of attacks involving logging into
> the root account might be avoided. I would like some feedback from the
> cryptography team as well as NIST finalists in order to better ascertain
> the implications of this behaviour.

Hmm, but afaiks, this is already done on install. What does you diff change?

-Otto

> 
> Index: src/distrib/miniroot/install.sub
> ===
> RCS file: /cvs/src/distrib/miniroot/install.sub,v
> retrieving revision 1.1032
> diff -u -p -r1.1032 install.sub
> --- src/distrib/miniroot/install.sub  8 Aug 2017 07:14:05 -   1.1032
> +++ src/distrib/miniroot/install.sub  28 Nov 2017 23:43:56 -
> @@ -2732,12 +2732,6 @@ do_install() {
>  
>   echo
>  
> + while :; do
> + ask_password "Password for root account?"
> + _rootpass="$_password"
> + [[ -n "$_password" ]] && break
> + echo "The root password must be set."
> + done
>  
>   # Ask for the root user public ssh key during autoinstall.
>   _rootkey=

kdump: fcntl(F_GETOWN) doesn't take an argument

[Philip Guenther]

When displaying fcntl() arguments, kdump knows that fcntl commands 
F_GETFD, F_GETFL, and F_ISATTY don't take an additional argument.  That's 
also true of F_GETOWN, so add it to that list.

This diff would be three lines, but I think it's cleaner to figure out 
whether an argument should be displayed in the main 'switch' in 
fcntlcmdname() instead of making the later 'if' conditional more 
complicated.  With this, adding more "no argument" fnctl commands becomes 
one liner diffs.

Example output with this:
 34617 fsetown  CALL  fcntl(3,F_GETOWN)
 34617 fsetown  RET   fcntl -34617/0x78c7

(It would be nice if we displayed the fnctl command's additional argument 
in the prefered form for its type, such as just decimal for pid/pgid args, 
and ditto for return values, but that requires more thought...)


ok?

Philip Guenther



Index: usr.bin/kdump/mksubr
===
RCS file: /data/src/openbsd/src/usr.bin/kdump/mksubr,v
retrieving revision 1.32
diff -u -p -r1.32 mksubr
--- usr.bin/kdump/mksubr28 Apr 2017 13:53:05 -  1.32
+++ usr.bin/kdump/mksubr27 Nov 2017 17:35:03 -
@@ -375,18 +375,25 @@ cat <<_EOF_
 void
 fcntlcmdname (int arg)
 {
+   int noarg = 0;
+
switch (arg1) {
 _EOF_
 egrep 
"^#[[:space:]]*define[[:space:]]+F_[A-Z_]+[[:space:]]+[0-9]+[[:space:]]*" \
$include_dir/sys/fcntl.h | \
-   awk 'BEGIN { o=0 } { for (i = 1; i <= NF; i++) \
+   awk 'BEGIN { o=0; \
+   noarg["F_GETFD"] = 1; \
+   noarg["F_GETFL"] = 1; \
+   noarg["F_ISATTY"] = 1; \
+   noarg["F_GETOWN"] = 1; \
+}{ for (i = 1; i <= NF; i++) \
if ($i ~ /define/) \
break; \
++i; \
-   if (o <= $(i+1)) \
-   printf "\tcase 
%s:\n\t\t(void)printf(\"%s\");\n\t\tbreak;\n", $i, $i; \
-   else \
+   if (o > $(i+1)) \
exit; \
+   printf "\tcase %s:\n\t\t(void)printf(\"%s\");%s\n\t\tbreak;\n", 
$i, $i, \
+   noarg[$i] ? "\n\t\tnoarg = 1;" : ""; \
o = $(i+1) }'
 cat <<_EOF_
default: /* Should not reach */
@@ -404,7 +411,7 @@ cat <<_EOF_
} else if (arg1 == F_SETFL) {
(void)putchar(',');
doflagsname(arg, 0);
-   } else if (!fancy || (arg1 != F_GETFD && arg1 != F_GETFL && arg1 != 
F_ISATTY))
+   } else if (!fancy || !noarg)
(void)printf(",%#x", arg);
 }
 

Re: Revise ACPI OperationRegion support

[Mike Larkin]

On Wed, Nov 29, 2017 at 04:30:01AM +0100, Mark Kettenis wrote:
> Diff below revises the OperationRegion support to allow chvgpio(4) to
> register its own OEM defined regions. This prevents a panic on a
> Lenovo 2-in-1 that mlarkin@ carried all the way to Elk Lakes cabin.
> It even makes the lid switch work somewhat.
> 
> This will also make it easier to add CMOS support and other
> OEM-specific regions in the future.
> 
> ok?
> 

Looks ok, no objections here.

-ml

> Index: dev/acpi/chvgpio.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/chvgpio.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 chvgpio.c
> --- dev/acpi/chvgpio.c25 Oct 2016 06:48:58 -  1.6
> +++ dev/acpi/chvgpio.c29 Nov 2017 03:18:06 -
> @@ -43,6 +43,9 @@
>  #define CHVGPIO_PAD_CFG1_INVRXTX_MASK0x00f0
>  #define CHVGPIO_PAD_CFG1_INVRXTX_RXDATA  0x0040
>  
> +/* OEM defined RegionSpace. */
> +#define CHVGPIO_REGIONSPACE_BASE 0x90
> +
>  struct chvgpio_intrhand {
>   int (*ih_func)(void *);
>   void *ih_arg;
> @@ -149,6 +152,7 @@ int   chvgpio_read_pin(void *, int);
>  void chvgpio_write_pin(void *, int, int);
>  void chvgpio_intr_establish(void *, int, int, int (*)(), void *);
>  int  chvgpio_intr(void *);
> +int  chvgpio_opreg_handler(void *, int, uint64_t, int, uint64_t *);
>  
>  int
>  chvgpio_match(struct device *parent, void *match, void *aux)
> @@ -247,7 +251,7 @@ chvgpio_attach(struct device *parent, st
>  
>   printf(", %d pins\n", sc->sc_npins);
>  
> - /* Register address space. */
> + /* Register GeneralPurposeIO address space. */
>   memset(, 0, sizeof(arg));
>   arg[0].type = AML_OBJTYPE_INTEGER;
>   arg[0].v_integer = ACPI_OPREG_GPIO;
> @@ -257,6 +261,9 @@ chvgpio_attach(struct device *parent, st
>   if (node && aml_evalnode(sc->sc_acpi, node, 2, arg, NULL))
>   printf("%s: _REG failed\n", sc->sc_dev.dv_xname);
>  
> + /* Register OEM defined address space. */
> + aml_register_regionspace(sc->sc_node, CHVGPIO_REGIONSPACE_BASE + uid,
> + sc, chvgpio_opreg_handler);
>   return;
>  
>  unmap:
> @@ -398,4 +405,22 @@ chvgpio_intr(void *arg)
>   }
>  
>   return rc;
> +}
> +
> +int
> +chvgpio_opreg_handler(void *cookie, int iodir, uint64_t address, int size,
> +uint64_t *value)
> +{
> + struct chvgpio_softc *sc = cookie;
> +
> + /* Only allow 32-bit access. */
> + if (size != 4 || address > sc->sc_size - size)
> + return -1;
> +
> + if (iodir == ACPI_IOREAD)
> + *value = bus_space_read_4(sc->sc_memt, sc->sc_memh, address);
> + else
> + bus_space_write_4(sc->sc_memt, sc->sc_memh, address, *value);
> +
> + return 0;
>  }
> Index: dev/acpi/dsdt.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> retrieving revision 1.235
> diff -u -p -r1.235 dsdt.c
> --- dev/acpi/dsdt.c   12 Oct 2017 07:24:46 -  1.235
> +++ dev/acpi/dsdt.c   29 Nov 2017 03:08:06 -
> @@ -2208,8 +2208,6 @@ void aml_createfield(struct aml_value *,
>  void aml_parsefieldlist(struct aml_scope *, int, int,
>  struct aml_value *, struct aml_value *, int);
>  
> -#define GAS_PCI_CFG_SPACE_UNEVAL  0xCC
> -
>  int
>  aml_evalhid(struct aml_node *node, struct aml_value *val)
>  {
> @@ -,7 +2220,73 @@ aml_evalhid(struct aml_node *node, struc
>   return (0);
>  }
>  
> -void aml_rwgas(struct aml_value *, int, int, struct aml_value *, int, int);
> +int
> +aml_opreg_sysmem_handler(void *cookie, int iodir, uint64_t address, int size,
> +uint64_t *value)
> +{
> + return acpi_gasio(acpi_softc, iodir, GAS_SYSTEM_MEMORY,
> + address, size, size, value);
> +}
> +
> +int
> +aml_opreg_sysio_handler(void *cookie, int iodir, uint64_t address, int size,
> +uint64_t *value)
> +{
> + return acpi_gasio(acpi_softc, iodir, GAS_SYSTEM_IOSPACE,
> + address, size, size, value);
> +}
> +
> +int
> +aml_opreg_pcicfg_handler(void *cookie, int iodir, uint64_t address, int size,
> +uint64_t *value)
> +{
> + return acpi_gasio(acpi_softc, iodir, GAS_PCI_CFG_SPACE,
> + address, size, size, value);
> +}
> +
> +int
> +aml_opreg_ec_handler(void *cookie, int iodir, uint64_t address, int size,
> +uint64_t *value)
> +{
> + return acpi_gasio(acpi_softc, iodir, GAS_EMBEDDED,
> + address, size, size, value);
> +}
> +
> +struct aml_regionspace {
> + void *cookie;
> + int (*handler)(void *, int, uint64_t, int, uint64_t *);
> +};
> +
> +struct aml_regionspace aml_regionspace[256] = {
> + [ACPI_OPREG_SYSMEM] = { NULL, aml_opreg_sysmem_handler },
> + [ACPI_OPREG_SYSIO] = { NULL, aml_opreg_sysio_handler },
> + [ACPI_OPREG_PCICFG] = { NULL, aml_opreg_pcicfg_handler },
> + [ACPI_OPREG_EC] = { NULL, aml_opreg_ec_handler },
> +};
> +
> +void
> +aml_register_regionspace(struct aml_node *node, 

Revise ACPI OperationRegion support

[Mark Kettenis]

Diff below revises the OperationRegion support to allow chvgpio(4) to
register its own OEM defined regions. This prevents a panic on a
Lenovo 2-in-1 that mlarkin@ carried all the way to Elk Lakes cabin.
It even makes the lid switch work somewhat.

This will also make it easier to add CMOS support and other
OEM-specific regions in the future.

ok?

Index: dev/acpi/chvgpio.c
===
RCS file: /cvs/src/sys/dev/acpi/chvgpio.c,v
retrieving revision 1.6
diff -u -p -r1.6 chvgpio.c
--- dev/acpi/chvgpio.c  25 Oct 2016 06:48:58 -  1.6
+++ dev/acpi/chvgpio.c  29 Nov 2017 03:18:06 -
@@ -43,6 +43,9 @@
 #define CHVGPIO_PAD_CFG1_INVRXTX_MASK  0x00f0
 #define CHVGPIO_PAD_CFG1_INVRXTX_RXDATA0x0040
 
+/* OEM defined RegionSpace. */
+#define CHVGPIO_REGIONSPACE_BASE   0x90
+
 struct chvgpio_intrhand {
int (*ih_func)(void *);
void *ih_arg;
@@ -149,6 +152,7 @@ int chvgpio_read_pin(void *, int);
 void   chvgpio_write_pin(void *, int, int);
 void   chvgpio_intr_establish(void *, int, int, int (*)(), void *);
 intchvgpio_intr(void *);
+intchvgpio_opreg_handler(void *, int, uint64_t, int, uint64_t *);
 
 int
 chvgpio_match(struct device *parent, void *match, void *aux)
@@ -247,7 +251,7 @@ chvgpio_attach(struct device *parent, st
 
printf(", %d pins\n", sc->sc_npins);
 
-   /* Register address space. */
+   /* Register GeneralPurposeIO address space. */
memset(, 0, sizeof(arg));
arg[0].type = AML_OBJTYPE_INTEGER;
arg[0].v_integer = ACPI_OPREG_GPIO;
@@ -257,6 +261,9 @@ chvgpio_attach(struct device *parent, st
if (node && aml_evalnode(sc->sc_acpi, node, 2, arg, NULL))
printf("%s: _REG failed\n", sc->sc_dev.dv_xname);
 
+   /* Register OEM defined address space. */
+   aml_register_regionspace(sc->sc_node, CHVGPIO_REGIONSPACE_BASE + uid,
+   sc, chvgpio_opreg_handler);
return;
 
 unmap:
@@ -398,4 +405,22 @@ chvgpio_intr(void *arg)
}
 
return rc;
+}
+
+int
+chvgpio_opreg_handler(void *cookie, int iodir, uint64_t address, int size,
+uint64_t *value)
+{
+   struct chvgpio_softc *sc = cookie;
+
+   /* Only allow 32-bit access. */
+   if (size != 4 || address > sc->sc_size - size)
+   return -1;
+
+   if (iodir == ACPI_IOREAD)
+   *value = bus_space_read_4(sc->sc_memt, sc->sc_memh, address);
+   else
+   bus_space_write_4(sc->sc_memt, sc->sc_memh, address, *value);
+
+   return 0;
 }
Index: dev/acpi/dsdt.c
===
RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
retrieving revision 1.235
diff -u -p -r1.235 dsdt.c
--- dev/acpi/dsdt.c 12 Oct 2017 07:24:46 -  1.235
+++ dev/acpi/dsdt.c 29 Nov 2017 03:08:06 -
@@ -2208,8 +2208,6 @@ void aml_createfield(struct aml_value *,
 void aml_parsefieldlist(struct aml_scope *, int, int,
 struct aml_value *, struct aml_value *, int);
 
-#define GAS_PCI_CFG_SPACE_UNEVAL  0xCC
-
 int
 aml_evalhid(struct aml_node *node, struct aml_value *val)
 {
@@ -,7 +2220,73 @@ aml_evalhid(struct aml_node *node, struc
return (0);
 }
 
-void aml_rwgas(struct aml_value *, int, int, struct aml_value *, int, int);
+int
+aml_opreg_sysmem_handler(void *cookie, int iodir, uint64_t address, int size,
+uint64_t *value)
+{
+   return acpi_gasio(acpi_softc, iodir, GAS_SYSTEM_MEMORY,
+   address, size, size, value);
+}
+
+int
+aml_opreg_sysio_handler(void *cookie, int iodir, uint64_t address, int size,
+uint64_t *value)
+{
+   return acpi_gasio(acpi_softc, iodir, GAS_SYSTEM_IOSPACE,
+   address, size, size, value);
+}
+
+int
+aml_opreg_pcicfg_handler(void *cookie, int iodir, uint64_t address, int size,
+uint64_t *value)
+{
+   return acpi_gasio(acpi_softc, iodir, GAS_PCI_CFG_SPACE,
+   address, size, size, value);
+}
+
+int
+aml_opreg_ec_handler(void *cookie, int iodir, uint64_t address, int size,
+uint64_t *value)
+{
+   return acpi_gasio(acpi_softc, iodir, GAS_EMBEDDED,
+   address, size, size, value);
+}
+
+struct aml_regionspace {
+   void *cookie;
+   int (*handler)(void *, int, uint64_t, int, uint64_t *);
+};
+
+struct aml_regionspace aml_regionspace[256] = {
+   [ACPI_OPREG_SYSMEM] = { NULL, aml_opreg_sysmem_handler },
+   [ACPI_OPREG_SYSIO] = { NULL, aml_opreg_sysio_handler },
+   [ACPI_OPREG_PCICFG] = { NULL, aml_opreg_pcicfg_handler },
+   [ACPI_OPREG_EC] = { NULL, aml_opreg_ec_handler },
+};
+
+void
+aml_register_regionspace(struct aml_node *node, int iospace, void *cookie,
+int (*handler)(void *, int, uint64_t, int, uint64_t *))
+{
+   struct aml_value arg[2];
+
+   KASSERT(iospace >= 0 && iospace < 256);
+
+   aml_regionspace[iospace].cookie = cookie;
+   aml_regionspace[iospace].handler = handler;
+
+   /* Register address space. 

Re: [patch] upon install of new operating system version, do not set root password to empty string

[Stefan Sperling]

On Tue, Nov 28, 2017 at 06:59:06PM -0500, Ian Sutton wrote:
> This is a highly theoretical and experimental mitigation which stops the
> root password on newly upgraded/installed systems from being an empty
> string. The thinking is that by not shipping an operating system with a
> known root password, certain classes of attacks involving logging into
> the root account might be avoided. I would like some feedback from the
> cryptography team as well as NIST finalists in order to better ascertain
> the implications of this behaviour.

Is this in response to
https://mobile.twitter.com/lemiorhan/status/935578694541770752 ?
 
> Index: src/distrib/miniroot/install.sub
> ===
> RCS file: /cvs/src/distrib/miniroot/install.sub,v
> retrieving revision 1.1032
> diff -u -p -r1.1032 install.sub
> --- src/distrib/miniroot/install.sub  8 Aug 2017 07:14:05 -   1.1032
> +++ src/distrib/miniroot/install.sub  28 Nov 2017 23:43:56 -
> @@ -2732,12 +2732,6 @@ do_install() {
>  
>   echo
>  
> + while :; do
> + ask_password "Password for root account?"
> + _rootpass="$_password"
> + [[ -n "$_password" ]] && break
> + echo "The root password must be set."
> + done
>  
>   # Ask for the root user public ssh key during autoinstall.
>   _rootkey=
> 

Re: iked, don't return NULL in print_host

[Jeremie Courreges-Anglas]

On Wed, Nov 29 2017, Claudio Jeker  wrote:
> On Wed, Nov 29, 2017 at 01:59:06AM +0100, Claudio Jeker wrote:
>> Seen in my log file:
>> Nov 28 17:47:22 dramaqueen iked: vfprintf %s NULL in "%s: %s %s from %s to
>> %s ms gid %u, %ld bytes%s"
>> 
>> and
>> 
>> Nov 29 01:02:39 dramaqueen iked[49967]: ikev2_msg_send: IKE_SA_INIT
>> request from (null) to 62.48.30.5:500 msgid 0, 438 bytes
>> 
>> The problem seems to be in print_host so try to not return NULL in there.
>> Maybe we could return something else but this is a start IMO.
>
> beck@ prefers to just print unknown instead  of the gai_strerror -- guess
> that is more sensible.

Why would getnameinfo(NI_NUMERICHOST) fail here?  The code in
ikev2_msg.c is:

--8<--
log_info("%s: %s %s from %s to %s msgid %u, %ld bytes%s", __func__,
print_map(exchange, ikev2_exchange_map),
(flags & IKEV2_FLAG_RESPONSE) ? "response" : "request",
print_host((struct sockaddr *)>msg_local, NULL, 0),
print_host((struct sockaddr *)>msg_peer, NULL, 0),
betoh32(hdr->ike_msgid),
ibuf_length(buf), isnatt ? ", NAT-T" : "");
-->8--

Maybe msg->msg_local is corrupt?

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

Re: iked, don't return NULL in print_host

[Bob Beck]

ok beck@

On Wed, Nov 29, 2017 at 02:17:21AM +0100, Claudio Jeker wrote:
> On Wed, Nov 29, 2017 at 01:59:06AM +0100, Claudio Jeker wrote:
> > Seen in my log file:
> > Nov 28 17:47:22 dramaqueen iked: vfprintf %s NULL in "%s: %s %s from %s to
> > %s ms gid %u, %ld bytes%s"
> > 
> > and
> > 
> > Nov 29 01:02:39 dramaqueen iked[49967]: ikev2_msg_send: IKE_SA_INIT
> > request from (null) to 62.48.30.5:500 msgid 0, 438 bytes
> > 
> > The problem seems to be in print_host so try to not return NULL in there.
> > Maybe we could return something else but this is a start IMO.
> 
> beck@ prefers to just print unknown instead  of the gai_strerror -- guess
> that is more sensible.
> 
> -- 
> :wq Claudio
> 
> Index: util.c
> ===
> RCS file: /cvs/src/sbin/iked/util.c,v
> retrieving revision 1.33
> diff -u -p -r1.33 util.c
> --- util.c9 Jan 2017 14:49:21 -   1.33
> +++ util.c29 Nov 2017 01:16:21 -
> @@ -654,8 +654,8 @@ print_host(struct sockaddr *sa, char *bu
>  
>   if (getnameinfo(sa, sa->sa_len,
>   buf, len, NULL, 0, NI_NUMERICHOST) != 0) {
> - buf[0] = '\0';
> - return (NULL);
> + strlcpy(buf, "unknown", len);
> + return (buf);
>   }
>  
>   if ((port = socket_getport(sa)) != 0) {
> 

Re: iked, don't return NULL in print_host

[Claudio Jeker]

On Wed, Nov 29, 2017 at 01:59:06AM +0100, Claudio Jeker wrote:
> Seen in my log file:
> Nov 28 17:47:22 dramaqueen iked: vfprintf %s NULL in "%s: %s %s from %s to
> %s ms gid %u, %ld bytes%s"
> 
> and
> 
> Nov 29 01:02:39 dramaqueen iked[49967]: ikev2_msg_send: IKE_SA_INIT
> request from (null) to 62.48.30.5:500 msgid 0, 438 bytes
> 
> The problem seems to be in print_host so try to not return NULL in there.
> Maybe we could return something else but this is a start IMO.

beck@ prefers to just print unknown instead  of the gai_strerror -- guess
that is more sensible.

-- 
:wq Claudio

Index: util.c
===
RCS file: /cvs/src/sbin/iked/util.c,v
retrieving revision 1.33
diff -u -p -r1.33 util.c
--- util.c  9 Jan 2017 14:49:21 -   1.33
+++ util.c  29 Nov 2017 01:16:21 -
@@ -654,8 +654,8 @@ print_host(struct sockaddr *sa, char *bu
 
if (getnameinfo(sa, sa->sa_len,
buf, len, NULL, 0, NI_NUMERICHOST) != 0) {
-   buf[0] = '\0';
-   return (NULL);
+   strlcpy(buf, "unknown", len);
+   return (buf);
}
 
if ((port = socket_getport(sa)) != 0) {

iked, don't return NULL in print_host

[Claudio Jeker]

Seen in my log file:
Nov 28 17:47:22 dramaqueen iked: vfprintf %s NULL in "%s: %s %s from %s to
%s ms gid %u, %ld bytes%s"

and

Nov 29 01:02:39 dramaqueen iked[49967]: ikev2_msg_send: IKE_SA_INIT
request from (null) to 62.48.30.5:500 msgid 0, 438 bytes

The problem seems to be in print_host so try to not return NULL in there.
Maybe we could return something else but this is a start IMO.
-- 
:wq Claudio


Index: util.c
===
RCS file: /cvs/src/sbin/iked/util.c,v
retrieving revision 1.33
diff -u -p -r1.33 util.c
--- util.c  9 Jan 2017 14:49:21 -   1.33
+++ util.c  29 Nov 2017 00:52:02 -
@@ -639,6 +639,7 @@ print_host(struct sockaddr *sa, char *bu
static int  idx = 0;
charpbuf[7];
in_port_t   port;
+   int rc;
 
if (buf == NULL) {
buf = sbuf[idx];
@@ -652,10 +653,10 @@ print_host(struct sockaddr *sa, char *bu
return (buf);
}
 
-   if (getnameinfo(sa, sa->sa_len,
-   buf, len, NULL, 0, NI_NUMERICHOST) != 0) {
-   buf[0] = '\0';
-   return (NULL);
+   if ((rc = getnameinfo(sa, sa->sa_len,
+   buf, len, NULL, 0, NI_NUMERICHOST)) != 0) {
+   snprintf(buf, len, "error %s", gai_strerror(rc));
+   return (buf);
}
 
if ((port = socket_getport(sa)) != 0) {

[patch] upon install of new operating system version, do not set root password to empty string

[Ian Sutton]

This is a highly theoretical and experimental mitigation which stops the
root password on newly upgraded/installed systems from being an empty
string. The thinking is that by not shipping an operating system with a
known root password, certain classes of attacks involving logging into
the root account might be avoided. I would like some feedback from the
cryptography team as well as NIST finalists in order to better ascertain
the implications of this behaviour.

Index: src/distrib/miniroot/install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.1032
diff -u -p -r1.1032 install.sub
--- src/distrib/miniroot/install.sub8 Aug 2017 07:14:05 -   1.1032
+++ src/distrib/miniroot/install.sub28 Nov 2017 23:43:56 -
@@ -2732,12 +2732,6 @@ do_install() {
 
echo
 
+   while :; do
+   ask_password "Password for root account?"
+   _rootpass="$_password"
+   [[ -n "$_password" ]] && break
+   echo "The root password must be set."
+   done
 
# Ask for the root user public ssh key during autoinstall.
_rootkey=

relayd, save some power by removing snmp session tracking

[Claudio Jeker]

So one of the things relayd's snmp agentx hook provieds is the relay
sessions. To do that every connection accepted by relayd does 2 IMSG and
the pfe is using linear lists to track them. Because of this the PFE
engine is using more CPU time than needed.

Is anyone using the fast changing session data in snmp? Else I propose to
remove the code.
-- 
:wq Claudio

? obj
Index: pfe.c
===
RCS file: /cvs/src/usr.sbin/relayd/pfe.c,v
retrieving revision 1.89
diff -u -p -r1.89 pfe.c
--- pfe.c   28 May 2017 10:39:15 -  1.89
+++ pfe.c   28 Nov 2017 18:45:31 -
@@ -252,9 +252,8 @@ pfe_dispatch_relay(int fd, struct privse
struct ctl_stats crs;
struct relay*rlay;
struct ctl_conn *c;
-   struct rsession  con, *s, *t;
+   struct rsession  con;
int  cid;
-   objid_t  sid;
 
switch (imsg->hdr.type) {
case IMSG_NATLOOK:
@@ -305,38 +304,6 @@ pfe_dispatch_relay(int fd, struct privse
/* Last ack for a previous request */
imsg_compose_event(>iev, IMSG_CTL_END,
0, 0, -1, NULL, 0);
-   }
-   break;
-   case IMSG_SESS_PUBLISH:
-   IMSG_SIZE_CHECK(imsg, s);
-   if ((s = calloc(1, sizeof(*s))) == NULL)
-   return (0); /* XXX */
-   memcpy(s, imsg->data, sizeof(*s));
-   TAILQ_FOREACH(t, >sc_sessions, se_entry) {
-   /* duplicate registration */
-   if (t->se_id == s->se_id) {
-   free(s);
-   return (0);
-   }
-   if (t->se_id > s->se_id)
-   break;
-   }
-   if (t)
-   TAILQ_INSERT_BEFORE(t, s, se_entry);
-   else
-   TAILQ_INSERT_TAIL(>sc_sessions, s, se_entry);
-   break;
-   case IMSG_SESS_UNPUBLISH:
-   IMSG_SIZE_CHECK(imsg, );
-   memcpy(, imsg->data, sizeof(sid));
-   TAILQ_FOREACH(s, >sc_sessions, se_entry)
-   if (s->se_id == sid)
-   break;
-   if (s) {
-   TAILQ_REMOVE(>sc_sessions, s, se_entry);
-   free(s);
-   } else {
-   DPRINTF("removal of unpublished session %i", sid);
}
break;
default:
Index: relay.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
retrieving revision 1.236
diff -u -p -r1.236 relay.c
--- relay.c 28 Nov 2017 01:51:47 -  1.236
+++ relay.c 28 Nov 2017 18:45:32 -
@@ -328,19 +328,6 @@ relay_init(struct privsep *ps, struct pr
 }
 
 void
-relay_session_publish(struct rsession *s)
-{
-   proc_compose(env->sc_ps, PROC_PFE, IMSG_SESS_PUBLISH, s, sizeof(*s));
-}
-
-void
-relay_session_unpublish(struct rsession *s)
-{
-   proc_compose(env->sc_ps, PROC_PFE, IMSG_SESS_UNPUBLISH,
-   >se_id, sizeof(s->se_id));
-}
-
-void
 relay_statistics(int fd, short events, void *arg)
 {
struct privsep  *ps = arg;
@@ -1120,7 +1107,6 @@ relay_accept(int fd, short event, void *
 
relay_sessions++;
SPLAY_INSERT(session_tree, >rl_sessions, con);
-   relay_session_publish(con);
 
/* Increment the per-relay session counter */
rlay->rl_stats[ps->ps_instance].last++;
@@ -1667,7 +1653,6 @@ relay_close(struct rsession *con, const 
struct protocol *proto = rlay->rl_proto;
 
SPLAY_REMOVE(session_tree, >rl_sessions, con);
-   relay_session_unpublish(con);
 
event_del(>se_ev);
if (con->se_in.bev != NULL)
Index: relay_udp.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay_udp.c,v
retrieving revision 1.47
diff -u -p -r1.47 relay_udp.c
--- relay_udp.c 4 Jul 2017 19:59:51 -   1.47
+++ relay_udp.c 28 Nov 2017 18:45:32 -
@@ -275,7 +275,6 @@ relay_udp_server(int fd, short sig, void
 
relay_sessions++;
SPLAY_INSERT(session_tree, >rl_sessions, con);
-   relay_session_publish(con);
 
/* Increment the per-relay session counter */
rlay->rl_stats[ps->ps_instance].last++;
Index: relayd.c
===
RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
retrieving revision 1.170
diff -u -p -r1.170 relayd.c
--- relayd.c27 Nov 2017 21:06:26 -  1.170
+++ relayd.c28 Nov 2017 18:45:33 -
@@ -182,7 +182,6 @@ main(int argc, char *argv[])
env->sc_conffile = conffile;
env->sc_conf.opts = opts;
TAILQ_INIT(>sc_hosts);
-   

Re: TCP/UDP/etc input w/o KERNEL_LOCK()

[Jeremie Courreges-Anglas]

On Tue, Nov 28 2017, Alexander Bluhm  wrote:
> On Tue, Nov 28, 2017 at 02:42:58PM +0100, Martin Pieuchot wrote:
>> > login: panic: kernel diagnostic assertion "_kernel_lock_held()" failed: 
>> > file "/usr/src/sys/kern/uipc_socket.c", line 1882
>> > Stopped at  db_enter+0x4:   popl%ebp
>
> Next crash, now during regress/usr.bin/openssl.  Looks like some
> ssl tests.
>
> === (Section) client/server operations 2017/11/28 17:28:11  
> [0/1984]
> === 
> [TEST] s_server ... start SSL/TLS test server
> :-) success.
> s_server pid = [ 28644 ]
> id_prefix 'APPSTEST.SH' set.
> [TEST] s_client ... connect to SSL/TLS test server
> :-) success. 
> [TEST] s_time ... connect to SSL/TLS test server
> Collecting connection statistics for 2 seconds
> 
> **
> 114 connections in 0.82s; 139.02 connections/user sec, bytes read 0
> 114 connections in 3 real seconds, 0 bytes read per connection
> Now timing with session id reuse.
> starting

Hopefully unrelated, but openssl(1) s_time uses
clock_gettime(CLOCK_MONOTONIC) instead of times(3) since a few days...

> login: uvm_fault(0xd0c49720, 0xefff1000, 0, 1) -> e
> kernel: page fault trap, code=0
> Stopped at  sowakeup+0x1f:  movl0x4(%eax),%eax
> ddb{1}> trace
> sowakeup(d48aa654,d48aa6a4) at sowakeup+0x1f
> sorwakeup_cb(d48aa654) at sorwakeup_cb+0x44
> taskq_thread(d0c08924) at taskq_thread+0x6c
> ddb{1}> ps
>PID TID   PPIDUID  S   FLAGS  WAIT  COMMAND
>  43123  418341  31210  0  20x12openssl
>  28644   72007  31210  0  70x12openssl
>  31210  280560  82764  0  30x10008a  pause sh
>  82764  152979  51322  0  30x10008a  pause make
>  51322  210233  34448  0  30x10008a  pause sh
>  34448  224788  32282  0  30x10008a  pause make
>  979309721  63243  0  30x100083  ttyin ksh
>  63243  146431  1  0  30x100080  kqreadtmux
>  61505  127179  84591  0  30x100083  kqreadtmux
>  845918868  72111  0  30x10008b  pause ksh
>  72111   99135  24574  0  30x92  selectsshd
>  51259  193379  17718  0  30x100082  piperdgzip
>  17718  452881  32282  0  30x100082  piperdpax
>  32282   85452  43519  0  30x82  piperdperl
>  43519  237534  81520  0  30x10008a  pause ksh
>  81520  513907  24574  0  30x92  selectsshd
>  11513  163133  1  0  30x100083  ttyin getty
>  68366   91198  1  0  30x100083  ttyin getty
>  223453539  1  0  30x100083  ttyin getty
>  91292   66425  1  0  30x100083  ttyin getty
>  26386   49610  1  0  30x100083  ttyin getty
>  96570   19814  1  0  30x100083  ttyin getty
>  68461  519579  1  0  30x100098  poll  cron
>  59656  398827  1 99  30x100090  poll  sndiod
>  65357   40070  1110  30x100090  poll  sndiod
>  50348  437814  91194 95  30x100092  kqreadsmtpd
>  26110   82956  91194103  30x100092  kqreadsmtpd
>   8062  205711  91194 95  30x100092  kqreadsmtpd
>  33360  291059  91194 95  30x100092  kqreadsmtpd
>  63652  458352  91194 95  30x100092  kqreadsmtpd
>  91166   55855  91194 95  30x100092  kqreadsmtpd
>  91194   93442  1  0  30x100080  kqreadsmtpd
>  24574  277618  1  0  30x80  selectsshd
>  17683  241641  0  0  3 0x14200  acct  acct
>  81154  213331  0  0  3 0x14280  nfsidlnfsio
>   3348  378560  0  0  3 0x14280  nfsidlnfsio
>  80015  161899  0  0  3 0x14280  nfsidlnfsio
>  54206  488651  0  0  3 0x14280  nfsidlnfsio
>  69417  207884  1  0  30x100080  poll  ntpd
>  75510  492539   9408 83  30x100092  poll  ntpd
>   9408   13284  1 83  30x100092  poll  ntpd
>  90703  344775  75175 74  30x100092  bpf   pflogd
>  75175   84723  1  0  30x80  netio pflogd
>  67321  443381  87861 73  20x100090syslogd
>  87861  416767  1  0  30x100082  netio syslogd
>  50957  208417  1 77  30x100090  poll  dhclient
>  42619   81728  1  0  30x80  poll  dhclient
>  97401  345152  0  0  3 0x14200  bored ttm_swap
>  15657  126262  0  0  3 0x14200  pgzerozerothread
>  39239   47549  0  0  3 0x14200  aiodoned  aiodoned
>   8482  427578  0 

Re: TCP/UDP/etc input w/o KERNEL_LOCK()

[Alexander Bluhm]

On Tue, Nov 28, 2017 at 02:42:58PM +0100, Martin Pieuchot wrote:
> > login: panic: kernel diagnostic assertion "_kernel_lock_held()" failed: 
> > file "/usr/src/sys/kern/uipc_socket.c", line 1882
> > Stopped at  db_enter+0x4:   popl%ebp

Next crash, now during regress/usr.bin/openssl.  Looks like some
ssl tests.

=== (Section) client/server operations 2017/11/28 17:28:11  [0/1984]
=== 
[TEST] s_server ... start SSL/TLS test server
:-) success.
s_server pid = [ 28644 ]
id_prefix 'APPSTEST.SH' set.
[TEST] s_client ... connect to SSL/TLS test server
:-) success. 
[TEST] s_time ... connect to SSL/TLS test server
Collecting connection statistics for 2 seconds

**
114 connections in 0.82s; 139.02 connections/user sec, bytes read 0
114 connections in 3 real seconds, 0 bytes read per connection
Now timing with session id reuse.
starting

login: uvm_fault(0xd0c49720, 0xefff1000, 0, 1) -> e
kernel: page fault trap, code=0
Stopped at  sowakeup+0x1f:  movl0x4(%eax),%eax
ddb{1}> trace
sowakeup(d48aa654,d48aa6a4) at sowakeup+0x1f
sorwakeup_cb(d48aa654) at sorwakeup_cb+0x44
taskq_thread(d0c08924) at taskq_thread+0x6c
ddb{1}> ps
   PID TID   PPIDUID  S   FLAGS  WAIT  COMMAND
 43123  418341  31210  0  20x12openssl
 28644   72007  31210  0  70x12openssl
 31210  280560  82764  0  30x10008a  pause sh
 82764  152979  51322  0  30x10008a  pause make
 51322  210233  34448  0  30x10008a  pause sh
 34448  224788  32282  0  30x10008a  pause make
 979309721  63243  0  30x100083  ttyin ksh
 63243  146431  1  0  30x100080  kqreadtmux
 61505  127179  84591  0  30x100083  kqreadtmux
 845918868  72111  0  30x10008b  pause ksh
 72111   99135  24574  0  30x92  selectsshd
 51259  193379  17718  0  30x100082  piperdgzip
 17718  452881  32282  0  30x100082  piperdpax
 32282   85452  43519  0  30x82  piperdperl
 43519  237534  81520  0  30x10008a  pause ksh
 81520  513907  24574  0  30x92  selectsshd
 11513  163133  1  0  30x100083  ttyin getty
 68366   91198  1  0  30x100083  ttyin getty
 223453539  1  0  30x100083  ttyin getty
 91292   66425  1  0  30x100083  ttyin getty
 26386   49610  1  0  30x100083  ttyin getty
 96570   19814  1  0  30x100083  ttyin getty
 68461  519579  1  0  30x100098  poll  cron
 59656  398827  1 99  30x100090  poll  sndiod
 65357   40070  1110  30x100090  poll  sndiod
 50348  437814  91194 95  30x100092  kqreadsmtpd
 26110   82956  91194103  30x100092  kqreadsmtpd
  8062  205711  91194 95  30x100092  kqreadsmtpd
 33360  291059  91194 95  30x100092  kqreadsmtpd
 63652  458352  91194 95  30x100092  kqreadsmtpd
 91166   55855  91194 95  30x100092  kqreadsmtpd
 91194   93442  1  0  30x100080  kqreadsmtpd
 24574  277618  1  0  30x80  selectsshd
 17683  241641  0  0  3 0x14200  acct  acct
 81154  213331  0  0  3 0x14280  nfsidlnfsio
  3348  378560  0  0  3 0x14280  nfsidlnfsio
 80015  161899  0  0  3 0x14280  nfsidlnfsio
 54206  488651  0  0  3 0x14280  nfsidlnfsio
 69417  207884  1  0  30x100080  poll  ntpd
 75510  492539   9408 83  30x100092  poll  ntpd
  9408   13284  1 83  30x100092  poll  ntpd
 90703  344775  75175 74  30x100092  bpf   pflogd
 75175   84723  1  0  30x80  netio pflogd
 67321  443381  87861 73  20x100090syslogd
 87861  416767  1  0  30x100082  netio syslogd
 50957  208417  1 77  30x100090  poll  dhclient
 42619   81728  1  0  30x80  poll  dhclient
 97401  345152  0  0  3 0x14200  bored ttm_swap
 15657  126262  0  0  3 0x14200  pgzerozerothread
 39239   47549  0  0  3 0x14200  aiodoned  aiodoned
  8482  427578  0  0  3 0x14200  syncerupdate
 78237  123810  0  0  3 0x14200  cleaner   cleaner
 57078  502488  0  0  3 0x14200  reaperreaper
 35375   23563  0  0  3 0x14200  pgdaemon  pagedaemon
 74622   81705  0  0  3 0x14200  bored crynlk
 45943  195618  0  0  3 

Re: fuse: vfs create does not map 1:1 to fuse create

[Otto Moerbeek]

On Tue, Nov 28, 2017 at 10:59:07PM +0800, Helg wrote:

One small comment from me:

>   /* already open i think all is ok */
>   if (ip->fufh[fufh_type].fh_type != FUFH_INVALID)
>   return (0);
>  
> + /*
> +  * The file has already been created and/or truncated so FUSE dictates
> +  * that no creation and truncation flags are passed to open.
> +  */
> + flags = OFLAGS(ap->a_mode) & ~(O_CREAT|O_EXCL|O_TRUNC);
> +
>   error = fusefs_file_open(fmp, ip, fufh_type, flags, isdir, ap->a_p);
>   if (error)
>   return (error);
>  
> - return (error);
> + return (0);

Just return (error) after the fusefs_file_open() call.

>  }

-Otto

Re: fuse: vfs create does not map 1:1 to fuse create

[Helg]

On Mon, Nov 27, 2017 at 10:48:04AM +0100, Martin Pieuchot wrote:
> On 23/11/17(Thu) 21:45, Helg wrote:
> > On Thu, Nov 23, 2017 at 12:09:34PM +, Helg Bredow wrote:
> > > - Forwarded message from Martin Pieuchot  -
> > > 
> > > Date: Sat, 18 Nov 2017 11:03:49 +0100
> > > From: Martin Pieuchot 
> > > To: Helg Bredow 
> > > CC: "tech@openbsd.org" 
> > > Subject: Re: fuse: vfs create does not map 1:1 to fuse create
> > > User-Agent: Mutt/1.9.1 (2017-09-22)
> > > 
> > > On 18/11/17(Sat) 02:22, Helg Bredow wrote:
> > > > On Fri, 10 Nov 2017 09:09:32 +0100
> > > > Martin Pieuchot  wrote:
> > > > 
> > > > > On 09/11/17(Thu) 01:20, Helg Bredow wrote:
> > > > > > > On 08/11/17(Wed) 14:12, Helg Bredow wrote:
> > > > > > > > There is a bug when creating a file in fuse-exfat and then 
> > > > > > > > deleting it
> > > > > > > > again without first unmounting the file system. The reason for 
> > > > > > > > this is
> > > > > > > > that fuse-exfat maintains strict reference counts and fuse 
> > > > > > > > currently
> > > > > > > > calls the file system create and open functions when it should 
> > > > > > > > only
> > > > > > > > call create. 
> > > > > > > > [...]
> > > > > > > 
> > > > [...]
> > > > > 
> > [...]
> > > > 
> > > > I now this caused some confusion but I believe the patch below is the
> > > > correct solution.
> > > 
> > > When you use the work "believing" you're asking us to trust you without
> > > argumenting.
> > > 
> > > >   Here's the mapping of file system calls so it's clear.
> > > > [...]
> > > 
> > > That's irrelevant.
> > > 
> > > There's no such thing as "atomic_open" from the userland point of view.
> > > So what you're discussing here is an "implementation details" of Linux
> > > VFS.
> > > 
> > > What matters is which syscall the application do.  I believe it is
> > > open(2).  So how its arguments are translated to the userland FS?  If
> > > I understand your diff correctly, OpenBSD will now do something like
> > > below (please correct the graph):
> > > 
> > > 
> > >   USER PROCESSfuse-zip
> > >   |   ^
> > >open(2)| 
> > >   |   FBT_MKNOD + FBT_OPEN 
> > >  \/ |
> > >   -
> > >   
> > > KERNEL
> > > 
> > > The only thing I understand is that you're changing the kernel behavior
> > > for all open(2) syscall operating on FUSE filesystem.  Before the kernel
> > > was generating a FBT_CREATE and FBT_OPEN messages, with some arguments.
> > > Now you want to generate FTB_MKNOD and FBT_OPEN with other arguments.
> > > 
> > > To me it sounds that you're working around a problem you don't
> > > understand.  If FUSE FSs want a FUSE 'create' operation then let's
> > > give them that!  What argument do they expect?
> > > 
> > > So let's start from the beginning:
> > > 
> > > - Which syscall is causing problem with which arguments?
> > > - Which FUSE operation(s) FUSE FSs are expecting for this exact syscall
> > >   with these arguments? 
> > > - Which FUSE operation(s) OpenBSD FUSE is currently generating with which
> > >   arguments?
> >  
> > The syscall that is causing the problem is open(2) with the O_CREAT flag.
> > With my change, this is the call graph.
> >  
> >  open(2)
> >|
> >  vn_open()
> >|
> >+--VOP_CREATE(9) when O_CREAT
> >|   |
> >|   +-->fusefs_create()-->FBT_MKNOD-->ifuse_ops_mknod()
> >|   |
> >|   +--> fs mknod()
> >|
> >+--VOP_SETATTR(9) when O_TRUNC
> >|   |
> >|   +-->fusefs_setattr()-->FBT_SETATTR-->ifuse_ops_setattr()
> >|   |
> >|   +--> fs truncate()
> >|
> >+--VOP_OPEN(9)   always
> >|
> >+--fusefs_open()-->FBT_OPEN-->ifuse_ops_open()
> >|
> >+--> fs open()
> >  
> >  
> > The file systems are expecting the following arguments.
> > 
> > mknod(path, mode, dev)
> >mode is that passed to open(2)
> >dev will be 0 for regular files. FUSE mknod can create regular files.
> > open(path, fuse_file_info)
> >fuse_file_info has (almost all) flags passed to open(2) plus other
> >things that are not set by ifuse_ops_open. The FS will also return
> >a handle to the file that was opened which it expects to receive
> >again for functions that operate on the same file.
> > create(path, mode, fuse_file_info)
> >mode is that passed to open(2)
> >fuse_file_info has (almost all) flags passed to open(2) plus other
> >things that are not set by ifuse_ops_open. The FS will also return
> >a handle to the file that was opened 

iked: don't include DH transform in IKE_AUTH msgs

[Patrick Wildt]

Hi,

turns out that, as specified in the RFC, the initial Child SA does not
do PFS and is assumed to be secured using the DH exchange in the first
handshake.  Thus there is no KE/N payload in the IKE_AUTH exchange and
we must not include a DH group other than None, which essentially means
we must not supply any DH transforms in the IKE_AUTH messages.

The diff skips adding the DH transforms for initiating and responding
to IKE_AUTH messages.  With this I have a bit more luck with connecting
to Strongswan, as Strongswan throws a no proposal chosen if we include
a DH transform.

ok?

Patrick

diff --git a/sbin/iked/ikev2.c b/sbin/iked/ikev2.c
index b505f763153..286e605fc32 100644
--- a/sbin/iked/ikev2.c
+++ b/sbin/iked/ikev2.c
@@ -114,7 +114,7 @@ int  ikev2_valid_proposal(struct iked_proposal *,
struct iked_transform **, struct iked_transform **, int *);
 
 ssize_t ikev2_add_proposals(struct iked *, struct iked_sa *, struct 
ibuf *,
-   struct iked_proposals *, uint8_t, int, int);
+   struct iked_proposals *, uint8_t, int, int, int);
 ssize_t ikev2_add_cp(struct iked *, struct iked_sa *, struct ibuf *);
 ssize_t ikev2_add_transform(struct ibuf *,
uint8_t, uint8_t, uint16_t, uint16_t);
@@ -971,7 +971,7 @@ ikev2_init_ike_sa_peer(struct iked *env, struct iked_policy 
*pol,
if ((pld = ikev2_add_payload(buf)) == NULL)
goto done;
if ((len = ikev2_add_proposals(env, sa, buf, >pol_proposals,
-   IKEV2_SAPROTO_IKE, sa->sa_hdr.sh_initiator, 0)) == -1)
+   IKEV2_SAPROTO_IKE, sa->sa_hdr.sh_initiator, 0, 0)) == -1)
goto done;
 
if (ikev2_next_payload(pld, len, IKEV2_PAYLOAD_KE) == -1)
@@ -1173,7 +1173,7 @@ ikev2_init_ike_auth(struct iked *env, struct iked_sa *sa)
if ((pld = ikev2_add_payload(e)) == NULL)
goto done;
if ((len = ikev2_add_proposals(env, sa, e, >pol_proposals, 0,
-   sa->sa_hdr.sh_initiator, 0)) == -1)
+   sa->sa_hdr.sh_initiator, 0, 1)) == -1)
goto done;
 
if ((len = ikev2_add_ts(e, , len, sa, 0)) == -1)
@@ -1940,7 +1940,7 @@ ikev2_add_cp(struct iked *env, struct iked_sa *sa, struct 
ibuf *buf)
 ssize_t
 ikev2_add_proposals(struct iked *env, struct iked_sa *sa, struct ibuf *buf,
 struct iked_proposals *proposals, uint8_t protoid, int initiator,
-int sendikespi)
+int sendikespi, int skipdh)
 {
struct ikev2_sa_proposal*sap;
struct iked_transform   *xform;
@@ -1949,7 +1949,7 @@ ikev2_add_proposals(struct iked *env, struct iked_sa *sa, 
struct ibuf *buf,
ssize_t  length = 0, saplength, xflen;
uint64_t spi64;
uint32_t spi32, spi;
-   unsigned int i;
+   unsigned int i, xfi, nxforms;
 
TAILQ_FOREACH(prop, proposals, prop_entry) {
if ((protoid && prop->prop_protoid != protoid) ||
@@ -1984,10 +1984,26 @@ ikev2_add_proposals(struct iked *env, struct iked_sa 
*sa, struct ibuf *buf,
prop->prop_localspi.spi_protoid = IKEV2_SAPROTO_IKE;
}
 
+   /*
+* RFC 7296: 1.2. The Initial Exchanges
+* IKE_AUTH messages do not contain KE/N payloads, thus
+* SA payloads cannot contain groups.
+*/
+   if (skipdh) {
+   nxforms = 0;
+   for (i = 0; i < prop->prop_nxforms; i++) {
+   xform = prop->prop_xforms + i;
+   if (xform->xform_type == IKEV2_XFORMTYPE_DH)
+   continue;
+   nxforms++;
+   }
+   } else
+   nxforms = prop->prop_nxforms;
+
sap->sap_proposalnr = prop->prop_id;
sap->sap_protoid = prop->prop_protoid;
sap->sap_spisize = prop->prop_localspi.spi_size;
-   sap->sap_transforms = prop->prop_nxforms;
+   sap->sap_transforms = nxforms;
saplength = sizeof(*sap);
 
switch (prop->prop_localspi.spi_size) {
@@ -2007,16 +2023,20 @@ ikev2_add_proposals(struct iked *env, struct iked_sa 
*sa, struct ibuf *buf,
break;
}
 
-   for (i = 0; i < prop->prop_nxforms; i++) {
+   for (i = 0, xfi = 0; i < prop->prop_nxforms; i++) {
xform = prop->prop_xforms + i;
 
+   if (skipdh && xform->xform_type == IKEV2_XFORMTYPE_DH)
+   continue;
+
if ((xflen = ikev2_add_transform(buf,
-   i == prop->prop_nxforms - 1 ?
+   xfi == nxforms - 1 ?
IKEV2_XFORM_LAST : 

Re: relayd/ctl alternative control socket

[Sebastian benoit]

Hi,

your diff looks good, but i would rather do it the way bgpd/bgpctl do it:

there the default is  /var/run/bgpd.sock. where  is 
the routing domain bgpctl is running in.  To administer bgpd(8) in a 
different routing domain, run bgpctl in said routing domain.


i.e. it detects the rdomain at startup, bgpctl does the same.

Can you do that in relayd? It was commited there in sometime in summer.

/Benno


On 11/28/17 11:54, Kapetanakis Giannis wrote:

Hi,

On June I've posted a patch about using alternative control socket for relayd 
and relayctl.
There was a comment from David Gwynne which was evaluated.

Is it OK to get this is in order to be able to control multiple relayd daemons 
on different rdomains?

thanks

Giannis

Index: config.c
===
RCS file: /cvs/src/usr.sbin/relayd/config.c,v
retrieving revision 1.35
diff -u -p -r1.35 config.c
--- config.c27 Nov 2017 23:21:16 -  1.35
+++ config.c28 Nov 2017 10:43:37 -
@@ -44,6 +44,7 @@ config_init(struct relayd *env)
env->sc_conf.interval.tv_usec = 0;
env->sc_conf.prefork_relay = RELAY_NUMPROC;
env->sc_conf.statinterval.tv_sec = RELAY_STATINTERVAL;
+   env->sc_ps->ps_csock.cs_name = RELAYD_SOCKET;
}
  
  	ps->ps_what[PROC_PARENT] = CONFIG_ALL;

Index: parse.y
===
RCS file: /cvs/src/usr.sbin/relayd/parse.y,v
retrieving revision 1.220
diff -u -p -r1.220 parse.y
--- parse.y 27 Nov 2017 23:21:16 -  1.220
+++ parse.y 28 Nov 2017 10:43:38 -
@@ -418,6 +418,9 @@ main: INTERVAL NUMBER   {
AGENTX_SOCKET,
sizeof(conf->sc_conf.snmp_path));
}
+   | SOCKET STRING {
+   conf->sc_ps->ps_csock.cs_name = $2;
+   }
;
  
  trap		: /* nothing */		{ $$ = 0; }

Index: relayd.c
===
RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
retrieving revision 1.170
diff -u -p -r1.170 relayd.c
--- relayd.c27 Nov 2017 21:06:26 -  1.170
+++ relayd.c28 Nov 2017 10:43:38 -
@@ -199,9 +199,6 @@ main(int argc, char *argv[])
if ((ps->ps_pw =  getpwnam(RELAYD_USER)) == NULL)
errx(1, "unknown user %s", RELAYD_USER);
  
-	/* Configure the control socket */

-   ps->ps_csock.cs_name = RELAYD_SOCKET;
-
log_init(debug, LOG_DAEMON);
log_setverbose(verbose);
  
Index: relayd.conf.5

===
RCS file: /cvs/src/usr.sbin/relayd/relayd.conf.5,v
retrieving revision 1.180
diff -u -p -r1.180 relayd.conf.5
--- relayd.conf.5   27 Nov 2017 23:21:16 -  1.180
+++ relayd.conf.5   28 Nov 2017 10:43:38 -
@@ -163,6 +163,12 @@ will be used.
  See
  .Xr snmpd.conf 5
  for more information about SNMP configuration.
+.It Ic socket Qo Ar path Qc
+Create a control socket at
+.Ar path .
+By default
+.Pa /var/run/relayd.sock
+is created and no other sockets are created.
  .It Ic timeout Ar number
  Set the global timeout in milliseconds for checks.
  This can be overridden by the timeout value in the table definitions.

Index: relayctl.8
===
RCS file: /cvs/src/usr.sbin/relayctl/relayctl.8,v
retrieving revision 1.32
diff -u -p -r1.32 relayctl.8
--- relayctl.8  28 Nov 2015 01:22:44 -  1.32
+++ relayctl.8  28 Nov 2017 10:43:22 -
@@ -23,6 +23,7 @@
  .Nd control the relay daemon
  .Sh SYNOPSIS
  .Nm
+.Op Fl s Ar socket
  .Ar command
  .Op Ar argument ...
  .Sh DESCRIPTION
@@ -31,6 +32,17 @@ The
  program controls the
  .Xr relayd 8
  daemon.
+.Pp
+The following options are available:
+.Bl -tag -width Ds
+.It Fl s Ar socket
+Use
+.Ar socket
+instead of the default
+.Pa /var/run/relayd.sock
+to communicate with
+.Xr relayd 8 .
+.El
  .Pp
  The following commands are available:
  .Bl -tag -width Ds
Index: relayctl.c
===
RCS file: /cvs/src/usr.sbin/relayctl/relayctl.c,v
retrieving revision 1.57
diff -u -p -r1.57 relayctl.c
--- relayctl.c  3 Sep 2016 14:44:21 -   1.57
+++ relayctl.c  28 Nov 2017 10:43:22 -
@@ -88,7 +88,8 @@ usage(void)
  {
extern char *__progname;
  
-	fprintf(stderr, "usage: %s command [argument ...]\n", __progname);

+   fprintf(stderr, "usage: %s [-s socket] command [argument ...]\n",
+   __progname);
exit(1);
  }
  
@@ -101,9 +102,25 @@ main(int argc, char *argv[])

int  ctl_sock;
int  done = 0;
int  n, verbose = 0;
+   int  ch;
+   const char  *sockname;
+
+   sockname = RELAYD_SOCKET;
+   while ((ch = 

Re: TCP/UDP/etc input w/o KERNEL_LOCK()

[Martin Pieuchot]

On 27/11/17(Mon) 14:01, Alexander Bluhm wrote:
> On Mon, Nov 27, 2017 at 12:20:34PM +0100, Martin Pieuchot wrote:
> > Questions, comments, tests?
> 
> New panic with regress.  I think it was sys/kern/sosplice this time.
> 
> login: panic: kernel diagnostic assertion "_kernel_lock_held()" failed: file 
> "/usr/src/sys/kern/uipc_socket.c", line 1882
> Stopped at  db_enter+0x4:   popl%ebp

Thanks, I'm now grabbing the KERNEL_LOCK() in sohasoutofband() instead
of asserting that we have it.

Index: kern/sys_socket.c
===
RCS file: /cvs/src/sys/kern/sys_socket.c,v
retrieving revision 1.34
diff -u -p -r1.34 sys_socket.c
--- kern/sys_socket.c   14 Nov 2017 16:01:55 -  1.34
+++ kern/sys_socket.c   27 Nov 2017 10:46:15 -
@@ -166,11 +166,11 @@ soo_poll(struct file *fp, int events, st
if (revents == 0) {
if (events & (POLLIN | POLLPRI | POLLRDNORM | POLLRDBAND)) {
selrecord(p, >so_rcv.sb_sel);
-   so->so_rcv.sb_flagsintr |= SB_SEL;
+   so->so_rcv.sb_flags |= SB_SEL;
}
if (events & (POLLOUT | POLLWRNORM)) {
selrecord(p, >so_snd.sb_sel);
-   so->so_snd.sb_flagsintr |= SB_SEL;
+   so->so_snd.sb_flags |= SB_SEL;
}
}
sounlock(s);
Index: kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.209
diff -u -p -r1.209 uipc_socket.c
--- kern/uipc_socket.c  23 Nov 2017 13:45:46 -  1.209
+++ kern/uipc_socket.c  28 Nov 2017 13:27:09 -
@@ -135,6 +135,8 @@ socreate(int dom, struct socket **aso, i
so->so_egid = p->p_ucred->cr_gid;
so->so_cpid = p->p_p->ps_pid;
so->so_proto = prp;
+   task_set(>so_rcv.sb_wtask, sorwakeup_cb, so);
+   task_set(>so_snd.sb_wtask, sowwakeup_cb, so);
 
s = solock(so);
error = (*prp->pr_attach)(so, proto);
@@ -194,7 +196,8 @@ sofree(struct socket *so)
 {
soassertlocked(so);
 
-   if (so->so_pcb || (so->so_state & SS_NOFDREF) == 0)
+   if ((so->so_pcb != NULL) ||
+   (so->so_state & (SS_NOFDREF|SS_NONOTIF)) != (SS_NOFDREF|SS_NONOTIF))
return;
if (so->so_head) {
/*
@@ -273,8 +276,15 @@ drop:
error = error2;
}
 discard:
-   if (so->so_state & SS_NOFDREF)
-   panic("soclose NOFDREF: so %p, so_type %d", so, so->so_type);
+   KASSERT((so->so_state & (SS_NOFDREF|SS_NONOTIF)) == 0);
+   /* Make sure possible delayed notification are delivered. */
+   so->so_state |= SS_NONOTIF;
+   if (!task_del(systq, >so_rcv.sb_wtask) ||
+   !task_del(systq, >so_snd.sb_wtask)) {
+   NET_UNLOCK();
+   taskq_barrier(systq);
+   NET_LOCK();
+   }
so->so_state |= SS_NOFDREF;
sofree(so);
sounlock(s);
@@ -296,9 +306,8 @@ soaccept(struct socket *so, struct mbuf 
int error = 0;
 
soassertlocked(so);
+   KASSERT(so->so_state & SS_NOFDREF);
 
-   if ((so->so_state & SS_NOFDREF) == 0)
-   panic("soaccept !NOFDREF: so %p, so_type %d", so, so->so_type);
so->so_state &= ~SS_NOFDREF;
if ((so->so_state & SS_ISDISCONNECTED) == 0 ||
(so->so_proto->pr_flags & PR_ABRTACPTDIS) == 0)
@@ -1052,12 +1061,8 @@ sorflush(struct socket *so)
sbunlock(so, sb);
aso.so_proto = pr;
aso.so_rcv = *sb;
-   memset(sb, 0, sizeof (*sb));
-   /* XXX - the memset stomps all over so_rcv */
-   if (aso.so_rcv.sb_flags & SB_KNOTE) {
-   sb->sb_sel.si_note = aso.so_rcv.sb_sel.si_note;
-   sb->sb_flags = SB_KNOTE;
-   }
+   memset(>sb_startzero, 0,
+   (caddr_t)>sb_endzero - (caddr_t)>sb_startzero);
if (pr->pr_flags & PR_RIGHTS && pr->pr_domain->dom_dispose)
(*pr->pr_domain->dom_dispose)(aso.so_rcv.sb_mb);
sbrelease(, _rcv);
@@ -1178,8 +1183,8 @@ sosplice(struct socket *so, int fd, off_
 * we sleep, the socket buffers are not marked as spliced yet.
 */
if (somove(so, M_WAIT)) {
-   so->so_rcv.sb_flagsintr |= SB_SPLICE;
-   sosp->so_snd.sb_flagsintr |= SB_SPLICE;
+   so->so_rcv.sb_flags |= SB_SPLICE;
+   sosp->so_snd.sb_flags |= SB_SPLICE;
}
 
  release:
@@ -1196,8 +1201,8 @@ sounsplice(struct socket *so, struct soc
 
task_del(sosplice_taskq, >so_splicetask);
timeout_del(>so_idleto);
-   sosp->so_snd.sb_flagsintr &= ~SB_SPLICE;
-   so->so_rcv.sb_flagsintr &= ~SB_SPLICE;
+   sosp->so_snd.sb_flags &= ~SB_SPLICE;
+   so->so_rcv.sb_flags &= ~SB_SPLICE;
so->so_sp->ssp_socket = sosp->so_sp->ssp_soback = NULL;
if (wakeup && soreadable(so))
  

Re: armv7/sxie: enable phy-supply

[Jonathan Gray]

On Sun, Nov 26, 2017 at 06:52:42PM +0200, Artturi Alm wrote:
> Hi,
> 
> unless i failed w/grep, only 4 boards with dts in u-boot/linux need this,
> but i've got one of those, so this would be much appreciated:)
> 
> -Artturi

This seems reasonable and there is no non-panic error path
there currently to add a regulator_disable() call to.

Changing the variable name to phy_supply would match the existing
code in sys/dev/fdt.

> 
> 
> diff --git a/sys/arch/armv7/sunxi/sxie.c b/sys/arch/armv7/sunxi/sxie.c
> index 116fda5f8d7..b5edab31a09 100644
> --- a/sys/arch/armv7/sunxi/sxie.c
> +++ b/sys/arch/armv7/sunxi/sxie.c
> @@ -51,6 +51,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  
>  /* configuration registers */
> @@ -212,6 +213,7 @@ sxie_attach(struct device *parent, struct device *self, 
> void *aux)
>   struct fdt_attach_args *faa = aux;
>   struct mii_data *mii;
>   struct ifnet *ifp;
> + int physupply;
>   int s;
>  
>   if (faa->fa_nreg < 1)
> @@ -233,6 +235,11 @@ sxie_attach(struct device *parent, struct device *self, 
> void *aux)
>   sxie_socware_init(sc);
>   sc->txf_inuse = 0;
>  
> + /* Power up PHY. */
> + physupply = OF_getpropint(faa->fa_node, "phy-supply", 0);
> + if (physupply)
> + regulator_enable(physupply);
> +
>   sc->sc_ih = arm_intr_establish_fdt(faa->fa_node, IPL_NET,
>   sxie_intr, sc, sc->sc_dev.dv_xname);
>  
> 

Re: sed.1: miscellaneous corrections

[kshe]

On Mon, 27 Nov 2017 10:41:05 +, Jason McIntyre wrote:
> On Sun, Nov 26, 2017 at 07:47:01PM +, kshe wrote:
> > Hi,
> >
> > I noticed a certain number of inaccuracies within the manual page for
> > sed.  The diff below corrects to most obvious ones, although further
> > improvements are certainly possible.
> >
> > Additionally, the script given in the EXAMPLES section being already
> > quite unnecessarily contrived (as it does in twelve commands what could
> > be done in merely two), I took the opportunity to make it slightly
> > simpler and easier to read.
>
> morning.
>
> this diff is too much for review (at least for me anyway). could you
> parcel it up into some logical parts and resend?

These changes fall into five different categories.  The manually split
diffs below include a succinct description of each, in the hope of
making review easier.

1.  Remove false claims about restrictions that do not exist in POSIX
nor in this implementation.  Also clarify that label names may be
terminated by semicolons, although not specified by the standard.

Index: sed.1
===
RCS file: /cvs/src/usr.bin/sed/sed.1,v
retrieving revision 1.50
diff -u -p -r1.50 sed.1
--- sed.1   19 Jul 2017 21:28:19 -  1.50
+++ sed.1   17 Nov 2017 02:50:36 -
@@ -57,10 +57,9 @@ utility reads the specified files, or th
 are specified, modifying the input as specified by a list of commands.
 The input is then written to the standard output.
 .Pp
-A single command may be specified as the first argument to
-.Nm sed .
-Multiple commands may be specified
-separated by newlines or semicolons,
+Commands are separated by newlines or semicolons and may be specified
+either as the first argument to
+.Nm
 or by using the
 .Fl e
 or
@@ -95,7 +94,6 @@ to the list of commands.
 Append the editing commands found in the file
 .Ar command_file
 to the list of commands.
-The editing commands should each be listed on a separate line.
 .It Fl i Ns Op Ar extension
 Edit files in place, saving backups with the specified
 .Ar extension .
@@ -264,23 +262,22 @@ Files are created
 before any input processing begins.
 .Pp
 The
-.Ic b ,
-.Ic r ,
-.Ic s ,
-.Ic t ,
-.Ic w ,
-.Ic y ,
+.Ar label
+argument to the
+.Ic \&: ,
+.Ic b
 and
-.Ic \&:
-functions all accept additional arguments.
-The synopses below indicate which arguments have to be separated from
+.Ic t
+commands may be terminated by either a newline or a semicolon,
+although the former should be prefered if portability is desired.
+.Pp
+For functions that accept additional arguments,
+the synopses below indicate which have to be separated from
 the function letters by whitespace characters.
 .Pp
 Functions can be combined to form a
-.Em function list ,
-a list of
-.Nm
-functions each followed by a newline, as follows:
+.Em function list
+as follows:
 .Bd -literal -offset indent
 { function
   function
@@ -569,12 +565,6 @@ specification.
 The flags
 .Op Fl aEiru
 are extensions to that specification.
-.Pp
-The use of newlines to separate multiple commands on the command line
-is non-portable;
-the use of newlines to separate multiple commands within a command file
-.Pq Fl f Ar command_file
-is portable.
 .Sh HISTORY
 A
 .Nm
@@ -583,8 +573,6 @@ command appeared in
 .Sh CAVEATS
 The use of semicolons to separate multiple commands
 is not permitted for the following commands:
-.Ic a , b , c ,
-.Ic i , r , t ,
-.Ic w , \&: ,
+.Ic a , c , i , r , w
 and
 .Ic # .

2.  Specifying a file name for commands reading from and writing to
files is obviously not optional.

$ sed w
sed: 1: "w": filename expected

Index: sed.1
===
RCS file: /cvs/src/usr.bin/sed/sed.1,v
retrieving revision 1.50
diff -u -p -r1.50 sed.1
--- sed.1   19 Jul 2017 21:28:19 -  1.50
+++ sed.1   17 Nov 2017 02:50:36 -
@@ -255,7 +253,7 @@ as well as the
 flag to the
 .Ic s
 function,
-take an optional
+take a
 .Ar file
 parameter,
 which should be separated from the function or flag by whitespace.

3.  Correct the description of the `y' command: a backslash only escapes
specific characters.

Index: sed.1
===
RCS file: /cvs/src/usr.bin/sed/sed.1,v
retrieving revision 1.50
diff -u -p -r1.50 sed.1
--- sed.1   19 Jul 2017 21:28:19 -  1.50
+++ sed.1   17 Nov 2017 02:50:36 -
@@ -486,10 +483,14 @@ Within
 .Ar string1
 and
 .Ar string2 ,
-a backslash followed by any character other than a newline is that literal
-character, and a backslash followed by an
+a backslash followed by another backslash
+is replaced by a single backslash,
+a backslash followed by an
 .Sq n
-is replaced by a newline character.
+is replaced by a newline character,
+and a backslash followed by the delimiting character
+is replaced by a that character,
+causing it to be treated literally.
 .It [0addr] Ns Ic \&: Ns Ar 

Re: Add Diffie-Hellman group negotiation to iked

[Patrick Wildt]

On Mon, Nov 27, 2017 at 06:12:22PM +0100, Patrick Wildt wrote:
> On Mon, Nov 27, 2017 at 04:21:08PM +0100, Patrick Wildt wrote:
> > On Wed, Nov 22, 2017 at 05:26:24PM +0100, Patrick Wildt wrote:
> > > On 2017/06/25 21:44, Tim Stewart wrote:
> > > > My first patch did, in fact, break Child SAs rekeying.  I have a new
> > > > patch at the end of this message that simply restricts DH group
> > > > negotiation to IKE SAs (I *think* that DH group guessing only applies to
> > > > IKE SAs, and perhaps only the IKE_SA_INIT exchange, but I'm still
> > > > working through the RFC).  This may not be the ultimate solution, but it
> > > > does allow us to move forward.
> > > 
> > > Reading RFC 7296 it looks like throwing INVALID_KE_PAYLOAD is fine for
> > > both establishing the IKE SA and rekeying the Child SAs.  If we select a
> > > proposal from the msg that uses a different DH group than the one that's
> > > used in the KEi (in the same msg) we need to throw INVALID_KE_PAYLOAD.
> > > 
> > > Since all messages subsequent to the initial exchange must be encrypted,
> > > the INVALID_KE_PAYLOAD message on rekeying Child SAs must be encrypted.
> > > Apparently with the previous diff the Child SA rekeying failed.  This is
> > > because the code sends the INVALID_KE_PAYLOAD response unencrypted.
> > > 
> > > Also I have found inconsistencies in handling INVALID_KE_PAYLOAD with us
> > > acting as initiator.  I will take a look at both cases and will follow
> > > up.
> > > 
> > > Patrick
> > 
> > Hi,
> > 
> > This diff adds suport to rejecting IKE SA messages.  This means that we
> > can reply to IKE SA INIT messages with no proposal chosen, as we already
> > do for Child SAs.  For that the error "adding" is done in a new function
> > shared by both send error handlers.  We need two "send error" functions
> > because the init error is unencrypted, while all later ones are not.
> > Now we can add more cases, like Child SA not found or that the DH group
> > is not what we expect.  I think that is quite an improvement.
> > 
> > One "issue" is retransmits.  The RFC specifies that IKEv2 is a reliable
> > protocol.  When a packet is lost, the initiator has to retransmit the
> > request and the responder must retransmit the saved reply.  On IKE SA
> > INIT error we don't save our response, which means that we will handle
> > the request as if we never had it.  The RFC says we shouldn't do that,
> > because DoS protection and so on.  What we should do is keep our reply
> > around and check if it is indeed a retransmit.  If so we reply with the
> > same reply, thus saving resources.  If it is an attempt to create a new
> > IKE SA, drop the old SA so that iked(8) can attempt to create a new SA.
> > 
> > Thoughts?
> > 
> > Patrick

Updated diff also responds with INVALID_KE_PAYLOAD when the responder
has a lower SA lifetime and initiates the CHILD_SA_CREATE.

Handling rejections (as in receiving INVALID_KE_PAYLOAD) is still a ToDo
I want to tackle next.

ok?

diff --git a/sbin/iked/iked.h b/sbin/iked/iked.h
index a0059682bc8..01f8ac00506 100644
--- a/sbin/iked/iked.h
+++ b/sbin/iked/iked.h
@@ -508,6 +508,7 @@ struct iked_message {
struct iked_proposalsmsg_proposals;
struct iked_spi  msg_rekey;
struct ibuf *msg_nonce; /* dh NONCE */
+   uint16_t msg_dhgroup;   /* dh group */
struct ibuf *msg_ke;/* dh key exchange */
struct iked_id   msg_auth;  /* AUTH payload */
struct iked_id   msg_id;
diff --git a/sbin/iked/ikev2.c b/sbin/iked/ikev2.c
index b505f763153..b8cef9061fc 100644
--- a/sbin/iked/ikev2.c
+++ b/sbin/iked/ikev2.c
@@ -76,6 +76,7 @@ intikev2_resp_ike_eap(struct iked *, struct iked_sa *, 
struct ibuf *);
 int ikev2_send_auth_failed(struct iked *, struct iked_sa *);
 int ikev2_send_error(struct iked *, struct iked_sa *,
struct iked_message *, uint8_t);
+int ikev2_send_init_error(struct iked *, struct iked_message *);
 
 int ikev2_send_create_child_sa(struct iked *, struct iked_sa *,
struct iked_spi *, uint8_t);
@@ -125,6 +126,7 @@ ssize_t  ikev2_add_certreq(struct ibuf *, struct 
ikev2_payload **, ssize_t,
 ssize_t ikev2_add_ipcompnotify(struct iked *, struct ibuf *,
struct ikev2_payload **, ssize_t, struct iked_sa *);
 ssize_t ikev2_add_ts_payload(struct ibuf *, unsigned int, struct 
iked_sa *);
+ssize_t ikev2_add_error(struct iked *, struct ibuf *, struct 
iked_message *);
 int ikev2_add_data(struct ibuf *, void *, size_t);
 int ikev2_add_buf(struct ibuf *buf, struct ibuf *);
 
@@ -455,6 +457,23 @@ ikev2_recv(struct iked *env, struct iked_message *msg)
if ((m = ikev2_msg_lookup(env, >sa_requests, msg, hdr)))
ikev2_msg_dispose(env, >sa_requests, m);
} else {
+   /*
+* IKE_SA_INIT is special since it always uses the message id 0.
+  

Re: dc(1): minor cleanup

[kshe]

On Tue, 28 Nov 2017 08:08:24 +, Otto Moerbeek wrote:
> On Sun, Nov 26, 2017 at 07:25:46PM +, kshe wrote:
> > Hi,
> >
> > The diff below encompasses three unrelated minor changes.
> >
> > 1.  Merge the not_equal(), not_less() and not_greater() functions into
> > their caller; these functions cannot be called from the jump table, so
> > it is confusing to define them as if they could.
> >
> > 2.  Make warnings consistent by using warnx(3) everywhere.
> >
> > 3.  Add a missing parenthesis in a comment.
>
> I committed this; but you diff does not apply, I had to fix it. Looks
> like you edited line numbers manually or something like that.

This is actually the opposite: I extracted this diff from a larger one
without editing line numbers, as I thought they did not matter much for
contextual diffs.  Apparently, this works well as long as two unrelated
changes are more than six lines apart; otherwise, well, they appear in
the same window and things get more complicated.

By the way, while fixing my broken patch, it seems that you forgot to
remove the newline at the end of the argument to warnx() in
not_compare().

Regards,

kshe

Hide carp pass and sppp authname from userland

[Jeremie Courreges-Anglas]

On Mon, Nov 27 2017, Stefan Sperling  wrote:
> On Mon, Nov 27, 2017 at 01:31:17AM +0100, Stefan Sperling wrote:
>> On Sun, Nov 26, 2017 at 06:17:14PM +0100, Jeremie Courreges-Anglas wrote:
>> > 
>> > I don't think anything has been committed regarding this issue, right?
>> 
>> Nope.
>> 
>> I've been discussing this with people in person.
>> Will summarize those discussions and send a new diff soon.
>
> Most people I've talked to seem to be OK with never exposing
> these secrets to userland in the first place.
>
> OK?

As discussed with stsp, who would rather focus on wifi, the carp and
sppp parts need more love.

To stop the "get" ioctls from passing sppp authname and carp pass to
userland, the "set" ioctls should test whether those are actually set in
the request, instead of zeroing them.

The sppp authname is a NUL-terminated string, but the carp pass is
treated as a blob so I'm just checking if it contains at least one
non-NUL byte.  I performed a bunch of tests but this could use more
eyes.


Index: net/if_spppsubr.c
===
RCS file: /d/cvs/src/sys/net/if_spppsubr.c,v
retrieving revision 1.173
diff -u -p -r1.173 if_spppsubr.c
--- net/if_spppsubr.c   20 Oct 2017 09:35:09 -  1.173
+++ net/if_spppsubr.c   28 Nov 2017 10:48:39 -
@@ -4588,12 +4588,14 @@ sppp_set_params(struct sppp *sp, struct 
auth->flags = spa->flags;
 
spa->name[AUTHMAXLEN - 1] = '\0';
-   len = strlen(spa->name) + 1;
-   p = malloc(len, M_DEVBUF, M_WAITOK);
-   strlcpy(p, spa->name, len);
-   if (auth->name != NULL)
-   free(auth->name, M_DEVBUF, 0);
-   auth->name = p;
+   if (spa->name[0] != '\0') {
+   len = strlen(spa->name) + 1;
+   p = malloc(len, M_DEVBUF, M_WAITOK);
+   strlcpy(p, spa->name, len);
+   if (auth->name != NULL)
+   free(auth->name, M_DEVBUF, 0);
+   auth->name = p;
+   }
 
if (spa->secret[0] != '\0') {
spa->secret[AUTHMAXLEN - 1] = '\0';
Index: netinet/ip_carp.c
===
RCS file: /d/cvs/src/sys/netinet/ip_carp.c,v
retrieving revision 1.320
diff -u -p -r1.320 ip_carp.c
--- netinet/ip_carp.c   23 Nov 2017 13:32:25 -  1.320
+++ netinet/ip_carp.c   28 Nov 2017 10:48:39 -
@@ -2118,7 +2118,14 @@ carp_ioctl(struct ifnet *ifp, u_long cmd
carp_set_enaddr(sc);
carp_update_lsmask(sc);
}
-   bcopy(carpr.carpr_key, sc->sc_key, sizeof(sc->sc_key));
+   /* only update the carp key if non-zero */
+   for (i = 0; i < CARP_KEY_LEN; i++) {
+   if (carpr.carpr_key[i] != '\0') {
+   bcopy(carpr.carpr_key, sc->sc_key,
+   sizeof(sc->sc_key));
+   break;
+   }
+   }
if (error > 0)
error = EINVAL;
else {


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

Re: relayd/ctl alternative control socket

[Kapetanakis Giannis]

Hi,

On June I've posted a patch about using alternative control socket for relayd 
and relayctl.
There was a comment from David Gwynne which was evaluated. 

Is it OK to get this is in order to be able to control multiple relayd daemons 
on different rdomains?

thanks

Giannis

Index: config.c
===
RCS file: /cvs/src/usr.sbin/relayd/config.c,v
retrieving revision 1.35
diff -u -p -r1.35 config.c
--- config.c27 Nov 2017 23:21:16 -  1.35
+++ config.c28 Nov 2017 10:43:37 -
@@ -44,6 +44,7 @@ config_init(struct relayd *env)
env->sc_conf.interval.tv_usec = 0;
env->sc_conf.prefork_relay = RELAY_NUMPROC;
env->sc_conf.statinterval.tv_sec = RELAY_STATINTERVAL;
+   env->sc_ps->ps_csock.cs_name = RELAYD_SOCKET;
}
 
ps->ps_what[PROC_PARENT] = CONFIG_ALL;
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/relayd/parse.y,v
retrieving revision 1.220
diff -u -p -r1.220 parse.y
--- parse.y 27 Nov 2017 23:21:16 -  1.220
+++ parse.y 28 Nov 2017 10:43:38 -
@@ -418,6 +418,9 @@ main: INTERVAL NUMBER   {
AGENTX_SOCKET,
sizeof(conf->sc_conf.snmp_path));
}
+   | SOCKET STRING {
+   conf->sc_ps->ps_csock.cs_name = $2;
+   }
;
 
 trap   : /* nothing */ { $$ = 0; }
Index: relayd.c
===
RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
retrieving revision 1.170
diff -u -p -r1.170 relayd.c
--- relayd.c27 Nov 2017 21:06:26 -  1.170
+++ relayd.c28 Nov 2017 10:43:38 -
@@ -199,9 +199,6 @@ main(int argc, char *argv[])
if ((ps->ps_pw =  getpwnam(RELAYD_USER)) == NULL)
errx(1, "unknown user %s", RELAYD_USER);
 
-   /* Configure the control socket */
-   ps->ps_csock.cs_name = RELAYD_SOCKET;
-
log_init(debug, LOG_DAEMON);
log_setverbose(verbose);
 
Index: relayd.conf.5
===
RCS file: /cvs/src/usr.sbin/relayd/relayd.conf.5,v
retrieving revision 1.180
diff -u -p -r1.180 relayd.conf.5
--- relayd.conf.5   27 Nov 2017 23:21:16 -  1.180
+++ relayd.conf.5   28 Nov 2017 10:43:38 -
@@ -163,6 +163,12 @@ will be used.
 See
 .Xr snmpd.conf 5
 for more information about SNMP configuration.
+.It Ic socket Qo Ar path Qc
+Create a control socket at
+.Ar path .
+By default
+.Pa /var/run/relayd.sock
+is created and no other sockets are created.
 .It Ic timeout Ar number
 Set the global timeout in milliseconds for checks.
 This can be overridden by the timeout value in the table definitions.

Index: relayctl.8
===
RCS file: /cvs/src/usr.sbin/relayctl/relayctl.8,v
retrieving revision 1.32
diff -u -p -r1.32 relayctl.8
--- relayctl.8  28 Nov 2015 01:22:44 -  1.32
+++ relayctl.8  28 Nov 2017 10:43:22 -
@@ -23,6 +23,7 @@
 .Nd control the relay daemon
 .Sh SYNOPSIS
 .Nm
+.Op Fl s Ar socket
 .Ar command
 .Op Ar argument ...
 .Sh DESCRIPTION
@@ -31,6 +32,17 @@ The
 program controls the
 .Xr relayd 8
 daemon.
+.Pp
+The following options are available:
+.Bl -tag -width Ds
+.It Fl s Ar socket
+Use
+.Ar socket
+instead of the default
+.Pa /var/run/relayd.sock
+to communicate with
+.Xr relayd 8 .
+.El
 .Pp
 The following commands are available:
 .Bl -tag -width Ds
Index: relayctl.c
===
RCS file: /cvs/src/usr.sbin/relayctl/relayctl.c,v
retrieving revision 1.57
diff -u -p -r1.57 relayctl.c
--- relayctl.c  3 Sep 2016 14:44:21 -   1.57
+++ relayctl.c  28 Nov 2017 10:43:22 -
@@ -88,7 +88,8 @@ usage(void)
 {
extern char *__progname;
 
-   fprintf(stderr, "usage: %s command [argument ...]\n", __progname);
+   fprintf(stderr, "usage: %s [-s socket] command [argument ...]\n",
+   __progname);
exit(1);
 }
 
@@ -101,9 +102,25 @@ main(int argc, char *argv[])
int  ctl_sock;
int  done = 0;
int  n, verbose = 0;
+   int  ch;
+   const char  *sockname;
+
+   sockname = RELAYD_SOCKET;
+   while ((ch = getopt(argc, argv, "s:")) != -1) {
+   switch (ch) {
+   case 's':
+   sockname = optarg;
+   break;
+   default:
+   usage();
+   /* NOTREACHED */
+   }
+   }
+   argc -= optind;
+   argv += optind;
 
/* parse options */
-   if ((res = parse(argc - 1, argv + 1)) == NULL)
+   if ((res = parse(argc, argv)) == 

Re: dc(1) mishandles fractional input in bases other than 10

[Otto Moerbeek]

On Sun, Nov 26, 2017 at 07:51:13PM +, kshe wrote:

> Hi,
> 
> The following behaviour seems unacceptable to me.
> 
>   $ dc -e '16dio .C .C0 f'
>   .C0
>   .B
> 
>   $ dc -e '8dio .3 .30 .300 f'
>   .3000
>   .275
>   .23
> 
> This bug affects all bases other than 10 and increasing the scale with
> the `k' command does not prevent it.  The only reliable way to input a
> rational number in these bases is therefore to input an integer first,
> and then divide it by the appropriate factor; alternatively, as
> illustrated above, adding meaningless zeros to the digit expansion can
> help, although I have not verified that this works in all cases nor how
> many zeros would need to be added to guarantee the expected result
> every time.
> 
> I could try to provide a patch for this, but I dislike how the current
> code gives special privileges to base 10 at the expense of all the
> others, even though it is supposed to accept input in any base; so my
> patch would probably be deemed too invasive as I would want to change
> too many things at once, including in areas irrelevant to this
> particular issue.
> 
> If a less intrusive workaround is not possible, then perhaps at least a
> warning about fractional input being essentially broken in bases other
> than 10 could be added to the manual.
> 
> Regards,
> 
> kshe

I implemented dc(1) to stay as close as possible to the original dc(1)
and GNU dc(1). Any change in the way fractional numbers are handled
(even though you might consider it broken) will break existing
programs.

-Otto

Re: dc(1): dead store

[Otto Moerbeek]

On Sun, Nov 26, 2017 at 07:43:22PM +, kshe wrote:

> Hi,
> 
> This assignment is useless.
> 
> Index: bcode.c
> ===
> RCS file: /cvs/src/usr.bin/dc/bcode.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 bcode.c
> --- bcode.c   26 Feb 2017 11:29:55 -  1.51
> +++ bcode.c   26 Oct 2017 04:44:01 -
> @@ -1630,7 +1611,7 @@ skip_until_mark(void)
>   free(read_string([bmachine.readsp]));
>   break;
>   case '!':
> - switch (ch = readch()) {
> + switch (readch()) {
>   case '<':
>   case '>':
>   case '=':
> 
> The above diff does not cause any change in the optimised executable
> output, because such removal was obviously already performed by the
> compiler.

I just committed a diff that just eliminates the 'int ch' altogether.

-Otto

> 
> Alternatively, while here, this function could be slightly shortened as
> follows.
> 
> Index: bcode.c
> ===
> RCS file: /cvs/src/usr.bin/dc/bcode.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 bcode.c
> --- bcode.c   26 Feb 2017 11:29:55 -  1.51
> +++ bcode.c   17 Nov 2017 02:38:12 -
> @@ -1610,7 +1590,13 @@ skip_until_mark(void)
>   return;
>   case EOF:
>   errx(1, "mark not found");
> - return;
> + case '!':
> + ch = readch();
> + if (ch != '<' && ch != '>' && ch != '=') {
> + free(readline());
> + break;
> + }
> + /* fall through */
>   case 'l':
>   case 'L':
>   case 's':
> @@ -1629,22 +1615,6 @@ skip_until_mark(void)
>   case '[':
>   free(read_string([bmachine.readsp]));
>   break;
> - case '!':
> - switch (ch = readch()) {
> - case '<':
> - case '>':
> - case '=':
> - (void)readreg();
> - if (readch() == 'e')
> - (void)readreg();
> - else
> - unreadch();
> - break;
> - default:
> - free(readline());
> - break;
> - }
> - break;
>   default:
>   break;
>   }
> 
> Regards,
> 
> kshe

Re: pf divert type

[Alexandr Nedvedicky]

Hello,

your change looks good to me as-is. Though I have one small
suggestion, which I don't insist on:


> Index: sys/netinet/raw_ip.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet/raw_ip.c,v
> retrieving revision 1.106
> diff -u -p -r1.106 raw_ip.c
> --- sys/netinet/raw_ip.c  20 Nov 2017 10:35:24 -  1.106
> +++ sys/netinet/raw_ip.c  27 Nov 2017 23:25:27 -
> @@ -149,7 +149,7 @@ rip_input(struct mbuf **mp, int *offp, i
>   /* XXX rdomain support */
>   if ((divert = pf_find_divert(m)) == NULL)
>   continue;
> - if (!divert->addr.v4.s_addr)
KASSERT((divert->type == PF_DIVERT_REPLY) ||
(divert-type == PF_DIVERT_TO));
> + if (divert->type == PF_DIVERT_REPLY)
>   goto divert_reply;
>   if (inp->inp_laddr.s_addr != divert->addr.v4.s_addr)
>   continue;
> Index: sys/netinet6/raw_ip6.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/netinet6/raw_ip6.c,v
> retrieving revision 1.123
> diff -u -p -r1.123 raw_ip6.c
> --- sys/netinet6/raw_ip6.c20 Nov 2017 10:35:24 -  1.123
> +++ sys/netinet6/raw_ip6.c27 Nov 2017 23:25:27 -
> @@ -152,7 +152,7 @@ rip6_input(struct mbuf **mp, int *offp, 
>   /* XXX rdomain support */
>   if ((divert = pf_find_divert(m)) == NULL)
>   continue;
KASSERT((divert->type == PF_DIVERT_REPLY) ||
(divert-type == PF_DIVERT_TO));
> - if (IN6_IS_ADDR_UNSPECIFIED(>addr.v6))
> + if (divert->type == PF_DIVERT_REPLY)
>   goto divert_reply;
>   if (!IN6_ARE_ADDR_EQUAL(>inp_laddr6,
>   >addr.v6))


OK sashan@

Re: filedesc's locking.

[Martin Pieuchot]

On 27/11/17(Mon) 19:32, Mathieu - wrote:
> Martin Pieuchot wrote:
> > On 27/11/17(Mon) 11:49, Mathieu - wrote:
> > > Hi everyone,
> > > 
> > > I was looking / poking around the filedesc handling in kern_descrip.c
> > > and found the locking a bit.. weird,
> > 
> > Can you define "weird"?  Is it a taste thing or did you find any bug?
> > If it's a bug how can you reproduce it?
> 
> Well it's a mix. Yeah a bit of a personnal taste (which means nothing
> here), but also the fact that the current locking looks buggy to me (see
> below).

Changes for personal taste should be avoided.

> > >  especially the fd_getfile function
> > > is touching protected members of the filedesc w/o taking any lock.
> > 
> > Which "protected members" are you talking about?  What makes you think
> > they are protected?  What do you mean with protected?  Why do you think
> > they need a lock?
> 
> I'm speaking of the fd_ofiles, fd_ofileflags and fd_(lo|hi)map members
> of the filedesc struct. And by protected I mean that their access needs
> to be done with the fd_lock held. (It's hinted in the struct commentaries,
> and the lock was introduced to prevent racy access of those members).

You're not giving any fact.  You're argumentation is poor.  What you're
saying is "it should be like that".  Well that might be true, but unless
you can say "why" nobody is going to work with you.

> > > This has been already hit previously in [1] and fixed by reordering the
> > > malloc / free operations in rev 1.138 of kern_descrip.c ([2])
> > 
> > The bug that has been found has been fixed.  What are you trying to do?
> 
> Yes it's been fixed but what I argue, is that fd_getfile is still
> touching fd_ofiles w/o serializing the access wrt to the writes.

Where, how, what?  Fact, fact, fact, fact!  Stop the bullshit.  Give
concrete examples, stop wasting our time :)

> So while the bug is fixed, that I agree with you, what I mean is that
> fd_getfile is error prone in its current form and can be wrongly used.

Still no fact, a lot of words.

> > > At the time it was proposed to actually lock fd_getfile which entailed
> > > fixing all the callers.
> > 
> > This could be useful for a different reason.  Can you guess which one?

Why didn't you answer this question?

> > > I tried to go this way but it proved difficult
> > > as sometimes the locking is done one level higher (e.g namei() being
> > > called with or without the filedesc lock held).
> > > called with or without the filedesc lock held). Introducing a _locked
> > > version of the file was also not feasible for the same reasons (and
> > > anyway it was a bit ugly).
> > 
> > Yes it's a complicated task.
> 
> It is, and, imho it makes things a lot more complex when trying to solve
> it this way.

Some times complexity is needed.  We cannot decide anything yet.  We
don't have the required information.

> > > So I went down the rabbit hole and tried changing the semantics of the
> > > whole filedesc manipulations in order to make this clearer.
> > 
> > Can you define "this"?
> 
> Sorry, english not being my first language this wasn't as clear as I
> intended it to be. By "this" I meant to say, "the locking semantics" (of
> filedesc).

You're English is fine, your reasoning is not.  It is too abstract.
What do you mean by "the locking semantics"?

> > > I know the diff is a bit big and scary because it touches sensitive
> > > paths... but I wanted to have an opinion on it before going further.
> > 
> > Before you go any further you need:
> > 
> >  - to explain what you want to achieve
> >  - to have a way to test your changes
> >  - to convince us that this is what we want
> > 
> > Moving the lock above a level might be something we want.  But every
> > time you move the lock/unlock dance to a caller function you might
> > introduce a deadlock.  So a big diff is not the way to go.  You might
> > also not need to fix all the code paths at once.  It depends what you're
> > trying to achieve.
> 
> 
> Well it all started with looking at file_desc handling for fun. My
> endgame, as it seems that's what you are asking, is to make the filedesc
> code mpsafe. That's a pretty big task, you are one to know!, so on the
> road to there what I wanted first was to get the locking more
> straightforward than it is (and that's why I mentionned FIF_LARVAL but
> let's not go this way ..)

I'm not sure what you mean with straightforward.  What I understand from
your diff is that it means "try to put everything under a lock without
understanding".  Understanding is more important than the diffs you're
sending.  Because then you can debug the problems.  If you don't
understand what needs to be protected from what in which situation
you're doomed to revert your diff and try, try and try without being
sure to succeed.

> > > It has been running here for a few days, on a workstation, I also ran
> > > the regress tests w/o any regressions.
> > 
> > On which 

Re: dc(1): minor cleanup

[Otto Moerbeek]

On Sun, Nov 26, 2017 at 07:25:46PM +, kshe wrote:

> Hi,
> 
> The diff below encompasses three unrelated minor changes.
> 
> 1.  Merge the not_equal(), not_less() and not_greater() functions into
> their caller; these functions cannot be called from the jump table, so
> it is confusing to define them as if they could.
> 
> 2.  Make warnings consistent by using warnx(3) everywhere.
> 
> 3.  Add a missing parenthesis in a comment.

I committed this; but you diff does not apply, I had to fix it. Looks
like you edited line numbers manually or something like that.

To make this process more smooth always cvs diff -pu against current. It
is also not very effective to send a bunch of diffs in burst mode. One
or two outstanding diffs works much better.

-Otto

> 
> Index: bcode.c
> ===
> RCS file: /cvs/src/usr.bin/dc/bcode.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 bcode.c
> --- bcode.c   26 Feb 2017 11:29:55 -  1.51
> +++ bcode.c   17 Nov 2017 02:38:12 -
> @@ -95,18 +95,14 @@ static void   bdiv(void);
>  static void  less_numbers(void);
>  static void  lesseq_numbers(void);
>  static void  equal(void);
> -static void  not_equal(void);
>  static void  less(void);
> -static void  not_less(void);
>  static void  greater(void);
> -static void  not_greater(void);
>  static void  not_compare(void);
>  static bool  compare_numbers(enum bcode_compare, struct number *,
>   struct number *);
> @@ -1195,7 +1207,7 @@ bexp(void)
>   negate(p);
>   rscale = bmachine.scale;
>   } else {
> - /* Posix bc says min(a.scale * b, max(a.scale, scale) */
> + /* Posix bc says min(a.scale * b, max(a.scale, scale)) */
>   u_long  b;
>   u_int   m;
>  
> @@ -1402,12 +1400,6 @@ lesseq_numbers(void)
>  }
>  
>  static void
> -not_equal(void)
> -{
> - compare(BCODE_NOT_EQUAL);
> -}
> -
> -static void
>  less(void)
>  {
>   compare(BCODE_LESS);
> @@ -1418,39 +1410,27 @@ not_compare(void)
>  {
>   switch (readch()) {
>   case '<':
> - not_less();
> + compare(BCODE_NOT_LESS);
>   break;
>   case '>':
> - not_greater();
> + compare(BCODE_NOT_GREATER);
>   break;
>   case '=':
> - not_equal();
> + compare(BCODE_NOT_EQUAL);
>   break;
>   default:
>   unreadch();
> - (void)fprintf(stderr, "! command is deprecated\n");
> + warnx("! command is deprecated");
>   break;
>   }
>  }
>  
>  static void
> -not_less(void)
> -{
> - compare(BCODE_NOT_LESS);
> -}
> -
> -static void
>  greater(void)
>  {
>   compare(BCODE_GREATER);
>  }
>  
> -static void
> -not_greater(void)
> -{
> - compare(BCODE_NOT_GREATER);
> -}
> -
>  static bool
>  compare_numbers(enum bcode_compare type, struct number *a, struct number *b)
>  {
> 
> Regards,
> 
> kshe