Re: ure(4): add support for rtl8153b
On Tue, Aug 27, 2019 at 10:50:05AM +0800, Kevin Lo wrote:
> Hi,
>
> The diff below adds support for RTL8153B to ure(4).
> Tested on Totolink u1003.
>
morning. the changes for ure.4 are fine, but can you adjust the entry in
usb.4 accordingly too, please?
thanks,
jmc
> Index: share/man/man4/ure.4
> ===
> RCS file: /cvs/src/share/man/man4/ure.4,v
> retrieving revision 1.5
> diff -u -p -u -p -r1.5 ure.4
> --- share/man/man4/ure.4 16 Apr 2017 20:26:34 - 1.5
> +++ share/man/man4/ure.4 27 Aug 2019 02:36:47 -
> @@ -31,7 +31,7 @@
> .Os
> .Sh NAME
> .Nm ure
> -.Nd RealTek RTL8152/RTL8153 10/100/Gigabit USB Ethernet device
> +.Nd RealTek RTL8152/RTL8153/RTL8153B 10/100/Gigabit USB Ethernet device
> .Sh SYNOPSIS
> .Cd "ure* at uhub?"
> .Cd "rgephy* at mii?"
> @@ -40,13 +40,12 @@
> The
> .Nm
> driver provides support for USB Ethernet adapters based on the RealTek
> -RTL8152 USB Fast Ethernet and RTL8153 USB Gigabit Ethernet controller
> -chips.
> +RTL8152, RTL8153 and RTL8153B chipsets.
> .Pp
> The RTL8152 contains an integrated Fast Ethernet MAC, which supports
> both 10 and 100Mbps speeds in either full or half duplex.
> -The RTL8153 has a Gigabit Ethernet MAC and additionally supports
> -1000Mbps speeds.
> +The RTL8153 and RTL8153B have Gigabit Ethernet MACs and additionally
> +support 1000Mbps speeds.
> .Pp
> For more information on configuring this device, see
> .Xr ifconfig 8 .
> Index: sys/dev/usb/if_ure.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_ure.c,v
> retrieving revision 1.10
> diff -u -p -u -p -r1.10 if_ure.c
> --- sys/dev/usb/if_ure.c 2 Nov 2018 21:32:30 - 1.10
> +++ sys/dev/usb/if_ure.c 27 Aug 2019 02:36:48 -
> @@ -1,6 +1,6 @@
> /* $OpenBSD: if_ure.c,v 1.10 2018/11/02 21:32:30 jcs Exp $ */
> /*-
> - * Copyright (c) 2015-2016 Kevin Lo
> + * Copyright (c) 2015, 2016, 2019 Kevin Lo
> * All rights reserved.
> *
> * Redistribution and use in source and binary forms, with or without
> @@ -124,13 +124,31 @@ voidure_tick(void *);
>
> int ure_ifmedia_upd(struct ifnet *);
> void ure_ifmedia_sts(struct ifnet *, struct ifmediareq *);
> +void ure_iff(struct ure_softc *);
> +void ure_rxvlan(struct ure_softc *);
> int ure_ioctl(struct ifnet *, u_long, caddr_t);
> void ure_rtl8152_init(struct ure_softc *);
> void ure_rtl8153_init(struct ure_softc *);
> +void ure_rtl8153b_init(struct ure_softc *);
> +void ure_rtl8152_nic_reset(struct ure_softc *);
> +void ure_rtl8153_nic_reset(struct ure_softc *);
> +void ure_rtl8153_phy_status(struct ure_softc *, int);
> +void ure_reset_bmu(struct ure_softc *);
> void ure_disable_teredo(struct ure_softc *);
> -void ure_init_fifo(struct ure_softc *);
> -
>
> +#define URE_SETBIT_1(sc, reg, index, x) \
> + ure_write_1(sc, reg, index, ure_read_1(sc, reg, index) | (x))
> +#define URE_SETBIT_2(sc, reg, index, x) \
> + ure_write_2(sc, reg, index, ure_read_2(sc, reg, index) | (x))
> +#define URE_SETBIT_4(sc, reg, index, x) \
> + ure_write_4(sc, reg, index, ure_read_4(sc, reg, index) | (x))
> +
> +#define URE_CLRBIT_1(sc, reg, index, x) \
> + ure_write_1(sc, reg, index, ure_read_1(sc, reg, index) & ~(x))
> +#define URE_CLRBIT_2(sc, reg, index, x) \
> + ure_write_2(sc, reg, index, ure_read_2(sc, reg, index) & ~(x))
> +#define URE_CLRBIT_4(sc, reg, index, x) \
> + ure_write_4(sc, reg, index, ure_read_4(sc, reg, index) & ~(x))
>
> int
> ure_ctl(struct ure_softc *sc, uint8_t rw, uint16_t val, uint16_t index,
> @@ -166,7 +184,6 @@ int
> ure_read_mem(struct ure_softc *sc, uint16_t addr, uint16_t index,
> void *buf, int len)
> {
> -
> return (ure_ctl(sc, URE_CTL_READ, addr, index, buf, len));
> }
>
> @@ -174,16 +191,15 @@ int
> ure_write_mem(struct ure_softc *sc, uint16_t addr, uint16_t index,
> void *buf, int len)
> {
> -
> return (ure_ctl(sc, URE_CTL_WRITE, addr, index, buf, len));
> }
>
> uint8_t
> ure_read_1(struct ure_softc *sc, uint16_t reg, uint16_t index)
> {
> - uint32_t val;
> - uint8_t temp[4];
> - uint8_t shift;
> + uint32_tval;
> + uint8_t temp[4];
> + uint8_t shift;
>
> shift = (reg & 3) << 3;
> reg &= ~3;
> @@ -198,9 +214,9 @@ ure_read_1(struct ure_softc *sc, uint16_
> uint16_t
> ure_read_2(struct ure_softc *sc, uint16_t reg, uint16_t index)
> {
> - uint32_t val;
> - uint8_t temp[4];
> - uint8_t shift;
> + uint32_tval;
> + uint8_t temp[4];
> + uint8_t shift;
>
> shift = (reg & 2) << 3;
> reg &= ~3;
> @@ -215,7 +231,7 @@ ure_read_2(struct ure_softc *sc, uint16_
> uint32_t
> ure_read_4(struct ure_softc *sc, uint16_t reg, uint16_t index)
> {
> - uint8_t
Re: ure(4): add support for rtl8153b
On Tue, Aug 27, 2019 at 12:34:55PM +0800, Kevin Lo wrote: > On Tue, Aug 27, 2019 at 10:50:05AM +0800, Kevin Lo wrote: > > > > Hi, > > > > The diff below adds support for RTL8153B to ure(4). > > Tested on Totolink u1003. > > I should mention that it's based on Linux r8152 driver. My bad, should be "study of".
Re: ure(4): add support for rtl8153b
On Tue, Aug 27, 2019 at 10:50:05AM +0800, Kevin Lo wrote: > > Hi, > > The diff below adds support for RTL8153B to ure(4). > Tested on Totolink u1003. I should mention that it's based on Linux r8152 driver.
ure(4): add support for rtl8153b
Hi,
The diff below adds support for RTL8153B to ure(4).
Tested on Totolink u1003.
Index: share/man/man4/ure.4
===
RCS file: /cvs/src/share/man/man4/ure.4,v
retrieving revision 1.5
diff -u -p -u -p -r1.5 ure.4
--- share/man/man4/ure.416 Apr 2017 20:26:34 - 1.5
+++ share/man/man4/ure.427 Aug 2019 02:36:47 -
@@ -31,7 +31,7 @@
.Os
.Sh NAME
.Nm ure
-.Nd RealTek RTL8152/RTL8153 10/100/Gigabit USB Ethernet device
+.Nd RealTek RTL8152/RTL8153/RTL8153B 10/100/Gigabit USB Ethernet device
.Sh SYNOPSIS
.Cd "ure* at uhub?"
.Cd "rgephy* at mii?"
@@ -40,13 +40,12 @@
The
.Nm
driver provides support for USB Ethernet adapters based on the RealTek
-RTL8152 USB Fast Ethernet and RTL8153 USB Gigabit Ethernet controller
-chips.
+RTL8152, RTL8153 and RTL8153B chipsets.
.Pp
The RTL8152 contains an integrated Fast Ethernet MAC, which supports
both 10 and 100Mbps speeds in either full or half duplex.
-The RTL8153 has a Gigabit Ethernet MAC and additionally supports
-1000Mbps speeds.
+The RTL8153 and RTL8153B have Gigabit Ethernet MACs and additionally
+support 1000Mbps speeds.
.Pp
For more information on configuring this device, see
.Xr ifconfig 8 .
Index: sys/dev/usb/if_ure.c
===
RCS file: /cvs/src/sys/dev/usb/if_ure.c,v
retrieving revision 1.10
diff -u -p -u -p -r1.10 if_ure.c
--- sys/dev/usb/if_ure.c2 Nov 2018 21:32:30 - 1.10
+++ sys/dev/usb/if_ure.c27 Aug 2019 02:36:48 -
@@ -1,6 +1,6 @@
/* $OpenBSD: if_ure.c,v 1.10 2018/11/02 21:32:30 jcs Exp $ */
/*-
- * Copyright (c) 2015-2016 Kevin Lo
+ * Copyright (c) 2015, 2016, 2019 Kevin Lo
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@@ -124,13 +124,31 @@ void ure_tick(void *);
inture_ifmedia_upd(struct ifnet *);
void ure_ifmedia_sts(struct ifnet *, struct ifmediareq *);
+void ure_iff(struct ure_softc *);
+void ure_rxvlan(struct ure_softc *);
inture_ioctl(struct ifnet *, u_long, caddr_t);
void ure_rtl8152_init(struct ure_softc *);
void ure_rtl8153_init(struct ure_softc *);
+void ure_rtl8153b_init(struct ure_softc *);
+void ure_rtl8152_nic_reset(struct ure_softc *);
+void ure_rtl8153_nic_reset(struct ure_softc *);
+void ure_rtl8153_phy_status(struct ure_softc *, int);
+void ure_reset_bmu(struct ure_softc *);
void ure_disable_teredo(struct ure_softc *);
-void ure_init_fifo(struct ure_softc *);
-
+#define URE_SETBIT_1(sc, reg, index, x) \
+ ure_write_1(sc, reg, index, ure_read_1(sc, reg, index) | (x))
+#define URE_SETBIT_2(sc, reg, index, x) \
+ ure_write_2(sc, reg, index, ure_read_2(sc, reg, index) | (x))
+#define URE_SETBIT_4(sc, reg, index, x) \
+ ure_write_4(sc, reg, index, ure_read_4(sc, reg, index) | (x))
+
+#define URE_CLRBIT_1(sc, reg, index, x) \
+ ure_write_1(sc, reg, index, ure_read_1(sc, reg, index) & ~(x))
+#define URE_CLRBIT_2(sc, reg, index, x) \
+ ure_write_2(sc, reg, index, ure_read_2(sc, reg, index) & ~(x))
+#define URE_CLRBIT_4(sc, reg, index, x) \
+ ure_write_4(sc, reg, index, ure_read_4(sc, reg, index) & ~(x))
int
ure_ctl(struct ure_softc *sc, uint8_t rw, uint16_t val, uint16_t index,
@@ -166,7 +184,6 @@ int
ure_read_mem(struct ure_softc *sc, uint16_t addr, uint16_t index,
void *buf, int len)
{
-
return (ure_ctl(sc, URE_CTL_READ, addr, index, buf, len));
}
@@ -174,16 +191,15 @@ int
ure_write_mem(struct ure_softc *sc, uint16_t addr, uint16_t index,
void *buf, int len)
{
-
return (ure_ctl(sc, URE_CTL_WRITE, addr, index, buf, len));
}
uint8_t
ure_read_1(struct ure_softc *sc, uint16_t reg, uint16_t index)
{
- uint32_t val;
- uint8_t temp[4];
- uint8_t shift;
+ uint32_tval;
+ uint8_t temp[4];
+ uint8_t shift;
shift = (reg & 3) << 3;
reg &= ~3;
@@ -198,9 +214,9 @@ ure_read_1(struct ure_softc *sc, uint16_
uint16_t
ure_read_2(struct ure_softc *sc, uint16_t reg, uint16_t index)
{
- uint32_t val;
- uint8_t temp[4];
- uint8_t shift;
+ uint32_tval;
+ uint8_t temp[4];
+ uint8_t shift;
shift = (reg & 2) << 3;
reg &= ~3;
@@ -215,7 +231,7 @@ ure_read_2(struct ure_softc *sc, uint16_
uint32_t
ure_read_4(struct ure_softc *sc, uint16_t reg, uint16_t index)
{
- uint8_t temp[4];
+ uint8_t temp[4];
ure_read_mem(sc, reg, index, , 4);
return (UGETDW(temp));
@@ -224,9 +240,9 @@ ure_read_4(struct ure_softc *sc, uint16_
int
ure_write_1(struct ure_softc *sc, uint16_t reg, uint16_t index, uint32_t val)
{
- uint16_t byen;
- uint8_t temp[4];
- uint8_t shift;
+ uint16_t
Re: unveil vmd(8)'s priv process
On Mon, Aug 26, 2019 at 11:01:26AM +0100, Ricardo Mestre wrote:
> Hi,
>
> Currently vmd(8) has 3 processes that run under chroot(2)/chdir(2), namely
> control, vmm and priv. From these both control and vmm already run under
> different pledge(2)s but without any filesystem access, priv in the other hand
> cannot use pledge due to forbidden ioctls.
>
> That being said the priv proc can instead just run with an unveil("/", "") to
> disable fs access, and while here remove the need of chroot/chdir dance since
> it's not needed any longer.
>
> I've been running this for more than a week now without issues. Any
> comments/objections? OK?
>
> /mestre
Tested here, no problems with my local VMs. Confirm that the priv
process is now unveiled.
root 12241 0.0 0.0 1224 1780 ?? SU 9:11AM0:00.01 vmd:
priv (vmd)
Just to clarify, this diff is removing chroot(2) from 3 processes:
* priv - (cannot be pledged)
* control - pledged "stdio,unix,sendfd,recvfd"
* vmm - pledged "stdio,proc,sendfd,recvfd,vmm"
I think this should be OK.
-Bryan.
> Index: priv.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/priv.c,v
> retrieving revision 1.15
> diff -u -p -u -r1.15 priv.c
> --- priv.c28 Jun 2019 13:32:51 - 1.15
> +++ priv.c19 Aug 2019 09:38:51 -
> @@ -66,8 +66,14 @@ priv_run(struct privsep *ps, struct priv
>
> /*
>* no pledge(2) in the "priv" process:
> - * write ioctls are not permitted by pledge.
> + * write ioctls are not permitted by pledge, but we can restrict fs
> + * access with unveil
>*/
> + /* no filesystem visibility */
> + if (unveil("/", "") == -1)
> + fatal("unveil");
> + if (unveil(NULL, NULL) == -1)
> + fatal("unveil");
>
> /* Open our own socket for generic interface ioctls */
> if ((env->vmd_fd = socket(AF_INET, SOCK_DGRAM, 0)) == -1)
> Index: proc.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/proc.c,v
> retrieving revision 1.18
> diff -u -p -u -r1.18 proc.c
> --- proc.c10 Sep 2018 10:36:01 - 1.18
> +++ proc.c19 Aug 2019 09:38:51 -
> @@ -524,7 +524,6 @@ proc_run(struct privsep *ps, struct priv
> void (*run)(struct privsep *, struct privsep_proc *, void *), void *arg)
> {
> struct passwd *pw;
> - const char *root;
> struct control_sock *rcs;
>
> log_procinit(p->p_title);
> @@ -545,17 +544,6 @@ proc_run(struct privsep *ps, struct priv
> pw = p->p_pw;
> else
> pw = ps->ps_pw;
> -
> - /* Change root directory */
> - if (p->p_chroot != NULL)
> - root = p->p_chroot;
> - else
> - root = pw->pw_dir;
> -
> - if (chroot(root) == -1)
> - fatal("%s: chroot", __func__);
> - if (chdir("/") == -1)
> - fatal("%s: chdir(\"/\")", __func__);
>
> privsep_process = p->p_id;
>
> Index: proc.h
> ===
> RCS file: /cvs/src/usr.sbin/vmd/proc.h,v
> retrieving revision 1.16
> diff -u -p -u -r1.16 proc.h
> --- proc.h10 Sep 2018 10:36:01 - 1.16
> +++ proc.h19 Aug 2019 09:38:51 -
> @@ -136,7 +136,6 @@ struct privsep_proc {
> void(*p_init)(struct privsep *,
> struct privsep_proc *);
> void(*p_shutdown)(void);
> - const char *p_chroot;
> struct passwd *p_pw;
> struct privsep *p_ps;
> };
> Index: vmd.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v
> retrieving revision 1.115
> diff -u -p -u -r1.115 vmd.c
> --- vmd.c 14 Aug 2019 07:34:49 - 1.115
> +++ vmd.c 19 Aug 2019 09:38:51 -
> @@ -789,9 +789,8 @@ main(int argc, char **argv)
> if ((ps->ps_pw = getpwnam(VMD_USER)) == NULL)
> fatal("unknown user %s", VMD_USER);
>
> - /* First proc runs as root without pledge but in default chroot */
> + /* First proc runs as root without pledge but with unveil */
> proc_priv->p_pw = _privpw; /* initialized to all 0 */
> - proc_priv->p_chroot = ps->ps_pw->pw_dir; /* from VMD_USER */
>
> /* Open /dev/vmm */
> if (env->vmd_noaction == 0) {
> Index: vmm.c
> ===
> RCS file: /cvs/src/usr.sbin/vmd/vmm.c,v
> retrieving revision 1.93
> diff -u -p -u -r1.93 vmm.c
> --- vmm.c 28 Jun 2019 13:32:51 - 1.93
> +++ vmm.c 19 Aug 2019 09:38:51 -
> @@ -655,7 +655,7 @@ vmm_start_vm(struct imsg *imsg, uint32_t
> if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, fds) == -1)
> fatal("socketpair");
>
> - /* Start child vmd for this VM (fork, chroot, drop privs) */
> + /* Start child
Re: Make filter line handling more developer friendly
On 8/26/19 11:42 AM, Sebastien Marie wrote:
> On Sun, Aug 25, 2019 at 07:01:01AM +0200, Martijn van Duren wrote:
>> Right now all we get is "Misbehaving filter", which doesn't tell the
>> developer a lot.
>>
>> Diff below does the following:
>> - Make the register_hooks actually fail on misbehaviour.
>> - Change some *ep = 0 to a more semantically correct ep[0] = '\0'
>> - Hoist some checks from lka_filter_process_response to processor_io
>> - Make lka_filter_process_response void (it fatals now)
>> - strtoull returns ULLONG_MAX on error, not ULONG_MAX
>> - change str{,n}casecmp to str{,n}cmp for consistency.
>> - change an fugly "nextline:" "goto nextline" loop to an actual while.
>> - restructure lka_filter_process_response to be shorter.
>> and most importantly
>> - Add a descriptive fatal() minefield.
>>
>> -12 LoC.
>>
>> Tested with filter-dnsbl with both a successful and rejected connection.
>>
>> OK?
>
> just one remark.
>
>> Index: lka_filter.c
>> ===
>> RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
>> retrieving revision 1.41
>> diff -u -p -r1.41 lka_filter.c
>> --- lka_filter.c 18 Aug 2019 16:52:02 - 1.41
>> +++ lka_filter.c 25 Aug 2019 04:47:47 -
>> @@ -429,88 +429,68 @@ lka_filter_process_response(const char *
>> struct filter_session *fs;
>>
>> (void)strlcpy(buffer, line, sizeof buffer);
>> -if ((ep = strchr(buffer, '|')) == NULL)
>> -return 0;
>> -*ep = 0;
>> +ep = strchr(buffer, '|');
>> +ep[0] = '\0';
>
> is it possible to buffer to not have '|' ? if yes, you could deference NULL.
>
>
You're absolutely right.
It's not a risk, since both would crash smtpd, but doing the check would
result in a clearer error message, which is the purpose of this
endeavour.
Index: lka_filter.c
===
RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
retrieving revision 1.41
diff -u -p -r1.41 lka_filter.c
--- lka_filter.c18 Aug 2019 16:52:02 - 1.41
+++ lka_filter.c26 Aug 2019 10:03:43 -
@@ -61,7 +61,7 @@ static void filter_result_reject(uint64_
static voidfilter_result_disconnect(uint64_t, const char *);
static voidfilter_session_io(struct io *, int, void *);
-intlka_filter_process_response(const char *, const char *);
+void lka_filter_process_response(const char *, const char *);
struct filter_session {
@@ -218,13 +218,13 @@ lka_filter_register_hook(const char *nam
hook += 8;
}
else
- return;
+ fatalx("Invalid message direction: %s", hook);
for (i = 0; i < nitems(filter_execs); i++)
if (strcmp(hook, filter_execs[i].phase_name) == 0)
break;
if (i == nitems(filter_execs))
- return;
+ fatalx("Unrecognized report name: %s", hook);
iter = NULL;
while (dict_iter(, , _name, (void **)))
@@ -414,7 +414,7 @@ filter_session_io(struct io *io, int evt
}
}
-int
+void
lka_filter_process_response(const char *name, const char *line)
{
uint64_t reqid;
@@ -430,87 +430,68 @@ lka_filter_process_response(const char *
(void)strlcpy(buffer, line, sizeof buffer);
if ((ep = strchr(buffer, '|')) == NULL)
- return 0;
- *ep = 0;
+ fatalx("Missing token: %s", line);
+ ep[0] = '\0';
kind = buffer;
- if (strcmp(kind, "register") == 0)
- return 1;
-
- if (strcmp(kind, "filter-result") != 0 &&
- strcmp(kind, "filter-dataline") != 0)
- return 0;
qid = ep+1;
if ((ep = strchr(qid, '|')) == NULL)
- return 0;
- *ep = 0;
+ fatalx("Missing reqid: %s", line);
+ ep[0] = '\0';
token = strtoull(qid, , 16);
if (qid[0] == '\0' || *ep != '\0')
- return 0;
- if (errno == ERANGE && token == ULONG_MAX)
- return 0;
+ fatalx("Invalid token: %s", line);
+ if (errno == ERANGE && token == ULLONG_MAX)
+ fatal("Invalid token: %s", line);
qid = ep+1;
if ((ep = strchr(qid, '|')) == NULL)
- return 0;
- *ep = 0;
+ fatal("Missing directive: %s", line);
+ ep[0] = '\0';
reqid = strtoull(qid, , 16);
if (qid[0] == '\0' || *ep != '\0')
- return 0;
- if (errno == ERANGE && reqid == ULONG_MAX)
- return 0;
+ fatalx("Invalid reqid: %s", line);
+ if (errno == ERANGE && reqid == ULLONG_MAX)
+ fatal("Invalid reqid: %s", line);
response = ep+1;
fs = tree_xget(, reqid);
if (strcmp(kind, "filter-dataline") == 0) {
if (fs->phase != FILTER_DATA_LINE)
- fatalx("misbehaving filter");
unveil vmd(8)'s priv process
Hi,
Currently vmd(8) has 3 processes that run under chroot(2)/chdir(2), namely
control, vmm and priv. From these both control and vmm already run under
different pledge(2)s but without any filesystem access, priv in the other hand
cannot use pledge due to forbidden ioctls.
That being said the priv proc can instead just run with an unveil("/", "") to
disable fs access, and while here remove the need of chroot/chdir dance since
it's not needed any longer.
I've been running this for more than a week now without issues. Any
comments/objections? OK?
/mestre
Index: priv.c
===
RCS file: /cvs/src/usr.sbin/vmd/priv.c,v
retrieving revision 1.15
diff -u -p -u -r1.15 priv.c
--- priv.c 28 Jun 2019 13:32:51 - 1.15
+++ priv.c 19 Aug 2019 09:38:51 -
@@ -66,8 +66,14 @@ priv_run(struct privsep *ps, struct priv
/*
* no pledge(2) in the "priv" process:
-* write ioctls are not permitted by pledge.
+* write ioctls are not permitted by pledge, but we can restrict fs
+* access with unveil
*/
+ /* no filesystem visibility */
+ if (unveil("/", "") == -1)
+ fatal("unveil");
+ if (unveil(NULL, NULL) == -1)
+ fatal("unveil");
/* Open our own socket for generic interface ioctls */
if ((env->vmd_fd = socket(AF_INET, SOCK_DGRAM, 0)) == -1)
Index: proc.c
===
RCS file: /cvs/src/usr.sbin/vmd/proc.c,v
retrieving revision 1.18
diff -u -p -u -r1.18 proc.c
--- proc.c 10 Sep 2018 10:36:01 - 1.18
+++ proc.c 19 Aug 2019 09:38:51 -
@@ -524,7 +524,6 @@ proc_run(struct privsep *ps, struct priv
void (*run)(struct privsep *, struct privsep_proc *, void *), void *arg)
{
struct passwd *pw;
- const char *root;
struct control_sock *rcs;
log_procinit(p->p_title);
@@ -545,17 +544,6 @@ proc_run(struct privsep *ps, struct priv
pw = p->p_pw;
else
pw = ps->ps_pw;
-
- /* Change root directory */
- if (p->p_chroot != NULL)
- root = p->p_chroot;
- else
- root = pw->pw_dir;
-
- if (chroot(root) == -1)
- fatal("%s: chroot", __func__);
- if (chdir("/") == -1)
- fatal("%s: chdir(\"/\")", __func__);
privsep_process = p->p_id;
Index: proc.h
===
RCS file: /cvs/src/usr.sbin/vmd/proc.h,v
retrieving revision 1.16
diff -u -p -u -r1.16 proc.h
--- proc.h 10 Sep 2018 10:36:01 - 1.16
+++ proc.h 19 Aug 2019 09:38:51 -
@@ -136,7 +136,6 @@ struct privsep_proc {
void(*p_init)(struct privsep *,
struct privsep_proc *);
void(*p_shutdown)(void);
- const char *p_chroot;
struct passwd *p_pw;
struct privsep *p_ps;
};
Index: vmd.c
===
RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v
retrieving revision 1.115
diff -u -p -u -r1.115 vmd.c
--- vmd.c 14 Aug 2019 07:34:49 - 1.115
+++ vmd.c 19 Aug 2019 09:38:51 -
@@ -789,9 +789,8 @@ main(int argc, char **argv)
if ((ps->ps_pw = getpwnam(VMD_USER)) == NULL)
fatal("unknown user %s", VMD_USER);
- /* First proc runs as root without pledge but in default chroot */
+ /* First proc runs as root without pledge but with unveil */
proc_priv->p_pw = _privpw; /* initialized to all 0 */
- proc_priv->p_chroot = ps->ps_pw->pw_dir; /* from VMD_USER */
/* Open /dev/vmm */
if (env->vmd_noaction == 0) {
Index: vmm.c
===
RCS file: /cvs/src/usr.sbin/vmd/vmm.c,v
retrieving revision 1.93
diff -u -p -u -r1.93 vmm.c
--- vmm.c 28 Jun 2019 13:32:51 - 1.93
+++ vmm.c 19 Aug 2019 09:38:51 -
@@ -655,7 +655,7 @@ vmm_start_vm(struct imsg *imsg, uint32_t
if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, fds) == -1)
fatal("socketpair");
- /* Start child vmd for this VM (fork, chroot, drop privs) */
+ /* Start child vmd for this VM (fork, drop privs) */
ret = fork();
/* Start child failed? - cleanup and leave */
Re: Make filter line handling more developer friendly
On Sun, Aug 25, 2019 at 07:01:01AM +0200, Martijn van Duren wrote:
> Right now all we get is "Misbehaving filter", which doesn't tell the
> developer a lot.
>
> Diff below does the following:
> - Make the register_hooks actually fail on misbehaviour.
> - Change some *ep = 0 to a more semantically correct ep[0] = '\0'
> - Hoist some checks from lka_filter_process_response to processor_io
> - Make lka_filter_process_response void (it fatals now)
> - strtoull returns ULLONG_MAX on error, not ULONG_MAX
> - change str{,n}casecmp to str{,n}cmp for consistency.
> - change an fugly "nextline:" "goto nextline" loop to an actual while.
> - restructure lka_filter_process_response to be shorter.
> and most importantly
> - Add a descriptive fatal() minefield.
>
> -12 LoC.
>
> Tested with filter-dnsbl with both a successful and rejected connection.
>
> OK?
just one remark.
> Index: lka_filter.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 lka_filter.c
> --- lka_filter.c 18 Aug 2019 16:52:02 - 1.41
> +++ lka_filter.c 25 Aug 2019 04:47:47 -
> @@ -429,88 +429,68 @@ lka_filter_process_response(const char *
> struct filter_session *fs;
>
> (void)strlcpy(buffer, line, sizeof buffer);
> - if ((ep = strchr(buffer, '|')) == NULL)
> - return 0;
> - *ep = 0;
> + ep = strchr(buffer, '|');
> + ep[0] = '\0';
is it possible to buffer to not have '|' ? if yes, you could deference NULL.
--
Sebastien Marie
Re: Make filter line handling more developer friendly
On Mon, Aug 26, 2019, at 00:30, gil...@poolp.org wrote:
> 25 août 2019 07:01 "Martijn van Duren" a
> écrit:
> > Right now all we get is "Misbehaving filter", which doesn't tell the
> > developer a lot.
> >
>
> Indeed
>
>
> > Diff below does the following:
> > - Make the register_hooks actually fail on misbehaviour.
> > - Change some *ep = 0 to a more semantically correct ep[0] = '\0'
> > - Hoist some checks from lka_filter_process_response to processor_io
> > - Make lka_filter_process_response void (it fatals now)
> > - strtoull returns ULLONG_MAX on error, not ULONG_MAX
> > - change str{,n}casecmp to str{,n}cmp for consistency.
> > - change an fugly "nextline:" "goto nextline" loop to an actual while.
> > - restructure lka_filter_process_response to be shorter.
> > and most importantly
> > - Add a descriptive fatal() minefield.
> >
> > -12 LoC.
> >
> > Tested with filter-dnsbl with both a successful and rejected connection.
> >
> > OK?
> >
> > martijn@
> >
>
> Reads fine but I'll give it a better read today and run with it on my
> machines for a day see if I observe any issues with filter-rspamd and
> filter-senderscore as well as some builtin filters.
>
>
> > Index: lka_filter.c
> > ===
> > RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
> > retrieving revision 1.41
> > diff -u -p -r1.41 lka_filter.c
> > --- lka_filter.c 18 Aug 2019 16:52:02 - 1.41
> > +++ lka_filter.c 25 Aug 2019 04:47:47 -
> > @@ -61,7 +61,7 @@ static void filter_result_reject(uint64_
> > static void filter_result_disconnect(uint64_t, const char *);
> >
> > static void filter_session_io(struct io *, int, void *);
> > -int lka_filter_process_response(const char *, const char *);
> > +void lka_filter_process_response(const char *, const char *);
> >
> > struct filter_session {
> > @@ -218,13 +218,13 @@ lka_filter_register_hook(const char *nam
> > hook += 8;
> > }
> > else
> > - return;
> > + fatalx("Invalid message direction: %s", hook);
> >
> > for (i = 0; i < nitems(filter_execs); i++)
> > if (strcmp(hook, filter_execs[i].phase_name) == 0)
> > break;
> > if (i == nitems(filter_execs))
> > - return;
> > + fatalx("Unrecognized report name: %s", hook);
> >
> > iter = NULL;
> > while (dict_iter(, , _name, (void **)))
> > @@ -414,7 +414,7 @@ filter_session_io(struct io *io, int evt
> > }
> > }
> >
> > -int
> > +void
> > lka_filter_process_response(const char *name, const char *line)
> > {
> > uint64_t reqid;
> > @@ -429,88 +429,68 @@ lka_filter_process_response(const char *
> > struct filter_session *fs;
> >
> > (void)strlcpy(buffer, line, sizeof buffer);
> > - if ((ep = strchr(buffer, '|')) == NULL)
> > - return 0;
> > - *ep = 0;
> > + ep = strchr(buffer, '|');
> > + ep[0] = '\0';
> >
> > kind = buffer;
> > - if (strcmp(kind, "register") == 0)
> > - return 1;
> > -
> > - if (strcmp(kind, "filter-result") != 0 &&
> > - strcmp(kind, "filter-dataline") != 0)
> > - return 0;
> >
> > qid = ep+1;
> > if ((ep = strchr(qid, '|')) == NULL)
> > - return 0;
> > - *ep = 0;
> > + fatalx("Missing reqid: %s", line);
> > + ep[0] = '\0';
> >
> > token = strtoull(qid, , 16);
> > if (qid[0] == '\0' || *ep != '\0')
> > - return 0;
> > - if (errno == ERANGE && token == ULONG_MAX)
> > - return 0;
> > + fatalx("Invalid token: %s", line);
> > + if (errno == ERANGE && token == ULLONG_MAX)
> > + fatal("Invalid token: %s", line);
> >
> > qid = ep+1;
> > if ((ep = strchr(qid, '|')) == NULL)
> > - return 0;
> > - *ep = 0;
> > + fatal("Missing directive: %s", line);
> > + ep[0] = '\0';
> >
> > reqid = strtoull(qid, , 16);
> > if (qid[0] == '\0' || *ep != '\0')
> > - return 0;
> > - if (errno == ERANGE && reqid == ULONG_MAX)
> > - return 0;
> > + fatalx("Invalid reqid: %s", line);
> > + if (errno == ERANGE && reqid == ULLONG_MAX)
> > + fatal("Invalid reqid: %s", line);
> >
> > response = ep+1;
> >
> > fs = tree_xget(, reqid);
> > if (strcmp(kind, "filter-dataline") == 0) {
> > if (fs->phase != FILTER_DATA_LINE)
> > - fatalx("misbehaving filter");
> > + fatalx("filter-dataline out of dataline phase");
> > filter_data_next(token, reqid, response);
> > - return 1;
> > + return;
> > }
> > if (fs->phase == FILTER_DATA_LINE)
> > - fatalx("misbehaving filter");
> > + fatalx("filter-result in dataline phase");
> >
> > if ((ep = strchr(response, '|'))) {
> > parameter = ep + 1;
> > - *ep = 0;
> > + ep[0] = '\0';
> > }
> >
> > - if (strcmp(response, "proceed") != 0 &&
> > - strcmp(response, "reject") != 0 &&
> > - strcmp(response, "disconnect") != 0 &&
> > - strcmp(response, "rewrite") != 0)
> > - return 0;
> > -
> > - if (strcmp(response, "proceed") == 0 &&
> > - parameter)
> > - return 0;
> > -
> > - if (strcmp(response, "proceed") != 0 &&
> > - parameter == NULL)
> > - return 0;
> > -
> > - if (strcmp(response, "rewrite") == 0) {
> > - filter_result_rewrite(reqid, parameter);
> > - return 1;
> > - }
> > -
> > - if (strcmp(response, "reject") == 0) {
> > -
Re: Make filter line handling more developer friendly
25 août 2019 07:01 "Martijn van Duren" a
écrit:
> Right now all we get is "Misbehaving filter", which doesn't tell the
> developer a lot.
>
Indeed
> Diff below does the following:
> - Make the register_hooks actually fail on misbehaviour.
> - Change some *ep = 0 to a more semantically correct ep[0] = '\0'
> - Hoist some checks from lka_filter_process_response to processor_io
> - Make lka_filter_process_response void (it fatals now)
> - strtoull returns ULLONG_MAX on error, not ULONG_MAX
> - change str{,n}casecmp to str{,n}cmp for consistency.
> - change an fugly "nextline:" "goto nextline" loop to an actual while.
> - restructure lka_filter_process_response to be shorter.
> and most importantly
> - Add a descriptive fatal() minefield.
>
> -12 LoC.
>
> Tested with filter-dnsbl with both a successful and rejected connection.
>
> OK?
>
> martijn@
>
Reads fine but I'll give it a better read today and run with it on my
machines for a day see if I observe any issues with filter-rspamd and
filter-senderscore as well as some builtin filters.
> Index: lka_filter.c
> ===
> RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 lka_filter.c
> --- lka_filter.c 18 Aug 2019 16:52:02 - 1.41
> +++ lka_filter.c 25 Aug 2019 04:47:47 -
> @@ -61,7 +61,7 @@ static void filter_result_reject(uint64_
> static void filter_result_disconnect(uint64_t, const char *);
>
> static void filter_session_io(struct io *, int, void *);
> -int lka_filter_process_response(const char *, const char *);
> +void lka_filter_process_response(const char *, const char *);
>
> struct filter_session {
> @@ -218,13 +218,13 @@ lka_filter_register_hook(const char *nam
> hook += 8;
> }
> else
> - return;
> + fatalx("Invalid message direction: %s", hook);
>
> for (i = 0; i < nitems(filter_execs); i++)
> if (strcmp(hook, filter_execs[i].phase_name) == 0)
> break;
> if (i == nitems(filter_execs))
> - return;
> + fatalx("Unrecognized report name: %s", hook);
>
> iter = NULL;
> while (dict_iter(, , _name, (void **)))
> @@ -414,7 +414,7 @@ filter_session_io(struct io *io, int evt
> }
> }
>
> -int
> +void
> lka_filter_process_response(const char *name, const char *line)
> {
> uint64_t reqid;
> @@ -429,88 +429,68 @@ lka_filter_process_response(const char *
> struct filter_session *fs;
>
> (void)strlcpy(buffer, line, sizeof buffer);
> - if ((ep = strchr(buffer, '|')) == NULL)
> - return 0;
> - *ep = 0;
> + ep = strchr(buffer, '|');
> + ep[0] = '\0';
>
> kind = buffer;
> - if (strcmp(kind, "register") == 0)
> - return 1;
> -
> - if (strcmp(kind, "filter-result") != 0 &&
> - strcmp(kind, "filter-dataline") != 0)
> - return 0;
>
> qid = ep+1;
> if ((ep = strchr(qid, '|')) == NULL)
> - return 0;
> - *ep = 0;
> + fatalx("Missing reqid: %s", line);
> + ep[0] = '\0';
>
> token = strtoull(qid, , 16);
> if (qid[0] == '\0' || *ep != '\0')
> - return 0;
> - if (errno == ERANGE && token == ULONG_MAX)
> - return 0;
> + fatalx("Invalid token: %s", line);
> + if (errno == ERANGE && token == ULLONG_MAX)
> + fatal("Invalid token: %s", line);
>
> qid = ep+1;
> if ((ep = strchr(qid, '|')) == NULL)
> - return 0;
> - *ep = 0;
> + fatal("Missing directive: %s", line);
> + ep[0] = '\0';
>
> reqid = strtoull(qid, , 16);
> if (qid[0] == '\0' || *ep != '\0')
> - return 0;
> - if (errno == ERANGE && reqid == ULONG_MAX)
> - return 0;
> + fatalx("Invalid reqid: %s", line);
> + if (errno == ERANGE && reqid == ULLONG_MAX)
> + fatal("Invalid reqid: %s", line);
>
> response = ep+1;
>
> fs = tree_xget(, reqid);
> if (strcmp(kind, "filter-dataline") == 0) {
> if (fs->phase != FILTER_DATA_LINE)
> - fatalx("misbehaving filter");
> + fatalx("filter-dataline out of dataline phase");
> filter_data_next(token, reqid, response);
> - return 1;
> + return;
> }
> if (fs->phase == FILTER_DATA_LINE)
> - fatalx("misbehaving filter");
> + fatalx("filter-result in dataline phase");
>
> if ((ep = strchr(response, '|'))) {
> parameter = ep + 1;
> - *ep = 0;
> + ep[0] = '\0';
> }
>
> - if (strcmp(response, "proceed") != 0 &&
> - strcmp(response, "reject") != 0 &&
> - strcmp(response, "disconnect") != 0 &&
> - strcmp(response, "rewrite") != 0)
> - return 0;
> -
> - if (strcmp(response, "proceed") == 0 &&
> - parameter)
> - return 0;
> -
> - if (strcmp(response, "proceed") != 0 &&
> - parameter == NULL)
> - return 0;
> -
> - if (strcmp(response, "rewrite") == 0) {
> - filter_result_rewrite(reqid, parameter);
> - return 1;
> - }
> -
> - if (strcmp(response, "reject") == 0) {
> - filter_result_reject(reqid, parameter);
> - return 1;
> - }
> -
> - if (strcmp(response, "disconnect") == 0) {
> - filter_result_disconnect(reqid, parameter);
> - return 1;
> + if (strcmp(response, "proceed") == 0) {
> + if (parameter != NULL)
> + fatalx("Unexpected parameter after proceed: %s", line);
> + filter_protocol_next(token, reqid, 0);
> + return;
> + } else {
> + if (parameter == NULL)
> +
