Re: ifconfig description for wireguard peers
On Thu, Nov 10, 2022 at 12:53:07AM +, Mikolaj Kucharski wrote:
> On Wed, Oct 20, 2021 at 10:20:09PM -0400, Noah Meier wrote:
> > Hi,
> >
> > While wireguard interfaces can have a description set by ifconfig,
> > wireguard peers currently cannot. I now have a lot of peers and
> > descriptions of them in ifconfig would be helpful.
> >
> > This diff adds a 'wgdesc' option to a 'wgpeer' in ifconfig (and a
> > corresponding '-wgdesc' option). Man page also updated.
> >
> > NM
>
> Now that my `ifconfig, wireguard output less verbose, unless -A or `
> diff is commited ( see https://marc.info/?t=16577915002=1=2 ),
> bump of an old thread.
>
> Below is rebased on -current and tiny modified by me, Noah's diff.
>
> You need both kernel and ifconfig with below code, otherwise you may see
> issues bringing up wg(4) interface. If you may loose access to machine
> behind wg(4) VPN, make sure you update on that machine both kernel and
> ifconfig(8) at the same time.
>
Typo, s/wgpesc/wgdesc/
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.457
diff -u -p -u -r1.457 ifconfig.c
--- sbin/ifconfig/ifconfig.c26 Oct 2022 17:06:31 - 1.457
+++ sbin/ifconfig/ifconfig.c10 Nov 2022 01:03:04 -
@@ -355,12 +355,14 @@ void setwgpeerep(const char *, const cha
void setwgpeeraip(const char *, int);
void setwgpeerpsk(const char *, int);
void setwgpeerpka(const char *, int);
+void setwgpeerdesc(const char *, int);
void setwgport(const char *, int);
void setwgkey(const char *, int);
void setwgrtable(const char *, int);
void unsetwgpeer(const char *, int);
void unsetwgpeerpsk(const char *, int);
+void unsetwgpeerdesc(const char *, int);
void unsetwgpeerall(const char *, int);
void wg_status(int);
@@ -620,11 +622,13 @@ const struct cmd {
{ "wgaip", NEXTARG,A_WIREGUARD,setwgpeeraip},
{ "wgpsk", NEXTARG,A_WIREGUARD,setwgpeerpsk},
{ "wgpka", NEXTARG,A_WIREGUARD,setwgpeerpka},
+ { "wgdesc", NEXTARG,A_WIREGUARD,setwgpeerdesc},
{ "wgport", NEXTARG,A_WIREGUARD,setwgport},
{ "wgkey", NEXTARG,A_WIREGUARD,setwgkey},
{ "wgrtable", NEXTARG,A_WIREGUARD,setwgrtable},
{ "-wgpeer",NEXTARG,A_WIREGUARD,unsetwgpeer},
{ "-wgpsk", 0, A_WIREGUARD,unsetwgpeerpsk},
+ { "-wgdesc",0, A_WIREGUARD,unsetwgpeerdesc},
{ "-wgpeerall", 0, A_WIREGUARD,unsetwgpeerall},
#else /* SMALL */
@@ -5843,6 +5847,16 @@ setwgpeerpka(const char *pka, int param)
}
void
+setwgpeerdesc(const char *wgdesc, int param)
+{
+ if (wg_peer == NULL)
+ errx(1, "wgdesc: wgpeer not set");
+ if (strlen(wgdesc))
+ strlcpy(wg_peer->p_description, wgdesc, IFDESCRSIZE);
+ wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
+}
+
+void
setwgport(const char *port, int param)
{
const char *errmsg = NULL;
@@ -5889,6 +5903,15 @@ unsetwgpeerpsk(const char *value, int pa
}
void
+unsetwgpeerdesc(const char *value, int param)
+{
+ if (wg_peer == NULL)
+ errx(1, "wgdesc: wgpeer not set");
+ strlcpy(wg_peer->p_description, "", IFDESCRSIZE);
+ wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
+}
+
+void
unsetwgpeerall(const char *value, int param)
{
ensurewginterface();
@@ -5948,6 +5971,9 @@ wg_status(int ifaliases)
b64_ntop(wg_peer->p_public, WG_KEY_LEN,
key, sizeof(key));
printf("\twgpeer %s\n", key);
+
+ if (strlen(wg_peer->p_description))
+ printf("\t\twgdesc %s\n",
wg_peer->p_description);
if (wg_peer->p_flags & WG_PEER_HAS_PSK)
printf("\t\twgpsk (present)\n");
Index: share/man/man4/wg.4
===
RCS file: /cvs/src/share/man/man4/wg.4,v
retrieving revision 1.10
diff -u -p -u -r1.10 wg.4
--- share/man/man4/wg.4 14 Mar 2021 10:08:38 - 1.10
+++ share/man/man4/wg.4 10 Nov 2022 01:03:04 -
@@ -42,6 +42,19 @@ configuration file for
.Xr netstart 8 .
The interface itself can be configured with
.Xr ifconfig 8 .
+To display
+.Cm wgpeer
+information for each
+.Nm wg
+interface option
+.Fl A
+to
+.Xr ifconfig 8
+should be used or
+.Nm wg
+interface should be specified as an argument to
+.Xr ifconfig 8
+command.
.Pp
.Nm wg
interfaces support the following
Index: sys/net/if_wg.c
===
RCS file: /cvs/src/sys/net/if_wg.c,v
retrieving revision 1.26
diff -u -p -u -r1.26 if_wg.c
--- sys/net/if_wg.c 21 Jul 2022 11:26:50 - 1.26
+++
Re: ifconfig description for wireguard peers
On Wed, Oct 20, 2021 at 10:20:09PM -0400, Noah Meier wrote:
> Hi,
>
> While wireguard interfaces can have a description set by ifconfig, wireguard
> peers currently cannot. I now have a lot of peers and descriptions of them in
> ifconfig would be helpful.
>
> This diff adds a 'wgdesc' option to a 'wgpeer' in ifconfig (and a
> corresponding '-wgdesc' option). Man page also updated.
>
> NM
Now that my `ifconfig, wireguard output less verbose, unless -A or `
diff is commited ( see https://marc.info/?t=16577915002=1=2 ),
bump of an old thread.
Below is rebased on -current and tiny modified by me, Noah's diff.
You need both kernel and ifconfig with below code, otherwise you may see
issues bringing up wg(4) interface. If you may loose access to machine
behind wg(4) VPN, make sure you update on that machine both kernel and
ifconfig(8) at the same time.
Index: sbin/ifconfig/ifconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/ifconfig.c,v
retrieving revision 1.457
diff -u -p -u -r1.457 ifconfig.c
--- sbin/ifconfig/ifconfig.c26 Oct 2022 17:06:31 - 1.457
+++ sbin/ifconfig/ifconfig.c5 Nov 2022 19:41:22 -
@@ -355,12 +355,14 @@ void setwgpeerep(const char *, const cha
void setwgpeeraip(const char *, int);
void setwgpeerpsk(const char *, int);
void setwgpeerpka(const char *, int);
+void setwgpeerdesc(const char *, int);
void setwgport(const char *, int);
void setwgkey(const char *, int);
void setwgrtable(const char *, int);
void unsetwgpeer(const char *, int);
void unsetwgpeerpsk(const char *, int);
+void unsetwgpeerdesc(const char *, int);
void unsetwgpeerall(const char *, int);
void wg_status(int);
@@ -620,11 +622,13 @@ const struct cmd {
{ "wgaip", NEXTARG,A_WIREGUARD,setwgpeeraip},
{ "wgpsk", NEXTARG,A_WIREGUARD,setwgpeerpsk},
{ "wgpka", NEXTARG,A_WIREGUARD,setwgpeerpka},
+ { "wgdesc", NEXTARG,A_WIREGUARD,setwgpeerdesc},
{ "wgport", NEXTARG,A_WIREGUARD,setwgport},
{ "wgkey", NEXTARG,A_WIREGUARD,setwgkey},
{ "wgrtable", NEXTARG,A_WIREGUARD,setwgrtable},
{ "-wgpeer",NEXTARG,A_WIREGUARD,unsetwgpeer},
{ "-wgpsk", 0, A_WIREGUARD,unsetwgpeerpsk},
+ { "-wgdesc",0, A_WIREGUARD,unsetwgpeerdesc},
{ "-wgpeerall", 0, A_WIREGUARD,unsetwgpeerall},
#else /* SMALL */
@@ -5843,6 +5847,16 @@ setwgpeerpka(const char *pka, int param)
}
void
+setwgpeerdesc(const char *wgdesc, int param)
+{
+ if (wg_peer == NULL)
+ errx(1, "wgdesc: wgpeer not set");
+ if (strlen(wgdesc))
+ strlcpy(wg_peer->p_description, wgdesc, IFDESCRSIZE);
+ wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
+}
+
+void
setwgport(const char *port, int param)
{
const char *errmsg = NULL;
@@ -5889,6 +5903,15 @@ unsetwgpeerpsk(const char *value, int pa
}
void
+unsetwgpeerdesc(const char *value, int param)
+{
+ if (wg_peer == NULL)
+ errx(1, "wgpesc: wgpeer not set");
+ strlcpy(wg_peer->p_description, "", IFDESCRSIZE);
+ wg_peer->p_flags |= WG_PEER_SET_DESCRIPTION;
+}
+
+void
unsetwgpeerall(const char *value, int param)
{
ensurewginterface();
@@ -5948,6 +5971,9 @@ wg_status(int ifaliases)
b64_ntop(wg_peer->p_public, WG_KEY_LEN,
key, sizeof(key));
printf("\twgpeer %s\n", key);
+
+ if (strlen(wg_peer->p_description))
+ printf("\t\twgdesc %s\n",
wg_peer->p_description);
if (wg_peer->p_flags & WG_PEER_HAS_PSK)
printf("\t\twgpsk (present)\n");
Index: share/man/man4/wg.4
===
RCS file: /cvs/src/share/man/man4/wg.4,v
retrieving revision 1.10
diff -u -p -u -r1.10 wg.4
--- share/man/man4/wg.4 14 Mar 2021 10:08:38 - 1.10
+++ share/man/man4/wg.4 5 Nov 2022 19:41:22 -
@@ -42,6 +42,19 @@ configuration file for
.Xr netstart 8 .
The interface itself can be configured with
.Xr ifconfig 8 .
+To display
+.Cm wgpeer
+information for each
+.Nm wg
+interface option
+.Fl A
+to
+.Xr ifconfig 8
+should be used or
+.Nm wg
+interface should be specified as an argument to
+.Xr ifconfig 8
+command.
.Pp
.Nm wg
interfaces support the following
Index: sys/net/if_wg.c
===
RCS file: /cvs/src/sys/net/if_wg.c,v
retrieving revision 1.26
diff -u -p -u -r1.26 if_wg.c
--- sys/net/if_wg.c 21 Jul 2022 11:26:50 - 1.26
+++ sys/net/if_wg.c 5 Nov 2022 19:41:22 -
@@ -221,6 +221,9 @@ struct wg_peer {
SLIST_ENTRY(wg_peer) p_start_list;
int
Re: relayd: uninitialized errstr
On Thu, Nov 10, 2022 at 01:35:09AM +0100, Moritz Buhl wrote:
> On Thu, Nov 10, 2022 at 01:29:13AM +0100, Theo Buehler wrote:
> > On Thu, Nov 10, 2022 at 01:10:51AM +0100, Moritz Buhl wrote:
> > > errstr is never set but fail does:
> > > RSA_meth_free(rsae_method);
> > > fatalx("%s: %s", __func__, errstr);
> > > Found by codechecker.
> > >
> > > OK?
> >
> > Needs more braces
>
> Indeed.
> Is this OK?
of course. thanks
>
> Index: ca.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/ca.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 ca.c
> --- ca.c 20 Jan 2022 17:56:35 - 1.39
> +++ ca.c 10 Nov 2022 00:33:35 -
> @@ -513,8 +513,10 @@ ca_engine_init(struct relayd *x_env)
> if (rsa_default != NULL)
> return;
>
> - if ((rsae_method = RSA_meth_new("RSA privsep engine", 0)) == NULL)
> + if ((rsae_method = RSA_meth_new("RSA privsep engine", 0)) == NULL) {
> + errstr = "RSA_meth_new";
> goto fail;
> + }
>
> RSA_meth_set_pub_enc(rsae_method, rsae_pub_enc);
> RSA_meth_set_pub_dec(rsae_method, rsae_pub_dec);
Re: relayd: uninitialized errstr
On Thu, Nov 10, 2022 at 01:29:13AM +0100, Theo Buehler wrote:
> On Thu, Nov 10, 2022 at 01:10:51AM +0100, Moritz Buhl wrote:
> > errstr is never set but fail does:
> > RSA_meth_free(rsae_method);
> > fatalx("%s: %s", __func__, errstr);
> > Found by codechecker.
> >
> > OK?
>
> Needs more braces
Indeed.
Is this OK?
Index: ca.c
===
RCS file: /cvs/src/usr.sbin/relayd/ca.c,v
retrieving revision 1.39
diff -u -p -r1.39 ca.c
--- ca.c20 Jan 2022 17:56:35 - 1.39
+++ ca.c10 Nov 2022 00:33:35 -
@@ -513,8 +513,10 @@ ca_engine_init(struct relayd *x_env)
if (rsa_default != NULL)
return;
- if ((rsae_method = RSA_meth_new("RSA privsep engine", 0)) == NULL)
+ if ((rsae_method = RSA_meth_new("RSA privsep engine", 0)) == NULL) {
+ errstr = "RSA_meth_new";
goto fail;
+ }
RSA_meth_set_pub_enc(rsae_method, rsae_pub_enc);
RSA_meth_set_pub_dec(rsae_method, rsae_pub_dec);
Re: relayd: uninitialized errstr
On Thu, Nov 10, 2022 at 01:10:51AM +0100, Moritz Buhl wrote:
> errstr is never set but fail does:
> RSA_meth_free(rsae_method);
> fatalx("%s: %s", __func__, errstr);
> Found by codechecker.
>
> OK?
Needs more braces
> mbuhl
>
> Index: ca.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/ca.c,v
> retrieving revision 1.39
> diff -u -p -r1.39 ca.c
> --- ca.c 20 Jan 2022 17:56:35 - 1.39
> +++ ca.c 10 Nov 2022 00:06:20 -
> @@ -514,6 +514,7 @@ ca_engine_init(struct relayd *x_env)
> return;
>
> if ((rsae_method = RSA_meth_new("RSA privsep engine", 0)) == NULL)
> + errstr = "RSA_meth_new";
> goto fail;
>
> RSA_meth_set_pub_enc(rsae_method, rsae_pub_enc);
>
relayd: uninitialized errstr
errstr is never set but fail does:
RSA_meth_free(rsae_method);
fatalx("%s: %s", __func__, errstr);
Found by codechecker.
OK?
mbuhl
Index: ca.c
===
RCS file: /cvs/src/usr.sbin/relayd/ca.c,v
retrieving revision 1.39
diff -u -p -r1.39 ca.c
--- ca.c20 Jan 2022 17:56:35 - 1.39
+++ ca.c10 Nov 2022 00:06:20 -
@@ -514,6 +514,7 @@ ca_engine_init(struct relayd *x_env)
return;
if ((rsae_method = RSA_meth_new("RSA privsep engine", 0)) == NULL)
+ errstr = "RSA_meth_new";
goto fail;
RSA_meth_set_pub_enc(rsae_method, rsae_pub_enc);
Re: relayd: always call va_end
Yes, OK millert@ - todd
relayd: always call va_end
The same code is in httpd but there it was fixed in
commit 6b535b529336a3fd1beb56c42ff5755b84ba9b03
Author: jung
Date: Sun May 22 19:19:21 2016 +
fix unbalanced va_start and va_end macros
from Hiltjo Posthuma
"do." deraadt
Found by codechecker
OK?
mbuhl
Index: usr.sbin/relayd/relayd.c
===
RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
retrieving revision 1.189
diff -u -p -r1.189 relayd.c
--- usr.sbin/relayd/relayd.c3 Sep 2022 20:07:31 - 1.189
+++ usr.sbin/relayd/relayd.c9 Nov 2022 23:51:57 -
@@ -656,11 +656,13 @@ kv_set(struct kv *kv, char *fmt, ...)
va_list ap;
char*value = NULL;
struct kv *ckv;
+ int ret;
va_start(ap, fmt);
- if (vasprintf(, fmt, ap) == -1)
- return (-1);
+ ret = vasprintf(, fmt, ap);
va_end(ap);
+ if (ret == -1)
+ return (-1);
/* Remove all children */
while ((ckv = TAILQ_FIRST(>kv_children)) != NULL) {
@@ -681,11 +683,13 @@ kv_setkey(struct kv *kv, char *fmt, ...)
{
va_list ap;
char*key = NULL;
+ int ret;
va_start(ap, fmt);
- if (vasprintf(, fmt, ap) == -1)
- return (-1);
+ ret = vasprintf(, fmt, ap);
va_end(ap);
+ if (ret == -1)
+ return (-1);
free(kv->kv_key);
kv->kv_key = key;
Re: replace SRP with SMR in the if_idxmap commit
I know what this is. The barrier at the end of if_idxmap_alloc is sleeping waiting for cpus to run that aren't running cos we haven't finished booting yet. I'll back it out and fix it up when I'm actually awake. dlg > On 10 Nov 2022, at 6:28 am, Hrvoje Popovski wrote: > > Hi all, > > I've checkout cvs half an hour ago on two boxes and both boxes won't > properly boot. > > First one stops here > > ppb10 at pci1 dev 28 function 4 "Intel 8 Series PCIE" rev 0xd5: msi > pci12 at ppb10 bus 13 > em4 at pci12 dev 0 function 0 "Intel I350" rev 0x01: msi, address > 00:25:90:5d:c9:9a > em5 at pci12 dev 0 function 1 "Intel I350" rev 0x01: msi > > > second one stop here > vmm0 at mainbus0: VMX/EPT > > so I've change if.c revision to r1.672 and with that change box boots > but with r1.673 won't boot... > I've compile kernel with WITNESS and WITNESS_LOCKTRACE but with that > there isn't any more information. > > Did anyone experience that or I just me? > > > > >
replace SRP with SMR in the if_idxmap commit
Hi all, I've checkout cvs half an hour ago on two boxes and both boxes won't properly boot. First one stops here ppb10 at pci1 dev 28 function 4 "Intel 8 Series PCIE" rev 0xd5: msi pci12 at ppb10 bus 13 em4 at pci12 dev 0 function 0 "Intel I350" rev 0x01: msi, address 00:25:90:5d:c9:9a em5 at pci12 dev 0 function 1 "Intel I350" rev 0x01: msi second one stop here vmm0 at mainbus0: VMX/EPT so I've change if.c revision to r1.672 and with that change box boots but with r1.673 won't boot... I've compile kernel with WITNESS and WITNESS_LOCKTRACE but with that there isn't any more information. Did anyone experience that or I just me?
Re: install.sub: fix softraid disks not being created before md_installboot()
On Wed, Nov 09, 2022 at 02:36:57PM +, ssnf wrote: > My softraid keydisk did not get initialized during the install process. > This fixes it. You need to be more specific, installing to a softraid volume has always required manual steps, so I have no idea what you did (not) do and when.
Re: i386: switch to clockintr(9)
On Sun, Nov 06, 2022 at 07:46:37PM +, Scott Cheloha wrote: > This patch switches i386 to clockintr(9). > > I have tested this on my Lenovo X1 Carbon 6th and Dell Optiplex 7070 > running in 32-bit compatibility mode. It has survived ~20 parallel > release builds and upgrades from the resulting bsd.rd. > > mlarkin@ has tested this in an ESXi VM and reports that ACPI hibernate > works. > > This needs additional testing. A couple things have not been > confirmed to work yet: > > - Accelerated graphics > > - ACPI suspend and hibernate on real hardware > > - APM suspend and hibernate > > - Real 586- and 686-class hardware > > - Real machines without a lapic > > Notes: > > - i386 machines with a lapic now have a randomized statclock(). > > - This includes a preliminary patch that disables/enables hpet_delay() > across suspend/resume. It will be committed separately. > > - In i8254-mode, profhz = 1024 does not divide evenly into one billion. > We could circumvent this problem by running the RTC at 512hz when > profiling is enabled. > > - The same behavior change described in the amd64 patch: > > https://marc.info/?l=openbsd-tech=166776339203279=2 > > applies to i386: > > > If we're using the i8254 as our interrupt clock, there is > > a small behavior change. > > > > In i8254-mode, there are two interrupt clocks, the i8254 and mc146818. > > > > Currently, the i8254 runs hardclock() and the mc146818 runs statclock(). > > With this patch, neither interrupt handler has a monopoly on which events > > are dispatched anymore. So if hardclock() is due, the mc146818 handler > > will dispatch it. The i8254 might dispatch statclock(), too. > > > > Preserving the existing behavior would require per-intrclock event > > queues. That is, the i8254 would have its own work schedule > > maintained separately from that of the mc146818. I pitched this idea > > to kettenis@ and he said we probably didn't need it. I agree with > > him. Of course, if there is interest in this feature we could explore > > it. The HPET pieces have been committed separately. Here is an updated patch. Index: sys/arch/i386/i386/acpi_machdep.c === RCS file: /cvs/src/sys/arch/i386/i386/acpi_machdep.c,v retrieving revision 1.85 diff -u -p -r1.85 acpi_machdep.c --- sys/arch/i386/i386/acpi_machdep.c 21 Feb 2022 10:24:28 - 1.85 +++ sys/arch/i386/i386/acpi_machdep.c 9 Nov 2022 19:07:10 - @@ -421,8 +421,6 @@ acpi_resume_cpu(struct acpi_softc *sc, i #if NLAPIC > 0 lapic_tpr = save_lapic_tpr; lapic_enable(); - if (initclock_func == lapic_initclocks) - lapic_startclock(); lapic_set_lvt(); #endif Index: sys/arch/i386/i386/apm.c === RCS file: /cvs/src/sys/arch/i386/i386/apm.c,v retrieving revision 1.127 diff -u -p -r1.127 apm.c --- sys/arch/i386/i386/apm.c21 Feb 2022 10:24:28 - 1.127 +++ sys/arch/i386/i386/apm.c9 Nov 2022 19:07:11 - @@ -44,6 +44,7 @@ #include #include #include +#include #include #include #include @@ -271,6 +272,11 @@ apm_suspend(int state) if (initclock_func == i8254_initclocks) rtcstart(); /* in i8254 mode, rtc is profclock */ inittodr(gettime()); + +#ifdef __HAVE_CLOCKINTR + clockintr_cpu_init(NULL); + clockintr_trigger(); +#endif config_suspend_all(DVACT_RESUME); cold = 0; Index: sys/arch/i386/i386/cpu.c === RCS file: /cvs/src/sys/arch/i386/i386/cpu.c,v retrieving revision 1.109 diff -u -p -r1.109 cpu.c --- sys/arch/i386/i386/cpu.c15 Aug 2022 04:17:50 - 1.109 +++ sys/arch/i386/i386/cpu.c9 Nov 2022 19:07:11 - @@ -701,7 +701,6 @@ cpu_hatch(void *v) cpu_init_idt(); lapic_enable(); - lapic_startclock(); lapic_set_lvt(); gdt_init_cpu(ci); @@ -727,6 +726,8 @@ cpu_hatch(void *v) ci->ci_dev->dv_xname, ci->ci_cpuid); nanouptime(>ci_schedstate.spc_runtime); splx(s); + + lapic_startclock(); SCHED_LOCK(s); cpu_switchto(NULL, sched_chooseproc()); Index: sys/arch/i386/i386/lapic.c === RCS file: /cvs/src/sys/arch/i386/i386/lapic.c,v retrieving revision 1.52 diff -u -p -r1.52 lapic.c --- sys/arch/i386/i386/lapic.c 10 Sep 2022 01:30:14 - 1.52 +++ sys/arch/i386/i386/lapic.c 9 Nov 2022 19:07:11 - @@ -34,7 +34,9 @@ #include #include +#include #include +#include #include @@ -68,7 +70,6 @@ struct evcount clk_count; struct evcount ipi_count; #endif -void lapic_delay(int); static u_int32_t lapic_gettick(void); void lapic_clockintr(void *); void lapic_initclocks(void); @@ -239,19 +240,43 @@ lapic_gettick(void) #include /* for hz */
adding a mutex to pf_state
hello,
diff below adds a mutex to pf_state. It fixes a NULL pointer dereference panic
reported by Hrvoje sometime ago [1]. Besides adding a mutex to state the diff
addresses a race between pfsync and state purge thread. What happened in this
particular case was that state expired and its state keys got detached while it
was waiting to be processed by pfsync. Once pfsync got to it found state
keys detached and tripped on null pointer dereference. This is the
race change below fixes.
I'm not too much worried about contention on newly introduced mutex.
the thing is it is not a global mutex it is a per state mutex (per object
mutex). I don't expect to see two cpu's will be updating same state
very often.
thanks and
regards
sashan
[1] https://marc.info/?l=openbsd-bugs=166006758231954=2
8<---8<---8<--8<
diff --git a/sys/net/if_pfsync.c b/sys/net/if_pfsync.c
index d279ede9cd6..5f92ae6ec45 100644
--- a/sys/net/if_pfsync.c
+++ b/sys/net/if_pfsync.c
@@ -157,16 +157,16 @@ const struct {
};
struct pfsync_q {
- void(*write)(struct pf_state *, void *);
+ int (*write)(struct pf_state *, void *);
size_t len;
u_int8_taction;
};
/* we have one of these for every PFSYNC_S_ */
-void pfsync_out_state(struct pf_state *, void *);
-void pfsync_out_iack(struct pf_state *, void *);
-void pfsync_out_upd_c(struct pf_state *, void *);
-void pfsync_out_del(struct pf_state *, void *);
+intpfsync_out_state(struct pf_state *, void *);
+intpfsync_out_iack(struct pf_state *, void *);
+intpfsync_out_upd_c(struct pf_state *, void *);
+intpfsync_out_del(struct pf_state *, void *);
struct pfsync_q pfsync_qs[] = {
{ pfsync_out_iack, sizeof(struct pfsync_ins_ack), PFSYNC_ACT_INS_ACK },
@@ -1301,24 +1301,26 @@ pfsyncioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
return (0);
}
-void
+int
pfsync_out_state(struct pf_state *st, void *buf)
{
struct pfsync_state *sp = buf;
pf_state_export(sp, st);
+ return (0);
}
-void
+int
pfsync_out_iack(struct pf_state *st, void *buf)
{
struct pfsync_ins_ack *iack = buf;
iack->id = st->id;
iack->creatorid = st->creatorid;
+ return (0);
}
-void
+int
pfsync_out_upd_c(struct pf_state *st, void *buf)
{
struct pfsync_upd_c *up = buf;
@@ -1329,9 +1331,10 @@ pfsync_out_upd_c(struct pf_state *st, void *buf)
pf_state_peer_hton(>dst, >dst);
up->creatorid = st->creatorid;
up->timeout = st->timeout;
+ return (0);
}
-void
+int
pfsync_out_del(struct pf_state *st, void *buf)
{
struct pfsync_del_c *dp = buf;
@@ -1340,6 +1343,7 @@ pfsync_out_del(struct pf_state *st, void *buf)
dp->creatorid = st->creatorid;
SET(st->state_flags, PFSTATE_NOSYNC);
+ return (0);
}
void
@@ -1664,8 +1668,8 @@ pfsync_sendout(void)
KASSERT(st->snapped == 1);
st->sync_state = PFSYNC_S_NONE;
st->snapped = 0;
- pfsync_qs[q].write(st, m->m_data + offset);
- offset += pfsync_qs[q].len;
+ if (pfsync_qs[q].write(st, m->m_data + offset) == 0)
+ offset += pfsync_qs[q].len;
pf_state_unref(st);
count++;
diff --git a/sys/net/if_pfsync.h b/sys/net/if_pfsync.h
index 5447b829d74..30c1df0de9c 100644
--- a/sys/net/if_pfsync.h
+++ b/sys/net/if_pfsync.h
@@ -326,7 +326,7 @@ int pfsync_sysctl(int *, u_int, void *,
size_t *,
#definePFSYNC_SI_CKSUM 0x02
#definePFSYNC_SI_ACK 0x04
intpfsync_state_import(struct pfsync_state *, int);
-void pfsync_state_export(struct pfsync_state *,
+intpfsync_state_export(struct pfsync_state *,
struct pf_state *);
void pfsync_insert_state(struct pf_state *);
diff --git a/sys/net/pf.c b/sys/net/pf.c
index c42f76dbc67..1083ee95b9a 100644
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -185,7 +185,8 @@ int pf_translate_icmp_af(struct pf_pdesc*,
int, void *);
voidpf_send_icmp(struct mbuf *, u_int8_t, u_int8_t, int,
sa_family_t, struct pf_rule *, u_int);
voidpf_detach_state(struct pf_state *);
-voidpf_state_key_detach(struct pf_state *, int);
+voidpf_state_key_detach(struct pf_state *,
+ struct pf_state_key *);
u_int32_t pf_tcp_iss(struct pf_pdesc *);
voidpf_rule_to_actions(struct pf_rule *,
struct pf_rule_actions *);
@@ -260,6 +261,9 @@ void
pf_state_key_unlink_inpcb(struct pf_state_key *);
void
acme-client memory leak in error case
Hi tech,
g is not freed in this error case.
Found by codechecker.
OK?
mbuhl
Index: usr.sbin/acme-client/netproc.c
===
RCS file: /cvs/src/usr.sbin/acme-client/netproc.c,v
retrieving revision 1.31
diff -u -p -r1.31 netproc.c
--- usr.sbin/acme-client/netproc.c 24 Aug 2021 10:07:30 - 1.31
+++ usr.sbin/acme-client/netproc.c 9 Nov 2022 18:11:17 -
@@ -222,6 +222,7 @@ again:
if ((st = http_head_get("Location", g->head, g->headsz)) ==
NULL) {
warnx("redirect without location header");
+ http_get_free(g);
return -1;
}
Re: rpki-client: error out if too many ipAddrBlocks in ROA payload
On Wed, Nov 09, 2022 at 06:07:47PM +, Job Snijders wrote:
> Hi all,
>
> The ASN.1 profile in draft-ietf-sidrops-rfc6482bis section 4
> https://datatracker.ietf.org/doc/html/draft-ietf-sidrops-rfc6482bis-01
> specifies that there must not be more than 2 ipAddrBlocks (one for IPv4,
> and one for IPv6). This changeset enforces that constraint. Compatible
> with all published ROAs.
>
> OK?
>
> Kind regards,
>
> Job
>
> Index: roa.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/roa.c,v
> retrieving revision 1.55
> diff -u -p -r1.55 roa.c
> --- roa.c 4 Nov 2022 09:43:13 - 1.55
> +++ roa.c 9 Nov 2022 18:04:59 -
> @@ -111,6 +111,7 @@ roa_parse_econtent(const unsigned char *
> long maxlen;
> struct ip_addr ipaddr;
> struct roa_ip *res;
> + int ipaddrblocksz;
> int i, j, rc = 0;
>
> if ((roa = d2i_RouteOriginAttestation(NULL, , dsz)) == NULL) {
> @@ -128,7 +129,14 @@ roa_parse_econtent(const unsigned char *
> goto out;
> }
>
> - for (i = 0; i < sk_ROAIPAddressFamily_num(roa->ipAddrBlocks); i++) {
> + ipaddrblocksz = sk_ROAIPAddressFamily_num(roa->ipAddrBlocks);
> + if (ipaddrblocksz > 2) {
> + warnx("%s: draft-rfc6482bis: too many ipAddrBlocks (got %i, "
Please use %d
> + "expected 1 or 2)", p->fn, ipaddrblocksz);
I'd prefer having the parens open and close on the same line
ok tb
> + goto out;
> + }
> +
> + for (i = 0; i < ipaddrblocksz; i++) {
> addrfam = sk_ROAIPAddressFamily_value(roa->ipAddrBlocks, i);
> addrs = addrfam->addresses;
> addrsz = sk_ROAIPAddress_num(addrs);
>
install.sub: fix softraid disks not being created before md_installboot()
My softraid keydisk did not get initialized during the install process. This fixes it. Index: install.sub === RCS file: /cvs/src/distrib/miniroot/install.sub,v retrieving revision 1.1214 diff -r1.1214 install.sub 2875c2875 < make_dev $(bioctl $ROOTDISK 2>/dev/null | sed -n 's/.*<\(.*\)>$/\1/p') --- > make_dev "$(bioctl $ROOTDISK 2>/dev/null | sed -n 's/.*<\(.*\)>$/\1/p')"
rpki-client: error out if too many ipAddrBlocks in ROA payload
Hi all,
The ASN.1 profile in draft-ietf-sidrops-rfc6482bis section 4
https://datatracker.ietf.org/doc/html/draft-ietf-sidrops-rfc6482bis-01
specifies that there must not be more than 2 ipAddrBlocks (one for IPv4,
and one for IPv6). This changeset enforces that constraint. Compatible
with all published ROAs.
OK?
Kind regards,
Job
Index: roa.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/roa.c,v
retrieving revision 1.55
diff -u -p -r1.55 roa.c
--- roa.c 4 Nov 2022 09:43:13 - 1.55
+++ roa.c 9 Nov 2022 18:04:59 -
@@ -111,6 +111,7 @@ roa_parse_econtent(const unsigned char *
long maxlen;
struct ip_addr ipaddr;
struct roa_ip *res;
+ int ipaddrblocksz;
int i, j, rc = 0;
if ((roa = d2i_RouteOriginAttestation(NULL, , dsz)) == NULL) {
@@ -128,7 +129,14 @@ roa_parse_econtent(const unsigned char *
goto out;
}
- for (i = 0; i < sk_ROAIPAddressFamily_num(roa->ipAddrBlocks); i++) {
+ ipaddrblocksz = sk_ROAIPAddressFamily_num(roa->ipAddrBlocks);
+ if (ipaddrblocksz > 2) {
+ warnx("%s: draft-rfc6482bis: too many ipAddrBlocks (got %i, "
+ "expected 1 or 2)", p->fn, ipaddrblocksz);
+ goto out;
+ }
+
+ for (i = 0; i < ipaddrblocksz; i++) {
addrfam = sk_ROAIPAddressFamily_value(roa->ipAddrBlocks, i);
addrs = addrfam->addresses;
addrsz = sk_ROAIPAddress_num(addrs);
apm garbage stack value could be accessed
Hi tech,
In case send or recv fail in send_command, reply.error is stack
garbage. This wouldn't be possible if reply was zeroed. But
checking for ret == 0 ensures that reply was fully written.
OK?
mbuhl
Index: usr.sbin/apm/apm.c
===
RCS file: /cvs/src/usr.sbin/apm/apm.c,v
retrieving revision 1.42
diff -u -p -r1.42 apm.c
--- usr.sbin/apm/apm.c 10 Sep 2022 10:10:09 - 1.42
+++ usr.sbin/apm/apm.c 9 Nov 2022 17:29:03 -
@@ -99,6 +99,8 @@ do_zzz(int fd, enum apm_action action)
char *msg;
int ret;
+ bzero(, sizeof reply);
+
switch (action) {
case NONE:
case SUSPEND:
@@ -119,7 +121,7 @@ do_zzz(int fd, enum apm_action action)
printf("%s...\n", msg);
ret = send_command(fd, , );
- if (reply.error)
+ if (ret == 0 && reply.error)
errx(1, "%s: %s", apm_state(reply.newstate),
strerror(reply.error));
exit(ret);
}
Re: pause.3: misc cleanup
On Wed, 09 Nov 2022 16:47:22 +, Scott Cheloha wrote: > I think this could use some sprucing. > > NAME > > - "stop until signal" -> "wait for a signal" > > Feels more natural to me. OK > DESCRIPTION > > - It's "the calling thread" that blocks, not "a process". > > - Nothing is "forced". > > - It "blocks", it doesn't "pause". Saying pause(3) "pauses" > is too circular. Block seems better to me too. Sleep would also work that may be less precise. > - No need to enumerate the ways a signal can be delivered. That is > better left to other manpages. Nix kill(2) and setitimer(2) from > the DESCRIPTION. > > - No need to mention that we don't return until the signal handler > returns either, that's just basic signal(3) stuff. OK. > - Should we explicitly mention the underlying implementation system > calls or not? I'm unsure. I don't think there is any reason to document the implementation details. > RETURN VALUES > > - Pull ERRORS into this section. No need to put the one error in a > .Bl/.El, we can just mention it inline. OK > SEE ALSO > > - Nix select(2) and setitimer(2), they aren't directly relevant. OK. The reason select(2) is there is probably because you can emulate pause() using select(). > - Add sigprocmask(2) and signal(3). They are very relevant. OK. > HISTORY > > - We still have sigpause(3) and sigblock(3) in userspace. Should > we .Xr them? They aren't systems calls anymore, but they were > at that time. Unsure what to do here. I think it is better to use .Fn for sigpause(3) and sigblock(3) rather than .Xr here. Note that in 4.3-Reno pause(3) was still implemented in terms of sigpause(3) and sigblock(3), it is just that those functions were themselves wrappers instead of system calls. Personally, I would drop the bit about 4.3-Reno since it is not really correct in my opinion. - todd
Push kernel lock into SIOCSIFGATTR
The global group list keeps being protected by the net lock, but calling into netinet/ip_carp.c:carp_ifgattr_ioctl() still needs the big lock. Only carp(4) uses group attributes, so this unlocks just a small part of if_setgroupattribs(), but it also clarifies what is locked and why. Feedback? OK? diff --git a/sys/net/if.c b/sys/net/if.c index 6b13c2800fb..7b5b27b776c 100644 --- a/sys/net/if.c +++ b/sys/net/if.c @@ -1910,11 +1910,9 @@ ifioctl(struct socket *so, u_long cmd, caddr_t data, struct proc *p) case SIOCSIFGATTR: if ((error = suser(p)) != 0) return (error); - KERNEL_LOCK(); NET_LOCK(); error = if_setgroupattribs(data); NET_UNLOCK(); - KERNEL_UNLOCK(); return (error); case SIOCGIFCONF: case SIOCIFGCLONERS: @@ -2964,8 +2962,10 @@ if_setgroupattribs(caddr_t data) ifg->ifg_carp_demoted += demote; + KERNEL_LOCK(); /* for carp_ifgattr_ioctl() only */ TAILQ_FOREACH(ifgm, >ifg_members, ifgm_next) ifgm->ifgm_ifp->if_ioctl(ifgm->ifgm_ifp, SIOCSIFGATTR, data); + KERNEL_UNLOCK(); return (0); }
Re: ftp: strnvis redirect uri
On Wed, 09 Nov 2022 17:33:54 +0100, Claudio Jeker wrote: > Like this? Yes, OK millert@ - todd
pause.3: misc cleanup
I think this could use some sprucing. NAME - "stop until signal" -> "wait for a signal" Feels more natural to me. DESCRIPTION - It's "the calling thread" that blocks, not "a process". - Nothing is "forced". - It "blocks", it doesn't "pause". Saying pause(3) "pauses" is too circular. - No need to enumerate the ways a signal can be delivered. That is better left to other manpages. Nix kill(2) and setitimer(2) from the DESCRIPTION. - No need to mention that we don't return until the signal handler returns either, that's just basic signal(3) stuff. - Should we explicitly mention the underlying implementation system calls or not? I'm unsure. RETURN VALUES - Pull ERRORS into this section. No need to put the one error in a .Bl/.El, we can just mention it inline. SEE ALSO - Nix select(2) and setitimer(2), they aren't directly relevant. - Add sigprocmask(2) and signal(3). They are very relevant. HISTORY - We still have sigpause(3) and sigblock(3) in userspace. Should we .Xr them? They aren't systems calls anymore, but they were at that time. Unsure what to do here. Index: pause.3 === RCS file: /cvs/src/lib/libc/gen/pause.3,v retrieving revision 1.16 diff -u -p -r1.16 pause.3 --- pause.3 9 Nov 2022 06:48:29 - 1.16 +++ pause.3 9 Nov 2022 16:44:33 - @@ -32,7 +32,7 @@ .Os .Sh NAME .Nm pause -.Nd stop until signal +.Nd wait for a signal .Sh SYNOPSIS .In unistd.h .Ft int @@ -40,40 +40,30 @@ .Sh DESCRIPTION .Bf -symbolic .Fn pause -is made obsolete by +is obsoleted by .Xr sigsuspend 2 . .Ef .Pp -The .Fn pause -function forces a process to pause until a signal is received from either the -.Xr kill 2 -function or an interval timer -(see -.Xr setitimer 2 ) . +blocks the calling thread until it receives an unmasked signal. .Pp -Upon termination of a signal handler started during a -.Fn pause , -the +On +.Ox , .Fn pause -call will return. +is implemented with +.Xr sigsuspend 2 . .Sh RETURN VALUES -Always returns \-1. -.Sh ERRORS -The +On receipt of a signal, .Fn pause -function may set the global variable +returns \-1 and sets .Va errno -to the following error: -.Bl -tag -width Er -.It Bq Er EINTR -The call was interrupted. -.El +to +.Er EINTR . .Sh SEE ALSO .Xr kill 2 , -.Xr select 2 , -.Xr setitimer 2 , -.Xr sigsuspend 2 +.Xr sigprocmask 2 , +.Xr sigsuspend 2 , +.Xr signal 3 .Sh HISTORY A .Fn pause @@ -82,15 +72,16 @@ system call first appeared outside of Be tape for .At v6 . It was first officially released with PWB/UNIX 1.0. -It was reimplemented as a wrapper around the -.Fn sigpause -and -.Fn sigblock -system calls in -.Bx 4.2 , -and around the -.Xr sigsuspend 2 +In +.Bx 4.2 +it was moved to userspace and reimplemented with +.Xr sigpause 3 and +.Xr sigblock 3 . +In +.Bx 4.3 Reno +it was reimplemented yet again, +this time with .Xr sigprocmask 2 -system calls in -.Bx 4.3 Reno . +and +.Xr sigsuspend 2 .
Re: ftp: strnvis redirect uri
On Wed, Nov 09, 2022 at 05:33:54PM +0100, Claudio Jeker wrote:
> On Wed, Nov 09, 2022 at 08:51:21AM -0700, Todd C. Miller wrote:
> > On Wed, 09 Nov 2022 15:46:06 +, Florian Obser wrote:
> >
> > > 200 might be a bit short for an URL, no?
> >
> > Perhaps stravis(3) is a better choice then?
> >
>
> Like this?
Yes, that looks better.
OK kn
>
> --
> :wq Claudio
>
> Index: fetch.c
> ===
> RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
> retrieving revision 1.211
> diff -u -p -r1.211 fetch.c
> --- fetch.c 9 Nov 2022 16:29:58 - 1.211
> +++ fetch.c 9 Nov 2022 16:33:01 -
> @@ -950,8 +950,13 @@ noslash:
> loctail = strchr(redirurl, '#');
> if (loctail != NULL)
> *loctail = '\0';
> - if (verbose)
> - fprintf(ttyout, "Redirected to %s\n", redirurl);
> + if (verbose) {
> + char *visbuf;
> + if (stravis(, redirurl, VIS_SAFE) == -1)
> + err(1, "Cannot vis redirect URL");
> + fprintf(ttyout, "Redirected to %s\n", visbuf);
> + free(visbuf);
> + }
> ftp_close(, , );
> rval = url_get(redirurl, proxyenv, savefile, lastfile);
> free(redirurl);
>
Re: ftp: strnvis redirect uri
On Wed, Nov 09, 2022 at 08:51:21AM -0700, Todd C. Miller wrote:
> On Wed, 09 Nov 2022 15:46:06 +, Florian Obser wrote:
>
> > 200 might be a bit short for an URL, no?
>
> Perhaps stravis(3) is a better choice then?
>
Like this?
--
:wq Claudio
Index: fetch.c
===
RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.211
diff -u -p -r1.211 fetch.c
--- fetch.c 9 Nov 2022 16:29:58 - 1.211
+++ fetch.c 9 Nov 2022 16:33:01 -
@@ -950,8 +950,13 @@ noslash:
loctail = strchr(redirurl, '#');
if (loctail != NULL)
*loctail = '\0';
- if (verbose)
- fprintf(ttyout, "Redirected to %s\n", redirurl);
+ if (verbose) {
+ char *visbuf;
+ if (stravis(, redirurl, VIS_SAFE) == -1)
+ err(1, "Cannot vis redirect URL");
+ fprintf(ttyout, "Redirected to %s\n", visbuf);
+ free(visbuf);
+ }
ftp_close(, , );
rval = url_get(redirurl, proxyenv, savefile, lastfile);
free(redirurl);
Re: ftp: allow for more spaces in http headers
On Wed, 09 Nov 2022 16:21:46 +0100, Claudio Jeker wrote: > This is the changes that I added to rpki-client http.c which I have not > yet merged into ftp(1) OK millert@ - todd
Re: ftp: strnvis redirect uri
On Wed, 09 Nov 2022 15:46:06 +, Florian Obser wrote: > 200 might be a bit short for an URL, no? Perhaps stravis(3) is a better choice then? - todd
Re: ftp: strnvis redirect uri
On Wed, 09 Nov 2022 16:34:20 +0100, Claudio Jeker wrote: > The redirect URI is untrusted input so strnvis it first before printing > it. OK millert@ - todd
Re: ftp: strnvis redirect uri
On Wed, Nov 09, 2022 at 04:34:20PM +0100, Claudio Jeker wrote:
> The redirect URI is untrusted input so strnvis it first before printing
> it.
OK kn
>
> --
> :wq Claudio
>
> Index: fetch.c
> ===
> RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
> retrieving revision 1.210
> diff -u -p -r1.210 fetch.c
> --- fetch.c 15 Sep 2022 12:47:10 - 1.210
> +++ fetch.c 9 Nov 2022 15:29:37 -
> @@ -949,8 +950,11 @@ noslash:
> loctail = strchr(redirurl, '#');
> if (loctail != NULL)
> *loctail = '\0';
> - if (verbose)
> - fprintf(ttyout, "Redirected to %s\n", redirurl);
> + if (verbose) {
> + strnvis(gerror, redirurl, sizeof gerror,
sizeof without parenthese always looks off.
> + VIS_SAFE);
> + fprintf(ttyout, "Redirected to %s\n", gerror);
> + }
> ftp_close(, , );
> rval = url_get(redirurl, proxyenv, savefile, lastfile);
> free(redirurl);
>
Re: ftp: strnvis redirect uri
200 might be a bit short for an URL, no?
On 2022-11-09 16:34 +01, Claudio Jeker wrote:
> The redirect URI is untrusted input so strnvis it first before printing
> it.
>
> --
> :wq Claudio
>
> Index: fetch.c
> ===
> RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
> retrieving revision 1.210
> diff -u -p -r1.210 fetch.c
> --- fetch.c 15 Sep 2022 12:47:10 - 1.210
> +++ fetch.c 9 Nov 2022 15:29:37 -
> @@ -949,8 +950,11 @@ noslash:
> loctail = strchr(redirurl, '#');
> if (loctail != NULL)
> *loctail = '\0';
> - if (verbose)
> - fprintf(ttyout, "Redirected to %s\n", redirurl);
> + if (verbose) {
> + strnvis(gerror, redirurl, sizeof gerror,
> + VIS_SAFE);
> + fprintf(ttyout, "Redirected to %s\n", gerror);
> + }
> ftp_close(, , );
> rval = url_get(redirurl, proxyenv, savefile, lastfile);
> free(redirurl);
>
--
I'm not entirely sure you are real.
Unlock SIOCGIFG{MEMB,ATTR,LIST}
See "Document global interface group list locking" on tech@ --- The global interface group list is also protected by the net lock and all access to it (all within if.c) take it accordingly. Getting all - membersof a group (SIOCGIFGMEMB), - attributes of a group (SIOCGIFGATTR), - groups(SIOCGIFGLIST) are each read-only operations on the global interface group `ifg_head'. The global interface list `ifnetlist' or its per-interface group lists are not used in these ioctls. Feedback? OK? --- sys/net/if.c | 6 -- 1 file changed, 6 deletions(-) diff --git a/sys/net/if.c b/sys/net/if.c index 5dd40f97518..6b13c2800fb 100644 --- a/sys/net/if.c +++ b/sys/net/if.c @@ -2390,25 +2390,19 @@ ifioctl_get(u_long cmd, caddr_t data) error = if_clone_list((struct if_clonereq *)data); return (error); case SIOCGIFGMEMB: - KERNEL_LOCK(); NET_LOCK_SHARED(); error = if_getgroupmembers(data); NET_UNLOCK_SHARED(); - KERNEL_UNLOCK(); return (error); case SIOCGIFGATTR: - KERNEL_LOCK(); NET_LOCK_SHARED(); error = if_getgroupattribs(data); NET_UNLOCK_SHARED(); - KERNEL_UNLOCK(); return (error); case SIOCGIFGLIST: - KERNEL_LOCK(); NET_LOCK_SHARED(); error = if_getgrouplist(data); NET_UNLOCK_SHARED(); - KERNEL_UNLOCK(); return (error); } -- 2.38.1
ftp: strnvis redirect uri
The redirect URI is untrusted input so strnvis it first before printing
it.
--
:wq Claudio
Index: fetch.c
===
RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.210
diff -u -p -r1.210 fetch.c
--- fetch.c 15 Sep 2022 12:47:10 - 1.210
+++ fetch.c 9 Nov 2022 15:29:37 -
@@ -949,8 +950,11 @@ noslash:
loctail = strchr(redirurl, '#');
if (loctail != NULL)
*loctail = '\0';
- if (verbose)
- fprintf(ttyout, "Redirected to %s\n", redirurl);
+ if (verbose) {
+ strnvis(gerror, redirurl, sizeof gerror,
+ VIS_SAFE);
+ fprintf(ttyout, "Redirected to %s\n", gerror);
+ }
ftp_close(, , );
rval = url_get(redirurl, proxyenv, savefile, lastfile);
free(redirurl);
Re: ftp: allow for more spaces in http headers
On Wed, Nov 09, 2022 at 04:21:46PM +0100, Claudio Jeker wrote: > This is the changes that I added to rpki-client http.c which I have not > yet merged into ftp(1) ok tb
ftp: allow for more spaces in http headers
This is the changes that I added to rpki-client http.c which I have not yet merged into ftp(1) This is rev 1.67 of http.c: date: 2022/09/08 13:52:36; author: claudio; state: Exp; lines: +6 -4; commitid: lZD6EB9xp7I5lQCo; In http_get_line() additionally strip any trailing space or tab from lines. In many places the HTTP allows for extra spaces which need to be ignored. Similar the chunked encoding extensions are separated from the chunk size by a ':' but the spec also allows for bad whitespaces in all shapes and forms. Adjust the logic in http_parse_chunked() to stop when the first space, tab or ':' is seen. There is no need to check for newlines since those are stripped by http_get_line(). OK tb@ -- :wq Claudio Index: fetch.c === RCS file: /cvs/src/usr.bin/ftp/fetch.c,v retrieving revision 1.210 diff -u -p -r1.210 fetch.c --- fetch.c 15 Sep 2022 12:47:10 - 1.210 +++ fetch.c 9 Nov 2022 15:10:41 - @@ -875,7 +875,8 @@ noslash: goto cleanup_url_get; } - while (len > 0 && (buf[len-1] == '\r' || buf[len-1] == '\n')) + while (len > 0 && (buf[len-1] == '\r' || buf[len-1] == '\n' || + buf[len-1] == ' ' || buf[len-1] == '\t')) buf[--len] = '\0'; if (len == 0) break; @@ -1161,7 +1162,7 @@ save_chunked(FILE *fin, struct tls *tls, if (getline(, , fin) == -1) break; /* strip CRLF and any optional chunk extension */ - header[strcspn(header, ";\r\n")] = '\0'; + header[strcspn(header, "; \t\r\n")] = '\0'; errno = 0; chunksize = strtoul(header, , 16); if (errno || header[0] == '\0' || *end != '\0' ||
Unlock SIOCGIFCONF
As netintro(4) explains, this copies a bunch of data from the global
interface list its per-interface address lists.
All of this is never written to by ifconf(), protected by the net lock
and documented as such in the struct comments already.
Feedback? OK?
diff --git a/sys/net/if.c b/sys/net/if.c
index f3697b78aed..5dd40f97518 100644
--- a/sys/net/if.c
+++ b/sys/net/if.c
@@ -2382,11 +2382,9 @@ ifioctl_get(u_long cmd, caddr_t data)
switch(cmd) {
case SIOCGIFCONF:
- KERNEL_LOCK();
NET_LOCK_SHARED();
error = ifconf(data);
NET_UNLOCK_SHARED();
- KERNEL_UNLOCK();
return (error);
case SIOCIFGCLONERS:
error = if_clone_list((struct if_clonereq *)data);
Re: netstart: do not try to create physical interfaces
On Tue, Nov 01, 2022 at 01:57:21PM +, Klemens Nanni wrote:
> vifscreate() is used to create all virtual interfaces up-front and is
> always called at the beginning of netstart, whether an explicit list of
> interfaces is passed or none, i.e. all are to be configured.
>
> Yet, to check the given interface exists, ifstart() uses ifcreate()
> which obviously tries to create interfaces.
>
> When ifstart() is run for every hostname.if(5) file, every virtual
> interface is guaranteed to exist thanks to vifscreate().
>
> Nonexistent physical interfaces with an existent config, e.g. urndis(4)
> and cdce(4), will be skipped by ifstart() due to the failed ifcreate()
> call, but not without ifcreate() trying the impossible:
>
> $ ifconfig urndis0
> urndis0: no such interface
> # sh /etc/netstart -n urndis0
> { ifconfig urndis0 || ifconfig urndis0 create; }
> ifconfig urndis0 inet6 autoconf
> ifconfig urndis0 inet autoconf
>
> This dry-run output does NOT match what netstart would really do:
>
> # sh -x /etc/netstart urndis0 2>&1 | tail -n4
> + vifscreate urndis0
> + ifstart urndis0
> + defaultroute
> + return
>
> Here, ifstart() runs but bails out on the failing ifcreate() call and
> thus skips configuring urndis0 entirely.
>
>
> So clarify the comment and replace the ifcreate() call with a simpler,
> more obvious `ifconfig' check, which is exactly what ifcreate() boils
> down to for existing interfaces:
>
> # sh ./netstart -n urndis0 ; echo $?
> 0
>
> Actual steps taken remain the same, i.e. none, as the new dry-run output
> truthfully tells:
>
> # sh -x ./netstart urndis0 2>&1 | tail -n4
> + vifscreate urndis0
> + ifstart urndis0
> + defaultroute
> + return
>
>
> Virtual interfaces are now also created only once:
>
> # sh /etc/netstart -n veb0
> { ifconfig veb0 || ifconfig veb0 create; }
> { ifconfig veb0 || ifconfig veb0 create; }
> ifconfig veb0 description 'vmd(4) uplink'
> ifconfig veb0 up
> # sh ./netstart -n veb0
> { ifconfig veb0 || ifconfig veb0 create; }
> ifconfig veb0 description 'vmd(4) uplink'
> ifconfig veb0 up
>
> Feedback? OK?
Ping.
netstart and the installer's copy still remain in sync as best as
possible: the installer keeps creating interfaces as it doesn't have
any of this create-virtual-interfaces-up-front logic at all.
Index: netstart
===
RCS file: /cvs/src/etc/netstart,v
retrieving revision 1.229
diff -u -p -r1.229 netstart
--- netstart5 Nov 2022 12:06:05 - 1.229
+++ netstart9 Nov 2022 13:14:46 -
@@ -152,8 +152,8 @@ ifstart() {
chown -LR root:wheel $_hn
fi
- # Check for ifconfig'able interface, except if -n option is specified.
- ifcreate $_if || return
+ # Skip missing physical interface, virtual ones were created up front.
+ ifconfig $_if >/dev/null 2>&1 || return
# Parse the hostname.if(5) file and fill _cmds array with interface
# configuration commands.
Re: route(8) example for "out of prefix" default gateway
Claudio Jeker wrote: > On Wed, Nov 09, 2022 at 07:37:50AM +, Stuart Henderson wrote: > > Seems some hosting providers have annoying "out of prefix" > > default gateways whuch are painful to configure > > (https://marc.info/?t=16678224225=1=2), should > > we give a pointer in route(8)? > > > > Index: route.8 > > === > > RCS file: /cvs/src/sbin/route/route.8,v > > retrieving revision 1.104 > > diff -u -p -r1.104 route.8 > > --- route.8 29 Jul 2022 18:28:32 - 1.104 > > +++ route.8 9 Nov 2022 07:29:59 - > > @@ -596,6 +596,14 @@ Delete the > > route to the 192.168.5.0/24 network: > > .Pp > > .Dl # route delete -inet 192.168.5.0/24 > > +.Pp > > +Add a static > > +.Xr inet6 4 > > +route to a host which is on the vio0 interface that is outside the prefix > > +configured on the interface, and use that host as a default gateway: > > +.Pp > > +.Dl # route add -inet6 2001:db8:efef::1 -cloning -link -iface vio0 > > +.Dl # route add -inet6 default 2001:db8:efef::1 > > .Sh DIAGNOSTICS > > .Bl -diag > > .It "%s: gateway %s flags %x" > > > > I'm fine with this for now. It would be great if we could make ifconfig do > the right thing but that is more complex. Setting a destination (like on > point-to-point interfaces) is shared with the broadcast address so > IFF_BROADCAST handling needs to be adjusted. Actually I prefer the route way, because it requires people to be explicit in these circumstances. If it is automatic, there is more chance of creating non-working configurations no?
Re: route(8) example for "out of prefix" default gateway
On Wed, Nov 09, 2022 at 07:37:50AM +, Stuart Henderson wrote: > Seems some hosting providers have annoying "out of prefix" > default gateways whuch are painful to configure > (https://marc.info/?t=16678224225=1=2), should > we give a pointer in route(8)? > > Index: route.8 > === > RCS file: /cvs/src/sbin/route/route.8,v > retrieving revision 1.104 > diff -u -p -r1.104 route.8 > --- route.8 29 Jul 2022 18:28:32 - 1.104 > +++ route.8 9 Nov 2022 07:29:59 - > @@ -596,6 +596,14 @@ Delete the > route to the 192.168.5.0/24 network: > .Pp > .Dl # route delete -inet 192.168.5.0/24 > +.Pp > +Add a static > +.Xr inet6 4 > +route to a host which is on the vio0 interface that is outside the prefix > +configured on the interface, and use that host as a default gateway: > +.Pp > +.Dl # route add -inet6 2001:db8:efef::1 -cloning -link -iface vio0 > +.Dl # route add -inet6 default 2001:db8:efef::1 > .Sh DIAGNOSTICS > .Bl -diag > .It "%s: gateway %s flags %x" > I'm fine with this for now. It would be great if we could make ifconfig do the right thing but that is more complex. Setting a destination (like on point-to-point interfaces) is shared with the broadcast address so IFF_BROADCAST handling needs to be adjusted. -- :wq Claudio
Re: pppx(4): decrease netlock pressure in pppxioctl()
ping...
On Tue, Nov 01, 2022 at 03:16:02PM +0300, Vitaliy Makkoveev wrote:
> Push netlock down to pppx_add_session(). The 'pppx_if' structure has
> the `pxi_ready' member to prevent access to incomplete `pxi', so we
> don't need to hold netlock during all initialisation process. This
> removes potential PR_WAITOK/M_WAITOK allocations impact on packet
> processing. Also this removes relock dances around if_attach() and
> if_detach() calls.
>
> Do not grab netlock for FIONREAD. mbuf(9) queue doesn't rely on it.
>
> Do not grab netlock around pipex_ioctl() call. pipex(4) has its own
> protection and doesn't rely on netlock. We need to unlink pipex(4)
> session before destroy associated `pxi', it can't be killed
> concurrently. Also this stops to block packet processing when npppd(8)
> periodically does PIPEXGCLOSED ioctl(2) commands.
>
> The dummy FIONBIO case doesn't require any lock to be held.
>
> The netlock remains to be taken around pppx_del_session() and
> pppx_set_session_descr() because pppx(4) data structures rely on it.
>
> Index: sys/net/if_pppx.c
> ===
> RCS file: /cvs/src/sys/net/if_pppx.c,v
> retrieving revision 1.122
> diff -u -p -r1.122 if_pppx.c
> --- sys/net/if_pppx.c 29 Aug 2022 07:51:45 - 1.122
> +++ sys/net/if_pppx.c 1 Nov 2022 10:08:37 -
> @@ -414,7 +414,6 @@ pppxioctl(dev_t dev, u_long cmd, caddr_t
> struct pppx_dev *pxd = pppx_dev2pxd(dev);
> int error = 0;
>
> - NET_LOCK();
> switch (cmd) {
> case PIPEXASESSION:
> error = pppx_add_session(pxd,
> @@ -422,13 +421,17 @@ pppxioctl(dev_t dev, u_long cmd, caddr_t
> break;
>
> case PIPEXDSESSION:
> + NET_LOCK();
> error = pppx_del_session(pxd,
> (struct pipex_session_close_req *)addr);
> + NET_UNLOCK();
> break;
>
> case PIPEXSIFDESCR:
> + NET_LOCK();
> error = pppx_set_session_descr(pxd,
> (struct pipex_session_descr_req *)addr);
> + NET_UNLOCK();
> break;
>
> case FIONBIO:
> @@ -441,7 +444,6 @@ pppxioctl(dev_t dev, u_long cmd, caddr_t
> error = pipex_ioctl(pxd, cmd, addr);
> break;
> }
> - NET_UNLOCK();
>
> return (error);
> }
> @@ -607,6 +609,7 @@ pppx_add_session(struct pppx_dev *pxd, s
>
> pxi->pxi_session = session;
>
> + NET_LOCK();
> /* try to set the interface up */
> unit = pppx_if_next_unit();
> if (unit < 0) {
> @@ -624,6 +627,7 @@ pppx_add_session(struct pppx_dev *pxd, s
> goto out;
> }
> LIST_INSERT_HEAD(>pxd_pxis, pxi, pxi_list);
> + NET_UNLOCK();
>
> snprintf(ifp->if_xname, sizeof(ifp->if_xname), "%s%d", "pppx", unit);
> ifp->if_mtu = req->pr_peer_mru; /* XXX */
> @@ -638,13 +642,12 @@ pppx_add_session(struct pppx_dev *pxd, s
> /* ifp->if_rdomain = req->pr_rdomain; */
> if_counters_alloc(ifp);
>
> - /* XXXSMP breaks atomicity */
> - NET_UNLOCK();
> if_attach(ifp);
> - NET_LOCK();
>
> + NET_LOCK();
> if_addgroup(ifp, "pppx");
> if_alloc_sadl(ifp);
> + NET_UNLOCK();
>
> #if NBPFILTER > 0
> bpfattach(>if_bpf, ifp, DLT_LOOP, sizeof(u_int32_t));
> @@ -680,6 +683,7 @@ pppx_add_session(struct pppx_dev *pxd, s
>
> ia->ia_netmask = ia->ia_sockmask.sin_addr.s_addr;
>
> + NET_LOCK();
> error = in_ifinit(ifp, ia, , 1);
> if (error) {
> printf("pppx: unable to set addresses for %s, error=%d\n",
> @@ -687,26 +691,29 @@ pppx_add_session(struct pppx_dev *pxd, s
> } else {
> if_addrhooks_run(ifp);
> }
> + NET_UNLOCK();
>
> error = pipex_link_session(session, ifp, pxd);
> if (error)
> goto detach;
>
> + NET_LOCK();
> SET(ifp->if_flags, IFF_RUNNING);
> pxi->pxi_ready = 1;
> + NET_UNLOCK();
>
> return (error);
>
> detach:
> - /* XXXSMP breaks atomicity */
> - NET_UNLOCK();
> if_detach(ifp);
> - NET_LOCK();
>
> + NET_LOCK();
> if (RBT_REMOVE(pppx_ifs, _ifs, pxi) == NULL)
> panic("%s: inconsistent RB tree", __func__);
> LIST_REMOVE(pxi, pxi_list);
> out:
> + NET_UNLOCK();
> +
> pool_put(_if_pl, pxi);
> pipex_rele_session(session);
>
Re: pause.3: Use Fn
On Tue, Nov 08, 2022 at 04:35:33PM -0500, Josiah Frentsos wrote: > Index: pause.3 > === > RCS file: /cvs/src/lib/libc/gen/pause.3,v > retrieving revision 1.15 > diff -u -p -r1.15 pause.3 > --- pause.3 2 Aug 2022 01:23:23 - 1.15 > +++ pause.3 8 Nov 2022 21:22:46 - > @@ -39,7 +39,8 @@ > .Fn pause void > .Sh DESCRIPTION > .Bf -symbolic > -Pause is made obsolete by > +.Fn pause > +is made obsolete by > .Xr sigsuspend 2 . > .Ef > .Pp > committed, thanks. jmc
Re: mg: handle prefix argument in shell-command{,-on-region}
bump
On 2022/10/25 14:30:51 +0200, Omar Polo wrote:
> On 2022/10/13 12:25:00 +0200, Omar Polo wrote:
> > shell-command (M-!) and shell-command-on-region (M-|) works by
> > displaying the output of the command in a new buffer, but in emacs
> > using a prefix argument (C-u) allows to operate on the current buffer.
> >
> > diff belows adds that for mg. I can finally C-u M-! got diff RET when
> > composing mails :)
> >
> > A possible drawback is that now the *Shell Command Output* buffer
> > gains an undo history. linsert is also possibly slower than addline
> > but on the plus side we're no more limited to BUFSIZ long lines.
> >
> > ok/comments/improvements?
>
> Here's a slightly tweaked version that adds a missing parens around a
> return value and uses ssize_t for some vars in preadin. it also changes
> the size read(2) from BUFSIZ-1 to BUFSIZ since we no longer need to NUL
> terminate it.
>
> This has been more useful than I originally expected. I wanted it to
> include diffs and the like more easily, now i'm using it also for all
> sorts of stuff that mg doesn't do out-of-the-box (like using C-u M-|
> sort RET instead of M-x sort-lines.)
>
> If it were for me, M-| and M-! would operate by default on the buffer
> and with C-u on a scratch one, but this is what emacs does and i'm
> probably several decades too late :)
diff 214e94d3085276f4e5c6b416bfd54b5d50a7bf91
cde294b6d4634ab98c7926103d373202007e23c2
commit - 214e94d3085276f4e5c6b416bfd54b5d50a7bf91
commit + cde294b6d4634ab98c7926103d373202007e23c2
blob - 21c5174f52d21103b9cd15942620eb746b5069b2
blob + 1bee60b37ceea3f9ec3ceb779727f877a7f15851
--- usr.bin/mg/region.c
+++ usr.bin/mg/region.c
@@ -26,14 +26,13 @@ static char leftover[BUFSIZ];
#define TIMEOUT 1
-static char leftover[BUFSIZ];
-
static int getregion(struct region *);
static int iomux(int, char * const, int, struct buffer *);
static int preadin(int, struct buffer *);
static voidpwriteout(int, char **, int *);
static int setsize(struct region *, RSIZE);
-static int shellcmdoutput(char * const[], char * const, int);
+static int shellcmdoutput(char * const[], char * const, int,
+ struct buffer *);
/*
* Kill the region. Ask "getregion" to figure out the bounds of the region.
@@ -419,14 +418,11 @@ piperegion(int f, int n)
piperegion(int f, int n)
{
struct region region;
+ struct buffer *bp = NULL;
int len;
char *cmd, cmdbuf[NFILEN], *text;
char *argv[] = {"sh", "-c", (char *) NULL, (char *) NULL};
- /* C-u M-| is not supported yet */
- if (n > 1)
- return (ABORT);
-
if (curwp->w_markp == NULL) {
dobeep();
ewprintf("The mark is not set now, so there is no region");
@@ -452,7 +448,13 @@ piperegion(int f, int n)
region_get_data(, text, len);
- return shellcmdoutput(argv, text, len);
+ if (n > 1) {
+ bp = curbp;
+ killregion(FFRAND, 1);
+ kdelete();
+ }
+
+ return (shellcmdoutput(argv, text, len, bp));
}
/*
@@ -462,12 +464,12 @@ shellcommand(int f, int n)
int
shellcommand(int f, int n)
{
-
+ struct buffer *bp = NULL;
char *cmd, cmdbuf[NFILEN];
char *argv[] = {"sh", "-c", (char *) NULL, (char *) NULL};
if (n > 1)
- return (ABORT);
+ bp = curbp;
if ((cmd = eread("Shell command: ", cmdbuf, sizeof(cmdbuf),
EFNEW | EFCR)) == NULL || (cmd[0] == '\0'))
@@ -475,36 +477,43 @@ shellcommand(int f, int n)
argv[2] = cmd;
- return shellcmdoutput(argv, NULL, 0);
+ return (shellcmdoutput(argv, NULL, 0, bp));
}
-
int
-shellcmdoutput(char* const argv[], char* const text, int len)
+shellcmdoutput(char* const argv[], char* const text, int len,
+struct buffer *bp)
{
-
- struct buffer *bp;
+ struct mgwin *wp;
char*shellp;
- int ret;
+ int ret, special = 0;
- bp = bfind("*Shell Command Output*", TRUE);
- bp->b_flag |= BFREADONLY;
- if (bclear(bp) != TRUE) {
- free(text);
- return (FALSE);
+ if (bp == NULL) {
+ special = 1;
+ bp = bfind("*Shell Command Output*", TRUE);
+ bp->b_flag &= ~BFREADONLY; /* disable read-only */
+ wp = popbuf(bp, WNONE);
+ if (wp == NULL || bclear(bp) != TRUE) {
+ free(text);
+ return (FALSE);
+ }
+ curbp = bp;
+ curwp = wp;
}
shellp = getenv("SHELL");
ret = pipeio(shellp, argv, text, len, bp);
-
if (ret == TRUE) {
eerase();
- if (lforw(bp->b_headp) == bp->b_headp)
+ if (special && lforw(bp->b_headp) == bp->b_headp)
addline(bp, "(Shell command succeeded with no
Re: route(8) example for "out of prefix" default gateway
OK florian On 9 November 2022 07:37:50 GMT, Stuart Henderson wrote: >Seems some hosting providers have annoying "out of prefix" >default gateways whuch are painful to configure >(https://marc.info/?t=16678224225=1=2), should >we give a pointer in route(8)? > >Index: route.8 >=== >RCS file: /cvs/src/sbin/route/route.8,v >retrieving revision 1.104 >diff -u -p -r1.104 route.8 >--- route.829 Jul 2022 18:28:32 - 1.104 >+++ route.89 Nov 2022 07:29:59 - >@@ -596,6 +596,14 @@ Delete the > route to the 192.168.5.0/24 network: > .Pp > .Dl # route delete -inet 192.168.5.0/24 >+.Pp >+Add a static >+.Xr inet6 4 >+route to a host which is on the vio0 interface that is outside the prefix >+configured on the interface, and use that host as a default gateway: >+.Pp >+.Dl # route add -inet6 2001:db8:efef::1 -cloning -link -iface vio0 >+.Dl # route add -inet6 default 2001:db8:efef::1 > .Sh DIAGNOSTICS > .Bl -diag > .It "%s: gateway %s flags %x" > -- Sent from a mobile device. Please excuse poor formatting.
