inteldrm: Use "the flush page mechanism" only on chips having it.
In the intel_gtt_chipset_setup() routine, called at initialization,
the driver maps pci space for certain chip models only, but later in
intel_gtt_chipset_flush() it calls bus_space_write() on it in cases
it's not initialized.
This results in crashes during boot with "Pineview" generation chips,
which are "gen == 3", but, according to intel_gtt_chipset_setup() have
no flush page mechanism.
Fix this by adding a flag indicating whether the feature is available
or not. Then use the flag to check if it's OK to write to the pci
space. This is the what the driver did before the 14 april update.
OK?
FWIW, first it looked surprising to do nothing in
intel_gtt_chipset_flush(), so I also tried to call the "gen < 3"
code. It works as well.
Index: intel_gtt.c
===
RCS file: /cvs/src/sys/dev/pci/drm/i915/intel_gtt.c,v
retrieving revision 1.2
diff -u -p -u -p -r1.2 intel_gtt.c
--- intel_gtt.c 14 Apr 2019 10:14:52 - 1.2
+++ intel_gtt.c 9 May 2019 05:37:09 -
@@ -120,6 +120,8 @@ intel_gtt_chipset_setup(struct drm_devic
struct inteldrm_softc *dev_priv = dev->dev_private;
struct pci_attach_args bpa;
+ dev_priv->has_ifp = 0;
+
if (INTEL_GEN(dev_priv) >= 6)
return;
@@ -133,8 +135,10 @@ intel_gtt_chipset_setup(struct drm_devic
if (IS_I915G(dev_priv) || IS_I915GM(dev_priv) || IS_I945G(dev_priv) ||
IS_I945GM(dev_priv)) {
i915_alloc_ifp(dev_priv, );
+ dev_priv->has_ifp = 1;
} else if (INTEL_GEN(dev_priv) >= 4 || IS_G33(dev_priv)) {
i965_alloc_ifp(dev_priv, );
+ dev_priv->has_ifp = 1;
} else {
int nsegs;
/*
@@ -192,7 +196,7 @@ intel_gtt_chipset_flush(void)
* The write will return when it is done.
*/
if (INTEL_GEN(dev_priv) >= 3) {
- if (dev_priv->ifp.i9xx.bsh != 0)
+ if (dev_priv->has_ifp && dev_priv->ifp.i9xx.bsh != 0)
bus_space_write_4(dev_priv->ifp.i9xx.bst,
dev_priv->ifp.i9xx.bsh, 0, 1);
} else {
less discriminatory battlestar
there are lists of annointed usernames in battlestar. this creates an unfair
playing field! worse, there is a list of "bad" people! and i'm almost one of
them!
-static const char *const badguys[] = {
- "wnj",
- "root",
- "ted",
- 0
-};
Index: com1.c
===
RCS file: /home/cvs/src/games/battlestar/com1.c,v
retrieving revision 1.15
diff -u -p -r1.15 com1.c
--- com1.c 31 Dec 2015 17:51:19 - 1.15
+++ com1.c 9 May 2019 03:31:36 -
@@ -129,7 +129,7 @@ news(void)
}
rythmn = ourtime - ourtime % CYCLE;
}
- if (!wiz && !tempwiz)
+ if (!tempwiz)
if ((TestBit(inven, TALISMAN) || TestBit(wear, TALISMAN)) &&
(TestBit(inven, MEDALION) || TestBit(wear, MEDALION)) && (TestBit(inven,
AMULET) || TestBit(wear, AMULET))) {
tempwiz = 1;
puts("The three amulets glow and reenforce each other
in power.\nYou are now a wizard.");
Index: com4.c
===
RCS file: /home/cvs/src/games/battlestar/com4.c,v
retrieving revision 1.15
diff -u -p -r1.15 com4.c
--- com4.c 31 Dec 2015 17:51:19 - 1.15
+++ com4.c 9 May 2019 03:32:49 -
@@ -53,7 +53,7 @@ take(unsigned int from[])
printf("%s:\n", objsht[value]);
heavy = (carrying + objwt[value]) <= WEIGHT;
bulky = (encumber + objcumber[value]) <= CUMBER;
- if ((TestBit(from, value) || wiz || tempwiz) && heavy
&& bulky && !TestBit(inven, value)) {
+ if ((TestBit(from, value) || tempwiz) && heavy && bulky
&& !TestBit(inven, value)) {
SetBit(inven, value);
carrying += objwt[value];
encumber += objcumber[value];
Index: com6.c
===
RCS file: /home/cvs/src/games/battlestar/com6.c,v
retrieving revision 1.24
diff -u -p -r1.24 com6.c
--- com6.c 7 Feb 2018 20:22:23 - 1.24
+++ com6.c 9 May 2019 03:33:19 -
@@ -130,13 +130,10 @@ post(char ch)
if (score_fp != NULL) {
fprintf(score_fp, "%s %31s %c%20s", date, username, ch,
rate());
- if (wiz)
- fprintf(score_fp, " wizard\n");
+ if (tempwiz)
+ fprintf(score_fp, " WIZARD!\n");
else
- if (tempwiz)
- fprintf(score_fp, " WIZARD!\n");
- else
- fprintf(score_fp, "\n");
+ fprintf(score_fp, "\n");
}
sigprocmask(SIG_SETMASK, , (sigset_t *)0);
}
Index: cypher.c
===
RCS file: /home/cvs/src/games/battlestar/cypher.c,v
retrieving revision 1.19
diff -u -p -r1.19 cypher.c
--- cypher.c31 Dec 2015 17:51:19 - 1.19
+++ cypher.c9 May 2019 03:34:13 -
@@ -105,7 +105,7 @@ cypher(void)
break;
case UP:
- if (location[position].access || wiz || tempwiz) {
+ if (location[position].access || tempwiz) {
if (!location[position].access)
puts("Zap! A gust of wind lifts you
up.");
if (!moveplayer(location[position].up, AHEAD))
@@ -318,7 +318,7 @@ cypher(void)
break;
case SU:
- if (wiz || tempwiz) {
+ if (tempwiz) {
getnum(, "\nRoom (was %d) = ",
position);
getnum(, "Time (was %d) = ", ourtime);
getnum(, "Fuel (was %d) = ", fuel);
@@ -326,8 +326,8 @@ cypher(void)
getnum(, "CUMBER (was %d) = ", CUMBER);
getnum(, "WEIGHT (was %d) = ", WEIGHT);
getnum(, "Clock (was %d) = ",
ourclock);
- if (getnum(, "Wizard (was %d, %d) = ",
wiz, tempwiz) != -1 && !junk)
- tempwiz = wiz = 0;
+ if (getnum(, "Wizard (was %d) = ",
tempwiz) != -1 && !junk)
+ tempwiz = 0;
printf("\nDONE.\n");
return (0); /* No commands after a SU */
} else
Index: extern.h
===
RCS file: /home/cvs/src/games/battlestar/extern.h,v
retrieving revision 1.20
diff -u -p -r1.20 extern.h
--- extern.h31 Dec 2015 17:51:19 - 1.20
+++ extern.h
Re: [PATCH] Parallelize sysupgrade downloads
On 2019-05-09, Christian Weisgerber wrote: >> Does unpriv do anything if you don't give it a command to run? > > It should return an error code and abort the script. And actually that's what it does. (Sorry, I confused myself there for a moment.) -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: iwm: fix ADD_STA status check
On Wed, May 08, 2019 at 03:26:00PM -0400, Stefan Sperling wrote: > > Correctly mask status bits in ADD_STA command response; remaining > bits are used by firmware to return the baid for a BA session. > > Dragonfly 74d41163ddac72b0d7ea7b7873d53fe134723a12 > Linux 837c4da98481d4e504b2aba077c8528fee1b6d13 > > Not sure if this matters for us yet, but it will likely matter > when we start doing Tx aggregation. > > Perhaps this will fix the 'could not add sta' problem we've seen > occasionally? Let's try... > > ok? ok kevlo@
Re: iwm: rx interrupt paranoia
On Wed, May 08, 2019 at 02:54:50PM -0400, Stefan Sperling wrote: > > Add two sanity checks to iwm's firmware notification interrupt handler: > > 1) Clamp the firmware-provided index into the rx ring to the size of the ring. > Linux started doing this, too, to work around HW bugs in the 9000 series. Right, this matches Linux commit 5eae443eb5e2b3777582ea37c6a002171ec134d5 > 2) Don't call iwm_cmd_done() if the firmware response in the Rx buffer > is not recognized. We should just skip such buffers, not act on them. > > Not tested on hardware yet; these changes evidenty shouldn't break anything. Tested: iwm0 at pci3 dev 0 function 0 "Intel Dual Band Wireless AC 3165" rev 0x81, msi iwm0: hw rev 0x210, fw ver 16.242414.0, address 08:d4:0c:xx:xx:xx > ok? ok kevlo@
sysupgrade: don't re-verify sets in the installer
Currently we verify all sets, reboot into bsd.upgrade, and there the
installer re-verifies them all over. The diff below eliminates this
redundant check.
In the installer, verification is triggered by the presence of
SHA256.sig. Later, SHA256.sig is saved, so sysupgrade can exit
early if no newer snapshot is available. For this check, SHA256
(no .sig) will do just the same.
* In the installer, stash away a copy of SHA256 and move that code
into install_files() where the sets are actually processed. There we
also have a source URL that allows us to copy SHA256 when no
verification of the sets happened.
Confirm in the response file that we want to proceed without
verifying the sets.
* In sysupgrade, remove SHA256.sig once the signature has been
verified. Compare SHA256 against a stored copy from the previous
install/upgrade.
OK?
Index: distrib/miniroot/install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.1121
diff -u -p -r1.1121 install.sub
--- distrib/miniroot/install.sub8 May 2019 15:53:31 - 1.1121
+++ distrib/miniroot/install.sub8 May 2019 23:09:46 -
@@ -1685,6 +1685,13 @@ install_files() {
[[ -d $_tmpsrc ]] && rm -f "$_tmpsrc/$_f"
done
[[ -d $_tmpsrc ]] && rm -rf "$_tmpsrc" || true
+
+ # Keep SHA256 from installed sets for sysupgrade(8).
+ if [[ -f $_cfile ]]; then
+ cp $_cfile /mnt/var/db/installed.SHA256
+ elif $_srclocal && [[ -f ${_src#file://}/SHA256 ]]; then
+ cp ${_src#file://}/SHA256 /mnt/var/db/installed.SHA256
+ fi
}
# Fetch install sets from an HTTP server possibly using a proxy.
@@ -2769,11 +2776,6 @@ finish_up() {
# ensure it references the kernel as /bsd.
sha256 /mnt/bsd | (umask 077; sed 's,/mnt,,' >/mnt/var/db/kernel.SHA256)
- # Keep SHA256.sig from installed sets for sysupgrade(8).
- if [[ -f /tmp/SHA256.sig ]]; then
- cp /tmp/SHA256.sig /mnt/var/db/installed.SHA256.sig
- fi
-
if [[ -f $_kernel_dir.tgz ]]; then
echo -n "Relinking to create unique kernel..."
(
@@ -3388,6 +3390,7 @@ elif $UU; then
Location of sets = disk
Pathname to the sets = /home/_sysupgrade/
Set name(s) = done
+Directory does not contain SHA256.sig. Continue without verification = yes
__EOT
get_responsefile
do_autoinstall
Index: usr.sbin/sysupgrade/sysupgrade.sh
===
RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
retrieving revision 1.16
diff -u -p -r1.16 sysupgrade.sh
--- usr.sbin/sysupgrade/sysupgrade.sh 8 May 2019 15:06:20 - 1.16
+++ usr.sbin/sysupgrade/sysupgrade.sh 8 May 2019 23:09:46 -
@@ -132,11 +132,6 @@ cd ${SETSDIR}
unpriv -f SHA256.sig ftp -Vmo SHA256.sig ${URL}SHA256.sig
-if cmp -s /var/db/installed.SHA256.sig SHA256.sig && ! $FORCE; then
- echo "Already on latest snapshot."
- exit 0
-fi
-
_KEY=openbsd-${_KERNV[0]%.*}${_KERNV[0]#*.}-base.pub
_NEXTKEY=openbsd-${NEXT_VERSION%.*}${NEXT_VERSION#*.}-base.pub
@@ -150,6 +145,12 @@ esac
[[ -f ${SIGNIFY_KEY} ]] || ug_err "cannot find ${SIGNIFY_KEY}"
unpriv -f SHA256 signify -Ve -p "${SIGNIFY_KEY}" -x SHA256.sig -m SHA256
+rm SHA256.sig
+
+if cmp -s /var/db/installed.SHA256 SHA256 && ! $FORCE; then
+ echo "Already on latest snapshot."
+ exit 0
+fi
# INSTALL.*, bsd*, *.tgz
SETS=$(sed -n -e 's/^SHA256 (\(.*\)) .*/\1/' \
@@ -157,7 +158,6 @@ SETS=$(sed -n -e 's/^SHA256 (\(.*\)) .*/
OLD_FILES=$(ls)
OLD_FILES=$(rmel SHA256 $OLD_FILES)
-OLD_FILES=$(rmel SHA256.sig $OLD_FILES)
DL=$SETS
for f in $SETS; do
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: [PATCH] Parallelize sysupgrade downloads
On 2019-05-08, Jarkko Oranen wrote:
>> -unpriv -f $f ftp -Vmo ${f} ${URL}${f}
>> +unpriv -f $f
>
> Does unpriv do anything if you don't give it a command to run?
It should return an error code and abort the script. I need to
investigate why that doesn't happen.
--
Christian "naddy" Weisgerber na...@mips.inka.de
Re: Avoid system(3) in ikectl
Reyk Floeter wrote: > I meant: could you use /* */ instead of //? oh, sure. done. > Yes, it looks slightly better. > > grumble OK reyk thanks. Matthew, thanks.
Re: ftpd(8): rm dead code and simplifies popen clone
On Wed, May 08, 2019 at 07:10:37PM -0400, Ted Unangst wrote:
> Jan Klemkow wrote:
> > - * Special version of popen which avoids call to shell. This ensures noone
> > + * Special version of popen which avoids call to shell. This ensures none
>
> If we don't like noone, the correct spelling is no one.
>
> Rest of the diff seems like a good improvement.
fixed.
Thanks,
Jan
Index: extern.h
===
RCS file: /cvs/src/libexec/ftpd/extern.h,v
retrieving revision 1.19
diff -u -p -r1.19 extern.h
--- extern.h4 Oct 2015 11:58:09 - 1.19
+++ extern.h8 May 2019 20:02:23 -
@@ -68,7 +68,7 @@ void delete(char *);
void dologout(int);
void fatal(char *);
intftpd_pclose(FILE *, pid_t);
-FILE *ftpd_popen(char *, char *, pid_t *);
+FILE *ftpd_ls(char *, char *, pid_t *);
int get_line(char *, int, FILE *);
void ftpdlogwtmp(char *, char *, char *);
void lreply(int, const char *, ...);
@@ -89,7 +89,8 @@ void renamecmd(char *, char *);
char *renamefrom(char *);
void reply(int, const char *, ...);
void reply_r(int, const char *, ...);
-void retrieve(char *, char *);
+enum ret_cmd { RET_FILE, RET_LIST };
+void retrieve(enum ret_cmd, char *);
void send_file_list(char *);
void setproctitle(const char *, ...);
void statcmd(void);
Index: ftpcmd.y
===
RCS file: /cvs/src/libexec/ftpd/ftpcmd.y,v
retrieving revision 1.66
diff -u -p -r1.66 ftpcmd.y
--- ftpcmd.y27 Apr 2017 13:30:54 - 1.66
+++ ftpcmd.y8 May 2019 20:03:29 -
@@ -342,7 +342,7 @@ cmd
| RETR check_login SP pathname CRLF
{
if ($2 && $4 != NULL)
- retrieve(NULL, $4);
+ retrieve(RET_FILE, $4);
if ($4 != NULL)
free($4);
}
@@ -374,12 +374,12 @@ cmd
| LIST check_login CRLF
{
if ($2)
- retrieve("/bin/ls -lgA", "");
+ retrieve(RET_LIST, NULL);
}
| LIST check_login SP pathname CRLF
{
if ($2 && $4 != NULL)
- retrieve("/bin/ls -lgA %s", $4);
+ retrieve(RET_LIST, $4);
if ($4 != NULL)
free($4);
}
Index: ftpd.8
===
RCS file: /cvs/src/libexec/ftpd/ftpd.8,v
retrieving revision 1.75
diff -u -p -r1.75 ftpd.8
--- ftpd.8 25 Oct 2015 23:10:53 - 1.75
+++ ftpd.8 8 May 2019 20:08:15 -
@@ -385,15 +385,6 @@ subtree be constructed with care, follow
Make the home directory owned by
.Dq root
and unwritable by anyone (mode 555).
-.It Pa ~ftp/bin
-Make this directory owned by
-.Dq root
-and unwritable by anyone (mode 511).
-This directory is optional unless you have commands you wish
-the anonymous FTP user to be able to run (the
-.Xr ls 1
-command exists as a built-in).
-Any programs in this directory should be mode 111 (executable only).
.It Pa ~ftp/etc
Make this directory owned by
.Dq root
Index: ftpd.c
===
RCS file: /cvs/src/libexec/ftpd/ftpd.c,v
retrieving revision 1.225
diff -u -p -r1.225 ftpd.c
--- ftpd.c 11 Dec 2018 18:19:55 - 1.225
+++ ftpd.c 8 May 2019 22:45:07 -
@@ -1113,36 +1113,32 @@ bad:
}
void
-retrieve(char *cmd, char *name)
+retrieve(enum ret_cmd cmd, char *name)
{
FILE *fin, *dout;
struct stat st;
pid_t pid;
time_t start;
- if (cmd == NULL) {
+ if (cmd == RET_FILE) {
fin = fopen(name, "r");
st.st_size = 0;
} else {
- char line[BUFSIZ];
-
- (void) snprintf(line, sizeof(line), cmd, name);
- name = line;
- fin = ftpd_popen(line, "r", );
+ fin = ftpd_ls("-lgA", name, );
st.st_size = -1;
st.st_blksize = BUFSIZ;
}
if (fin == NULL) {
if (errno != 0) {
perror_reply(550, name);
- if (cmd == NULL) {
+ if (cmd == RET_FILE) {
LOGCMD("get", name);
}
}
return;
}
byte_count = -1;
- if (cmd == NULL &&
+ if (cmd == RET_FILE &&
(fstat(fileno(fin), ) < 0 || !S_ISREG(st.st_mode))) {
reply(550, "%s: not a plain file.", name);
goto done;
@@ -1175,8 +1171,8 @@ retrieve(char *cmd, char *name)
goto done;
time();
send_data(fin, dout, st.st_blksize, st.st_size,
-
Chromebook serial
Modern chromebooks expose a debugging interface on one of their USB
Type-C connectors. Part of this interface are *three* USB serial
devices that connect to microcontrollers and a serial port on the SoC.
It all works nicely with ucom(4) but we need a bit of glue to actually
make that attach. What I ended up is pretty close to uscom(4) but the
discovery of the descriptors is different and I don't immediately see
a way to fold this into that driver.
Should this be enabled on more architectures than amd64?
ok?
Index: arch/amd64/conf/GENERIC
===
RCS file: /cvs/src/sys/arch/amd64/conf/GENERIC,v
retrieving revision 1.470
diff -u -p -r1.470 GENERIC
--- arch/amd64/conf/GENERIC 4 May 2019 17:59:40 - 1.470
+++ arch/amd64/conf/GENERIC 8 May 2019 22:03:21 -
@@ -234,6 +234,8 @@ uslcom* at uhub?# Silicon Laboratories
ucom* at uslcom?
uscom* at uhub?# Simple USB serial adapters
ucom* at uscom?
+ucrcom*at uhub?# Chromebook serial
+ucom* at ucrcom?
uark* at uhub?# Arkmicro ARK3116 serial
ucom* at uark?
moscom*at uhub?# MosChip MCS7703 serial
Index: dev/usb/files.usb
===
RCS file: /cvs/src/sys/dev/usb/files.usb,v
retrieving revision 1.137
diff -u -p -r1.137 files.usb
--- dev/usb/files.usb 27 Mar 2019 22:08:51 - 1.137
+++ dev/usb/files.usb 8 May 2019 22:02:02 -
@@ -342,6 +342,11 @@ device uscom: ucombus
attach uscom at uhub
file dev/usb/uscom.c uscom
+# Chromebook serial
+device ucrcom: ucombus
+attach ucrcom at uhub
+file dev/usb/ucrcom.cucrcom
+
# Exar XR21V1410
device uxrcom: ucombus
attach uxrcom at uhub
Index: dev/usb/ucrcom.c
===
RCS file: dev/usb/ucrcom.c
diff -N dev/usb/ucrcom.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ dev/usb/ucrcom.c8 May 2019 22:49:57 -
@@ -0,0 +1,136 @@
+/* $OpenBSD$ */
+
+/*
+ * Copyright (c) 2019 Mark Kettenis
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+
+#include
+
+#define UCRCOMBUFSZ64
+
+struct ucrcom_softc {
+ struct device sc_dev;
+ struct device *sc_subdev;
+};
+
+struct ucom_methods ucrcom_methods = { NULL };
+
+int ucrcom_match(struct device *, void *, void *);
+void ucrcom_attach(struct device *, struct device *, void *);
+int ucrcom_detach(struct device *, int);
+
+struct cfdriver ucrcom_cd = {
+ NULL, "ucrcom", DV_DULL
+};
+
+const struct cfattach ucrcom_ca = {
+ sizeof(struct ucrcom_softc), ucrcom_match, ucrcom_attach, ucrcom_detach
+};
+
+int
+ucrcom_match(struct device *parent, void *match, void *aux)
+{
+ struct usb_attach_arg *uaa = aux;
+ usb_interface_descriptor_t *id;
+ usb_device_descriptor_t *dd;
+
+ if (uaa->iface == NULL)
+ return UMATCH_NONE;
+
+ id = usbd_get_interface_descriptor(uaa->iface);
+ dd = usbd_get_device_descriptor(uaa->device);
+ if (id == NULL || dd == NULL)
+ return UMATCH_NONE;
+
+ if (UGETW(dd->idVendor) == USB_VENDOR_GOOGLE &&
+ id->bInterfaceClass == UICLASS_VENDOR &&
+ id->bInterfaceSubClass == 0x50 &&
+ id->bInterfaceProtocol == 1)
+ return UMATCH_VENDOR_IFACESUBCLASS_IFACEPROTO;
+
+ return UMATCH_NONE;
+}
+
+void
+ucrcom_attach(struct device *parent, struct device *self, void *aux)
+{
+ struct ucrcom_softc *sc = (struct ucrcom_softc *)self;
+ struct usb_attach_arg *uaa = aux;
+ struct ucom_attach_args uca;
+ usb_interface_descriptor_t *id;
+ usb_endpoint_descriptor_t *ed;
+ int i;
+
+ id = usbd_get_interface_descriptor(uaa->iface);
+
+ memset(, 0, sizeof(uca));
+ uca.bulkin = uca.bulkout = -1;
+ for (i = 0; i < id->bNumEndpoints; i++) {
+ ed = usbd_interface2endpoint_descriptor(uaa->iface, i);
+ if (ed == NULL) {
+ printf("%s: no endpoint descriptor for
Re: Avoid system(3) in ikectl
On Wed, May 08, 2019 at 07:05:24PM -0400, Ted Unangst wrote:
> Reyk Floeter wrote:
> > On Wed, May 08, 2019 at 06:44:32PM -0400, Ted Unangst wrote:
> > > Ted Unangst wrote:
> > > > Matthew Martin wrote:
> > > > > I did that originally [1], but Reyk preferred the varargs approach
> > > > > [2],
> > > > > so I changed the patch to match.
> > > >
> > > > Sorry, only wading into the thread at this point. Seems not everybody
> > > > has the
> > > > same taste in code... Well, we have the original. Let me bring that
> > > > back.
> > >
> > > OK, here's diff one, but with run() renamed to ca_exec(). I think picking
> > > a
> > > better name was at least one improvement. :)
> > >
> > > And the file: comment added.
> > >
> >
> > His first diff included other things that got cleaned up, please don't
> > revert to this one (eg. the // comment).
>
> I looked at history to pick that up. Anything else?
>
I meant: could you use /* */ instead of //?
> I think returning an error code is the right thing, even if not checked. A
> void function means can't fail, not fails silently.
>
> ca_exec or ca_system I could go either way, but there's no longer sh involved,
> so that's why I went back to exec.
>
OK, that's a point.
> > If you want to switch to an array instead of varargs, fine. But the
> > definition of char *cmd[] somewhere in the function, especially in an
> > extra {} block, looks very ugly to me. That's why I suggested the
> > varargs in the first place to avoid such a thing.
>
> I do think it's less ugly without the extra {}. Here's a version with less of
> that.
>
Yes, it looks slightly better.
grumble OK reyk
>
> Index: ikeca.c
> ===
> RCS file: /home/cvs/src/usr.sbin/ikectl/ikeca.c,v
> retrieving revision 1.48
> diff -u -p -r1.48 ikeca.c
> --- ikeca.c 26 Feb 2019 14:21:30 - 1.48
> +++ ikeca.c 8 May 2019 22:59:09 -
> @@ -18,6 +18,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -63,12 +64,12 @@
>
> struct ca {
> char sslpath[PATH_MAX];
> - char passfile[PATH_MAX];
> + char passfile[PATH_MAX + 5]; // Includes the "file:" prefix
> char index[PATH_MAX];
> char serial[PATH_MAX];
> char sslcnf[PATH_MAX];
> char extcnf[PATH_MAX];
> - char batch[PATH_MAX];
> + char*batch;
> char*caname;
> };
>
> @@ -116,6 +117,7 @@ void ca_setenv(const char *, const cha
> void ca_clrenv(void);
> void ca_setcnf(struct ca *, const char *);
> void ca_create_index(struct ca *);
> +int staticca_exec(char *const []);
>
> /* util.c */
> int expand_string(char *, size_t, const char *, const char *);
> @@ -130,7 +132,6 @@ int
> ca_key_create(struct ca *ca, char *keyname)
> {
> struct stat st;
> - char cmd[PATH_MAX * 2];
> char path[PATH_MAX];
>
> snprintf(path, sizeof(path), "%s/private/%s.key", ca->sslpath, keyname);
> @@ -140,10 +141,8 @@ ca_key_create(struct ca *ca, char *keyna
> return (0);
> }
>
> - snprintf(cmd, sizeof(cmd),
> - "%s genrsa -out %s 2048",
> - PATH_OPENSSL, path);
> - system(cmd);
> + char *cmd[] = { PATH_OPENSSL, "genrsa", "-out", path, "2048", NULL };
> + ca_exec(cmd);
> chmod(path, 0600);
>
> return (0);
> @@ -200,9 +199,9 @@ ca_delkey(struct ca *ca, char *keyname)
> int
> ca_request(struct ca *ca, char *keyname, int type)
> {
> - charcmd[PATH_MAX * 2];
> charhostname[HOST_NAME_MAX+1];
> charname[128];
> + charkey[PATH_MAX];
> charpath[PATH_MAX];
>
> ca_setenv("$ENV::CERT_CN", keyname);
> @@ -226,13 +225,12 @@ ca_request(struct ca *ca, char *keyname,
>
> ca_setcnf(ca, keyname);
>
> + snprintf(key, sizeof(key), "%s/private/%s.key", ca->sslpath, keyname);
> snprintf(path, sizeof(path), "%s/private/%s.csr", ca->sslpath, keyname);
> - snprintf(cmd, sizeof(cmd), "%s req %s-new"
> - " -key %s/private/%s.key -out %s -config %s",
> - PATH_OPENSSL, ca->batch, ca->sslpath, keyname,
> - path, ca->sslcnf);
>
> - system(cmd);
> + char *cmd[] = { PATH_OPENSSL, "req", "-new", "-key", key, "-out", path,
> + "-config", ca->sslcnf, ca->batch, NULL };
> + ca_exec(cmd);
> chmod(path, 0600);
>
> return (0);
> @@ -241,8 +239,11 @@ ca_request(struct ca *ca, char *keyname,
> int
> ca_sign(struct ca *ca, char *keyname, int type)
> {
> - charcmd[PATH_MAX * 2];
> - const char *extensions = NULL;
> + charcakey[PATH_MAX];
> + charcacrt[PATH_MAX];
> + charout[PATH_MAX];
>
Re: ftpd(8): rm dead code and simplifies popen clone
Jan Klemkow wrote: > - * Special version of popen which avoids call to shell. This ensures noone > + * Special version of popen which avoids call to shell. This ensures none If we don't like noone, the correct spelling is no one. Rest of the diff seems like a good improvement.
Re: Avoid system(3) in ikectl
Ted Unangst wrote: > ca_exec or ca_system I could go either way, but there's no longer sh involved, > so that's why I went back to exec. Even more accurate to name it ca_execv(), since this is a argv[] interface.
Re: relayd websocket
ok benno@
Reyk Floeter(r...@openbsd.org) on 2019.05.08 20:35:46 +0200:
> On Wed, May 08, 2019 at 07:07:43PM +0200, Reyk Floeter wrote:
> > On Wed, May 08, 2019 at 06:26:45PM +0200, Reyk Floeter wrote:
> > > On Wed, Mar 06, 2019 at 05:36:32PM +0100, Sebastian Benoit wrote:
> > > > Rivo Nurges(rivo.nur...@smit.ee) on 2019.03.05 22:42:13 +:
> > > > > Hi!
> > > > >
> > > > > On 3/5/19 10:36 PM, Claudio Jeker wrote:
> > > > > > I guess that this would need strcasestr() instead of strcasecmp(),
> > > > > > since you
> > > > > > are looking for the substring "Upgrade" in value. Maybe more is
> > > > > > needed if
> > > > > > we want to be sure that 'Connection: Upgrade-maybe' does not match.
> > > > >
> > > > > You are correct about strcasestr. "Connection: Upgrade-maybe" would
> > > > > need
> > > > > to have correct "Upgrade: websocket". Anyway, lets be strict.
> > > > >
> > > > > Does something like this make sense?
> > > >
> > > > i think the seperator list needs to include '\t'
> > > > because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB.
> > > >
> > > > And i dont think you can mix "," with " \t" seperators,
> > > > because otherwise "Foo Upgrade, Bar" will match.
> > > >
> > > > Something more is needed to parse elements of a header.
> > > >
> > >
> > > I would reshuffle the websocket handling a bit as I don't think that
> > > we need the http priv struct. We can lookup the parsed headers later.
> > >
> > > The attached diff moves it all into one places and introduces a new
> > > function kv_find_value() that is able to to match headers with
> > > multiple values (I think we might need it elsewhere. And even if not,
> > > I would prefer to have this in a function intead of stuffing it into
> > > relay_read_http).
> > >
> > > A minor question is if the lookup should be done before or after
> > > applying the filters (relay_test - my diff does it afterwards, the
> > > current code does it before).
> > >
> > > Tests? Comments? Ok?
> > >
> >
> > I keep on replying to my own diffs... the updated one below uses
> > strcasecmp instead of strcasestr in kv_find_value(). There is no need
> > for substring- or fn- matching in this function.
> >
>
> Next one...
>
> benno@ pointed out that the RFC allows whitespace before the comma.
> We also don't have to strip \r\n as it is done by relayd's kv parser.
>
> OK?
>
> Reyk
>
> Index: usr.sbin/relayd/http.h
> ===
> RCS file: /cvs/src/usr.sbin/relayd/http.h,v
> retrieving revision 1.10
> diff -u -p -u -p -r1.10 http.h
> --- usr.sbin/relayd/http.h4 Mar 2019 21:25:03 - 1.10
> +++ usr.sbin/relayd/http.h8 May 2019 18:34:09 -
> @@ -251,10 +251,4 @@ struct http_descriptor {
> struct kvtreehttp_headers;
> };
>
> -struct relay_http_priv {
> -#define HTTP_CONNECTION_UPGRADE 0x01
> -#define HTTP_UPGRADE_WEBSOCKET 0x02
> - int http_upgrade_req;
> -};
> -
> #endif /* HTTP_H */
> Index: usr.sbin/relayd/relay.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
> retrieving revision 1.242
> diff -u -p -u -p -r1.242 relay.c
> --- usr.sbin/relayd/relay.c 4 Mar 2019 21:25:03 - 1.242
> +++ usr.sbin/relayd/relay.c 8 May 2019 18:34:09 -
> @@ -1410,13 +1410,7 @@ relay_session(struct rsession *con)
> return;
> }
>
> - if (rlay->rl_proto->type == RELAY_PROTO_HTTP) {
> - if (relay_http_priv_init(con) == -1) {
> - relay_close(con,
> - "failed to allocate http session data", 1);
> - return;
> - }
> - } else {
> + if (rlay->rl_proto->type != RELAY_PROTO_HTTP) {
> if (rlay->rl_conf.fwdmode == FWD_TRANS)
> relay_bindanyreq(con, 0, IPPROTO_TCP);
> else if (relay_connect(con) == -1) {
> Index: usr.sbin/relayd/relay_http.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
> retrieving revision 1.72
> diff -u -p -u -p -r1.72 relay_http.c
> --- usr.sbin/relayd/relay_http.c 4 Mar 2019 21:25:03 - 1.72
> +++ usr.sbin/relayd/relay_http.c 8 May 2019 18:34:09 -
> @@ -110,17 +110,6 @@ relay_http_init(struct relay *rlay)
> }
>
> int
> -relay_http_priv_init(struct rsession *con)
> -{
> - struct relay_http_priv *p;
> - if ((p = calloc(1, sizeof(struct relay_http_priv))) == NULL)
> - return (-1);
> -
> - con->se_priv = p;
> - return (0);
> -}
> -
> -int
> relay_httpdesc_init(struct ctl_relay_event *cre)
> {
> struct http_descriptor *desc;
> @@ -163,13 +152,13 @@ relay_read_http(struct bufferevent *bev,
> struct relay*rlay = con->se_relay;
> struct protocol *proto = rlay->rl_proto;
> struct
Re: Avoid system(3) in ikectl
Reyk Floeter wrote:
> On Wed, May 08, 2019 at 06:44:32PM -0400, Ted Unangst wrote:
> > Ted Unangst wrote:
> > > Matthew Martin wrote:
> > > > I did that originally [1], but Reyk preferred the varargs approach [2],
> > > > so I changed the patch to match.
> > >
> > > Sorry, only wading into the thread at this point. Seems not everybody has
> > > the
> > > same taste in code... Well, we have the original. Let me bring that back.
> >
> > OK, here's diff one, but with run() renamed to ca_exec(). I think picking a
> > better name was at least one improvement. :)
> >
> > And the file: comment added.
> >
>
> His first diff included other things that got cleaned up, please don't
> revert to this one (eg. the // comment).
I looked at history to pick that up. Anything else?
I think returning an error code is the right thing, even if not checked. A
void function means can't fail, not fails silently.
ca_exec or ca_system I could go either way, but there's no longer sh involved,
so that's why I went back to exec.
> If you want to switch to an array instead of varargs, fine. But the
> definition of char *cmd[] somewhere in the function, especially in an
> extra {} block, looks very ugly to me. That's why I suggested the
> varargs in the first place to avoid such a thing.
I do think it's less ugly without the extra {}. Here's a version with less of
that.
Index: ikeca.c
===
RCS file: /home/cvs/src/usr.sbin/ikectl/ikeca.c,v
retrieving revision 1.48
diff -u -p -r1.48 ikeca.c
--- ikeca.c 26 Feb 2019 14:21:30 - 1.48
+++ ikeca.c 8 May 2019 22:59:09 -
@@ -18,6 +18,7 @@
#include
#include
+#include
#include
#include
#include
@@ -63,12 +64,12 @@
struct ca {
char sslpath[PATH_MAX];
- char passfile[PATH_MAX];
+ char passfile[PATH_MAX + 5]; // Includes the "file:" prefix
char index[PATH_MAX];
char serial[PATH_MAX];
char sslcnf[PATH_MAX];
char extcnf[PATH_MAX];
- char batch[PATH_MAX];
+ char*batch;
char*caname;
};
@@ -116,6 +117,7 @@ void ca_setenv(const char *, const cha
voidca_clrenv(void);
voidca_setcnf(struct ca *, const char *);
voidca_create_index(struct ca *);
+int static ca_exec(char *const []);
/* util.c */
int expand_string(char *, size_t, const char *, const char *);
@@ -130,7 +132,6 @@ int
ca_key_create(struct ca *ca, char *keyname)
{
struct stat st;
- char cmd[PATH_MAX * 2];
char path[PATH_MAX];
snprintf(path, sizeof(path), "%s/private/%s.key", ca->sslpath, keyname);
@@ -140,10 +141,8 @@ ca_key_create(struct ca *ca, char *keyna
return (0);
}
- snprintf(cmd, sizeof(cmd),
- "%s genrsa -out %s 2048",
- PATH_OPENSSL, path);
- system(cmd);
+ char *cmd[] = { PATH_OPENSSL, "genrsa", "-out", path, "2048", NULL };
+ ca_exec(cmd);
chmod(path, 0600);
return (0);
@@ -200,9 +199,9 @@ ca_delkey(struct ca *ca, char *keyname)
int
ca_request(struct ca *ca, char *keyname, int type)
{
- charcmd[PATH_MAX * 2];
charhostname[HOST_NAME_MAX+1];
charname[128];
+ charkey[PATH_MAX];
charpath[PATH_MAX];
ca_setenv("$ENV::CERT_CN", keyname);
@@ -226,13 +225,12 @@ ca_request(struct ca *ca, char *keyname,
ca_setcnf(ca, keyname);
+ snprintf(key, sizeof(key), "%s/private/%s.key", ca->sslpath, keyname);
snprintf(path, sizeof(path), "%s/private/%s.csr", ca->sslpath, keyname);
- snprintf(cmd, sizeof(cmd), "%s req %s-new"
- " -key %s/private/%s.key -out %s -config %s",
- PATH_OPENSSL, ca->batch, ca->sslpath, keyname,
- path, ca->sslcnf);
- system(cmd);
+ char *cmd[] = { PATH_OPENSSL, "req", "-new", "-key", key, "-out", path,
+ "-config", ca->sslcnf, ca->batch, NULL };
+ ca_exec(cmd);
chmod(path, 0600);
return (0);
@@ -241,8 +239,11 @@ ca_request(struct ca *ca, char *keyname,
int
ca_sign(struct ca *ca, char *keyname, int type)
{
- charcmd[PATH_MAX * 2];
- const char *extensions = NULL;
+ charcakey[PATH_MAX];
+ charcacrt[PATH_MAX];
+ charout[PATH_MAX];
+ charin[PATH_MAX];
+ char*extensions = NULL;
if (type == HOST_IPADDR) {
extensions = "x509v3_IPAddr";
@@ -258,19 +259,16 @@ ca_sign(struct ca *ca, char *keyname, in
ca_setenv("$ENV::CASERIAL", ca->serial);
ca_setcnf(ca, keyname);
- snprintf(cmd, sizeof(cmd),
- "%s ca
ftpd(8): rm dead code and simplifies popen clone
Hi,
This diff removes dead code from ftpd(8). In the past ftpd executes
ls(1) for directory listings. But, quite a while now It justs calls the
main function of the linked in ls(1) directly. The code path to the
main function of ls(1) as well as the read-only mode are hard coded in
the current version of ftpd. All callers of ftpd_popen() calls it with
"/bin/ls" as program path. The diff below renames ftpd_popen() to
ftpd_ls() and simplifies the function. Also, the manpage section to
"~ftp/bin" is removed, because ftpd is not able to exec other programs.
I don't know if its usual?! But, I used an enum to signal file transfer
or listing for the retrieve function. The code seems to be more
readable for for me.
bye,
Jan
Index: extern.h
===
RCS file: /cvs/src/libexec/ftpd/extern.h,v
retrieving revision 1.19
diff -u -p -r1.19 extern.h
--- extern.h4 Oct 2015 11:58:09 - 1.19
+++ extern.h8 May 2019 20:02:23 -
@@ -68,7 +68,7 @@ void delete(char *);
void dologout(int);
void fatal(char *);
intftpd_pclose(FILE *, pid_t);
-FILE *ftpd_popen(char *, char *, pid_t *);
+FILE *ftpd_ls(char *, char *, pid_t *);
int get_line(char *, int, FILE *);
void ftpdlogwtmp(char *, char *, char *);
void lreply(int, const char *, ...);
@@ -89,7 +89,8 @@ void renamecmd(char *, char *);
char *renamefrom(char *);
void reply(int, const char *, ...);
void reply_r(int, const char *, ...);
-void retrieve(char *, char *);
+enum ret_cmd { RET_FILE, RET_LIST };
+void retrieve(enum ret_cmd, char *);
void send_file_list(char *);
void setproctitle(const char *, ...);
void statcmd(void);
Index: ftpcmd.y
===
RCS file: /cvs/src/libexec/ftpd/ftpcmd.y,v
retrieving revision 1.66
diff -u -p -r1.66 ftpcmd.y
--- ftpcmd.y27 Apr 2017 13:30:54 - 1.66
+++ ftpcmd.y8 May 2019 20:03:29 -
@@ -342,7 +342,7 @@ cmd
| RETR check_login SP pathname CRLF
{
if ($2 && $4 != NULL)
- retrieve(NULL, $4);
+ retrieve(RET_FILE, $4);
if ($4 != NULL)
free($4);
}
@@ -374,12 +374,12 @@ cmd
| LIST check_login CRLF
{
if ($2)
- retrieve("/bin/ls -lgA", "");
+ retrieve(RET_LIST, NULL);
}
| LIST check_login SP pathname CRLF
{
if ($2 && $4 != NULL)
- retrieve("/bin/ls -lgA %s", $4);
+ retrieve(RET_LIST, $4);
if ($4 != NULL)
free($4);
}
Index: ftpd.8
===
RCS file: /cvs/src/libexec/ftpd/ftpd.8,v
retrieving revision 1.75
diff -u -p -r1.75 ftpd.8
--- ftpd.8 25 Oct 2015 23:10:53 - 1.75
+++ ftpd.8 8 May 2019 20:08:15 -
@@ -385,15 +385,6 @@ subtree be constructed with care, follow
Make the home directory owned by
.Dq root
and unwritable by anyone (mode 555).
-.It Pa ~ftp/bin
-Make this directory owned by
-.Dq root
-and unwritable by anyone (mode 511).
-This directory is optional unless you have commands you wish
-the anonymous FTP user to be able to run (the
-.Xr ls 1
-command exists as a built-in).
-Any programs in this directory should be mode 111 (executable only).
.It Pa ~ftp/etc
Make this directory owned by
.Dq root
Index: ftpd.c
===
RCS file: /cvs/src/libexec/ftpd/ftpd.c,v
retrieving revision 1.225
diff -u -p -r1.225 ftpd.c
--- ftpd.c 11 Dec 2018 18:19:55 - 1.225
+++ ftpd.c 8 May 2019 22:45:07 -
@@ -1113,36 +1113,32 @@ bad:
}
void
-retrieve(char *cmd, char *name)
+retrieve(enum ret_cmd cmd, char *name)
{
FILE *fin, *dout;
struct stat st;
pid_t pid;
time_t start;
- if (cmd == NULL) {
+ if (cmd == RET_FILE) {
fin = fopen(name, "r");
st.st_size = 0;
} else {
- char line[BUFSIZ];
-
- (void) snprintf(line, sizeof(line), cmd, name);
- name = line;
- fin = ftpd_popen(line, "r", );
+ fin = ftpd_ls("-lgA", name, );
st.st_size = -1;
st.st_blksize = BUFSIZ;
}
if (fin == NULL) {
if (errno != 0) {
perror_reply(550, name);
- if (cmd == NULL) {
+ if (cmd == RET_FILE) {
LOGCMD("get", name);
}
}
return;
}
byte_count = -1;
- if (cmd ==
Re: Avoid system(3) in ikectl
On Wed, May 08, 2019 at 06:44:32PM -0400, Ted Unangst wrote:
> Ted Unangst wrote:
> > Matthew Martin wrote:
> > > I did that originally [1], but Reyk preferred the varargs approach [2],
> > > so I changed the patch to match.
> >
> > Sorry, only wading into the thread at this point. Seems not everybody has
> > the
> > same taste in code... Well, we have the original. Let me bring that back.
>
> OK, here's diff one, but with run() renamed to ca_exec(). I think picking a
> better name was at least one improvement. :)
>
> And the file: comment added.
>
His first diff included other things that got cleaned up, please don't
revert to this one (eg. the // comment).
If you want to switch to an array instead of varargs, fine. But the
definition of char *cmd[] somewhere in the function, especially in an
extra {} block, looks very ugly to me. That's why I suggested the
varargs in the first place to avoid such a thing.
And we settled on renaming run() to ca_system() instead of
ca_exec()... but that is just BS.
I'm not ok with this diff, if this has any weight at all.
Reyk
> Index: ikeca.c
> ===
> RCS file: /home/cvs/src/usr.sbin/ikectl/ikeca.c,v
> retrieving revision 1.48
> diff -u -p -r1.48 ikeca.c
> --- ikeca.c 26 Feb 2019 14:21:30 - 1.48
> +++ ikeca.c 8 May 2019 22:41:08 -
> @@ -18,6 +18,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -63,12 +64,12 @@
>
> struct ca {
> char sslpath[PATH_MAX];
> - char passfile[PATH_MAX];
> + char passfile[PATH_MAX + 5]; // Includes the "file:" prefix
> char index[PATH_MAX];
> char serial[PATH_MAX];
> char sslcnf[PATH_MAX];
> char extcnf[PATH_MAX];
> - char batch[PATH_MAX];
> + char*batch;
> char*caname;
> };
>
> @@ -116,6 +117,7 @@ void ca_setenv(const char *, const cha
> void ca_clrenv(void);
> void ca_setcnf(struct ca *, const char *);
> void ca_create_index(struct ca *);
> +int staticca_exec(char *const []);
>
> /* util.c */
> int expand_string(char *, size_t, const char *, const char *);
> @@ -130,7 +132,6 @@ int
> ca_key_create(struct ca *ca, char *keyname)
> {
> struct stat st;
> - char cmd[PATH_MAX * 2];
> char path[PATH_MAX];
>
> snprintf(path, sizeof(path), "%s/private/%s.key", ca->sslpath, keyname);
> @@ -140,10 +141,8 @@ ca_key_create(struct ca *ca, char *keyna
> return (0);
> }
>
> - snprintf(cmd, sizeof(cmd),
> - "%s genrsa -out %s 2048",
> - PATH_OPENSSL, path);
> - system(cmd);
> + char *cmd[] = { PATH_OPENSSL, "genrsa", "-out", path, "2048", NULL };
> + ca_exec(cmd);
> chmod(path, 0600);
>
> return (0);
> @@ -200,9 +199,9 @@ ca_delkey(struct ca *ca, char *keyname)
> int
> ca_request(struct ca *ca, char *keyname, int type)
> {
> - charcmd[PATH_MAX * 2];
> charhostname[HOST_NAME_MAX+1];
> charname[128];
> + charkey[PATH_MAX];
> charpath[PATH_MAX];
>
> ca_setenv("$ENV::CERT_CN", keyname);
> @@ -226,13 +225,12 @@ ca_request(struct ca *ca, char *keyname,
>
> ca_setcnf(ca, keyname);
>
> + snprintf(key, sizeof(key), "%s/private/%s.key", ca->sslpath, keyname);
> snprintf(path, sizeof(path), "%s/private/%s.csr", ca->sslpath, keyname);
> - snprintf(cmd, sizeof(cmd), "%s req %s-new"
> - " -key %s/private/%s.key -out %s -config %s",
> - PATH_OPENSSL, ca->batch, ca->sslpath, keyname,
> - path, ca->sslcnf);
>
> - system(cmd);
> + char *cmd[] = { PATH_OPENSSL, "req", "-new", "-key", key, "-out", path,
> + "-config", ca->sslcnf, ca->batch, NULL };
> + ca_exec(cmd);
> chmod(path, 0600);
>
> return (0);
> @@ -241,8 +239,11 @@ ca_request(struct ca *ca, char *keyname,
> int
> ca_sign(struct ca *ca, char *keyname, int type)
> {
> - charcmd[PATH_MAX * 2];
> - const char *extensions = NULL;
> + charcakey[PATH_MAX];
> + charcacrt[PATH_MAX];
> + charout[PATH_MAX];
> + charin[PATH_MAX];
> + char*extensions = NULL;
>
> if (type == HOST_IPADDR) {
> extensions = "x509v3_IPAddr";
> @@ -258,19 +259,16 @@ ca_sign(struct ca *ca, char *keyname, in
> ca_setenv("$ENV::CASERIAL", ca->serial);
> ca_setcnf(ca, keyname);
>
> - snprintf(cmd, sizeof(cmd),
> - "%s ca -config %s -keyfile %s/private/ca.key"
> - " -cert %s/ca.crt"
> - " -extfile %s -extensions %s -out %s/%s.crt"
> - " -in %s/private/%s.csr"
> - " -passin file:%s
Re: Avoid system(3) in ikectl
This feels approximately right to me.
Re: iwm: move mbuf hacks down in iwm_rx_rx_mpdu()
On Thu, May 09, 2019 at 08:31:29AM +1000, Jonathan Matthew wrote: > On Wed, May 08, 2019 at 03:01:05PM -0400, Stefan Sperling wrote: > > Don't write to mbuf memory before we've actually completed all > > sanity checks and iwm_rx_addbuf() has succcessfully put a new > > buffer on the ring. > > > > Same as dragonfly commit 96eaecf93d9f731459a0df8efc72cfad034320bd > > "Avoids leaving the mbuf in a weird state when dropping a packet." > > This doesn't look right to me. Doesn't iwm_rx_addbuf() replace data->m > with a new mbuf? In dragonfly, iwm_mvm_rx_rx_mpdu() already has the mbuf > in a parameter called 'm', so it doesn't do quite the same thing. > Indeed, thanks! I'll revisit this.
Re: Avoid system(3) in ikectl
Ted Unangst wrote:
> Matthew Martin wrote:
> > I did that originally [1], but Reyk preferred the varargs approach [2],
> > so I changed the patch to match.
>
> Sorry, only wading into the thread at this point. Seems not everybody has the
> same taste in code... Well, we have the original. Let me bring that back.
OK, here's diff one, but with run() renamed to ca_exec(). I think picking a
better name was at least one improvement. :)
And the file: comment added.
Index: ikeca.c
===
RCS file: /home/cvs/src/usr.sbin/ikectl/ikeca.c,v
retrieving revision 1.48
diff -u -p -r1.48 ikeca.c
--- ikeca.c 26 Feb 2019 14:21:30 - 1.48
+++ ikeca.c 8 May 2019 22:41:08 -
@@ -18,6 +18,7 @@
#include
#include
+#include
#include
#include
#include
@@ -63,12 +64,12 @@
struct ca {
char sslpath[PATH_MAX];
- char passfile[PATH_MAX];
+ char passfile[PATH_MAX + 5]; // Includes the "file:" prefix
char index[PATH_MAX];
char serial[PATH_MAX];
char sslcnf[PATH_MAX];
char extcnf[PATH_MAX];
- char batch[PATH_MAX];
+ char*batch;
char*caname;
};
@@ -116,6 +117,7 @@ void ca_setenv(const char *, const cha
voidca_clrenv(void);
voidca_setcnf(struct ca *, const char *);
voidca_create_index(struct ca *);
+int static ca_exec(char *const []);
/* util.c */
int expand_string(char *, size_t, const char *, const char *);
@@ -130,7 +132,6 @@ int
ca_key_create(struct ca *ca, char *keyname)
{
struct stat st;
- char cmd[PATH_MAX * 2];
char path[PATH_MAX];
snprintf(path, sizeof(path), "%s/private/%s.key", ca->sslpath, keyname);
@@ -140,10 +141,8 @@ ca_key_create(struct ca *ca, char *keyna
return (0);
}
- snprintf(cmd, sizeof(cmd),
- "%s genrsa -out %s 2048",
- PATH_OPENSSL, path);
- system(cmd);
+ char *cmd[] = { PATH_OPENSSL, "genrsa", "-out", path, "2048", NULL };
+ ca_exec(cmd);
chmod(path, 0600);
return (0);
@@ -200,9 +199,9 @@ ca_delkey(struct ca *ca, char *keyname)
int
ca_request(struct ca *ca, char *keyname, int type)
{
- charcmd[PATH_MAX * 2];
charhostname[HOST_NAME_MAX+1];
charname[128];
+ charkey[PATH_MAX];
charpath[PATH_MAX];
ca_setenv("$ENV::CERT_CN", keyname);
@@ -226,13 +225,12 @@ ca_request(struct ca *ca, char *keyname,
ca_setcnf(ca, keyname);
+ snprintf(key, sizeof(key), "%s/private/%s.key", ca->sslpath, keyname);
snprintf(path, sizeof(path), "%s/private/%s.csr", ca->sslpath, keyname);
- snprintf(cmd, sizeof(cmd), "%s req %s-new"
- " -key %s/private/%s.key -out %s -config %s",
- PATH_OPENSSL, ca->batch, ca->sslpath, keyname,
- path, ca->sslcnf);
- system(cmd);
+ char *cmd[] = { PATH_OPENSSL, "req", "-new", "-key", key, "-out", path,
+ "-config", ca->sslcnf, ca->batch, NULL };
+ ca_exec(cmd);
chmod(path, 0600);
return (0);
@@ -241,8 +239,11 @@ ca_request(struct ca *ca, char *keyname,
int
ca_sign(struct ca *ca, char *keyname, int type)
{
- charcmd[PATH_MAX * 2];
- const char *extensions = NULL;
+ charcakey[PATH_MAX];
+ charcacrt[PATH_MAX];
+ charout[PATH_MAX];
+ charin[PATH_MAX];
+ char*extensions = NULL;
if (type == HOST_IPADDR) {
extensions = "x509v3_IPAddr";
@@ -258,19 +259,16 @@ ca_sign(struct ca *ca, char *keyname, in
ca_setenv("$ENV::CASERIAL", ca->serial);
ca_setcnf(ca, keyname);
- snprintf(cmd, sizeof(cmd),
- "%s ca -config %s -keyfile %s/private/ca.key"
- " -cert %s/ca.crt"
- " -extfile %s -extensions %s -out %s/%s.crt"
- " -in %s/private/%s.csr"
- " -passin file:%s -outdir %s -batch",
- PATH_OPENSSL, ca->sslcnf, ca->sslpath,
- ca->sslpath,
- ca->extcnf, extensions, ca->sslpath, keyname,
- ca->sslpath, keyname,
- ca->passfile, ca->sslpath);
-
- system(cmd);
+ snprintf(cakey, sizeof(cakey), "%s/private/ca.key", ca->sslpath);
+ snprintf(cacrt, sizeof(cacrt), "%s/ca.crt", ca->sslpath);
+ snprintf(out, sizeof(out), "%s/%s.crt", ca->sslpath, keyname);
+ snprintf(in, sizeof(in), "%s/private/%s.csr", ca->sslpath, keyname);
+
+ char *cmd[] = { PATH_OPENSSL, "ca", "-config", ca->sslcnf,
+ "-keyfile", cakey, "-cert", cacrt, "-extfile", ca->extcnf,
+ "-extensions",
Re: Avoid system(3) in ikectl
Matthew Martin wrote: > I did that originally [1], but Reyk preferred the varargs approach [2], > so I changed the patch to match. Sorry, only wading into the thread at this point. Seems not everybody has the same taste in code... Well, we have the original. Let me bring that back.
Re: iwm: move mbuf hacks down in iwm_rx_rx_mpdu()
On Wed, May 08, 2019 at 03:01:05PM -0400, Stefan Sperling wrote: > Don't write to mbuf memory before we've actually completed all > sanity checks and iwm_rx_addbuf() has succcessfully put a new > buffer on the ring. > > Same as dragonfly commit 96eaecf93d9f731459a0df8efc72cfad034320bd > "Avoids leaving the mbuf in a weird state when dropping a packet." This doesn't look right to me. Doesn't iwm_rx_addbuf() replace data->m with a new mbuf? In dragonfly, iwm_mvm_rx_rx_mpdu() already has the mbuf in a parameter called 'm', so it doesn't do quite the same thing. > > ok? > > diff 74c6ffce73f932dd8464cc1def1ee4fcaa3e671f /usr/src > blob - 335033d21091a23511403804f09d1548b109b104 > file + sys/dev/pci/if_iwm.c > --- sys/dev/pci/if_iwm.c > +++ sys/dev/pci/if_iwm.c > @@ -3465,10 +3465,6 @@ iwm_rx_rx_mpdu(struct iwm_softc *sc, struct iwm_rx_pac > rx_pkt_status = le32toh(*(uint32_t *)(pkt->data + > sizeof(*rx_res) + len)); > > - m = data->m; > - m->m_data = pkt->data + sizeof(*rx_res); > - m->m_pkthdr.len = m->m_len = len; > - > if (__predict_false(phy_info->cfg_phy_cnt > 20)) > return; > > @@ -3488,6 +3484,10 @@ iwm_rx_rx_mpdu(struct iwm_softc *sc, struct iwm_rx_pac > > if (iwm_rx_addbuf(sc, IWM_RBUF_SIZE, sc->rxq.cur) != 0) > return; > + > + m = data->m; > + m->m_data = pkt->data + sizeof(*rx_res); > + m->m_pkthdr.len = m->m_len = len; > > chanidx = letoh32(phy_info->channel); > if (chanidx < 0 || chanidx >= nitems(ic->ic_channels)) >
Re: Avoid system(3) in ikectl
Matthew Martin wrote: > On Wed, May 08, 2019 at 04:22:16PM -0600, Theo de Raadt wrote: > > Isn't something like better -- to avoid marshalling code to convert > > arguments -> array? > > > > char *pkcs_args[] = > > PATH_OPENSSL, > > "pkcs12", > > "-export", > > "-caname", > > ca->caname, > > "-name", > > ca->caname, > > "-cacerts", > > "-nokeys", > > "-in", > > cacrt, > > "-out", > > capfx, > > "-passout", > > "env:EXPASS", > > "-passin", > > ca->passfile, > > NULL > > }; > > > > ca_system(pkcs_args); > > I did that originally [1], but Reyk preferred the varargs approach [2], > so I changed the patch to match. Your first mistake was to listen to Reyk
Re: Avoid system(3) in ikectl
On Wed, May 08, 2019 at 04:22:16PM -0600, Theo de Raadt wrote: > Isn't something like better -- to avoid marshalling code to convert > arguments -> array? > > char *pkcs_args[] = > PATH_OPENSSL, > "pkcs12", > "-export", > "-caname", > ca->caname, > "-name", > ca->caname, > "-cacerts", > "-nokeys", > "-in", > cacrt, > "-out", > capfx, > "-passout", > "env:EXPASS", > "-passin", > ca->passfile, > NULL > }; > > ca_system(pkcs_args); I did that originally [1], but Reyk preferred the varargs approach [2], so I changed the patch to match. 1: https://marc.info/?l=openbsd-tech=155193386415623=2 2: https://marc.info/?l=openbsd-tech=155203806116489=2
Re: Avoid system(3) in ikectl
Ted Unangst wrote: > Theo de Raadt wrote: > > Isn't something like better -- to avoid marshalling code to convert > > arguments -> array? > > this requires mixing declarations and code, but all our compilers are c99 > compliant now, and this does make ca_system simpler. I dislike mixing decl and code, but this is a special case where I hate the marshalling code more.
Re: Avoid system(3) in ikectl
Theo de Raadt wrote: > Isn't something like better -- to avoid marshalling code to convert > arguments -> array? this requires mixing declarations and code, but all our compilers are c99 compliant now, and this does make ca_system simpler. > > char *pkcs_args[] = > PATH_OPENSSL, > "pkcs12", > "-export", > "-caname", > ca->caname, > "-name", > ca->caname, > "-cacerts", > "-nokeys", > "-in", > cacrt, > "-out", > capfx, > "-passout", > "env:EXPASS", > "-passin", > ca->passfile, > NULL > }; > > ca_system(pkcs_args); >
Re: Avoid system(3) in ikectl
Ted Unangst wrote:
> Matthew Martin wrote:
> > ping
> >
> > On Thu, Apr 25, 2019 at 11:21:00PM -0500, Matthew Martin wrote:
> > > On Thu, Apr 25, 2019 at 08:59:56PM -0600, Theo de Raadt wrote:
> > > > > + argv = alloca((n + 1) * sizeof(*argv));
> > > >
> > > > Our source tree is exceedingly sparing in the use of alloca().
> > > > This will not do.
> > >
> > > Was staying as close as possible to exec.c, but avoiding alloca is
> > > preferable; replaced with reallocarray. err message is "reallocarray" to
> > > match style with the calloc call in the same file.
> > >
> > >
> > - if (keyname != NULL) {
> > - snprintf(cmd, sizeof(cmd), "env EXPASS=%s %s pkcs12 -export"
> > - " -name %s -CAfile %s/ca.crt -inkey %s/private/%s.key"
> > - " -in %s/%s.crt -out %s/private/%s.pfx -passout env:EXPASS"
> > - " -passin file:%s", pass, PATH_OPENSSL, keyname,
> > - ca->sslpath, ca->sslpath, keyname, ca->sslpath, keyname,
> > - ca->sslpath, oname, ca->passfile);
> > - system(cmd);
> > - }
> > -
> > - snprintf(cmd, sizeof(cmd), "env EXPASS=%s %s pkcs12 -export"
> > - " -caname '%s' -name '%s' -cacerts -nokeys"
> > - " -in %s/ca.crt -out %s/ca.pfx -passout env:EXPASS -passin file:%s",
> > - pass, PATH_OPENSSL, ca->caname, ca->caname, ca->sslpath,
> > - ca->sslpath, ca->passfile);
> > - system(cmd);
> > + snprintf(cacrt, sizeof(cacrt), "%s/ca.crt", ca->sslpath);
> > + snprintf(capfx, sizeof(capfx), "%s/ca.pfx", ca->sslpath);
> > + snprintf(key, sizeof(key), "%s/private/%s.key", ca->sslpath, keyname);
> > + snprintf(crt, sizeof(crt), "%s/%s.crt", ca->sslpath, keyname);
> > + snprintf(pfx, sizeof(pfx), "%s/private/%s.pfx", ca->sslpath, oname);
> > +
> > + snprintf(passenv, sizeof(passenv), "EXPASS=%s", pass);
> > + putenv(passenv);
> > +
> > + if (keyname != NULL)
> > + ca_system(PATH_OPENSSL, "pkcs12", "-export", "-name", keyname,
> > + "-CAfile", cacrt, "-inkey", key, "-in", crt, "-out", pfx,
> > + "-passout", "env:EXPASS", "-passin", ca->passfile, NULL);
> > +
> > + ca_system(PATH_OPENSSL, "pkcs12", "-export", "-caname", ca->caname,
> > + "-name", ca->caname, "-cacerts", "-nokeys", "-in", cacrt,
> > + "-out", capfx, "-passout", "env:EXPASS", "-passin", ca->passfile,
> > + NULL);
> > +
> > + unsetenv("EXPASS");
> > + explicit_bzero(passenv, sizeof(passenv));
>
> This looks weird, but it's still probably an improvement over the original
> code, which is trying to avoid leaking password via argv but only mostly
> succeeds.
>
> There's probably a better way to do environment handling, but I think this
> diff is better than using system() now. ok with me.
Isn't something like better -- to avoid marshalling code to convert
arguments -> array?
char *pkcs_args[] =
PATH_OPENSSL,
"pkcs12",
"-export",
"-caname",
ca->caname,
"-name",
ca->caname,
"-cacerts",
"-nokeys",
"-in",
cacrt,
"-out",
capfx,
"-passout",
"env:EXPASS",
"-passin",
ca->passfile,
NULL
};
ca_system(pkcs_args);
Re: httpd rewrite support and REQUEST_URI
On Tue, Jul 31, 2018 at 06:27:39AM -0500, Tim Baumgard wrote: > I was testing the new rewrite support in httpd in conjunction with the > "fastcgi" > option and noticed that the REQUEST_URI CGI variable is set to the rewritten > path and query string instead of the requested URI and query string. The patch > below mimics what happens with http_descriptor->http_path_alias to change > this. > > I also noticed that $QUERY_STRING macro isn't being encoded when it gets > expanded during a rewrite. I saw the recent commits making changes to this > macro, so I thought I'd mention it but leave it alone. Mischa reminded us of your diffs: https://raw.githubusercontent.com/tbaumgard/openbsd-httpd-rewrite/master/openbsd-httpd-rewrite-current.patch They are now committed. Many thanks!
SMR lists for bridge(4)
Diff below pushes the KERNEL_LOCK() further down into bridge(4).
With it bridge_enqueue() now only takes the lock for rules. Rules could
easily be protected by a mutex but I wanted to keep this change small.
The list of interface and span interfaces are now using SMR list.
Removing a `bif' from such list now calls smr_barrier() before tearing
down the object and freeing the memory. That means we might call
if_enqueue() on an interface while it is being removed/destroyed.
This is OK because if_detach() will purge the queue.
If my use of SMR list is correct I'll convert bridge_input() next.
Tests, reviews and oks welcome!
Index: net/bridgectl.c
===
RCS file: /cvs/src/sys/net/bridgectl.c,v
retrieving revision 1.18
diff -u -p -r1.18 bridgectl.c
--- net/bridgectl.c 28 Apr 2019 22:15:57 - 1.18
+++ net/bridgectl.c 8 May 2019 15:31:18 -
@@ -725,6 +725,10 @@ bridge_filterrule(struct brl_head *h, st
struct brl_node *n;
u_int8_t flags;
+ if (SIMPLEQ_EMPTY(h))
+ return (BRL_ACTION_PASS);
+
+ KERNEL_LOCK();
SIMPLEQ_FOREACH(n, h, brl_next) {
if (!bridge_arpfilter(n, eh, m))
continue;
@@ -753,9 +757,11 @@ bridge_filterrule(struct brl_head *h, st
goto return_action;
}
}
+ KERNEL_UNLOCK();
return (BRL_ACTION_PASS);
return_action:
+ KERNEL_UNLOCK();
#if NPF > 0
pf_tag_packet(m, n->brl_tag, -1);
#endif
@@ -781,6 +787,9 @@ bridge_addrule(struct bridge_iflist *bif
else
n->brl_tag = 0;
#endif
+
+ KERNEL_ASSERT_LOCKED();
+
if (out) {
n->brl_flags &= ~BRL_FLAG_IN;
n->brl_flags |= BRL_FLAG_OUT;
@@ -797,6 +806,8 @@ void
bridge_flushrule(struct bridge_iflist *bif)
{
struct brl_node *p;
+
+ KERNEL_ASSERT_LOCKED();
while (!SIMPLEQ_EMPTY(>bif_brlin)) {
p = SIMPLEQ_FIRST(>bif_brlin);
Index: net/if_bridge.c
===
RCS file: /cvs/src/sys/net/if_bridge.c,v
retrieving revision 1.329
diff -u -p -r1.329 if_bridge.c
--- net/if_bridge.c 3 May 2019 16:53:07 - 1.329
+++ net/if_bridge.c 8 May 2019 20:59:01 -
@@ -137,7 +137,6 @@ int bridge_ipsec(struct ifnet *, struct
#endif
int bridge_clone_create(struct if_clone *, int);
intbridge_clone_destroy(struct ifnet *);
-intbridge_delete(struct bridge_softc *, struct bridge_iflist *);
#defineETHERADDR_IS_IP_MCAST(a) \
/* struct etheraddr *a; */ \
@@ -173,8 +172,8 @@ bridge_clone_create(struct if_clone *ifc
sc->sc_brtmax = BRIDGE_RTABLE_MAX;
sc->sc_brttimeout = BRIDGE_RTABLE_TIMEOUT;
timeout_set(>sc_brtimeout, bridge_rtage, sc);
- SLIST_INIT(>sc_iflist);
- SLIST_INIT(>sc_spanlist);
+ SMR_SLIST_INIT(>sc_iflist);
+ SMR_SLIST_INIT(>sc_spanlist);
mtx_init(>sc_mtx, IPL_MPFLOOR);
for (i = 0; i < BRIDGE_RTABLE_SIZE; i++)
LIST_INIT(>sc_rts[i]);
@@ -220,9 +219,9 @@ bridge_clone_destroy(struct ifnet *ifp)
bridge_stop(sc);
bridge_rtflush(sc, IFBF_FLUSHALL);
- while ((bif = SLIST_FIRST(>sc_iflist)) != NULL)
+ while ((bif = SMR_SLIST_FIRST_LOCKED(>sc_iflist)) != NULL)
bridge_ifremove(bif);
- while ((bif = SLIST_FIRST(>sc_spanlist)) != NULL)
+ while ((bif = SMR_SLIST_FIRST_LOCKED(>sc_spanlist)) != NULL)
bridge_spanremove(bif);
bstp_destroy(sc->sc_stp);
@@ -241,26 +240,6 @@ bridge_clone_destroy(struct ifnet *ifp)
}
int
-bridge_delete(struct bridge_softc *sc, struct bridge_iflist *bif)
-{
- int error;
-
- if (bif->bif_flags & IFBIF_STP)
- bstp_delete(bif->bif_stp);
-
- bif->ifp->if_bridgeidx = 0;
- error = ifpromisc(bif->ifp, 0);
- hook_disestablish(bif->ifp->if_detachhooks, bif->bif_dhcookie);
-
- if_ih_remove(bif->ifp, bridge_input, NULL);
- bridge_rtdelete(sc, bif->ifp, 0);
- bridge_flushrule(bif);
- free(bif, M_DEVBUF, sizeof *bif);
-
- return (error);
-}
-
-int
bridge_ioctl(struct ifnet *ifp, u_long cmd, caddr_t data)
{
struct bridge_softc *sc = (struct bridge_softc *)ifp->if_softc;
@@ -299,7 +278,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c
}
/* If it's in the span list, it can't be a member. */
- SLIST_FOREACH(bif, >sc_spanlist, bif_next) {
+ SMR_SLIST_FOREACH_LOCKED(bif, >sc_spanlist, bif_next) {
if (bif->ifp == ifs)
break;
}
@@ -334,7 +313,7 @@ bridge_ioctl(struct ifnet *ifp, u_long c
bif->bif_dhcookie = hook_establish(ifs->if_detachhooks, 0,
bridge_ifdetach, bif);
Re: update to PF pfctl(8) and pf.conf(5) manpages
On Wed, May 08, 2019 at 05:31:37PM +0200, Ingo Schwarze wrote: > Thank you again for doing this work. +1 > Either way, this seems mature enough for getting it in. > IMHO, no need to resend it again. I concur; Ingo's nits are fine. Feel free to go ahead and commit with OK kn.
Re: enable pfctl to flush all rules and tables
Hello,
diff 3/3 makes patch complete. It uses pfctl_recurse() introduced in
2/3 to implement operations as follows:
pfctl -a "*" -Fa(applies pfctl_call_clearanchors())
pfctl -a "*" -Fr(applies pfctl_call_clearrules())
pfctl -a "*" -FT(applies pfctl_call_cleartables())
All above use pfctl_recurse(), which calls desired fucntion
to execute requested operation.
thanks and
regards
sashan
8<---8<---8<--8<
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index cb71dd99a82..cb0adf742a3 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -65,7 +65,7 @@ intpfctl_disable(int, int);
voidpfctl_clear_queues(struct pf_qihead *);
voidpfctl_clear_stats(int, const char *, int);
voidpfctl_clear_interface_flags(int, int);
-voidpfctl_clear_rules(int, int, char *);
+int pfctl_clear_rules(int, int, char *);
voidpfctl_clear_src_nodes(int, int);
voidpfctl_clear_states(int, const char *, int);
struct addrinfo *
@@ -115,6 +115,9 @@ int pfctl_walk_anchors(int, int, char *, void *,
struct pfr_anchors *
pfctl_get_anchors(int, int);
intpfctl_recurse(int, int, int(*nuke)(int, int, struct pfr_anchoritem *));
+intpfctl_call_clearrules(int, int, struct pfr_anchoritem *);
+intpfctl_call_cleartables(int, int, struct pfr_anchoritem *);
+intpfctl_call_clearanchors(int, int, struct pfr_anchoritem *);
const char *clearopt;
char *rulesopt;
@@ -244,7 +247,6 @@ static const char *optiopt_list[] = {
struct pf_qihead qspecs = TAILQ_HEAD_INITIALIZER(qspecs);
struct pf_qihead rootqs = TAILQ_HEAD_INITIALIZER(rootqs);
-
__dead void
usage(void)
{
@@ -364,7 +366,7 @@ pfctl_clear_interface_flags(int dev, int opts)
int
pfctl_clear_rules(int dev, int opts, char *anchorname)
{
- struct pfr_buffer t;
+ struct pfr_buffer t;
int rv = 0;
memset(, 0, sizeof(t));
@@ -2289,6 +2291,30 @@ pfctl_get_anchors(int dev, int opts)
return ();
}
+int
+pfctl_call_cleartables(int dev, int opts, struct pfr_anchoritem *pfra)
+{
+ return ((pfctl_clear_tables(pfra->pfra_anchorname, opts) == -1) ?
+ 1 : 0);
+}
+
+int
+pfctl_call_clearrules(int dev, int opts, struct pfr_anchoritem *pfra)
+{
+ return (pfctl_clear_rules(dev, opts, pfra->pfra_anchorname));
+}
+
+int
+pfctl_call_clearanchors(int dev, int opts, struct pfr_anchoritem *pfra)
+{
+ int rv = 0;
+
+ rv |= pfctl_call_cleartables(dev, opts, pfra);
+ rv |= pfctl_clear_rules(dev, opts, pfra->pfra_anchorname);
+
+ return (rv);
+}
+
int
pfctl_recurse(int dev, int opts, int(*nuke)(int, int, struct pfr_anchoritem *))
{
@@ -2297,6 +2323,10 @@ pfctl_recurse(int dev, int opts, int(*nuke)(int, int,
struct pfr_anchoritem *))
struct pfr_anchoritem *pfra, *pfra_save;
anchors = pfctl_get_anchors(dev, opts);
+ /*
+* don't let pfctl_clear_*() function to fail with exit
+*/
+ opts |= PF_OPT_IGNFAIL;
SLIST_FOREACH_SAFE(pfra, anchors, pfra_sle, pfra_save) {
rv |= nuke(dev, opts, pfra);
SLIST_REMOVE(anchors, pfra, pfr_anchoritem, pfra_sle);
@@ -2733,7 +2763,11 @@ main(int argc, char *argv[])
if (clearopt != NULL) {
switch (*clearopt) {
case 'r':
- pfctl_clear_rules(dev, opts, anchorname);
+ if ((opts & PF_OPT_RECURSE) == 0)
+ pfctl_clear_rules(dev, opts, anchorname);
+ else
+ pfctl_recurse(dev, opts,
+ pfctl_call_clearrules);
break;
case 's':
pfctl_clear_states(dev, ifaceopt, opts);
@@ -2750,9 +2784,14 @@ main(int argc, char *argv[])
usage();
/* NOTREACHED */
}
- pfctl_clear_tables(anchorname, opts);
- pfctl_clear_rules(dev, opts, anchorname);
- if (!*anchorname) {
+ if ((opts & PF_OPT_RECURSE) == 0) {
+ pfctl_clear_tables(anchorname, opts);
+ pfctl_clear_rules(dev, opts, anchorname);
+ } else
+ pfctl_recurse(dev, opts,
+ pfctl_call_clearanchors);
+
+ if ((!*anchorname) || (opts & PF_OPT_RECURSE)) {
pfctl_clear_states(dev, ifaceopt, opts);
pfctl_clear_src_nodes(dev, opts);
pfctl_clear_stats(dev, ifaceopt, opts);
@@ -2764,7 +2803,11 @@ main(int argc, char *argv[])
pfctl_clear_fingerprints(dev, opts);
break;
Re: relayd websocket
Hi!
Seems to work fine.
Rivo
On 2019-05-08 21:37, Reyk Floeter wrote:
> On Wed, May 08, 2019 at 07:07:43PM +0200, Reyk Floeter wrote:
>> On Wed, May 08, 2019 at 06:26:45PM +0200, Reyk Floeter wrote:
>>> On Wed, Mar 06, 2019 at 05:36:32PM +0100, Sebastian Benoit wrote:
Rivo Nurges(rivo.nur...@smit.ee) on 2019.03.05 22:42:13 +:
> Hi!
>
> On 3/5/19 10:36 PM, Claudio Jeker wrote:
>> I guess that this would need strcasestr() instead of strcasecmp(), since
>> you
>> are looking for the substring "Upgrade" in value. Maybe more is needed if
>> we want to be sure that 'Connection: Upgrade-maybe' does not match.
>
> You are correct about strcasestr. "Connection: Upgrade-maybe" would need
> to have correct "Upgrade: websocket". Anyway, lets be strict.
>
> Does something like this make sense?
i think the seperator list needs to include '\t'
because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB.
And i dont think you can mix "," with " \t" seperators,
because otherwise "Foo Upgrade, Bar" will match.
Something more is needed to parse elements of a header.
>>>
>>> I would reshuffle the websocket handling a bit as I don't think that
>>> we need the http priv struct. We can lookup the parsed headers later.
>>>
>>> The attached diff moves it all into one places and introduces a new
>>> function kv_find_value() that is able to to match headers with
>>> multiple values (I think we might need it elsewhere. And even if not,
>>> I would prefer to have this in a function intead of stuffing it into
>>> relay_read_http).
>>>
>>> A minor question is if the lookup should be done before or after
>>> applying the filters (relay_test - my diff does it afterwards, the
>>> current code does it before).
>>>
>>> Tests? Comments? Ok?
>>>
>>
>> I keep on replying to my own diffs... the updated one below uses
>> strcasecmp instead of strcasestr in kv_find_value(). There is no need
>> for substring- or fn- matching in this function.
>>
>
> Next one...
>
> benno@ pointed out that the RFC allows whitespace before the comma.
> We also don't have to strip \r\n as it is done by relayd's kv parser.
>
> OK?
>
> Reyk
>
> Index: usr.sbin/relayd/http.h
> ===
> RCS file: /cvs/src/usr.sbin/relayd/http.h,v
> retrieving revision 1.10
> diff -u -p -u -p -r1.10 http.h
> --- usr.sbin/relayd/http.h4 Mar 2019 21:25:03 - 1.10
> +++ usr.sbin/relayd/http.h8 May 2019 18:34:09 -
> @@ -251,10 +251,4 @@ struct http_descriptor {
> struct kvtreehttp_headers;
> };
>
> -struct relay_http_priv {
> -#define HTTP_CONNECTION_UPGRADE 0x01
> -#define HTTP_UPGRADE_WEBSOCKET 0x02
> - int http_upgrade_req;
> -};
> -
> #endif /* HTTP_H */
> Index: usr.sbin/relayd/relay.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
> retrieving revision 1.242
> diff -u -p -u -p -r1.242 relay.c
> --- usr.sbin/relayd/relay.c 4 Mar 2019 21:25:03 - 1.242
> +++ usr.sbin/relayd/relay.c 8 May 2019 18:34:09 -
> @@ -1410,13 +1410,7 @@ relay_session(struct rsession *con)
> return;
> }
>
> - if (rlay->rl_proto->type == RELAY_PROTO_HTTP) {
> - if (relay_http_priv_init(con) == -1) {
> - relay_close(con,
> - "failed to allocate http session data", 1);
> - return;
> - }
> - } else {
> + if (rlay->rl_proto->type != RELAY_PROTO_HTTP) {
> if (rlay->rl_conf.fwdmode == FWD_TRANS)
> relay_bindanyreq(con, 0, IPPROTO_TCP);
> else if (relay_connect(con) == -1) {
> Index: usr.sbin/relayd/relay_http.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
> retrieving revision 1.72
> diff -u -p -u -p -r1.72 relay_http.c
> --- usr.sbin/relayd/relay_http.c 4 Mar 2019 21:25:03 - 1.72
> +++ usr.sbin/relayd/relay_http.c 8 May 2019 18:34:09 -
> @@ -110,17 +110,6 @@ relay_http_init(struct relay *rlay)
> }
>
> int
> -relay_http_priv_init(struct rsession *con)
> -{
> - struct relay_http_priv *p;
> - if ((p = calloc(1, sizeof(struct relay_http_priv))) == NULL)
> - return (-1);
> -
> - con->se_priv = p;
> - return (0);
> -}
> -
> -int
> relay_httpdesc_init(struct ctl_relay_event *cre)
> {
> struct http_descriptor *desc;
> @@ -163,13 +152,13 @@ relay_read_http(struct bufferevent *bev,
> struct relay*rlay = con->se_relay;
> struct protocol *proto = rlay->rl_proto;
> struct evbuffer *src = EVBUFFER_INPUT(bev);
> - struct relay_http_priv *priv = con->se_priv;
> char*line = NULL, *key, *value;
>
[2/3] enable pfctl to flush all rules and tables
Hello,
second diff makes current implementation of 'pfctl -vsA' (show rules) more
generic. It changes current pfctl_show_anchors() to pfctl_walk_anchors()
which accepts a callback as argument (pfctl_show_anchor()) to show anchor
found in kernel. So existing pfctl_show_anchors() is changed
to this:
int
pfctl_show_anchors(int dev, int opts, char *anchorname)
{
int rv;
rv = pfctl_walk_anchors(dev, opts, anchorname, NULL, pfctl_walk_show);
return (rv);
}
Diff also introduces pfctl_get_anchors() which also calls pfctl_walk_anchors()
to collect existing anchors to linked list, which might be processed
in pfctl_recurse(). This mechanism here is currently unused it is foot step
for diff 3/3.
thanks and
regards
sashan
8<---8<---8<--8<
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index f56f6f9e90b..5daba743b34 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -106,6 +106,13 @@ const char *pfctl_lookup_option(char *, const char **);
void pfctl_state_store(int, const char *);
void pfctl_state_load(int, const char *);
void pfctl_reset(int, int);
+intpfctl_walk_show(int, struct pfioc_ruleset *, void *);
+intpfctl_walk_get(int, struct pfioc_ruleset *, void *);
+intpfctl_walk_anchors(int, int, char *, void *,
+int(*)(int, struct pfioc_ruleset *, void *));
+struct pfr_anchors *
+ pfctl_get_anchors(int, int);
+intpfctl_recurse(int, int, int(*nuke)(int, int, struct pfr_anchoritem *));
const char *clearopt;
char *rulesopt;
@@ -2109,13 +2116,59 @@ pfctl_debug(int dev, u_int32_t level, int opts)
}
int
-pfctl_show_anchors(int dev, int opts, char *anchorname)
+pfctl_walk_show(int opts, struct pfioc_ruleset *pr, void *warg)
+{
+ if (pr->path[0]) {
+ if (pr->path[0] != '_' || (opts & PF_OPT_VERBOSE))
+ printf(" %s/%s\n", pr->path, pr->name);
+ } else if (pr->name[0] != '_' || (opts & PF_OPT_VERBOSE))
+ printf(" %s\n", pr->name);
+
+ return (0);
+}
+
+int
+pfctl_walk_get(int opts, struct pfioc_ruleset *pr, void *warg)
+{
+ struct pfr_anchoritem *pfra;
+ unsigned int len;
+ struct {
+ struct pfr_anchoritem *pfra;
+ } *tail_pfra = warg;
+
+ pfra = malloc(sizeof(*pfra));
+ if (pfra == NULL)
+ err(1, "%s", __func__);
+
+ len = strlen(pr->path) + 1;
+ len += strlen(pr->name) + 1;
+ pfra->pfra_anchorname = malloc(len);
+ if (pfra->pfra_anchorname == NULL)
+ err(1, "%s", __func__);
+
+ if (pr->path[0])
+ snprintf(pfra->pfra_anchorname, len, "%s/%s",
+ pr->path, pr->name);
+ else
+ snprintf(pfra->pfra_anchorname, len, "%s", pr->name);
+
+ if (tail_pfra->pfra != NULL)
+ SLIST_INSERT_AFTER(tail_pfra->pfra, pfra, pfra_sle);
+
+ tail_pfra->pfra = pfra;
+
+ return (0);
+}
+
+int
+pfctl_walk_anchors(int dev, int opts, char *anchorname, void *warg,
+int(walkf)(int, struct pfioc_ruleset *, void *))
{
struct pfioc_ruleset pr;
u_int32_tmnr, nr;
memset(, 0, sizeof(pr));
- memcpy(pr.path, anchorname, sizeof(pr.path));
+ strlcpy(pr.path, anchorname, sizeof(pr.path));
if (ioctl(dev, DIOCGETRULESETS, )) {
if (errno == EINVAL)
fprintf(stderr, "Anchor '%s' not found.\n",
@@ -2134,19 +2187,85 @@ pfctl_show_anchors(int dev, int opts, char *anchorname)
if (!strcmp(pr.name, PF_RESERVED_ANCHOR))
continue;
sub[0] = '\0';
- if (pr.path[0]) {
- strlcat(sub, pr.path, sizeof(sub));
- strlcat(sub, "/", sizeof(sub));
- }
- strlcat(sub, pr.name, sizeof(sub));
- if (sub[0] != '_' || (opts & PF_OPT_VERBOSE))
- printf(" %s\n", sub);
- if ((opts & PF_OPT_VERBOSE) && pfctl_show_anchors(dev, opts,
sub))
+
+ if (walkf(opts, , warg))
return (-1);
+
+ if (opts & PF_OPT_VERBOSE) {
+ charsub[PATH_MAX];
+
+ if (pr.path[0])
+ snprintf(sub, sizeof(sub), "%s/%s",
+ pr.path, pr.name);
+ else
+ snprintf(sub, sizeof(sub), "%s",
+ pr.name);
+ if (pfctl_walk_anchors(dev, opts, sub, warg, walkf))
+ return (-1);
+ }
}
return (0);
}
+int
+pfctl_show_anchors(int dev, int opts, char *anchorname)
+{
+ int rv;
+
+ rv = pfctl_walk_anchors(dev, opts, anchorname, NULL, pfctl_walk_show);
+ return (rv);
+}
+
+struct pfr_anchors *
Re: vm.conf: boot-device
On Wed, May 08, 2019 at 09:41:42PM +0200, Anton Lindqvist wrote:
> On Wed, May 08, 2019 at 07:19:45PM +0200, Reyk Floeter wrote:
> > On Wed, May 08, 2019 at 06:47:53PM +0200, Anton Lindqvist wrote:
> > > Hi,
> > > A first stab at adding support for option `-B device' to vm.conf(5).
> > > With the diff below, I'm able to add a dedicated VM to be used with
> > > autoinstall(5):
> > >
> > > vm "amd64-install" {
> > > disable
> > > boot $snapshots "bsd.rd"
> > > disk $home "amd64.qcow2"
> > > boot-device net
> > > local interface
> > > }
> > >
> > > The name `boot-device' is of course up for debate. Also, should allow
> > > support
> > > also be considered?
> > >
> > > Comments? OK?
> > >
> >
> > Nice, thanks for adding this. When I did vm.conf vs vmctl, I always
> > liked to have vm.conf feature-complete or even to have it as a
> > superset of vmctl - it is just nicer to add complex options with a
> > grammar than command line flags (yay getsubopt(3)!).
> >
> > Could you adjust the manpage based on vmctl(8)? It is much more
> > verbose and why should vm.conf(5) lack the details?
> >
> > For the grammar, I think it should be done without the "-"...
> > - boot "bsd.foo"
> > - boot device net
> >
> > ...as the parser should be able to handle this. We should have made
> > it more flexible but I guess we don't want to change the grammar now?
> > - boot image "bsd.foo"
> > - boot device net
>
> Thanks for the feedback, new diff:
>
> * Grammar changed to `boot device' according to feedback
>
> * Manual tweaks, thanks jmc@ and sthen@
>
> * I also took a stab at incorporating more of details from the vmctl
> manual. This part could probably be improved...
>
I like the diff, but there is a larger issue lurking ... :)
When I shuffled the man page bits last week, I just moved wording around, and
actually didn't notice we call out PXE in the documentation. The problem here
is that we don't actually support PXE in -current. I have a diff in my tree
that fixes this but it's not committed yet. It breaks Linux guests, and every
time I think I've nailed it, something else breaks. I had intended to commit
it after AsisBSDcon last month but then I ran into that bug.
When claudio committed the "net boot" changes last year, he was pretty clear
what it was intended to do:
revision 1.7
date: 2018/12/06 09:20:06; author: claudio; state: Exp; lines: +7 -6;
commitid: 3sAzaMHdn0pTPnpR;
Make it possible to define the bootdevice in vmd. This information is used
currently only when booting a OpenBSD kernel. If VMBOOTDEV_NET is used the
internal dhcp server will pass "auto_install" as boot file to the client and
the boot loader passes the MAC of the first interface to the kernel to indicate
PXE booting. Adding boot order support to SeaBIOS is not yet implemented.
Ok ccardenas@
So what this does is sets up the bootarg page to tell the kernel that it
had been booted from PXE (even though it wasn't). That's used in conjunction
with the vmd-dhcp server to provide auto_install options automatically to
the VM. The fact that the term "PXE" was included in the commit may have
misled some people to think this actually means PXE is supported now, which
it is not.
So, while the diff is good, we should probably clean up the documentation
to reflect what we really can and cannot do. Once I clean up iPXE and
(really) get it working, we can go back and rev the documentation again.
Claudio, please correct me if I am wrong in the interpretation.
-ml
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/vmd/parse.y,v
> retrieving revision 1.50
> diff -u -p -r1.50 parse.y
> --- parse.y 13 Feb 2019 22:57:08 - 1.50
> +++ parse.y 8 May 2019 19:36:04 -
> @@ -120,12 +120,13 @@ typedef struct {
>
>
> %token INCLUDE ERROR
> -%token ADD ALLOW BOOT CDROM DISABLE DISK DOWN ENABLE FORMAT GROUP INET6
> -%token INSTANCE INTERFACE LLADDR LOCAL LOCKED MEMORY NIFS OWNER PATH
> PREFIX
> -%token RDOMAIN SIZE SOCKET SWITCH UP VM VMID
> +%token ADD ALLOW BOOT CDROM DEVICE DISABLE DISK DOWN ENABLE FORMAT
> GROUP
> +%token INET6 INSTANCE INTERFACE LLADDR LOCAL LOCKED MEMORY NET NIFS
> OWNER
> +%token PATH PREFIX RDOMAIN SIZE SOCKET SWITCH UP VM VMID
> %token NUMBER
> %token STRING
> %type lladdr
> +%type bootdevice
> %type disable
> %type image_format
> %type local
> @@ -456,6 +457,9 @@ vm_opts : disable {
> }
> vmc.vmc_flags |= VMOP_CREATE_KERNEL;
> }
> + | BOOT DEVICE bootdevice{
> + vmc.vmc_bootdevice = $3;
> + }
> | CDROM string {
> if
[1/3] Re: enable pfctl to flush all rules and tables
Hello,
looks like the diff is too big for review. so let me slice it
to smaller chunks. I've tested the complete diff I have not
tested the partial diffs.
no issues were discovered by running regress on pfctl. furthermore doing
'pfctl -a "*" -Fa' with my changes in did expected thing: it removed all
leftovers after pfctl regress.
Diff below introduces PF_OPT_IGNFAIL, which optionally relaxes err()/errx()
to warn()/warnx(), where my following changes need that. I have not done
'global' replace of err()/errx() to pfctl_err()/pfctl_errx() to keep changes
small.
regards
sashan
8<---8<---8<--8<
diff --git a/sbin/pfctl/pfctl.c b/sbin/pfctl/pfctl.c
index f56f6f9e90b..d21d2a8dbb2 100644
--- a/sbin/pfctl/pfctl.c
+++ b/sbin/pfctl/pfctl.c
@@ -54,6 +54,8 @@
#include
+#include
+
#include "pfctl_parser.h"
#include "pfctl.h"
@@ -125,6 +127,7 @@ char*state_kill[2];
int dev = -1;
int first_title = 1;
int labels = 0;
+int exit_val = 0;
#define INDENT(d, o) do {\
if (o) {\
@@ -251,6 +254,40 @@ usage(void)
exit(1);
}
+void
+pfctl_err(int opts, int eval, const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+
+ if ((opts & PF_OPT_IGNFAIL) == 0)
+ verr(1, fmt, ap);
+ else
+ vwarn(fmt, ap);
+
+ va_end(ap);
+
+ exit_val = eval;
+}
+
+void
+pfctl_errx(int opts, int eval, const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+
+ if ((opts & PF_OPT_IGNFAIL) == 0)
+ verrx(1, fmt, ap);
+ else
+ vwarnx(fmt, ap);
+
+ va_end(ap);
+
+ exit_val = eval;
+}
+
int
pfctl_enable(int dev, int opts)
{
@@ -289,10 +326,10 @@ pfctl_clear_stats(int dev, const char *iface, int opts)
memset(, 0, sizeof(pi));
if (iface != NULL && strlcpy(pi.pfiio_name, iface,
sizeof(pi.pfiio_name)) >= sizeof(pi.pfiio_name))
- errx(1, "invalid interface: %s", iface);
+ pfctl_errx(opts, 1, "invalid interface: %s", iface);
if (ioctl(dev, DIOCCLRSTATUS, ))
- err(1, "DIOCCLRSTATUS");
+ pfctl_err(opts, 1, "DIOCCLRSTATUS");
if ((opts & PF_OPT_QUIET) == 0) {
fprintf(stderr, "pf: statistics cleared");
if (iface != NULL)
@@ -311,32 +348,36 @@ pfctl_clear_interface_flags(int dev, int opts)
pi.pfiio_flags = PFI_IFLAG_SKIP;
if (ioctl(dev, DIOCCLRIFFLAG, ))
- err(1, "DIOCCLRIFFLAG");
+ pfctl_err(opts, 1, "DIOCCLRIFFLAG");
if ((opts & PF_OPT_QUIET) == 0)
fprintf(stderr, "pf: interface flags reset\n");
}
}
-void
+int
pfctl_clear_rules(int dev, int opts, char *anchorname)
{
struct pfr_buffer t;
+ int rv = 0;
memset(, 0, sizeof(t));
t.pfrb_type = PFRB_TRANS;
if (pfctl_add_trans(, PF_TRANS_RULESET, anchorname) ||
pfctl_trans(dev, , DIOCXBEGIN, 0) ||
- pfctl_trans(dev, , DIOCXCOMMIT, 0))
- err(1, "pfctl_clear_rules");
- if ((opts & PF_OPT_QUIET) == 0)
+ pfctl_trans(dev, , DIOCXCOMMIT, 0)) {
+ pfctl_err(opts, 1, "%s", __func__);
+ rv = 1;
+ } else if ((opts & PF_OPT_QUIET) == 0)
fprintf(stderr, "rules cleared\n");
+
+ return (rv);
}
void
pfctl_clear_src_nodes(int dev, int opts)
{
if (ioctl(dev, DIOCCLRSRCNODES))
- err(1, "DIOCCLRSRCNODES");
+ pfctl_err(opts, 1, "DIOCCLRSRCNODES");
if ((opts & PF_OPT_QUIET) == 0)
fprintf(stderr, "source tracking entries cleared\n");
}
@@ -349,10 +390,10 @@ pfctl_clear_states(int dev, const char *iface, int opts)
memset(, 0, sizeof(psk));
if (iface != NULL && strlcpy(psk.psk_ifname, iface,
sizeof(psk.psk_ifname)) >= sizeof(psk.psk_ifname))
- errx(1, "invalid interface: %s", iface);
+ pfctl_errx(opts, 1, "invalid interface: %s", iface);
if (ioctl(dev, DIOCCLRSTATES, ))
- err(1, "DIOCCLRSTATES");
+ pfctl_err(opts, 1, "DIOCCLRSTATES");
if ((opts & PF_OPT_QUIET) == 0)
fprintf(stderr, "%d states cleared\n", psk.psk_killed);
}
diff --git a/sbin/pfctl/pfctl.h b/sbin/pfctl/pfctl.h
index 7981cf66fdb..669ffcb3784 100644
--- a/sbin/pfctl/pfctl.h
+++ b/sbin/pfctl/pfctl.h
@@ -96,5 +96,7 @@ u_int32_t
int pfctl_trans(int, struct pfr_buffer *, u_long, int);
int pfctl_show_queues(int, const char *, int, int);
+voidpfctl_err(int, int, const char *, ...);
+voidpfctl_errx(int, int, const char *, ...);
#endif /* _PFCTL_H_ */
diff --git
Re: vm.conf: boot-device
On Wed, May 08, 2019 at 07:19:45PM +0200, Reyk Floeter wrote:
> On Wed, May 08, 2019 at 06:47:53PM +0200, Anton Lindqvist wrote:
> > Hi,
> > A first stab at adding support for option `-B device' to vm.conf(5).
> > With the diff below, I'm able to add a dedicated VM to be used with
> > autoinstall(5):
> >
> > vm "amd64-install" {
> > disable
> > boot $snapshots "bsd.rd"
> > disk $home "amd64.qcow2"
> > boot-device net
> > local interface
> > }
> >
> > The name `boot-device' is of course up for debate. Also, should allow
> > support
> > also be considered?
> >
> > Comments? OK?
> >
>
> Nice, thanks for adding this. When I did vm.conf vs vmctl, I always
> liked to have vm.conf feature-complete or even to have it as a
> superset of vmctl - it is just nicer to add complex options with a
> grammar than command line flags (yay getsubopt(3)!).
>
> Could you adjust the manpage based on vmctl(8)? It is much more
> verbose and why should vm.conf(5) lack the details?
>
> For the grammar, I think it should be done without the "-"...
> - boot "bsd.foo"
> - boot device net
>
> ...as the parser should be able to handle this. We should have made
> it more flexible but I guess we don't want to change the grammar now?
> - boot image "bsd.foo"
> - boot device net
Thanks for the feedback, new diff:
* Grammar changed to `boot device' according to feedback
* Manual tweaks, thanks jmc@ and sthen@
* I also took a stab at incorporating more of details from the vmctl
manual. This part could probably be improved...
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/vmd/parse.y,v
retrieving revision 1.50
diff -u -p -r1.50 parse.y
--- parse.y 13 Feb 2019 22:57:08 - 1.50
+++ parse.y 8 May 2019 19:36:04 -
@@ -120,12 +120,13 @@ typedef struct {
%token INCLUDE ERROR
-%token ADD ALLOW BOOT CDROM DISABLE DISK DOWN ENABLE FORMAT GROUP INET6
-%token INSTANCE INTERFACE LLADDR LOCAL LOCKED MEMORY NIFS OWNER PATH PREFIX
-%token RDOMAIN SIZE SOCKET SWITCH UP VM VMID
+%token ADD ALLOW BOOT CDROM DEVICE DISABLE DISK DOWN ENABLE FORMAT GROUP
+%token INET6 INSTANCE INTERFACE LLADDR LOCAL LOCKED MEMORY NET NIFS OWNER
+%token PATH PREFIX RDOMAIN SIZE SOCKET SWITCH UP VM VMID
%token NUMBER
%token STRING
%typelladdr
+%typebootdevice
%typedisable
%typeimage_format
%typelocal
@@ -456,6 +457,9 @@ vm_opts : disable {
}
vmc.vmc_flags |= VMOP_CREATE_KERNEL;
}
+ | BOOT DEVICE bootdevice{
+ vmc.vmc_bootdevice = $3;
+ }
| CDROM string {
if (vcp->vcp_cdrom[0] != '\0') {
yyerror("cdrom specified more than once");
@@ -703,6 +707,11 @@ disable: ENABLE{ $$ =
0; }
| DISABLE { $$ = 1; }
;
+bootdevice : CDROM { $$ = VMBOOTDEV_CDROM; }
+ | DISK { $$ = VMBOOTDEV_DISK; }
+ | NET { $$ = VMBOOTDEV_NET; }
+ ;
+
optcomma : ','
|
;
@@ -756,6 +765,7 @@ lookup(char *s)
{ "allow", ALLOW },
{ "boot", BOOT },
{ "cdrom", CDROM },
+ { "device", DEVICE },
{ "disable",DISABLE },
{ "disk", DISK },
{ "down", DOWN },
@@ -772,6 +782,7 @@ lookup(char *s)
{ "local", LOCAL },
{ "locked", LOCKED },
{ "memory", MEMORY },
+ { "net",NET },
{ "owner", OWNER },
{ "prefix", PREFIX },
{ "rdomain",RDOMAIN },
Index: vm.conf.5
===
RCS file: /cvs/src/usr.sbin/vmd/vm.conf.5,v
retrieving revision 1.42
diff -u -p -r1.42 vm.conf.5
--- vm.conf.5 7 Mar 2019 18:54:06 - 1.42
+++ vm.conf.5 8 May 2019 19:36:04 -
@@ -144,6 +144,34 @@ See
Kernel or BIOS image to load when booting the VM.
If not specified, the default is to boot using the BIOS image in
.Pa /etc/firmware/vmm-bios .
+.It Cm boot device Ar device
+Force VM to boot from
+.Ar device ,
+valid values are:
+.Bl -tag -width "cdrom"
+.It Ar cdrom
+Boot the ISO image file specified using the
+.Ic cdrom
+parameter.
+.It Ar disk
+Boot from the disk image file specified using the
+.Ic disk
+parameter.
+.It Ar net
+Perform a PXE boot using the first network interface specified using the
iwm: fix ADD_STA status check
Correctly mask status bits in ADD_STA command response; remaining
bits are used by firmware to return the baid for a BA session.
Dragonfly 74d41163ddac72b0d7ea7b7873d53fe134723a12
Linux 837c4da98481d4e504b2aba077c8528fee1b6d13
Not sure if this matters for us yet, but it will likely matter
when we start doing Tx aggregation.
Perhaps this will fix the 'could not add sta' problem we've seen
occasionally? Let's try...
ok?
diff 63562686d340902505feb02c68d09c5d34652880 /usr/src
blob - 335033d21091a23511403804f09d1548b109b104
file + sys/dev/pci/if_iwm.c
--- sys/dev/pci/if_iwm.c
+++ sys/dev/pci/if_iwm.c
@@ -2566,7 +2566,7 @@ iwm_sta_rx_agg(struct iwm_softc *sc, struct ieee80211_
);
s = splnet();
- if (err == 0 && status == IWM_ADD_STA_SUCCESS) {
+ if (!err && (status & IWM_ADD_STA_STATUS_MASK) == IWM_ADD_STA_SUCCESS) {
if (start) {
sc->sc_rx_ba_sessions++;
ieee80211_addba_req_accept(ic, ni, tid);
@@ -4675,7 +4675,7 @@ iwm_add_sta_cmd(struct iwm_softc *sc, struct iwm_node
status = IWM_ADD_STA_SUCCESS;
err = iwm_send_cmd_pdu_status(sc, IWM_ADD_STA, sizeof(add_sta_cmd),
_sta_cmd, );
- if (err == 0 && status != IWM_ADD_STA_SUCCESS)
+ if (!err && (status & IWM_ADD_STA_STATUS_MASK) != IWM_ADD_STA_SUCCESS)
err = EIO;
return err;
@@ -4702,7 +4702,7 @@ iwm_add_aux_sta(struct iwm_softc *sc)
status = IWM_ADD_STA_SUCCESS;
err = iwm_send_cmd_pdu_status(sc, IWM_ADD_STA, sizeof(cmd), ,
);
- if (err == 0 && status != IWM_ADD_STA_SUCCESS)
+ if (!err && (status & IWM_ADD_STA_STATUS_MASK) != IWM_ADD_STA_SUCCESS)
err = EIO;
return err;
Re: Avoid system(3) in ikectl
On Wed, May 08, 2019 at 01:06:30PM -0500, Matthew Martin wrote:
> ping
>
The diff looks good now. I otherwise agree with tedu.
OK reyk@
> On Thu, Apr 25, 2019 at 11:21:00PM -0500, Matthew Martin wrote:
> > On Thu, Apr 25, 2019 at 08:59:56PM -0600, Theo de Raadt wrote:
> > > > + argv = alloca((n + 1) * sizeof(*argv));
> > >
> > > Our source tree is exceedingly sparing in the use of alloca().
> > > This will not do.
> >
> > Was staying as close as possible to exec.c, but avoiding alloca is
> > preferable; replaced with reallocarray. err message is "reallocarray" to
> > match style with the calloc call in the same file.
> >
> >
> diff --git usr.sbin/ikectl/ikeca.c usr.sbin/ikectl/ikeca.c
> index bac76ab9c2f..eda957d1b2f 100644
> --- usr.sbin/ikectl/ikeca.c
> +++ usr.sbin/ikectl/ikeca.c
> @@ -18,11 +18,13 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -63,12 +65,12 @@
>
> struct ca {
> char sslpath[PATH_MAX];
> - char passfile[PATH_MAX];
> + char passfile[PATH_MAX + 5]; // Includes the "file:" prefix
> char index[PATH_MAX];
> char serial[PATH_MAX];
> char sslcnf[PATH_MAX];
> char extcnf[PATH_MAX];
> - char batch[PATH_MAX];
> + char*batch;
> char*caname;
> };
>
> @@ -116,6 +118,7 @@ void ca_setenv(const char *, const char *);
> void ca_clrenv(void);
> void ca_setcnf(struct ca *, const char *);
> void ca_create_index(struct ca *);
> +static void ca_system(char *, ...);
>
> /* util.c */
> int expand_string(char *, size_t, const char *, const char *);
> @@ -130,7 +133,6 @@ int
> ca_key_create(struct ca *ca, char *keyname)
> {
> struct stat st;
> - char cmd[PATH_MAX * 2];
> char path[PATH_MAX];
>
> snprintf(path, sizeof(path), "%s/private/%s.key", ca->sslpath, keyname);
> @@ -140,10 +142,7 @@ ca_key_create(struct ca *ca, char *keyname)
> return (0);
> }
>
> - snprintf(cmd, sizeof(cmd),
> - "%s genrsa -out %s 2048",
> - PATH_OPENSSL, path);
> - system(cmd);
> + ca_system(PATH_OPENSSL, "genrsa", "-out", path, "2048", NULL);
> chmod(path, 0600);
>
> return (0);
> @@ -200,9 +199,9 @@ ca_delkey(struct ca *ca, char *keyname)
> int
> ca_request(struct ca *ca, char *keyname, int type)
> {
> - charcmd[PATH_MAX * 2];
> charhostname[HOST_NAME_MAX+1];
> charname[128];
> + charkey[PATH_MAX];
> charpath[PATH_MAX];
>
> ca_setenv("$ENV::CERT_CN", keyname);
> @@ -226,13 +225,11 @@ ca_request(struct ca *ca, char *keyname, int type)
>
> ca_setcnf(ca, keyname);
>
> + snprintf(key, sizeof(key), "%s/private/%s.key", ca->sslpath, keyname);
> snprintf(path, sizeof(path), "%s/private/%s.csr", ca->sslpath, keyname);
> - snprintf(cmd, sizeof(cmd), "%s req %s-new"
> - " -key %s/private/%s.key -out %s -config %s",
> - PATH_OPENSSL, ca->batch, ca->sslpath, keyname,
> - path, ca->sslcnf);
>
> - system(cmd);
> + ca_system(PATH_OPENSSL, "req", "-new", "-key", key, "-out", path,
> + "-config", ca->sslcnf, ca->batch, NULL);
> chmod(path, 0600);
>
> return (0);
> @@ -241,8 +238,11 @@ ca_request(struct ca *ca, char *keyname, int type)
> int
> ca_sign(struct ca *ca, char *keyname, int type)
> {
> - charcmd[PATH_MAX * 2];
> - const char *extensions = NULL;
> + charcakey[PATH_MAX];
> + charcacrt[PATH_MAX];
> + charout[PATH_MAX];
> + charin[PATH_MAX];
> + char*extensions = NULL;
>
> if (type == HOST_IPADDR) {
> extensions = "x509v3_IPAddr";
> @@ -258,19 +258,15 @@ ca_sign(struct ca *ca, char *keyname, int type)
> ca_setenv("$ENV::CASERIAL", ca->serial);
> ca_setcnf(ca, keyname);
>
> - snprintf(cmd, sizeof(cmd),
> - "%s ca -config %s -keyfile %s/private/ca.key"
> - " -cert %s/ca.crt"
> - " -extfile %s -extensions %s -out %s/%s.crt"
> - " -in %s/private/%s.csr"
> - " -passin file:%s -outdir %s -batch",
> - PATH_OPENSSL, ca->sslcnf, ca->sslpath,
> - ca->sslpath,
> - ca->extcnf, extensions, ca->sslpath, keyname,
> - ca->sslpath, keyname,
> - ca->passfile, ca->sslpath);
> + snprintf(cakey, sizeof(cakey), "%s/private/ca.key", ca->sslpath);
> + snprintf(cacrt, sizeof(cacrt), "%s/ca.crt", ca->sslpath);
> + snprintf(out, sizeof(out), "%s/%s.crt", ca->sslpath, keyname);
> + snprintf(in, sizeof(in), "%s/private/%s.csr", ca->sslpath, keyname);
>
iwm: rx interrupt paranoia
Add two sanity checks to iwm's firmware notification interrupt handler:
1) Clamp the firmware-provided index into the rx ring to the size of the ring.
Linux started doing this, too, to work around HW bugs in the 9000 series.
2) Don't call iwm_cmd_done() if the firmware response in the Rx buffer
is not recognized. We should just skip such buffers, not act on them.
Not tested on hardware yet; these changes evidenty shouldn't break anything.
ok?
diff 74c6ffce73f932dd8464cc1def1ee4fcaa3e671f /usr/src
blob - 335033d21091a23511403804f09d1548b109b104
file + sys/dev/pci/if_iwm.c
--- sys/dev/pci/if_iwm.c
+++ sys/dev/pci/if_iwm.c
@@ -7000,10 +7000,11 @@ iwm_notif_intr(struct iwm_softc *sc)
0, sc->rxq.stat_dma.size, BUS_DMASYNC_POSTREAD);
hw = le16toh(sc->rxq.stat->closed_rb_num) & 0xfff;
+ hw &= (IWM_RX_RING_COUNT - 1);
while (sc->rxq.cur != hw) {
struct iwm_rx_data *data = >rxq.data[sc->rxq.cur];
struct iwm_rx_packet *pkt;
- int qid, idx, code;
+ int qid, idx, code, handled = 1;
bus_dmamap_sync(sc->sc_dmat, data->map, 0, sizeof(*pkt),
BUS_DMASYNC_POSTREAD);
@@ -7256,6 +7257,7 @@ iwm_notif_intr(struct iwm_softc *sc)
}
default:
+ handled = 0;
printf("%s: unhandled firmware response 0x%x/0x%x "
"rx ring %d[%d]\n",
DEVNAME(sc), pkt->hdr.code, pkt->len_n_flags, qid,
@@ -7270,7 +7272,7 @@ iwm_notif_intr(struct iwm_softc *sc)
* For example, uCode issues IWM_REPLY_RX when it sends a
* received frame to the driver.
*/
- if (!(pkt->hdr.qid & (1 << 7))) {
+ if (handled && !(pkt->hdr.qid & (1 << 7))) {
iwm_cmd_done(sc, pkt);
}
iwm: move mbuf hacks down in iwm_rx_rx_mpdu()
Don't write to mbuf memory before we've actually completed all sanity checks and iwm_rx_addbuf() has succcessfully put a new buffer on the ring. Same as dragonfly commit 96eaecf93d9f731459a0df8efc72cfad034320bd "Avoids leaving the mbuf in a weird state when dropping a packet." ok? diff 74c6ffce73f932dd8464cc1def1ee4fcaa3e671f /usr/src blob - 335033d21091a23511403804f09d1548b109b104 file + sys/dev/pci/if_iwm.c --- sys/dev/pci/if_iwm.c +++ sys/dev/pci/if_iwm.c @@ -3465,10 +3465,6 @@ iwm_rx_rx_mpdu(struct iwm_softc *sc, struct iwm_rx_pac rx_pkt_status = le32toh(*(uint32_t *)(pkt->data + sizeof(*rx_res) + len)); - m = data->m; - m->m_data = pkt->data + sizeof(*rx_res); - m->m_pkthdr.len = m->m_len = len; - if (__predict_false(phy_info->cfg_phy_cnt > 20)) return; @@ -3488,6 +3484,10 @@ iwm_rx_rx_mpdu(struct iwm_softc *sc, struct iwm_rx_pac if (iwm_rx_addbuf(sc, IWM_RBUF_SIZE, sc->rxq.cur) != 0) return; + + m = data->m; + m->m_data = pkt->data + sizeof(*rx_res); + m->m_pkthdr.len = m->m_len = len; chanidx = letoh32(phy_info->channel); if (chanidx < 0 || chanidx >= nitems(ic->ic_channels))
Re: relayd websocket
On Wed, May 08, 2019 at 07:07:43PM +0200, Reyk Floeter wrote:
> On Wed, May 08, 2019 at 06:26:45PM +0200, Reyk Floeter wrote:
> > On Wed, Mar 06, 2019 at 05:36:32PM +0100, Sebastian Benoit wrote:
> > > Rivo Nurges(rivo.nur...@smit.ee) on 2019.03.05 22:42:13 +:
> > > > Hi!
> > > >
> > > > On 3/5/19 10:36 PM, Claudio Jeker wrote:
> > > > > I guess that this would need strcasestr() instead of strcasecmp(),
> > > > > since you
> > > > > are looking for the substring "Upgrade" in value. Maybe more is
> > > > > needed if
> > > > > we want to be sure that 'Connection: Upgrade-maybe' does not match.
> > > >
> > > > You are correct about strcasestr. "Connection: Upgrade-maybe" would
> > > > need
> > > > to have correct "Upgrade: websocket". Anyway, lets be strict.
> > > >
> > > > Does something like this make sense?
> > >
> > > i think the seperator list needs to include '\t'
> > > because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB.
> > >
> > > And i dont think you can mix "," with " \t" seperators,
> > > because otherwise "Foo Upgrade, Bar" will match.
> > >
> > > Something more is needed to parse elements of a header.
> > >
> >
> > I would reshuffle the websocket handling a bit as I don't think that
> > we need the http priv struct. We can lookup the parsed headers later.
> >
> > The attached diff moves it all into one places and introduces a new
> > function kv_find_value() that is able to to match headers with
> > multiple values (I think we might need it elsewhere. And even if not,
> > I would prefer to have this in a function intead of stuffing it into
> > relay_read_http).
> >
> > A minor question is if the lookup should be done before or after
> > applying the filters (relay_test - my diff does it afterwards, the
> > current code does it before).
> >
> > Tests? Comments? Ok?
> >
>
> I keep on replying to my own diffs... the updated one below uses
> strcasecmp instead of strcasestr in kv_find_value(). There is no need
> for substring- or fn- matching in this function.
>
Next one...
benno@ pointed out that the RFC allows whitespace before the comma.
We also don't have to strip \r\n as it is done by relayd's kv parser.
OK?
Reyk
Index: usr.sbin/relayd/http.h
===
RCS file: /cvs/src/usr.sbin/relayd/http.h,v
retrieving revision 1.10
diff -u -p -u -p -r1.10 http.h
--- usr.sbin/relayd/http.h 4 Mar 2019 21:25:03 - 1.10
+++ usr.sbin/relayd/http.h 8 May 2019 18:34:09 -
@@ -251,10 +251,4 @@ struct http_descriptor {
struct kvtreehttp_headers;
};
-struct relay_http_priv {
-#define HTTP_CONNECTION_UPGRADE0x01
-#define HTTP_UPGRADE_WEBSOCKET 0x02
- int http_upgrade_req;
-};
-
#endif /* HTTP_H */
Index: usr.sbin/relayd/relay.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
retrieving revision 1.242
diff -u -p -u -p -r1.242 relay.c
--- usr.sbin/relayd/relay.c 4 Mar 2019 21:25:03 - 1.242
+++ usr.sbin/relayd/relay.c 8 May 2019 18:34:09 -
@@ -1410,13 +1410,7 @@ relay_session(struct rsession *con)
return;
}
- if (rlay->rl_proto->type == RELAY_PROTO_HTTP) {
- if (relay_http_priv_init(con) == -1) {
- relay_close(con,
- "failed to allocate http session data", 1);
- return;
- }
- } else {
+ if (rlay->rl_proto->type != RELAY_PROTO_HTTP) {
if (rlay->rl_conf.fwdmode == FWD_TRANS)
relay_bindanyreq(con, 0, IPPROTO_TCP);
else if (relay_connect(con) == -1) {
Index: usr.sbin/relayd/relay_http.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
retrieving revision 1.72
diff -u -p -u -p -r1.72 relay_http.c
--- usr.sbin/relayd/relay_http.c4 Mar 2019 21:25:03 - 1.72
+++ usr.sbin/relayd/relay_http.c8 May 2019 18:34:09 -
@@ -110,17 +110,6 @@ relay_http_init(struct relay *rlay)
}
int
-relay_http_priv_init(struct rsession *con)
-{
- struct relay_http_priv *p;
- if ((p = calloc(1, sizeof(struct relay_http_priv))) == NULL)
- return (-1);
-
- con->se_priv = p;
- return (0);
-}
-
-int
relay_httpdesc_init(struct ctl_relay_event *cre)
{
struct http_descriptor *desc;
@@ -163,13 +152,13 @@ relay_read_http(struct bufferevent *bev,
struct relay*rlay = con->se_relay;
struct protocol *proto = rlay->rl_proto;
struct evbuffer *src = EVBUFFER_INPUT(bev);
- struct relay_http_priv *priv = con->se_priv;
char*line = NULL, *key, *value;
char*urlproto, *host, *path;
int action, unique, ret;
Re: Avoid system(3) in ikectl
Matthew Martin wrote:
> ping
>
> On Thu, Apr 25, 2019 at 11:21:00PM -0500, Matthew Martin wrote:
> > On Thu, Apr 25, 2019 at 08:59:56PM -0600, Theo de Raadt wrote:
> > > > + argv = alloca((n + 1) * sizeof(*argv));
> > >
> > > Our source tree is exceedingly sparing in the use of alloca().
> > > This will not do.
> >
> > Was staying as close as possible to exec.c, but avoiding alloca is
> > preferable; replaced with reallocarray. err message is "reallocarray" to
> > match style with the calloc call in the same file.
> >
> >
> - if (keyname != NULL) {
> - snprintf(cmd, sizeof(cmd), "env EXPASS=%s %s pkcs12 -export"
> - " -name %s -CAfile %s/ca.crt -inkey %s/private/%s.key"
> - " -in %s/%s.crt -out %s/private/%s.pfx -passout env:EXPASS"
> - " -passin file:%s", pass, PATH_OPENSSL, keyname,
> - ca->sslpath, ca->sslpath, keyname, ca->sslpath, keyname,
> - ca->sslpath, oname, ca->passfile);
> - system(cmd);
> - }
> -
> - snprintf(cmd, sizeof(cmd), "env EXPASS=%s %s pkcs12 -export"
> - " -caname '%s' -name '%s' -cacerts -nokeys"
> - " -in %s/ca.crt -out %s/ca.pfx -passout env:EXPASS -passin file:%s",
> - pass, PATH_OPENSSL, ca->caname, ca->caname, ca->sslpath,
> - ca->sslpath, ca->passfile);
> - system(cmd);
> + snprintf(cacrt, sizeof(cacrt), "%s/ca.crt", ca->sslpath);
> + snprintf(capfx, sizeof(capfx), "%s/ca.pfx", ca->sslpath);
> + snprintf(key, sizeof(key), "%s/private/%s.key", ca->sslpath, keyname);
> + snprintf(crt, sizeof(crt), "%s/%s.crt", ca->sslpath, keyname);
> + snprintf(pfx, sizeof(pfx), "%s/private/%s.pfx", ca->sslpath, oname);
> +
> + snprintf(passenv, sizeof(passenv), "EXPASS=%s", pass);
> + putenv(passenv);
> +
> + if (keyname != NULL)
> + ca_system(PATH_OPENSSL, "pkcs12", "-export", "-name", keyname,
> + "-CAfile", cacrt, "-inkey", key, "-in", crt, "-out", pfx,
> + "-passout", "env:EXPASS", "-passin", ca->passfile, NULL);
> +
> + ca_system(PATH_OPENSSL, "pkcs12", "-export", "-caname", ca->caname,
> + "-name", ca->caname, "-cacerts", "-nokeys", "-in", cacrt,
> + "-out", capfx, "-passout", "env:EXPASS", "-passin", ca->passfile,
> + NULL);
> +
> + unsetenv("EXPASS");
> + explicit_bzero(passenv, sizeof(passenv));
This looks weird, but it's still probably an improvement over the original
code, which is trying to avoid leaking password via argv but only mostly
succeeds.
There's probably a better way to do environment handling, but I think this
diff is better than using system() now. ok with me.
Re: Avoid system(3) in ikectl
ping
On Thu, Apr 25, 2019 at 11:21:00PM -0500, Matthew Martin wrote:
> On Thu, Apr 25, 2019 at 08:59:56PM -0600, Theo de Raadt wrote:
> > > + argv = alloca((n + 1) * sizeof(*argv));
> >
> > Our source tree is exceedingly sparing in the use of alloca().
> > This will not do.
>
> Was staying as close as possible to exec.c, but avoiding alloca is
> preferable; replaced with reallocarray. err message is "reallocarray" to
> match style with the calloc call in the same file.
>
>
diff --git usr.sbin/ikectl/ikeca.c usr.sbin/ikectl/ikeca.c
index bac76ab9c2f..eda957d1b2f 100644
--- usr.sbin/ikectl/ikeca.c
+++ usr.sbin/ikectl/ikeca.c
@@ -18,11 +18,13 @@
#include
#include
+#include
#include
#include
#include
#include
#include
+#include
#include
#include
#include
@@ -63,12 +65,12 @@
struct ca {
char sslpath[PATH_MAX];
- char passfile[PATH_MAX];
+ char passfile[PATH_MAX + 5]; // Includes the "file:" prefix
char index[PATH_MAX];
char serial[PATH_MAX];
char sslcnf[PATH_MAX];
char extcnf[PATH_MAX];
- char batch[PATH_MAX];
+ char*batch;
char*caname;
};
@@ -116,6 +118,7 @@ void ca_setenv(const char *, const char *);
voidca_clrenv(void);
voidca_setcnf(struct ca *, const char *);
voidca_create_index(struct ca *);
+static void ca_system(char *, ...);
/* util.c */
int expand_string(char *, size_t, const char *, const char *);
@@ -130,7 +133,6 @@ int
ca_key_create(struct ca *ca, char *keyname)
{
struct stat st;
- char cmd[PATH_MAX * 2];
char path[PATH_MAX];
snprintf(path, sizeof(path), "%s/private/%s.key", ca->sslpath, keyname);
@@ -140,10 +142,7 @@ ca_key_create(struct ca *ca, char *keyname)
return (0);
}
- snprintf(cmd, sizeof(cmd),
- "%s genrsa -out %s 2048",
- PATH_OPENSSL, path);
- system(cmd);
+ ca_system(PATH_OPENSSL, "genrsa", "-out", path, "2048", NULL);
chmod(path, 0600);
return (0);
@@ -200,9 +199,9 @@ ca_delkey(struct ca *ca, char *keyname)
int
ca_request(struct ca *ca, char *keyname, int type)
{
- charcmd[PATH_MAX * 2];
charhostname[HOST_NAME_MAX+1];
charname[128];
+ charkey[PATH_MAX];
charpath[PATH_MAX];
ca_setenv("$ENV::CERT_CN", keyname);
@@ -226,13 +225,11 @@ ca_request(struct ca *ca, char *keyname, int type)
ca_setcnf(ca, keyname);
+ snprintf(key, sizeof(key), "%s/private/%s.key", ca->sslpath, keyname);
snprintf(path, sizeof(path), "%s/private/%s.csr", ca->sslpath, keyname);
- snprintf(cmd, sizeof(cmd), "%s req %s-new"
- " -key %s/private/%s.key -out %s -config %s",
- PATH_OPENSSL, ca->batch, ca->sslpath, keyname,
- path, ca->sslcnf);
- system(cmd);
+ ca_system(PATH_OPENSSL, "req", "-new", "-key", key, "-out", path,
+ "-config", ca->sslcnf, ca->batch, NULL);
chmod(path, 0600);
return (0);
@@ -241,8 +238,11 @@ ca_request(struct ca *ca, char *keyname, int type)
int
ca_sign(struct ca *ca, char *keyname, int type)
{
- charcmd[PATH_MAX * 2];
- const char *extensions = NULL;
+ charcakey[PATH_MAX];
+ charcacrt[PATH_MAX];
+ charout[PATH_MAX];
+ charin[PATH_MAX];
+ char*extensions = NULL;
if (type == HOST_IPADDR) {
extensions = "x509v3_IPAddr";
@@ -258,19 +258,15 @@ ca_sign(struct ca *ca, char *keyname, int type)
ca_setenv("$ENV::CASERIAL", ca->serial);
ca_setcnf(ca, keyname);
- snprintf(cmd, sizeof(cmd),
- "%s ca -config %s -keyfile %s/private/ca.key"
- " -cert %s/ca.crt"
- " -extfile %s -extensions %s -out %s/%s.crt"
- " -in %s/private/%s.csr"
- " -passin file:%s -outdir %s -batch",
- PATH_OPENSSL, ca->sslcnf, ca->sslpath,
- ca->sslpath,
- ca->extcnf, extensions, ca->sslpath, keyname,
- ca->sslpath, keyname,
- ca->passfile, ca->sslpath);
+ snprintf(cakey, sizeof(cakey), "%s/private/ca.key", ca->sslpath);
+ snprintf(cacrt, sizeof(cacrt), "%s/ca.crt", ca->sslpath);
+ snprintf(out, sizeof(out), "%s/%s.crt", ca->sslpath, keyname);
+ snprintf(in, sizeof(in), "%s/private/%s.csr", ca->sslpath, keyname);
- system(cmd);
+ ca_system(PATH_OPENSSL, "ca", "-config", ca->sslcnf, "-keyfile", cakey,
+ "-cert", cacrt, "-extfile", ca->extcnf, "-extensions", extensions,
+ "-out", out, "-in", in, "-passin", ca->passfile,
+
Re: vm.conf: boot-device
On Wed, May 08, 2019 at 06:47:53PM +0200, Anton Lindqvist wrote:
> Hi,
> A first stab at adding support for option `-B device' to vm.conf(5).
> With the diff below, I'm able to add a dedicated VM to be used with
> autoinstall(5):
>
> vm "amd64-install" {
> disable
> boot $snapshots "bsd.rd"
> disk $home "amd64.qcow2"
> boot-device net
> local interface
> }
>
> The name `boot-device' is of course up for debate. Also, should allow support
> also be considered?
>
> Comments? OK?
>
Nice, thanks for adding this. When I did vm.conf vs vmctl, I always
liked to have vm.conf feature-complete or even to have it as a
superset of vmctl - it is just nicer to add complex options with a
grammar than command line flags (yay getsubopt(3)!).
Could you adjust the manpage based on vmctl(8)? It is much more
verbose and why should vm.conf(5) lack the details?
For the grammar, I think it should be done without the "-"...
- boot "bsd.foo"
- boot device net
...as the parser should be able to handle this. We should have made
it more flexible but I guess we don't want to change the grammar now?
- boot image "bsd.foo"
- boot device net
Reyk
> Index: parse.y
> ===
> RCS file: /cvs/src/usr.sbin/vmd/parse.y,v
> retrieving revision 1.50
> diff -u -p -r1.50 parse.y
> --- parse.y 13 Feb 2019 22:57:08 - 1.50
> +++ parse.y 28 Apr 2019 13:54:50 -
> @@ -120,12 +120,13 @@ typedef struct {
>
>
> %token INCLUDE ERROR
> -%token ADD ALLOW BOOT CDROM DISABLE DISK DOWN ENABLE FORMAT GROUP INET6
> -%token INSTANCE INTERFACE LLADDR LOCAL LOCKED MEMORY NIFS OWNER PATH
> PREFIX
> -%token RDOMAIN SIZE SOCKET SWITCH UP VM VMID
> +%token ADD ALLOW BOOT BOOTDEVICE CDROM DISABLE DISK DOWN ENABLE FORMAT
> GROUP
> +%token INET6 INSTANCE INTERFACE LLADDR LOCAL LOCKED MEMORY NET NIFS
> OWNER
> +%token PATH PREFIX RDOMAIN SIZE SOCKET SWITCH UP VM VMID
> %token NUMBER
> %token STRING
> %type lladdr
> +%type bootdevice
> %type disable
> %type image_format
> %type local
> @@ -456,6 +457,9 @@ vm_opts : disable {
> }
> vmc.vmc_flags |= VMOP_CREATE_KERNEL;
> }
> + | BOOTDEVICE bootdevice {
> + vmc.vmc_bootdevice = $2;
> + }
> | CDROM string {
> if (vcp->vcp_cdrom[0] != '\0') {
> yyerror("cdrom specified more than once");
> @@ -703,6 +707,11 @@ disable : ENABLE{ $$ =
> 0; }
> | DISABLE { $$ = 1; }
> ;
>
> +bootdevice : CDROM { $$ = VMBOOTDEV_CDROM; }
> + | DISK { $$ = VMBOOTDEV_DISK; }
> + | NET { $$ = VMBOOTDEV_NET; }
> + ;
> +
> optcomma : ','
> |
> ;
> @@ -755,6 +764,7 @@ lookup(char *s)
> { "add",ADD },
> { "allow", ALLOW },
> { "boot", BOOT },
> + { "boot-device",BOOTDEVICE },
> { "cdrom", CDROM },
> { "disable",DISABLE },
> { "disk", DISK },
> @@ -772,6 +782,7 @@ lookup(char *s)
> { "local", LOCAL },
> { "locked", LOCKED },
> { "memory", MEMORY },
> + { "net",NET },
> { "owner", OWNER },
> { "prefix", PREFIX },
> { "rdomain",RDOMAIN },
> Index: vm.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/vmd/vm.conf.5,v
> retrieving revision 1.42
> diff -u -p -r1.42 vm.conf.5
> --- vm.conf.5 7 Mar 2019 18:54:06 - 1.42
> +++ vm.conf.5 28 Apr 2019 13:54:50 -
> @@ -144,6 +144,13 @@ See
> Kernel or BIOS image to load when booting the VM.
> If not specified, the default is to boot using the BIOS image in
> .Pa /etc/firmware/vmm-bios .
> +.It Cm boot-device Ar device
> +Force VM to boot of
> +.Ar device
> +which may be either
> +.Ar cdrom , disk
> +or
> +.Ar net .
> .It Cm cdrom Ar path
> ISO image file.
> .It Cm enable
>
Re: ld.so speedup (part 1)
On Mon 6, May 2019 at 00:16:57, Nathanael Rensen wrote:
> On Sun, 5 May 2019 at 06:41, Martin Pieuchot wrote:
[snip]
> > Do I understand correctly that you're not recursing if not object is
> > inserted?
>
> Yes.
>
> You have probably also noticed that using a single recurse flag is not
> optimally efficient. It is only necessary to recurse child nodes where
> n->data->grpsym_gen != _dl_grpsym_gen
> With my approach if any child node satisfies that then all child nodes
> are recursed.
[snip]
> What do you think about the approach below?
>
> static void
> _dl_insert_grpsym(elf_object_t *object)
> {
> struct dep_node *n;
>
> n = _dl_malloc(sizeof *n);
> if (n == NULL)
> _dl_oom();
> n->data = object;
> TAILQ_INSERT_TAIL(&_dl_loading_object->grpsym_list, n, next_sib);
> }
>
> void
> _dl_link_grpsym(elf_object_t *object)
> {
> struct dep_node *n;
>
> TAILQ_FOREACH(n, &_dl_loading_object->grpsym_list, next_sib)
> if (n->data == object && object->grpsym_gen == _dl_grpsym_gen)
> return; /* found, dont bother adding */
>
> _dl_insert_grpsym(object);
> object->grpsym_gen = _dl_grpsym_gen;
> }
>
> void
> _dl_cache_grpsym_list(elf_object_t *object)
> {
> struct dep_node *n;
>
> /*
>* grpsym_list is an ordered list of all child libs of the
>* _dl_loading_object with no dups. The order is equivalent
>* to a breadth-first traversal of the child list without dups.
>*/
>
> TAILQ_FOREACH(n, >child_list, next_sib)
> if (n->data->grpsym_gen != _dl_grpsym_gen)
> _dl_insert_grpsym(n->data);
>
> TAILQ_FOREACH(n, >child_list, next_sib)
> if (n->data->grpsym_gen != _dl_grpsym_gen) {
> object->grpsym_gen = _dl_grpsym_gen;
> _dl_cache_grpsym_list(n->data);
> }
> }
For the record, the approach above is wrong. It is essential that
object->grpsym_gen is updated by _dl_insert_grpsym() otherwise a tree
such as that below would result in z being added to grpsym_list twice:
x -> y,z
y -> z
While processing x both children y and z are inserted without updating
grpsym_gen. Then while recursing into y, z is added a second time.
This can be fixed using a status flag as follows:
static void
_dl_insert_grpsym(elf_object_t *object)
{
struct dep_node *n;
object->grpsym_gen = _dl_grpsym_gen;
n = _dl_malloc(sizeof *n);
if (n == NULL)
_dl_oom();
n->data = object;
TAILQ_INSERT_TAIL(&_dl_loading_object->grpsym_list, n, next_sib);
}
void
_dl_link_grpsym(elf_object_t *object)
{
struct dep_node *n;
TAILQ_FOREACH(n, &_dl_loading_object->grpsym_list, next_sib)
if (n->data == object && object->grpsym_gen == _dl_grpsym_gen)
return; /* found, dont bother adding */
_dl_insert_grpsym(object);
}
void
_dl_cache_grpsym_list(elf_object_t *object)
{
struct dep_node *n;
/*
* grpsym_list is an ordered list of all child libs of the
* _dl_loading_object with no dups. The order is equivalent
* to a breadth-first traversal of the child list without dups.
*/
TAILQ_FOREACH(n, >child_list, next_sib)
if (n->data->grpsym_gen != _dl_grpsym_gen) {
n->data->status |= STAT_GS_RECURSE;
_dl_insert_grpsym(n->data);
}
TAILQ_FOREACH(n, >child_list, next_sib)
if (n->data->status & STAT_GS_RECURSE) {
n->data->status &= ~STAT_GS_RECURSE;
_dl_cache_grpsym_list(n->data);
}
}
This approach maintains the integrity of the grpsym_gen mechanism ensuring
a node will not be added to the grpsym_list multiple times. The
STAT_GS_RECURSE flag is used to record the requirement to recurse for
those nodes added on this iteration. This extends the per-iteration
recurse flag of the original diff to a per-child flag without damaging
the integrity of the grpsym_gen mechanism.
For the example tree above, while processing x both y and z are inserted
and STAT_GS_RECURSE is set on both. Then when recursing y, z is not
inserted again because grpsym_gen has already been updated.
The flaw described above is not present in the original part 1 diff, only
in the variation I proposed in response to mpi@'s questions.
Nathanael
Re: relayd websocket
On Wed, May 08, 2019 at 06:26:45PM +0200, Reyk Floeter wrote:
> On Wed, Mar 06, 2019 at 05:36:32PM +0100, Sebastian Benoit wrote:
> > Rivo Nurges(rivo.nur...@smit.ee) on 2019.03.05 22:42:13 +:
> > > Hi!
> > >
> > > On 3/5/19 10:36 PM, Claudio Jeker wrote:
> > > > I guess that this would need strcasestr() instead of strcasecmp(),
> > > > since you
> > > > are looking for the substring "Upgrade" in value. Maybe more is needed
> > > > if
> > > > we want to be sure that 'Connection: Upgrade-maybe' does not match.
> > >
> > > You are correct about strcasestr. "Connection: Upgrade-maybe" would need
> > > to have correct "Upgrade: websocket". Anyway, lets be strict.
> > >
> > > Does something like this make sense?
> >
> > i think the seperator list needs to include '\t'
> > because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB.
> >
> > And i dont think you can mix "," with " \t" seperators,
> > because otherwise "Foo Upgrade, Bar" will match.
> >
> > Something more is needed to parse elements of a header.
> >
>
> I would reshuffle the websocket handling a bit as I don't think that
> we need the http priv struct. We can lookup the parsed headers later.
>
> The attached diff moves it all into one places and introduces a new
> function kv_find_value() that is able to to match headers with
> multiple values (I think we might need it elsewhere. And even if not,
> I would prefer to have this in a function intead of stuffing it into
> relay_read_http).
>
> A minor question is if the lookup should be done before or after
> applying the filters (relay_test - my diff does it afterwards, the
> current code does it before).
>
> Tests? Comments? Ok?
>
I keep on replying to my own diffs... the updated one below uses
strcasecmp instead of strcasestr in kv_find_value(). There is no need
for substring- or fn- matching in this function.
Reyk
Index: usr.sbin/relayd/http.h
===
RCS file: /cvs/src/usr.sbin/relayd/http.h,v
retrieving revision 1.10
diff -u -p -u -p -r1.10 http.h
--- usr.sbin/relayd/http.h 4 Mar 2019 21:25:03 - 1.10
+++ usr.sbin/relayd/http.h 8 May 2019 16:55:59 -
@@ -251,10 +251,4 @@ struct http_descriptor {
struct kvtreehttp_headers;
};
-struct relay_http_priv {
-#define HTTP_CONNECTION_UPGRADE0x01
-#define HTTP_UPGRADE_WEBSOCKET 0x02
- int http_upgrade_req;
-};
-
#endif /* HTTP_H */
Index: usr.sbin/relayd/relay.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
retrieving revision 1.242
diff -u -p -u -p -r1.242 relay.c
--- usr.sbin/relayd/relay.c 4 Mar 2019 21:25:03 - 1.242
+++ usr.sbin/relayd/relay.c 8 May 2019 16:56:00 -
@@ -1410,13 +1410,7 @@ relay_session(struct rsession *con)
return;
}
- if (rlay->rl_proto->type == RELAY_PROTO_HTTP) {
- if (relay_http_priv_init(con) == -1) {
- relay_close(con,
- "failed to allocate http session data", 1);
- return;
- }
- } else {
+ if (rlay->rl_proto->type != RELAY_PROTO_HTTP) {
if (rlay->rl_conf.fwdmode == FWD_TRANS)
relay_bindanyreq(con, 0, IPPROTO_TCP);
else if (relay_connect(con) == -1) {
Index: usr.sbin/relayd/relay_http.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
retrieving revision 1.72
diff -u -p -u -p -r1.72 relay_http.c
--- usr.sbin/relayd/relay_http.c4 Mar 2019 21:25:03 - 1.72
+++ usr.sbin/relayd/relay_http.c8 May 2019 16:56:01 -
@@ -110,17 +110,6 @@ relay_http_init(struct relay *rlay)
}
int
-relay_http_priv_init(struct rsession *con)
-{
- struct relay_http_priv *p;
- if ((p = calloc(1, sizeof(struct relay_http_priv))) == NULL)
- return (-1);
-
- con->se_priv = p;
- return (0);
-}
-
-int
relay_httpdesc_init(struct ctl_relay_event *cre)
{
struct http_descriptor *desc;
@@ -163,13 +152,13 @@ relay_read_http(struct bufferevent *bev,
struct relay*rlay = con->se_relay;
struct protocol *proto = rlay->rl_proto;
struct evbuffer *src = EVBUFFER_INPUT(bev);
- struct relay_http_priv *priv = con->se_priv;
char*line = NULL, *key, *value;
char*urlproto, *host, *path;
int action, unique, ret;
const char *errstr;
size_t size, linelen;
struct kv *hdr = NULL;
+ struct kv *upgrade = NULL, *upgrade_ws = NULL;
getmonotime(>se_tv_last);
cre->timedout = 0;
@@ -398,17 +387,6 @@ relay_read_http(struct bufferevent
vm.conf: boot-device
Hi,
A first stab at adding support for option `-B device' to vm.conf(5).
With the diff below, I'm able to add a dedicated VM to be used with
autoinstall(5):
vm "amd64-install" {
disable
boot $snapshots "bsd.rd"
disk $home "amd64.qcow2"
boot-device net
local interface
}
The name `boot-device' is of course up for debate. Also, should allow support
also be considered?
Comments? OK?
Index: parse.y
===
RCS file: /cvs/src/usr.sbin/vmd/parse.y,v
retrieving revision 1.50
diff -u -p -r1.50 parse.y
--- parse.y 13 Feb 2019 22:57:08 - 1.50
+++ parse.y 28 Apr 2019 13:54:50 -
@@ -120,12 +120,13 @@ typedef struct {
%token INCLUDE ERROR
-%token ADD ALLOW BOOT CDROM DISABLE DISK DOWN ENABLE FORMAT GROUP INET6
-%token INSTANCE INTERFACE LLADDR LOCAL LOCKED MEMORY NIFS OWNER PATH PREFIX
-%token RDOMAIN SIZE SOCKET SWITCH UP VM VMID
+%token ADD ALLOW BOOT BOOTDEVICE CDROM DISABLE DISK DOWN ENABLE FORMAT GROUP
+%token INET6 INSTANCE INTERFACE LLADDR LOCAL LOCKED MEMORY NET NIFS OWNER
+%token PATH PREFIX RDOMAIN SIZE SOCKET SWITCH UP VM VMID
%token NUMBER
%token STRING
%typelladdr
+%typebootdevice
%typedisable
%typeimage_format
%typelocal
@@ -456,6 +457,9 @@ vm_opts : disable {
}
vmc.vmc_flags |= VMOP_CREATE_KERNEL;
}
+ | BOOTDEVICE bootdevice {
+ vmc.vmc_bootdevice = $2;
+ }
| CDROM string {
if (vcp->vcp_cdrom[0] != '\0') {
yyerror("cdrom specified more than once");
@@ -703,6 +707,11 @@ disable: ENABLE{ $$ =
0; }
| DISABLE { $$ = 1; }
;
+bootdevice : CDROM { $$ = VMBOOTDEV_CDROM; }
+ | DISK { $$ = VMBOOTDEV_DISK; }
+ | NET { $$ = VMBOOTDEV_NET; }
+ ;
+
optcomma : ','
|
;
@@ -755,6 +764,7 @@ lookup(char *s)
{ "add",ADD },
{ "allow", ALLOW },
{ "boot", BOOT },
+ { "boot-device",BOOTDEVICE },
{ "cdrom", CDROM },
{ "disable",DISABLE },
{ "disk", DISK },
@@ -772,6 +782,7 @@ lookup(char *s)
{ "local", LOCAL },
{ "locked", LOCKED },
{ "memory", MEMORY },
+ { "net",NET },
{ "owner", OWNER },
{ "prefix", PREFIX },
{ "rdomain",RDOMAIN },
Index: vm.conf.5
===
RCS file: /cvs/src/usr.sbin/vmd/vm.conf.5,v
retrieving revision 1.42
diff -u -p -r1.42 vm.conf.5
--- vm.conf.5 7 Mar 2019 18:54:06 - 1.42
+++ vm.conf.5 28 Apr 2019 13:54:50 -
@@ -144,6 +144,13 @@ See
Kernel or BIOS image to load when booting the VM.
If not specified, the default is to boot using the BIOS image in
.Pa /etc/firmware/vmm-bios .
+.It Cm boot-device Ar device
+Force VM to boot of
+.Ar device
+which may be either
+.Ar cdrom , disk
+or
+.Ar net .
.It Cm cdrom Ar path
ISO image file.
.It Cm enable
Re: relayd websocket
On Wed, Mar 06, 2019 at 05:36:32PM +0100, Sebastian Benoit wrote:
> Rivo Nurges(rivo.nur...@smit.ee) on 2019.03.05 22:42:13 +:
> > Hi!
> >
> > On 3/5/19 10:36 PM, Claudio Jeker wrote:
> > > I guess that this would need strcasestr() instead of strcasecmp(), since
> > > you
> > > are looking for the substring "Upgrade" in value. Maybe more is needed if
> > > we want to be sure that 'Connection: Upgrade-maybe' does not match.
> >
> > You are correct about strcasestr. "Connection: Upgrade-maybe" would need
> > to have correct "Upgrade: websocket". Anyway, lets be strict.
> >
> > Does something like this make sense?
>
> i think the seperator list needs to include '\t'
> because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB.
>
> And i dont think you can mix "," with " \t" seperators,
> because otherwise "Foo Upgrade, Bar" will match.
>
> Something more is needed to parse elements of a header.
>
I would reshuffle the websocket handling a bit as I don't think that
we need the http priv struct. We can lookup the parsed headers later.
The attached diff moves it all into one places and introduces a new
function kv_find_value() that is able to to match headers with
multiple values (I think we might need it elsewhere. And even if not,
I would prefer to have this in a function intead of stuffing it into
relay_read_http).
A minor question is if the lookup should be done before or after
applying the filters (relay_test - my diff does it afterwards, the
current code does it before).
Tests? Comments? Ok?
Reyk
Index: usr.sbin/relayd/http.h
===
RCS file: /cvs/src/usr.sbin/relayd/http.h,v
retrieving revision 1.10
diff -u -p -u -p -r1.10 http.h
--- usr.sbin/relayd/http.h 4 Mar 2019 21:25:03 - 1.10
+++ usr.sbin/relayd/http.h 8 May 2019 16:17:38 -
@@ -251,10 +251,4 @@ struct http_descriptor {
struct kvtreehttp_headers;
};
-struct relay_http_priv {
-#define HTTP_CONNECTION_UPGRADE0x01
-#define HTTP_UPGRADE_WEBSOCKET 0x02
- int http_upgrade_req;
-};
-
#endif /* HTTP_H */
Index: usr.sbin/relayd/relay.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
retrieving revision 1.242
diff -u -p -u -p -r1.242 relay.c
--- usr.sbin/relayd/relay.c 4 Mar 2019 21:25:03 - 1.242
+++ usr.sbin/relayd/relay.c 8 May 2019 16:17:39 -
@@ -1410,13 +1410,7 @@ relay_session(struct rsession *con)
return;
}
- if (rlay->rl_proto->type == RELAY_PROTO_HTTP) {
- if (relay_http_priv_init(con) == -1) {
- relay_close(con,
- "failed to allocate http session data", 1);
- return;
- }
- } else {
+ if (rlay->rl_proto->type != RELAY_PROTO_HTTP) {
if (rlay->rl_conf.fwdmode == FWD_TRANS)
relay_bindanyreq(con, 0, IPPROTO_TCP);
else if (relay_connect(con) == -1) {
Index: usr.sbin/relayd/relay_http.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
retrieving revision 1.72
diff -u -p -u -p -r1.72 relay_http.c
--- usr.sbin/relayd/relay_http.c4 Mar 2019 21:25:03 - 1.72
+++ usr.sbin/relayd/relay_http.c8 May 2019 16:17:40 -
@@ -110,17 +110,6 @@ relay_http_init(struct relay *rlay)
}
int
-relay_http_priv_init(struct rsession *con)
-{
- struct relay_http_priv *p;
- if ((p = calloc(1, sizeof(struct relay_http_priv))) == NULL)
- return (-1);
-
- con->se_priv = p;
- return (0);
-}
-
-int
relay_httpdesc_init(struct ctl_relay_event *cre)
{
struct http_descriptor *desc;
@@ -163,13 +152,13 @@ relay_read_http(struct bufferevent *bev,
struct relay*rlay = con->se_relay;
struct protocol *proto = rlay->rl_proto;
struct evbuffer *src = EVBUFFER_INPUT(bev);
- struct relay_http_priv *priv = con->se_priv;
char*line = NULL, *key, *value;
char*urlproto, *host, *path;
int action, unique, ret;
const char *errstr;
size_t size, linelen;
struct kv *hdr = NULL;
+ struct kv *upgrade = NULL, *upgrade_ws = NULL;
getmonotime(>se_tv_last);
cre->timedout = 0;
@@ -398,17 +387,6 @@ relay_read_http(struct bufferevent *bev,
unique = 0;
if (cre->line != 1) {
- if (cre->dir == RELAY_DIR_REQUEST) {
- if (strcasecmp("Connection", key) == 0 &&
- strcasecmp("Upgrade", value) == 0)
-
Re: arm64: fix DEBUG_AGINTC
> Date: Wed, 8 May 2019 07:21:06 -0700
> From: Carlos Cardenas
>
> Fix kernel compile with DEBUG_AGINTC.
>
> OK?
Sure!
> +--+
> Carlos
>
> [2:text/plain Hide]
>
> Index: agintc.c
> ===
> RCS file: /cvs/src/sys/arch/arm64/dev/agintc.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 agintc.c
> --- agintc.c 7 Dec 2018 21:33:28 - 1.15
> +++ agintc.c 8 May 2019 03:44:51 -
> @@ -845,7 +845,6 @@ agintc_route(struct agintc_softc *sc, in
> #ifdef DEBUG_AGINTC
> printf("router %x irq %d val %016llx\n", GICD_IROUTER(irq),
> irq, ci->ci_mpidr & MPIDR_AFF);
> - val);
> #endif
> bus_space_write_8(sc->sc_iot, sc->sc_d_ioh,
> GICD_IROUTER(irq), ci->ci_mpidr & MPIDR_AFF);
Re: [PATCH] Parallelize sysupgrade downloads
On 5/7/19 10:39 PM, Ville Valkonen wrote:
Hello,
thanks for the great new tool, sysupgrade. Works like a charm.
While on it, I came with this patch to speed up the downloading.
It uses xargs -P to parallelize downloads (max 6, chosen from top of my head).
Also, removed trailing spaces in two lines.
Without parallel patch:
4m20.91s real 0m00.44s user 0m24.26s system
With parallel patch:
3m14.14s real 0m00.32s user 0m13.80s system
I wonder if saving a minute is worth the trouble. However, there seems
to be a more important problem with the patch. See below
diff --git usr.sbin/sysupgrade/sysupgrade.sh usr.sbin/sysupgrade/sysupgrade.sh
index 87de9ccf432..635b48186f5 100644
--- usr.sbin/sysupgrade/sysupgrade.sh
+++ usr.sbin/sysupgrade/sysupgrade.sh
@@ -44,7 +44,7 @@ unpriv()
_file=$2
shift 2
fi
- if [[ -n ${_file} ]]; then
+if [[ -n ${_file} ]]; then
>${_file}
chown "${_user}" "${_file}"
fi
@@ -69,6 +69,16 @@ rmel() {
echo -n "$_c"
}
+generate_urls()
+{
+OFS=$IFS
+IFS=$'\n'
+printf "${2}\n" | while read -r f; do
+printf "%s%s\n" "${1}" "${f}"
+done
+IFS=$OFS
+}
+
RELEASE=false
SNAP=false
FORCE=false
@@ -122,7 +132,7 @@ if [[ -e ${SETSDIR} ]]; then
ug_err "${SETSDIR} needs to be owned by root:wheel"
[[ $st_gid -eq 0 ]] ||
ug_err "${SETSDIR} needs to be owned by root:wheel"
-[[ $st_mode -eq 040755 ]] ||
+[[ $st_mode -eq 040755 ]] ||
ug_err "${SETSDIR} is not a directory with permissions 0755"
else
mkdir -p ${SETSDIR}
@@ -167,9 +177,13 @@ for f in $SETS; do
done
[[ -n ${OLD_FILES} ]] && rm ${OLD_FILES}
+cd ${SETSDIR}
+generate_urls $URL "$(echo ${DL} |tr ' ' '\n')" \
+| xargs -P 6 -n 1 ftp -VCm
Doesn't this download the files as root without using unpriv to drop the
downloader's privileges? This doesn't seem correct.
for f in ${DL}; do
-unpriv -f $f ftp -Vmo ${f} ${URL}${f}
+unpriv -f $f
Does unpriv do anything if you don't give it a command to run? These are
probably no-ops.
done
+cd -
echo Verifying sets.
[[ -n ${DL} ]] && unpriv cksum -qC SHA256 ${DL}
--
Kind regards,
Ville Valkonen
--
Jarkko
Re: update to PF pfctl(8) and pf.conf(5) manpages
Hi Alexandr,
Alexandr Nedvedicky wrote on Wed, May 08, 2019 at 04:55:57PM +0200:
> below is third iteration of pf.conf.5 manpage.
OK schwarze@
A few final nits inline.
Thank you again for doing this work.
Ingo
> diff --git a/share/man/man5/pf.conf.5 b/share/man/man5/pf.conf.5
[...]
> @@ -1237,6 +1244,22 @@ Various limits can be combined on a single line:
> .Bd -literal -offset indent
> set limit { states 2, frags 2000, src-nodes 2000 }
> .Ed
> +.Pp
> +.Xr pf 4
> +has the following defaults:
> +.Bl -column table-entries PFR_KENTRY_HIWAT_SMALL platform_dependent
> +.It states Ta Dv PFSTATE_HIWAT Ta Pq 10
> +.It tables Ta Dv PFR_KTABLE_HIWAT Ta Pq 1000
> +.It table-entries Ta Dv PFR_KENTRY_HIWAT Ta Pq 20
> +.It table-entries Ta Dv PFR_KENTRY_HIWAT_SMALL Ta Pq 10
> +.It frags Ta Dv NMBCLUSTERS/32 Ta Pq platform dependent
Ta Dv NMBCLUSTERS Ns /32 Ta
would be even better.
> +.El
> +.Pp
> +.Cm NMBCLUSTERS
Please make this line
.Dv NMBCLUSTERS
> +defines the total number of packets which can exist in-system at any one
> time.
> +Refer to
> +.In machine/param.h
> +to find out a value for system sepcific values.
s/sepcific/specific/
The following would sound better; is it accurate?
Refer to
.In machine/param.h
for the platform-specific value.
> @@ -1393,12 +1425,13 @@ set syncookies adaptive (start 25%, end 12%)
> .It Ic set Cm timeout Ar variable value
> .Bl -tag -width "src.track" -compact
> .It Cm frag
> -Seconds before an unassembled fragment is expired.
> +Seconds before an unassembled fragment is expired (60 by default).
> .It Cm interval
> -Interval between purging expired states and fragments.
> +Interval between purging expired states and fragments (10 seconds by
> default).
> .It Cm src.track
> Length of time to retain a source tracking entry after the last state
> -expires.
> +expires (0 by default, which means there is no global limit.
> +The value is defined by rule which creates state.).
I think articles are needed:
The value is defined by the rule which creates the state).
> @@ -1493,6 +1526,10 @@ set limit states 10
> .Pp
> With 9000 state table entries, the timeout values are scaled to 50%
> (tcp.first 60, tcp.established 43200).
> +.Pp
> +.Dq pfctl -F Reset
> +restores default values for those options: debug, all limit options,
> +loginterface, reassemble, skip, syncookies, all timeouts.
> .El
> .Sh QUEUEING
> Packets can be assigned to queues for the purpose of bandwidth
I think this would sound better:
restores default values for the following options: debug, all limit options,
loginterface, reassemble, skip, syncookies, and all timeouts.
Also, the new lines should go after the .El, not before it, like this:
.Pp
With 9000 state table entries, the timeout values are scaled to 50%
(tcp.first 60, tcp.established 43200).
.El
.Pp
.Dq pfctl -F Reset
restores default values for the following options: debug, all limit options,
loginterface, reassemble, skip, syncookies, and all timeouts.
.Sh QUEUEING
Either way, this seems mature enough for getting it in.
IMHO, no need to resend it again.
Yours,
Ingo
Re: update to PF pfctl(8) and pf.conf(5) manpages
Hello,
below is third iteration of pf.conf.5 manpage. The new addresses
issues as follows:
all excessive "'pfctl -F Reset' ..." are gone.
I've added simple sentence/paragraph at the end of OPTIONS
section:
+.Pp
+.Dq pfctl -F Reset
+restores default values for those options: debug, all limit options,
+loginterface, reassemble, skip, syncookies, all timeouts.
also using '.Dq' instead of cross reference
reference to 'OPTIONS' section in pfctl(8) got fixed to:
+Reset limits, timeouts and other options back to default settings.
+See the OPTIONS section in
+.Xr pf.conf 5
+for details.
those two above should address all comments raised by Ingo, Jason and Klemens.
thanks and
regards
sashan
8<---8<---8<--8<
diff --git a/sbin/pfctl/pfctl.8 b/sbin/pfctl/pfctl.8
index b7e941991ba..a207f95321c 100644
--- a/sbin/pfctl/pfctl.8
+++ b/sbin/pfctl/pfctl.8
@@ -198,7 +198,10 @@ Flush the tables.
.It Fl F Cm osfp
Flush the passive operating system fingerprints.
.It Fl F Cm Reset
-Reset limits, timeouts and options back to default settings.
+Reset limits, timeouts and other options back to default settings.
+See the OPTIONS section in
+.Xr pf.conf 5
+for details.
.It Fl F Cm all
Flush all of the above.
.El
diff --git a/share/man/man5/pf.conf.5 b/share/man/man5/pf.conf.5
index c3fa0d07e58..9b7b01c4306 100644
--- a/share/man/man5/pf.conf.5
+++ b/share/man/man5/pf.conf.5
@@ -1148,6 +1148,9 @@ A TCP RST is returned for blocked TCP packets,
an ICMP UNREACHABLE is returned for blocked UDP packets,
and all other packets are silently dropped.
.El
+.Pp
+The default value is
+.Cm drop .
.It Ic set Cm debug Ar level
Set the debug
.Ar level ,
@@ -1167,6 +1170,8 @@ and
These keywords correspond to the similar (LOG_) values specified to the
.Xr syslog 3
library routine.
+The default value is
+.Cm err .
.It Cm set Cm fingerprints Ar filename
Load fingerprints of known operating systems from the given
.Ar filename .
@@ -1177,6 +1182,8 @@ but can be overridden via this option.
Setting this option may leave a small period of time where the fingerprints
referenced by the currently active ruleset are inconsistent until the new
ruleset finishes loading.
+The default location for fingerprints is
+.Pa /etc/pf.os .
.It Ic set Cm hostid Ar number
The 32-bit hostid
.Ar number
@@ -1237,6 +1244,22 @@ Various limits can be combined on a single line:
.Bd -literal -offset indent
set limit { states 2, frags 2000, src-nodes 2000 }
.Ed
+.Pp
+.Xr pf 4
+has the following defaults:
+.Bl -column table-entries PFR_KENTRY_HIWAT_SMALL platform_dependent
+.It states Ta Dv PFSTATE_HIWAT Ta Pq 10
+.It tables Ta Dv PFR_KTABLE_HIWAT Ta Pq 1000
+.It table-entries Ta Dv PFR_KENTRY_HIWAT Ta Pq 20
+.It table-entries Ta Dv PFR_KENTRY_HIWAT_SMALL Ta Pq 10
+.It frags Ta Dv NMBCLUSTERS/32 Ta Pq platform dependent
+.El
+.Pp
+.Cm NMBCLUSTERS
+defines the total number of packets which can exist in-system at any one time.
+Refer to
+.In machine/param.h
+to find out a value for system sepcific values.
.It Ic set Cm loginterface Ar interface | Cm none
Enable collection of packet and byte count statistics for the given
interface or interface group.
@@ -1253,6 +1276,9 @@ collects statistics on the interface named dc0:
One can disable the loginterface using:
.Pp
.Dl set loginterface none
+.Pp
+The default value is
+.Cm none .
.It Ic set Cm optimization Ar environment
Optimize state timeouts for one of the following network environments:
.Pp
@@ -1275,6 +1301,9 @@ Suitable for almost all networks.
Alias for
.Cm high-latency .
.El
+.Pp
+The default value is
+.Cm normal .
.It Ic set Cm reassemble yes | no Op Cm no-df
The
.Cm reassemble
@@ -1292,6 +1321,8 @@ instead of being dropped;
the reassembled packet will have the
.Dq dont-fragment
bit cleared.
+The default value is
+.Cm yes .
.It Ic set Cm ruleset-optimization Ar level
.Bl -tag -width profile -compact
.It Cm basic
@@ -1341,6 +1372,7 @@ packet filtering is not desired and can have unexpected
effects.
.Ar ifspec
is only evaluated when the ruleset is loaded; interfaces created
later will not be skipped.
+PF filters traffic on all interfaces by default.
.It Ic set Cm state-defaults Ar state-option , ...
The
.Cm state-defaults
@@ -1393,12 +1425,13 @@ set syncookies adaptive (start 25%, end 12%)
.It Ic set Cm timeout Ar variable value
.Bl -tag -width "src.track" -compact
.It Cm frag
-Seconds before an unassembled fragment is expired.
+Seconds before an unassembled fragment is expired (60 by default).
.It Cm interval
-Interval between purging expired states and fragments.
+Interval between purging expired states and fragments (10 seconds by default).
.It Cm src.track
Length of time to retain a source tracking entry after the last state
-expires.
+expires (0 by default, which means there is no global limit.
+The value is
relayd: fix filter rules with forward to statement
Hi,
the attached diff fixes filter rules with "forward to" statement in
persistent (keep-alive) connections. See the XXX comment below.
```relayd.conf
log connection
table {
127.0.0.1
}
table {
127.0.0.1
}
table {
127.0.0.1
}
http protocol pathfwd {
return error
# XXX The following workaround is not needed anymore:
#match header set "Connection" value "close"
pass path "/a/*" forward to
pass path "/b/*" forward to
#match request path log "*"
}
relay pathfwd {
listen on 0.0.0.0 port 80
protocol pathfwd
forward to port 8082
forward to port 8080
forward to port 8081
}
```
OK?
reyk
Index: usr.sbin/relayd/relay.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
retrieving revision 1.242
diff -u -p -u -p -r1.242 relay.c
--- usr.sbin/relayd/relay.c 4 Mar 2019 21:25:03 - 1.242
+++ usr.sbin/relayd/relay.c 8 May 2019 14:26:40 -
@@ -76,11 +76,14 @@ int relay_tls_ctx_create(struct relay
voidrelay_tls_transaction(struct rsession *,
struct ctl_relay_event *);
voidrelay_tls_handshake(int, short, void *);
-voidrelay_connect_retry(int, short, void *);
voidrelay_tls_connected(struct ctl_relay_event *);
voidrelay_tls_readcb(int, short, void *);
voidrelay_tls_writecb(int, short, void *);
+voidrelay_connect_retry(int, short, void *);
+voidrelay_connect_state(struct rsession *,
+ struct ctl_relay_event *, enum relay_state);
+
extern void bufferevent_read_pressure_cb(struct evbuffer *, size_t,
size_t, void *);
@@ -654,6 +657,7 @@ relay_socket_listen(struct sockaddr_stor
void
relay_connected(int fd, short sig, void *arg)
{
+ char obuf[128];
struct rsession *con = arg;
struct relay*rlay = con->se_relay;
struct protocol *proto = rlay->rl_proto;
@@ -696,6 +700,22 @@ relay_connected(int fd, short sig, void
DPRINTF("%s: session %d: successful", __func__, con->se_id);
+ /* Log destination if it was changed in a keep-alive connection */
+ if ((con->se_table != con->se_table0) &&
+ (env->sc_conf.opts & (RELAYD_OPT_LOGCON|RELAYD_OPT_LOGCONERR))) {
+ con->se_table0 = con->se_table;
+ memset(, 0, sizeof(obuf));
+ (void)print_host(>se_out.ss, obuf, sizeof(obuf));
+ if (asprintf(, " -> %s:%d",
+ obuf, ntohs(con->se_out.port)) == -1) {
+ relay_abort_http(con, 500,
+ "connection changed and asprintf failed", 0);
+ return;
+ }
+ relay_log(con, msg);
+ free(msg);
+ }
+
switch (rlay->rl_proto->type) {
case RELAY_PROTO_HTTP:
if (relay_httpdesc_init(out) == -1) {
@@ -1465,6 +1485,17 @@ relay_bindany(int fd, short event, void
}
void
+relay_connect_state(struct rsession *con, struct ctl_relay_event *cre,
+enum relay_state new)
+{
+ DPRINTF("%s: session %d: %s state %s -> %s",
+ __func__, con->se_id,
+ cre->dir == RELAY_DIR_REQUEST ? "accept" : "connect",
+ relay_state(cre->state), relay_state(new));
+ cre->state = new;
+}
+
+void
relay_connect_retry(int fd, short sig, void *arg)
{
struct timeval evtpause = { 1, 0 };
@@ -1533,9 +1564,9 @@ relay_connect_retry(int fd, short sig, v
}
if (rlay->rl_conf.flags & F_TLSINSPECT)
- con->se_out.state = STATE_PRECONNECT;
+ relay_connect_state(con, >se_out, STATE_PRECONNECT);
else
- con->se_out.state = STATE_CONNECTED;
+ relay_connect_state(con, >se_out, STATE_CONNECTED);
relay_inflight--;
DPRINTF("%s: inflight decremented, now %d",__func__, relay_inflight);
@@ -1560,7 +1591,7 @@ relay_preconnect(struct rsession *con)
con->se_id, privsep_process);
rv = relay_connect(con);
if (con->se_out.state == STATE_CONNECTED)
- con->se_out.state = STATE_PRECONNECT;
+ relay_connect_state(con, >se_out, STATE_PRECONNECT);
return (rv);
}
@@ -1585,7 +1616,7 @@ relay_connect(struct rsession *con)
return (-1);
}
relay_connected(con->se_out.s, EV_WRITE, con);
- con->se_out.state = STATE_CONNECTED;
+ relay_connect_state(con, >se_out, STATE_CONNECTED);
return (0);
}
@@ -1642,7 +1673,7 @@ relay_connect(struct rsession *con)
evtimer_add(>rl_evt, );
/* this connect is pending */
- con->se_out.state = STATE_PENDING;
+
less(1) UTF-8 cleanup: cvt.c
Hi Todd,
Todd C. Miller wrote on Wed, May 08, 2019 at 06:36:30AM -0600:
> On Tue, 07 May 2019 23:32:20 +0200, Ingo Schwarze wrote:
>> Here is basic cleanup of the last major function in line.c, pshift().
> Looks good. OK millert@
Thanks for checking, i will commit the line.c patch later today.
Here is a patch cleaning up all major issues in the file cvt.c.
They are all in the function cvt_text(), which is used for
setting up patterns and for searching. One minor issue remains,
usage of the not-so-nice function is_ansi_middle(), but this doesn't
seem the right time yet to worry about that detail.
This patch deletes the abominable function put_wchar() which accepts
many invalid codepoints and happily produces invalid results,
replacing its only call with the standard function wctomb(3). Note
that uncharacteristically, no error handling is needed after the
call to wctomb(3) because the wchar_t passed into it just came out
of either mbtowc(3) with a positive return value or out of towlower(3),
so we already know it is valid and not L'\0'.
The patch also gets rid of one automatic LWCHAR variable and one
call to the bad function step_char().
Note that memory allocation before calls to cvt_text() - in file
pattern.c, function compile_pattern() and in file search.c, function
search_range() - looks suspicious. In theory, the UTF-8 representation
of an uppercase character might contain fewer bytes than the UTF-8
representation of the lowercase character towlower() converts it
to, in which case cvt_text() would write beyond the end of the dst
buffer. I don't know whether uppercase characters with this property
actually exist in Unicode or might appear in the future - but no
doubt the Unicode character table is dark and full of terrors. Then
again, that's a different (potential) issue which i won't try to
fix with the present patch.
I tested that searching for strings containing non-ASCII UTF-8
characters still works with -I, with -i, and without either, in all
cases with the correct case-related behaviour.
OK?
Ingo
Index: charset.c
===
RCS file: /cvs/src/usr.bin/less/charset.c,v
retrieving revision 1.22
diff -u -p -r1.22 charset.c
--- charset.c 7 May 2019 14:26:38 - 1.22
+++ charset.c 8 May 2019 14:03:01 -
@@ -329,48 +329,6 @@ get_wchar(const char *p)
}
/*
- * Store a character into a UTF-8 string.
- */
-void
-put_wchar(char **pp, LWCHAR ch)
-{
- if (!utf_mode || ch < 0x80) {
- /* 0xxx */
- *(*pp)++ = (char)ch;
- } else if (ch < 0x800) {
- /* 110x 10xx */
- *(*pp)++ = (char)(0xC0 | ((ch >> 6) & 0x1F));
- *(*pp)++ = (char)(0x80 | (ch & 0x3F));
- } else if (ch < 0x1) {
- /* 1110 10xx 10xx */
- *(*pp)++ = (char)(0xE0 | ((ch >> 12) & 0x0F));
- *(*pp)++ = (char)(0x80 | ((ch >> 6) & 0x3F));
- *(*pp)++ = (char)(0x80 | (ch & 0x3F));
- } else if (ch < 0x20) {
- /* 0xxx 10xx 10xx 10xx */
- *(*pp)++ = (char)(0xF0 | ((ch >> 18) & 0x07));
- *(*pp)++ = (char)(0x80 | ((ch >> 12) & 0x3F));
- *(*pp)++ = (char)(0x80 | ((ch >> 6) & 0x3F));
- *(*pp)++ = (char)(0x80 | (ch & 0x3F));
- } else if (ch < 0x400) {
- /* 10xx 10xx 10xx 10xx 10xx */
- *(*pp)++ = (char)(0xF0 | ((ch >> 24) & 0x03));
- *(*pp)++ = (char)(0x80 | ((ch >> 18) & 0x3F));
- *(*pp)++ = (char)(0x80 | ((ch >> 12) & 0x3F));
- *(*pp)++ = (char)(0x80 | ((ch >> 6) & 0x3F));
- *(*pp)++ = (char)(0x80 | (ch & 0x3F));
- } else {
- /* 110x 10xx 10xx 10xx 10xx 10xx */
- *(*pp)++ = (char)(0xF0 | ((ch >> 30) & 0x01));
- *(*pp)++ = (char)(0x80 | ((ch >> 24) & 0x3F));
- *(*pp)++ = (char)(0x80 | ((ch >> 18) & 0x3F));
- *(*pp)++ = (char)(0x80 | ((ch >> 12) & 0x3F));
- *(*pp)++ = (char)(0x80 | ((ch >> 6) & 0x3F));
- *(*pp)++ = (char)(0x80 | (ch & 0x3F));
- }
-}
-
-/*
* Step forward or backward one character in a string.
*/
LWCHAR
Index: cvt.c
===
RCS file: /cvs/src/usr.bin/less/cvt.c,v
retrieving revision 1.9
diff -u -p -r1.9 cvt.c
--- cvt.c 26 Feb 2019 11:01:54 - 1.9
+++ cvt.c 8 May 2019 14:03:01 -
@@ -60,7 +60,8 @@ cvt_text(char *odst, char *osrc, int *ch
char *edst = odst;
char *src;
char *src_end;
- LWCHAR ch;
+ wchar_t ch;
+ int len;
if (lenp != NULL)
src_end = osrc + *lenp;
@@ -70,8 +71,10 @@ cvt_text(char *odst, char *osrc, int *ch
for (src = osrc, dst = odst; src < src_end; ) {
int src_pos
arm64: fix DEBUG_AGINTC
Fix kernel compile with DEBUG_AGINTC.
OK?
+--+
Carlos
Index: agintc.c
===
RCS file: /cvs/src/sys/arch/arm64/dev/agintc.c,v
retrieving revision 1.15
diff -u -p -r1.15 agintc.c
--- agintc.c7 Dec 2018 21:33:28 - 1.15
+++ agintc.c8 May 2019 03:44:51 -
@@ -845,7 +845,6 @@ agintc_route(struct agintc_softc *sc, in
#ifdef DEBUG_AGINTC
printf("router %x irq %d val %016llx\n", GICD_IROUTER(irq),
irq, ci->ci_mpidr & MPIDR_AFF);
- val);
#endif
bus_space_write_8(sc->sc_iot, sc->sc_d_ioh,
GICD_IROUTER(irq), ci->ci_mpidr & MPIDR_AFF);
Re: relayd websocket
Hi!
Anyone?
Rivo
On 2019-03-12 21:50, Rivo Nurges wrote:
> Hi!
>
> Bump...
>
> Rivo
>
> On 3/6/19 10:57 PM, Rivo Nurges wrote:
>> Hi!
>>
>>
>> On 3/6/19 10:20 PM, Rivo Nurges wrote:
>>> On 3/6/19 6:36 PM, Sebastian Benoit wrote:
> Does something like this make sense?
i think the seperator list needs to include '\t'
because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB.
And i dont think you can mix "," with " \t" seperators,
because otherwise "Foo Upgrade, Bar" will match.
Something more is needed to parse elements of a header.
>>>
>>> Oh yeah. I'll work on that.
>>
>> So here comes the next version. Works with both spaces and tabs.
>>
>> Index: usr.sbin/relayd/relay_http.c
>> ===
>> RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
>> retrieving revision 1.72
>> diff -u -p -r1.72 relay_http.c
>> --- usr.sbin/relayd/relay_http.c 4 Mar 2019 21:25:03 - 1.72
>> +++ usr.sbin/relayd/relay_http.c 6 Mar 2019 20:53:59 -
>> @@ -36,6 +36,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> #include "relayd.h"
>> #include "http.h"
>> @@ -166,6 +167,7 @@ relay_read_http(struct bufferevent *bev,
>> struct relay_http_priv *priv = con->se_priv;
>> char*line = NULL, *key, *value;
>> char*urlproto, *host, *path;
>> +char*valuecopy, *valuepart;
>> int action, unique, ret;
>> const char *errstr;
>> size_t size, linelen;
>> @@ -399,10 +401,19 @@ relay_read_http(struct bufferevent *bev,
>>
>> if (cre->line != 1) {
>> if (cre->dir == RELAY_DIR_REQUEST) {
>> -if (strcasecmp("Connection", key) == 0 &&
>> -strcasecmp("Upgrade", value) == 0)
>> -priv->http_upgrade_req |=
>> -HTTP_CONNECTION_UPGRADE;
>> +if (strcasecmp("Connection", key) == 0) {
>> +valuecopy = strdup(value);
>> +while ((valuepart = strsep(,
>> +",")) != NULL) {
>> +while (isblank(*valuepart))
>> +valuepart = [1];
>> +if (strcasecmp("Upgrade", valuepart)
>> +== 0)
>> +priv->http_upgrade_req |=
>> +HTTP_CONNECTION_UPGRADE;
>> +}
>> +free(valuecopy);
>> +}
>> if (strcasecmp("Upgrade", key) == 0 &&
>> strcasecmp("websocket", value) == 0)
>> priv->http_upgrade_req |=
>>
>>
>> begin-base64 644 websocket3.diff
>> SW5kZXg6IHVzci5zYmluL3JlbGF5ZC9yZWxheV9odHRwLmMKPT09PT09PT09PT09PT09PT09PT09
>> PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQpSQ1MgZmlsZTog
>> L2N2cy9zcmMvdXNyLnNiaW4vcmVsYXlkL3JlbGF5X2h0dHAuYyx2CnJldHJpZXZpbmcgcmV2aXNp
>> b24gMS43MgpkaWZmIC11IC1wIC1yMS43MiByZWxheV9odHRwLmMKLS0tIHVzci5zYmluL3JlbGF5
>> ZC9yZWxheV9odHRwLmMJNCBNYXIgMjAxOSAyMToyNTowMyAtMDAwMAkxLjcyCisrKyB1c3Iuc2Jp
>> bi9yZWxheWQvcmVsYXlfaHR0cC5jCTYgTWFyIDIwMTkgMjA6NTM6NTkgLTAwMDAKQEAgLTM2LDYg
>> KzM2LDcgQEAKICNpbmNsdWRlIDxzaXBoYXNoLmg+CiAjaW5jbHVkZSA8aW1zZy5oPgogI2luY2x1
>> ZGUgPHVuaXN0ZC5oPgorI2luY2x1ZGUgPGN0eXBlLmg+CiAKICNpbmNsdWRlICJyZWxheWQuaCIK
>> ICNpbmNsdWRlICJodHRwLmgiCkBAIC0xNjYsNiArMTY3LDcgQEAgcmVsYXlfcmVhZF9odHRwKHN0
>> cnVjdCBidWZmZXJldmVudCAqYmV2LAogCXN0cnVjdCByZWxheV9odHRwX3ByaXYJKnByaXYgPSBj
>> b24tPnNlX3ByaXY7CiAJY2hhcgkJCSpsaW5lID0gTlVMTCwgKmtleSwgKnZhbHVlOwogCWNoYXIJ
>> CQkqdXJscHJvdG8sICpob3N0LCAqcGF0aDsKKwljaGFyCQkJKnZhbHVlY29weSwgKnZhbHVlcGFy
>> dDsKIAlpbnQJCQkgYWN0aW9uLCB1bmlxdWUsIHJldDsKIAljb25zdCBjaGFyCQkqZXJyc3RyOwog
>> CXNpemVfdAkJCSBzaXplLCBsaW5lbGVuOwpAQCAtMzk5LDEwICs0MDEsMTkgQEAgcmVsYXlfcmVh
>> ZF9odHRwKHN0cnVjdCBidWZmZXJldmVudCAqYmV2LAogCiAJCWlmIChjcmUtPmxpbmUgIT0gMSkg
>> ewogCQkJaWYgKGNyZS0+ZGlyID09IFJFTEFZX0RJUl9SRVFVRVNUKSB7Ci0JCQkJaWYgKHN0cmNh
>> c2VjbXAoIkNvbm5lY3Rpb24iLCBrZXkpID09IDAgJiYKLQkJCQkgICAgc3RyY2FzZWNtcCgiVXBn
>> cmFkZSIsIHZhbHVlKSA9PSAwKQotCQkJCQlwcml2LT5odHRwX3VwZ3JhZGVfcmVxIHw9Ci0JCQkJ
>> CSAgICBIVFRQX0NPTk5FQ1RJT05fVVBHUkFERTsKKwkJCQlpZiAoc3RyY2FzZWNtcCgiQ29ubmVj
>> dGlvbiIsIGtleSkgPT0gMCkgeworCQkJCSAgICB2YWx1ZWNvcHkgPSBzdHJkdXAodmFsdWUpOwor
>> CQkJCSAgICB3aGlsZSAoKHZhbHVlcGFydCA9IHN0cnNlcCgmdmFsdWVjb3B5LAorCQkJCQkiLCIp
>> KSAhPSBOVUxMKSB7CisJCQkJCXdoaWxlIChpc2JsYW5rKCp2YWx1ZXBhcnQpKQorCQkJCQkgICAg
>> dmFsdWVwYXJ0ID0gJnZhbHVlcGFydFsxXTsKKwkJCQkgICAgCWlmIChzdHJjYXNlY21wKCJVcGdy
>>
Re: less(1) UTF-8 cleanup: pshift()
On Tue, 07 May 2019 23:32:20 +0200, Ingo Schwarze wrote: > Here is basic cleanup of the last major function in line.c, pshift(). > Several minor issues still remain in the file, but those are for later. > > This gets rid of two LWCHAR variables, one call to utf_len(), > get_wchar(), is_composing_char(), is_combining_char(), and > control_char() each and two calls to is_wide_char(). > > Note the change from "continue" to "break" in the "width == 2" > block. It is an improvement because the double-width character did > not move off-screen completely, its second half is still there, > represented as a single-width space. So whatever follows it does > absolutely not move off-screen, not even if what follows is zero > width, so it must not be inspected by the loop. > > Since the variable names are far from intuitive, improve and add > comments. Also, the comment above pshift() is simply wrong. > The function does not discard a specific number of characters. Looks good. OK millert@ - todd
