Re: /etc/netstart diff
On November 9, 2017 7:02:54 PM GMT+01:00, Robert Peichaer wrote: >On Wed, Nov 08, 2017 at 10:47:43PM +0100, Holger Mikolon wrote: >> The veriable $HN_DIR is set in /etc/netstart on line 166 but used >only >> once (line 78). The diff below makes use of $HN_DIR in the other >cases >> where netstart cares of ip address configuration. >> >> With below change I can maintain different sets (think "profiles") of >> hostname.if(5) files in separate directories and use them e.g. like >this: >> "env HN_DIR=/etc/myprofile sh /etc/netstart" >> >> Even without such use case it's at least a consistency fix. >> >> Regards >> Holger >> ;-se > >Hallo Holger > >This "feature" was introduced for a specific purpose some time ago to >assist the transition to the new hostname.if(5) parser code in the >netstart script. People were able to run netstart with the -n option >and this HN_DIR environment variable to verify that the new parser >works with their possibly non-trivial hostname.if(5) configurations. >We considered this to be a rather delicate transition because we did >not want to break peoples setups. > >Now that the parser code proved to not break anything, I rather want >to remove this funcionality, than to extend it. I totally agree.
try to bundle multiple packets on an ifq before sending
this makes ifq_start try to wait for 4 packets before calling if->if_qstart. this is based on work sephe did in dragonflybsd, and described in a comment in their sys/net/if.c. there's a link to it here: https://github.com/DragonFlyBSD/DragonFlyBSD/blob/master/sys/net/if.c#L2976-L2987 the tests we've done generally show a performance bump, but otherwise no degradation in performance. the mechanism for bundling packets is to but schedule a task to service the queue later. if 4 packets get accumulated before the task runs, it's cancelled and the code runs the start routine directly. the most significant difference this implementation has to the dfly one is that our ifqs dont (currently) track the number of bytes on the q. dfly sends if it can bundle 4 packets, or up to mtu bytes worth of packets. this implementation only looks at the number of packets. the taskq the ifq uses is one of the softnets, which is assigned when the ifq is initted and unconditionally used during the ifq's lifetime. because ifq work could now be pending in a softnet taskq, ifq_barrier also needs to put a barrier in the taskq. this is implemented using taskq_barrier, which i wrote ages ago but didn't have a use case for at the time. tests? ok? Index: share/man/man9/task_add.9 === RCS file: /cvs/src/share/man/man9/task_add.9,v retrieving revision 1.16 diff -u -p -r1.16 task_add.9 --- share/man/man9/task_add.9 14 Sep 2015 15:14:55 - 1.16 +++ share/man/man9/task_add.9 10 Nov 2017 00:45:41 - @@ -20,6 +20,7 @@ .Sh NAME .Nm taskq_create , .Nm taskq_destroy , +.Nm taskq_barrier , .Nm task_set , .Nm task_add , .Nm task_del , @@ -37,6 +38,8 @@ .Ft void .Fn taskq_destroy "struct taskq *tq" .Ft void +.Fn taskq_barrier "struct taskq *tq" +.Ft void .Fn task_set "struct task *t" "void (*fn)(void *)" "void *arg" .Ft int .Fn task_add "struct taskq *tq" "struct task *t" @@ -88,6 +91,15 @@ Calling against the system taskq is an error and will lead to undefined behaviour or a system fault. .Pp +.Fn taskq_barrier +guarantees that any task that was running on the +.Fa tq +taskq when the barrier was called has finished by the time the barrier +returns. +.Fn taskq_barrier +is only supported on taskqs serviced by 1 thread, +and may not be called by a task running in the specified taskq. +.Pp It is the responsibility of the caller to provide the .Fn task_set , .Fn task_add , @@ -163,6 +175,8 @@ argument given in and .Fn taskq_destroy can be called during autoconf, or from process context. +.Fn taskq_barrier +can be called from process context. .Fn task_set , .Fn task_add , and Index: sys/sys/task.h === RCS file: /cvs/src/sys/sys/task.h,v retrieving revision 1.11 diff -u -p -r1.11 task.h --- sys/sys/task.h 7 Jun 2016 07:53:33 - 1.11 +++ sys/sys/task.h 10 Nov 2017 00:45:41 - @@ -43,6 +43,7 @@ extern struct taskq *const systqmp; struct taskq *taskq_create(const char *, unsigned int, int, unsigned int); voidtaskq_destroy(struct taskq *); +voidtaskq_barrier(struct taskq *); voidtask_set(struct task *, void (*)(void *), void *); int task_add(struct taskq *, struct task *); Index: sys/kern/kern_task.c === RCS file: /cvs/src/sys/kern/kern_task.c,v retrieving revision 1.20 diff -u -p -r1.20 kern_task.c --- sys/kern/kern_task.c30 Oct 2017 14:01:42 - 1.20 +++ sys/kern/kern_task.c10 Nov 2017 00:45:41 - @@ -22,6 +22,7 @@ #include #include #include +#include #define TASK_ONQUEUE 1 @@ -68,6 +69,7 @@ struct taskq *const systqmp = &taskq_sys void taskq_init(void); /* called in init_main.c */ void taskq_create_thread(void *); +void taskq_barrier_task(void *); inttaskq_sleep(const volatile void *, struct mutex *, int, const char *, int); inttaskq_next_work(struct taskq *, struct task *, sleepfn); @@ -176,6 +178,30 @@ taskq_create_thread(void *arg) } while (tq->tq_running < tq->tq_nthreads); mtx_leave(&tq->tq_mtx); +} + +void +taskq_barrier(struct taskq *tq) +{ + struct sleep_state sls; + unsigned int notdone = 1; + struct task t = TASK_INITIALIZER(taskq_barrier_task, ¬done); + + task_add(tq, &t); + + while (notdone) { + sleep_setup(&sls, ¬done, PWAIT, "tqbar"); + sleep_finish(&sls, notdone); + } +} + +void +taskq_barrier_task(void *p) +{ + unsigned int *notdone = p; + + *notdone = 0; + wakeup_one(notdone); } void Index: sys/net/ifq.c === RCS file: /cvs/src/sys/net/ifq.c,v retrieving revision 1.12 diff -u -p -r1.12 ifq.c --- sys/net/ifq.c 2 Jun 2017 00:07:12 - 1.12 +++ sys/net/ifq.c 10 Nov 2017 00:45:41
iked: extend default transform list
Hi, the list of default transform parameters for iked is rather small. In practice that means that users will have to find out which parameters the peer uses and then create a config with specific parameters. And then another peer comes along that uses different settings and you bite into your desk. I think we should extend the list of default parameters that iked accepts. First of all we should allow elliptic curve groups with at least 256 bits, for the IKE SA and the Child SA. While there, maybe we should allow the same DH Groups for both IKE SA and Child SA, which should gives us to ability to use PFS by default. I do think adding higher bit integrity transforms and adding AES-GCM makes sense as well. ok? Patrick diff --git a/sbin/iked/parse.y b/sbin/iked/parse.y index 708165a7808..7615d20f8c9 100644 --- a/sbin/iked/parse.y +++ b/sbin/iked/parse.y @@ -129,13 +129,20 @@ struct iked_transform ikev2_default_ike_transforms[] = { { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_CBC, 192 }, { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_CBC, 128 }, { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_3DES }, + { IKEV2_XFORMTYPE_PRF, IKEV2_XFORMPRF_HMAC_SHA2_512 }, + { IKEV2_XFORMTYPE_PRF, IKEV2_XFORMPRF_HMAC_SHA2_384 }, { IKEV2_XFORMTYPE_PRF, IKEV2_XFORMPRF_HMAC_SHA2_256 }, { IKEV2_XFORMTYPE_PRF, IKEV2_XFORMPRF_HMAC_SHA1 }, + { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_512_256 }, + { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_384_192 }, { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_256_128 }, { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA1_96 }, { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_MODP_2048 }, { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_MODP_1536 }, { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_MODP_1024 }, + { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_ECP_521 }, + { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_ECP_384 }, + { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_ECP_256 }, { 0 } }; size_t ikev2_default_nike_transforms = ((sizeof(ikev2_default_ike_transforms) / @@ -145,8 +152,19 @@ struct iked_transform ikev2_default_esp_transforms[] = { { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_CBC, 256 }, { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_CBC, 192 }, { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_CBC, 128 }, + { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_GCM_16, 256 }, + { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_GCM_16, 192 }, + { IKEV2_XFORMTYPE_ENCR, IKEV2_XFORMENCR_AES_GCM_16, 128 }, + { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_512_256 }, + { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_384_192 }, { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA2_256_128 }, { IKEV2_XFORMTYPE_INTEGR, IKEV2_XFORMAUTH_HMAC_SHA1_96 }, + { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_MODP_2048 }, + { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_MODP_1536 }, + { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_MODP_1024 }, + { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_ECP_521 }, + { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_ECP_384 }, + { IKEV2_XFORMTYPE_DH, IKEV2_XFORMDH_ECP_256 }, { IKEV2_XFORMTYPE_ESN, IKEV2_XFORMESN_ESN }, { IKEV2_XFORMTYPE_ESN, IKEV2_XFORMESN_NONE }, { 0 }
armv7/arm64: enable fifo in com_fdt for "snps,dw-apb-uart"
Hi, does also remove ti,omapX-uart compatibles from arm64 com_fdt, as they're never to be found there. iirc. the lenght is based on minimal lenght i found from any datasheet for SoCs w/"snps,dw-apb-uart", and conclusions drawn from gpl-sources. -Artturi diff --git a/sys/arch/arm64/dev/com_fdt.c b/sys/arch/arm64/dev/com_fdt.c index 9f2a8eb760a..1dd67c7c3e0 100644 --- a/sys/arch/arm64/dev/com_fdt.c +++ b/sys/arch/arm64/dev/com_fdt.c @@ -66,9 +66,7 @@ com_fdt_init_cons(void) void *node; if ((node = fdt_find_cons("brcm,bcm2835-aux-uart")) == NULL && - (node = fdt_find_cons("snps,dw-apb-uart")) == NULL && - (node = fdt_find_cons("ti,omap3-uart")) == NULL && - (node = fdt_find_cons("ti,omap4-uart")) == NULL) + (node = fdt_find_cons("snps,dw-apb-uart")) == NULL) return; if (fdt_get_reg(node, 0, ®)) return; @@ -95,9 +93,7 @@ com_fdt_match(struct device *parent, void *match, void *aux) struct fdt_attach_args *faa = aux; return (OF_is_compatible(faa->fa_node, "brcm,bcm2835-aux-uart") || - OF_is_compatible(faa->fa_node, "snps,dw-apb-uart") || - OF_is_compatible(faa->fa_node, "ti,omap3-uart") || - OF_is_compatible(faa->fa_node, "ti,omap4-uart")); + OF_is_compatible(faa->fa_node, "snps,dw-apb-uart")); } void @@ -136,12 +132,11 @@ com_fdt_attach(struct device *parent, struct device *self, void *aux) sc->sc.sc_uarttype = COM_UART_16550; sc->sc.sc_frequency = freq ? freq : COM_FREQ; - if (OF_is_compatible(faa->fa_node, "snps,dw-apb-uart")) + if (OF_is_compatible(faa->fa_node, "snps,dw-apb-uart")) { + sc->sc.sc_uarttype = COM_UART_16550A; + sc->sc.sc_fifolen = 64; intr = com_fdt_intr_designware; - - if (OF_is_compatible(faa->fa_node, "ti,omap3-uart") || - OF_is_compatible(faa->fa_node, "ti,omap4-uart")) - sc->sc.sc_uarttype = COM_UART_TI16750; + } if (stdout_node == faa->fa_node) { SET(sc->sc.sc_hwflags, COM_HW_CONSOLE); diff --git a/sys/arch/armv7/dev/com_fdt.c b/sys/arch/armv7/dev/com_fdt.c index 00504801850..97c535a5a05 100644 --- a/sys/arch/armv7/dev/com_fdt.c +++ b/sys/arch/armv7/dev/com_fdt.c @@ -140,8 +140,11 @@ com_fdt_attach(struct device *parent, struct device *self, void *aux) sc->sc.sc_uarttype = COM_UART_16550; sc->sc.sc_frequency = freq ? freq : COM_FREQ; - if (OF_is_compatible(faa->fa_node, "snps,dw-apb-uart")) + if (OF_is_compatible(faa->fa_node, "snps,dw-apb-uart")) { + sc->sc.sc_uarttype = COM_UART_16550A; + sc->sc.sc_fifolen = 64; intr = com_fdt_intr_designware; + } if (OF_is_compatible(faa->fa_node, "ti,omap3-uart") || OF_is_compatible(faa->fa_node, "ti,omap4-uart"))
Re: Add Diffie-Hellman group negotiation to iked
On 2017/06/25 21:44, Tim Stewart wrote: > Hi, > > In this message I've tried to encode everything I've done to allow > strongSwan on Android to connect with iked, including the latest patch. > I have also verified that it breaks neither initial negotiation nor > Child SA rekeying for OpenBSD, Windows, and strongSwan (on Android) > clients. > > Stuart Henderson writes: > > > On 2017/05/22 01:52, Tim Stewart wrote: > >> Hello again, > >> > >> Tim Stewart writes: > >> > >> > Tim Stewart writes: > >> > > >> >> This patch teaches iked to reject a KE with a Notify payload of type > >> >> INVALID_KE_PAYLOAD when the KE uses a different Diffie-Hellman group > >> >> than is configured locally. The rejection indicates the desired > >> >> group. > >> >> > >> >> In my environment, this patch allows stock strongSwan on Android from > >> >> the Google Play store to interop with iked. strongSwan's logs show > >> >> the following once iked is patched: > >> >> > >> >> [IKE] initiating IKE_SA android[7] to 192.0.2.1 > >> >> [ENC] generating IKE_SA_INIT request 0 [ SA KE No N(NATD_S_IP) > >> >> N(NATD_D_IP) N(FRAG_SUP) N(HASH_ALG) N(REDIR_SUP) ] > >> >> [ENC] parsed IKE_SA_INIT response 0 [ N(INVAL_KE) ] > >> >> [IKE] peer didn't accept DH group ECP_256, it requested MODP_2048 > >> >> [IKE] initiating IKE_SA android[7] to 192.0.2.1 > >> >> [ENC] generating IKE_SA_INIT request 0 [ SA KE No N(NATD_S_IP) > >> >> N(NATD_D_IP) N(FRAG_SUP) N(HASH_ALG) N(REDIR_SUP) ] > >> >> [ENC] parsed IKE_SA_INIT response 0 [ SA KE No N(NATD_S_IP) > >> >> N(NATD_D_IP) CERTREQ N(HASH_ALG) ] > >> >> > >> >> I'm happy to iterate on this patch to get it into proper shape for > >> >> inclusion. > >> > > >> > I discovered a bug in the previous patch that broke renegotiation of > >> > CHILD SAs. I was ignoring "other than NONE" in the following sentence > >> > from RFC 5996 section 3.4: > >> > > >> > If the selected proposal uses a different Diffie-Hellman group > >> > (other than NONE), the message MUST be rejected with a Notify > >> > payload of type INVALID_KE_PAYLOAD. > >> > > >> > The new patch below repairs the flaw. > >> > >> After re-reading relevant parts of the RFC I'm not convinced that my fix > >> (rejecting with INVALID_KE_PAYLOAD unless msg->msg_dhgroup is > >> IKEV2_XFORMDH_NONE) is correct. It happens to resolve my local issue > >> but I think it may accidentally work due to a side effect of the code > >> path for rekeying a child SA. > >> > >> I will look at it more closely this week. > > My first patch did, in fact, break Child SAs rekeying. I have a new > patch at the end of this message that simply restricts DH group > negotiation to IKE SAs (I *think* that DH group guessing only applies to > IKE SAs, and perhaps only the IKE_SA_INIT exchange, but I'm still > working through the RFC). This may not be the ultimate solution, but it > does allow us to move forward. > > > Hi, I'm interested in this, but wasn't able to get strongswan to connect > > with either of your patches (and had iked exiting on one attempt, though > > I haven't been able to repeat that). > > After making changes I usually rebuild all of /usr/src/sbin/iked/ (make > clean && make). I started doing this after experiencing similar exits > and the problem went away. That could just be a coincidence, though! > > > If you have any updates please do send them here, it can be a bit slow > > getting feedback on iked diffs at times but it definitely is worth sending > > them out. > > I'll summarize what I know about strongSwan (on Android) and iked > interop. strongSwan chooses ECP_256 for its DH group guess when it > starts the IKE_SA_INIT exchange. The negotiation gets pretty far if I > specify `group ecp256' in the ikesa directive, but there there is some > incompatibility between iked and strongSwan that causes the following: > > ... > ikev2_recv: IKE_AUTH request from initiator 5.6.7.8:49317 to 1.2.3.4:4500 > policy 'default' id 1, 2040 bytes > ikev2_recv: ispi 0x4a0221ee629489c0 rspi 0x2459b2780209a1c8 > ikev2_recv: updated SA to peer 5.6.7.8:49317 local 1.2.3.4:4500 > ikev2_pld_parse: header ispi 0x4a0221ee629489c0 rspi 0x2459b2780209a1c8 > nextpayload SK version 0x20 exchange IKE_AUTH flags 0x08 msgid 1 length 2040 > response 0 > ikev2_pld_payloads: payload SK nextpayload IDi critical 0x00 length 2012 > ikev2_msg_decrypt: IV length 16 > ikev2_msg_decrypt: encrypted payload length 1968 > ikev2_msg_decrypt: integrity checksum length 24 > ikev2_msg_decrypt: integrity check failed > ikev2_resp_recv: failed to parse message > ... > > I suspect this is the failure many people hit if they try to configure > iked to match strongSwan on Android's built-in settings. I don't know > what the issue is with ecp256, but I needed to use modp2048 anyway so I > decided to write this patch. Presumably due to the commit on 27 Oct, this part of things is now fixed. With the following in config ... ikesa auth hmac-sha2-256 enc ae
Re: README patch
On Thu, Nov 09, 2017 at 08:42:32PM +0100, Ingo Schwarze wrote: > Hi, > > Jason McIntyre wrote on Thu, Nov 09, 2017 at 07:26:32PM +: > > On Wed, Nov 08, 2017 at 08:14:24PM -0600, Edgar Pettijohn wrote: > > >> --- README.orig 2017-11-08 20:11:47.091955000 -0600 > >> +++ README 2017-11-08 20:12:19.787639000 -0600 > >> @@ -49,8 +49,8 @@ > >> > >> No major funding or cost-sharing of the project comes from any company > >> or educational institution. Theo works full-time on improving OpenBSD > >> -and paying bills, many other developers expend spend significant > >> -quantities of time as well. > >> +and paying bills, many other developers spend significant quantities > >> +of time as well. > >> > >> For those unable to make their contributions as straightforward gifts, > >> the OpenBSD Foundation (http://OpenBSDFoundation.org) is a Canadian > >> > >> > >> Or perhaps it should be expend instead. Not sure, but both sound weird. > > > both suggested fixes would be fine. > > No it would not. The first sentence is no longer true, see > > http://www.openbsdfoundation.org/contributors.html > > So this would need a much larger fix. > fair enough. my comment was about the grammar really, rather than the content. > > however i have no idea which README this is... > > Indeed, i see no evidence that this old text still exists anywhere. > At least > > locate README | xargs grep -F 'No major funding' > > turns up nothing for me. > win win! jmc
Re: README patch
Hi, Jason McIntyre wrote on Thu, Nov 09, 2017 at 07:26:32PM +: > On Wed, Nov 08, 2017 at 08:14:24PM -0600, Edgar Pettijohn wrote: >> --- README.orig 2017-11-08 20:11:47.091955000 -0600 >> +++ README 2017-11-08 20:12:19.787639000 -0600 >> @@ -49,8 +49,8 @@ >> >> No major funding or cost-sharing of the project comes from any company >> or educational institution. Theo works full-time on improving OpenBSD >> -and paying bills, many other developers expend spend significant >> -quantities of time as well. >> +and paying bills, many other developers spend significant quantities >> +of time as well. >> >> For those unable to make their contributions as straightforward gifts, >> the OpenBSD Foundation (http://OpenBSDFoundation.org) is a Canadian >> >> >> Or perhaps it should be expend instead. Not sure, but both sound weird. > both suggested fixes would be fine. No it would not. The first sentence is no longer true, see http://www.openbsdfoundation.org/contributors.html So this would need a much larger fix. > however i have no idea which README this is... Indeed, i see no evidence that this old text still exists anywhere. At least locate README | xargs grep -F 'No major funding' turns up nothing for me. Yours, Ingo
Re: README patch
On Wed, Nov 08, 2017 at 08:14:24PM -0600, Edgar Pettijohn wrote: > EADME.orig 2017-11-08 20:11:47.091955000 -0600 > +++ README 2017-11-08 20:12:19.787639000 -0600 > @@ -49,8 +49,8 @@ > > No major funding or cost-sharing of the project comes from any company > or educational institution. Theo works full-time on improving OpenBSD > -and paying bills, many other developers expend spend significant > -quantities of time as well. > +and paying bills, many other developers spend significant quantities > +of time as well. > > For those unable to make their contributions as straightforward gifts, > the OpenBSD Foundation (http://OpenBSDFoundation.org) is a Canadian > > > Or perhaps it should be expend instead. Not sure, but both sound weird. > both suggested fixes would be fine. however i have no idea which README this is... jmc
Re: /etc/netstart diff
On Wed, Nov 08, 2017 at 10:47:43PM +0100, Holger Mikolon wrote: > The veriable $HN_DIR is set in /etc/netstart on line 166 but used only > once (line 78). The diff below makes use of $HN_DIR in the other cases > where netstart cares of ip address configuration. > > With below change I can maintain different sets (think "profiles") of > hostname.if(5) files in separate directories and use them e.g. like this: > "env HN_DIR=/etc/myprofile sh /etc/netstart" > > Even without such use case it's at least a consistency fix. > > Regards > Holger > ;-se Hallo Holger This "feature" was introduced for a specific purpose some time ago to assist the transition to the new hostname.if(5) parser code in the netstart script. People were able to run netstart with the -n option and this HN_DIR environment variable to verify that the new parser works with their possibly non-trivial hostname.if(5) configurations. We considered this to be a rather delicate transition because we did not want to break peoples setups. Now that the parser code proved to not break anything, I rather want to remove this funcionality, than to extend it. -- -=[rpe]=-
Re: dwiic: add pci attachment
Hi, this breaks my ASUS E200HA because of missing acpi_attach_deps in dwiic_acpi_found_hid, see https://marc.info/?l=openbsd-bugs&m=149504791212937&w=2 - C. On Nov 9, 2017 14:07, "joshua stein" wrote: On Thu, 09 Nov 2017 at 10:14:16 +0100, Remi Locherer wrote: > On Fri, Nov 03, 2017 at 12:01:15PM -0500, joshua stein wrote: > >> Intel 100 Series laptops have the DesignWare I2C controller >> attaching via PCI instead of ACPI, so move the guts of dwiic(4) into >> ic/ and add dwiic_acpi and dwiic_pci files. Unfortunately the PCI >> attachment still needs to reach into ACPI to discover client >> devices. >> >> There is still an issue with interrupt setup on these machines >> (unrelated to dwiic as far as I can tell), where ioapic interrupts >> registered for client devices like ihidev never fire, but those for >> dwiic (via PCI) do. This diff sets a flag for PCI-attached dwiic >> devices so that ihidev can do polling for client devices, which will >> come in a separate diff. >> >> I need some tests on machines where dwiic/ihidev attaches over ACPI. >> > > With your patch dwiic and also imt do attach. > > dwiic0 at pci0 dev 21 function 0 "Intel 100 Series I2C" rev 0x21: apic 2 > int 16 > iic0 at dwiic0 > dwiic1 at pci0 dev 21 function 1 "Intel 100 Series I2C" rev 0x21: apic 2 > int 17 > iic1 at dwiic1 > ihidev0 at iic1 addr 0x15, vendor 0x4f3 product 0x3035, ELAN1301 > ihidev0: 11 report ids > imt0 at ihidev0: clickpad, 5 contacts > wsmouse0 at imt0 mux 0 > > The touchpad does not work yet. > Let me know if you are interested in anything else from this machine. > Thanks. Has anyone tested this on currently-working machines with ACPI attachment? I am more concerned about not breaking anything right now.
Re: Sh FILES without Pa
On Thu, Nov 09, 2017 at 12:36:29PM +0100, Jan Stary wrote: > Hi Ingo, > > currently, mandoc DIAGNOSTICS recognizes an AUTHORS section without An. > Would it be similarly useful to recognize a FILES section without Pa? > personally this is one warning that i don;t like from mandoc. it flags up things when there is no issue. i like the idea of "No Pa" even less. i'd rather see the current AUTHORS warning removed than this be expanded. jmc
Re: dwiic: add pci attachment
On Thu, 09 Nov 2017 at 10:14:16 +0100, Remi Locherer wrote: On Fri, Nov 03, 2017 at 12:01:15PM -0500, joshua stein wrote: Intel 100 Series laptops have the DesignWare I2C controller attaching via PCI instead of ACPI, so move the guts of dwiic(4) into ic/ and add dwiic_acpi and dwiic_pci files. Unfortunately the PCI attachment still needs to reach into ACPI to discover client devices. There is still an issue with interrupt setup on these machines (unrelated to dwiic as far as I can tell), where ioapic interrupts registered for client devices like ihidev never fire, but those for dwiic (via PCI) do. This diff sets a flag for PCI-attached dwiic devices so that ihidev can do polling for client devices, which will come in a separate diff. I need some tests on machines where dwiic/ihidev attaches over ACPI. With your patch dwiic and also imt do attach. dwiic0 at pci0 dev 21 function 0 "Intel 100 Series I2C" rev 0x21: apic 2 int 16 iic0 at dwiic0 dwiic1 at pci0 dev 21 function 1 "Intel 100 Series I2C" rev 0x21: apic 2 int 17 iic1 at dwiic1 ihidev0 at iic1 addr 0x15, vendor 0x4f3 product 0x3035, ELAN1301 ihidev0: 11 report ids imt0 at ihidev0: clickpad, 5 contacts wsmouse0 at imt0 mux 0 The touchpad does not work yet. Let me know if you are interested in anything else from this machine. Thanks. Has anyone tested this on currently-working machines with ACPI attachment? I am more concerned about not breaking anything right now.
Re: Sh FILES without Pa
On Nov 09 12:36:29, h...@stare.cz wrote: > Question: > > static int > child_an(const struct roff_node *n) > { > for (n = n->child; n != NULL; n = n->next) > if ((n->tok == MDOC_An && n->child != NULL) || child_an(n)) > return 1; > return 0; > } > > If I'm reading this right, it tests whether the given node has an An child; > the child_pa() below is an obvious modification. Does the recursive > child_an(n) mean that an An node can itself have an An child? > Similar question for child_pa() then. Ah, just a child of a child, like an An inside a Bl inside AUTHORS. Sorry for the confusion. Jan > > Index: mandoc.1 > === > RCS file: /cvs/src/usr.bin/mandoc/mandoc.1,v > retrieving revision 1.143 > diff -u -p -r1.143 mandoc.1 > --- mandoc.1 7 Sep 2017 14:22:58 - 1.143 > +++ mandoc.1 9 Nov 2017 11:28:44 - > @@ -1171,6 +1171,12 @@ An AUTHORS sections contains no > .Ic \&An > macros, or only empty ones. > Probably, there are author names lacking markup. > +.It Sy "FILES section without Pa macro" > +.Pq mdoc > +A FILES sections contains no > +.Ic \&Pa > +macros, or only empty ones. > +Probably, there are file names lacking markup. > .El > .Ss "Warnings related to macros and nesting" > .Bl -ohang > Index: mandoc.h > === > RCS file: /cvs/src/usr.bin/mandoc/mandoc.h,v > retrieving revision 1.187 > diff -u -p -r1.187 mandoc.h > --- mandoc.h 8 Jul 2017 14:51:01 - 1.187 > +++ mandoc.h 9 Nov 2017 11:28:44 - > @@ -105,6 +105,7 @@ enum mandocerr { > MANDOCERR_XR_ORDER, /* unusual Xr order: ... after ... */ > MANDOCERR_XR_PUNCT, /* unusual Xr punctuation: ... after ... */ > MANDOCERR_AN_MISSING, /* AUTHORS section without An macro */ > + MANDOCERR_PA_MISSING, /* FILES section without Pa macro */ > > /* related to macros and nesting */ > MANDOCERR_MACRO_OBS, /* obsolete macro: macro */ > Index: mdoc_validate.c > === > RCS file: /cvs/src/usr.bin/mandoc/mdoc_validate.c,v > retrieving revision 1.268 > diff -u -p -r1.268 mdoc_validate.c > --- mdoc_validate.c 12 Sep 2017 18:20:32 - 1.268 > +++ mdoc_validate.c 9 Nov 2017 11:28:44 - > @@ -105,6 +105,7 @@ staticvoid post_sh_head(POST_ARGS); > static void post_sh_name(POST_ARGS); > static void post_sh_see_also(POST_ARGS); > static void post_sh_authors(POST_ARGS); > +static void post_sh_files(POST_ARGS); > static void post_sm(POST_ARGS); > static void post_st(POST_ARGS); > static void post_std(POST_ARGS); > @@ -2080,6 +2081,9 @@ post_sh(POST_ARGS) > case SEC_AUTHORS: > post_sh_authors(mdoc); > break; > + case SEC_FILES: > + post_sh_files(mdoc); > + break; > default: > break; > } > @@ -2216,6 +2220,25 @@ post_sh_authors(POST_ARGS) > > if ( ! child_an(mdoc->last)) > mandoc_msg(MANDOCERR_AN_MISSING, mdoc->parse, > + mdoc->last->line, mdoc->last->pos, NULL); > +} > + > +static int > +child_pa(const struct roff_node *n) > +{ > + > + for (n = n->child; n != NULL; n = n->next) > + if ((n->tok == MDOC_Pa && n->child != NULL) || child_pa(n)) > + return 1; > + return 0; > +} > + > +static void > +post_sh_files(POST_ARGS) > +{ > + > + if ( ! child_pa(mdoc->last)) > + mandoc_msg(MANDOCERR_PA_MISSING, mdoc->parse, > mdoc->last->line, mdoc->last->pos, NULL); > } > > Index: read.c > === > RCS file: /cvs/src/usr.bin/mandoc/read.c,v > retrieving revision 1.164 > diff -u -p -r1.164 read.c > --- read.c20 Jul 2017 14:36:32 - 1.164 > +++ read.c9 Nov 2017 11:28:44 - > @@ -141,6 +141,7 @@ staticconst char * const mandocerrs[MAN > "unusual Xr order", > "unusual Xr punctuation", > "AUTHORS section without An macro", > + "FILES section without Pa macro", > > /* related to macros and nesting */ > "obsolete macro", >
Sh FILES without Pa
Hi Ingo, currently, mandoc DIAGNOSTICS recognizes an AUTHORS section without An. Would it be similarly useful to recognize a FILES section without Pa? I left the empty lines after the opening { in case they are intended, just like child_an() and post_sh_authors() have them, but feel free to remove them all if they are not. Question: static int child_an(const struct roff_node *n) { for (n = n->child; n != NULL; n = n->next) if ((n->tok == MDOC_An && n->child != NULL) || child_an(n)) return 1; return 0; } If I'm reading this right, it tests whether the given node has an An child; the child_pa() below is an obvious modification. Does the recursive child_an(n) mean that an An node can itself have an An child? Similar question for child_pa() then. Jan Index: mandoc.1 === RCS file: /cvs/src/usr.bin/mandoc/mandoc.1,v retrieving revision 1.143 diff -u -p -r1.143 mandoc.1 --- mandoc.17 Sep 2017 14:22:58 - 1.143 +++ mandoc.19 Nov 2017 11:28:44 - @@ -1171,6 +1171,12 @@ An AUTHORS sections contains no .Ic \&An macros, or only empty ones. Probably, there are author names lacking markup. +.It Sy "FILES section without Pa macro" +.Pq mdoc +A FILES sections contains no +.Ic \&Pa +macros, or only empty ones. +Probably, there are file names lacking markup. .El .Ss "Warnings related to macros and nesting" .Bl -ohang Index: mandoc.h === RCS file: /cvs/src/usr.bin/mandoc/mandoc.h,v retrieving revision 1.187 diff -u -p -r1.187 mandoc.h --- mandoc.h8 Jul 2017 14:51:01 - 1.187 +++ mandoc.h9 Nov 2017 11:28:44 - @@ -105,6 +105,7 @@ enummandocerr { MANDOCERR_XR_ORDER, /* unusual Xr order: ... after ... */ MANDOCERR_XR_PUNCT, /* unusual Xr punctuation: ... after ... */ MANDOCERR_AN_MISSING, /* AUTHORS section without An macro */ + MANDOCERR_PA_MISSING, /* FILES section without Pa macro */ /* related to macros and nesting */ MANDOCERR_MACRO_OBS, /* obsolete macro: macro */ Index: mdoc_validate.c === RCS file: /cvs/src/usr.bin/mandoc/mdoc_validate.c,v retrieving revision 1.268 diff -u -p -r1.268 mdoc_validate.c --- mdoc_validate.c 12 Sep 2017 18:20:32 - 1.268 +++ mdoc_validate.c 9 Nov 2017 11:28:44 - @@ -105,6 +105,7 @@ static void post_sh_head(POST_ARGS); static void post_sh_name(POST_ARGS); static void post_sh_see_also(POST_ARGS); static void post_sh_authors(POST_ARGS); +static void post_sh_files(POST_ARGS); static void post_sm(POST_ARGS); static void post_st(POST_ARGS); static void post_std(POST_ARGS); @@ -2080,6 +2081,9 @@ post_sh(POST_ARGS) case SEC_AUTHORS: post_sh_authors(mdoc); break; + case SEC_FILES: + post_sh_files(mdoc); + break; default: break; } @@ -2216,6 +2220,25 @@ post_sh_authors(POST_ARGS) if ( ! child_an(mdoc->last)) mandoc_msg(MANDOCERR_AN_MISSING, mdoc->parse, + mdoc->last->line, mdoc->last->pos, NULL); +} + +static int +child_pa(const struct roff_node *n) +{ + + for (n = n->child; n != NULL; n = n->next) + if ((n->tok == MDOC_Pa && n->child != NULL) || child_pa(n)) + return 1; + return 0; +} + +static void +post_sh_files(POST_ARGS) +{ + + if ( ! child_pa(mdoc->last)) + mandoc_msg(MANDOCERR_PA_MISSING, mdoc->parse, mdoc->last->line, mdoc->last->pos, NULL); } Index: read.c === RCS file: /cvs/src/usr.bin/mandoc/read.c,v retrieving revision 1.164 diff -u -p -r1.164 read.c --- read.c 20 Jul 2017 14:36:32 - 1.164 +++ read.c 9 Nov 2017 11:28:44 - @@ -141,6 +141,7 @@ static const char * const mandocerrs[MAN "unusual Xr order", "unusual Xr punctuation", "AUTHORS section without An macro", + "FILES section without Pa macro", /* related to macros and nesting */ "obsolete macro",
iked: support MOBIKE (RFC 4555)
Hi, this diff implements MOBIKE (RFC 4555) support in iked(8), with us acting as responder. In practice this support means that clients like iPhones can roam in different networks (LTE, WiFi) and change their external address without having to re-do the whole handshake. It allows the client to choose how and when to change the external tunnel endpoint addresses on demand, depending on which network is better or even is connected at all. This diff already had a few iterations where race conditions were found and ironed out, so this should be a rather stable version which has been working really well for me now. Key changes with this diff are: * MOBIKE is on by default, but can be turned off (if neccessary) using a config key. * MOBIKE support is then announced to the initiator. * Peers can use different IP addresses in the IKE messages, but the Tunnel is only updated when we receive IKEV2_N_UPDATE_SA_ADDRESSES. * This means we need to store two peer addresses. The one he talked to us last, so that we know where to respond to. And the one that the kernel knows, so that we know if we have to update the SAs and flows. * We need to update the SA as well as the flow. For the SAs I added a way of updating destination addresses of an existing TDB in June. The flows simply need a reload after the SA changed. * COOKIE2 is used for return routability checks. strongswan seems to use it. It's encrypted and should be sent back to the initiator. When you, for instance, use your phone and turn on/off WiFi, you should see that the VPN stays connected and iked should log that the address has changed. Especially `ipsecctl -sa` should show updated SAs and flows. OKs? Objections? Feedback? Patrick diff --git a/sbin/iked/config.c b/sbin/iked/config.c index 590e4d7f4da..da8d0745f3c 100644 --- a/sbin/iked/config.c +++ b/sbin/iked/config.c @@ -814,6 +814,29 @@ config_getcompile(struct iked *env, struct imsg *imsg) return (0); } +int +config_setmobike(struct iked *env) +{ + unsigned int boolval; + + boolval = env->sc_mobike; + proc_compose(&env->sc_ps, PROC_IKEV2, IMSG_CTL_MOBIKE, + &boolval, sizeof(boolval)); + return (0); +} + +int +config_getmobike(struct iked *env, struct imsg *imsg) +{ + unsigned int boolval; + + IMSG_SIZE_CHECK(imsg, &boolval); + memcpy(&boolval, imsg->data, sizeof(boolval)); + env->sc_mobike = boolval; + log_debug("%s: %smobike", __func__, env->sc_mobike ? "" : "no "); + return (0); +} + int config_setocsp(struct iked *env) { diff --git a/sbin/iked/iked.c b/sbin/iked/iked.c index 09fef3ea877..548da77d014 100644 --- a/sbin/iked/iked.c +++ b/sbin/iked/iked.c @@ -250,6 +250,7 @@ parent_configure(struct iked *env) if (pledge("stdio rpath proc dns inet route sendfd", NULL) == -1) fatal("pledge"); + config_setmobike(env); config_setcoupled(env, env->sc_decoupled ? 0 : 1); config_setmode(env, env->sc_passive ? 1 : 0); config_setocsp(env); @@ -280,6 +281,7 @@ parent_reload(struct iked *env, int reset, const char *filename) /* Re-compile policies and skip steps */ config_setcompile(env, PROC_IKEV2); + config_setmobike(env); config_setcoupled(env, env->sc_decoupled ? 0 : 1); config_setmode(env, env->sc_passive ? 1 : 0); config_setocsp(env); diff --git a/sbin/iked/iked.conf.5 b/sbin/iked/iked.conf.5 index 8c77f24d603..be0e8ef30f6 100644 --- a/sbin/iked/iked.conf.5 +++ b/sbin/iked/iked.conf.5 @@ -136,6 +136,15 @@ This is the default. .It Ic set decouple Don't load the negotiated SAs and flows from the kernel. This mode is only useful for testing and debugging. +.It Ic set mobike +Enable MOBIKE (RFC 4555) support. +This is the default. +MOBIKE allows the change of the peer IP address for IKE and IPsec SAs. +Currenly +.Xr iked 8 +only supports MOBIKE when acting as a responder. +.It Ic set nomobike +Disables MOBIKE support. .It Ic set ocsp Ar URL Enable OCSP and set the URL of the OCSP responder. Please note that the matching responder and issuer certificates diff --git a/sbin/iked/iked.h b/sbin/iked/iked.h index b536d58e157..c2030ea7460 100644 --- a/sbin/iked/iked.h +++ b/sbin/iked/iked.h @@ -157,6 +157,8 @@ struct iked_flow { RB_ENTRY(iked_flow) flow_node; TAILQ_ENTRY(iked_flow) flow_entry; + + int flow_replacing; /* cf flow_replace() */ int flow_ipcomp; }; RB_HEAD(iked_flows, iked_flow); @@ -371,6 +373,7 @@ struct iked_sa { #define IKED_SATYPE_LOCAL 1 /* Local SA */ struct iked_addr sa_peer; + struct iked_addr sa_peer_loaded;/* MOBIKE */ struct iked_addr sa_local; int sa_fd; @@ -441,6 +444
Re: dwiic: add pci attachment
On Fri, Nov 03, 2017 at 12:01:15PM -0500, joshua stein wrote: > Intel 100 Series laptops have the DesignWare I2C controller > attaching via PCI instead of ACPI, so move the guts of dwiic(4) into > ic/ and add dwiic_acpi and dwiic_pci files. Unfortunately the PCI > attachment still needs to reach into ACPI to discover client > devices. > > There is still an issue with interrupt setup on these machines > (unrelated to dwiic as far as I can tell), where ioapic interrupts > registered for client devices like ihidev never fire, but those for > dwiic (via PCI) do. This diff sets a flag for PCI-attached dwiic > devices so that ihidev can do polling for client devices, which will > come in a separate diff. > > I need some tests on machines where dwiic/ihidev attaches over ACPI. With your patch dwiic and also imt do attach. dwiic0 at pci0 dev 21 function 0 "Intel 100 Series I2C" rev 0x21: apic 2 int 16 iic0 at dwiic0 dwiic1 at pci0 dev 21 function 1 "Intel 100 Series I2C" rev 0x21: apic 2 int 17 iic1 at dwiic1 ihidev0 at iic1 addr 0x15, vendor 0x4f3 product 0x3035, ELAN1301 ihidev0: 11 report ids imt0 at ihidev0: clickpad, 5 contacts wsmouse0 at imt0 mux 0 The touchpad does not work yet. Let me know if you are interested in anything else from this machine. Remi $ wsconsctl | grep mouse mouse.type=elantech mouse.rawmode=0 mouse.scale=0,3208,0,1829,0,31,31 mouse.tp.tapping=0 mouse.tp.scaling=0.249 mouse.tp.swapsides=0 mouse.tp.disable=0 $ xinput + Virtual core pointer id=2[master pointer (3)] | + Virtual core XTEST pointerid=4[slave pointer (2)] | + clickpad id=6[slave pointer (2)] | + /dev/wsmouse id=8[slave pointer (2)] + Virtual core keyboard id=3[master keyboard (2)] + Virtual core XTEST keyboard id=5[slave keyboard (3)] + /dev/wskbdid=7[slave keyboard (3)] $ dmesg OpenBSD 6.2-current (GENERIC.MP) #97: Wed Nov 8 08:15:57 CET 2017 r...@mistral.relo.ch:/usr/src/sys/arch/amd64/compile/GENERIC.MP real mem = 17041661952 (16252MB) avail mem = 16518344704 (15753MB) mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 3.0 @ 0x9b16d000 (31 entries) bios0: vendor American Megatrends Inc. version "UX390UAK.311" date 02/23/2017 bios0: ASUSTeK COMPUTER INC. UX390UAK acpi0 at bios0: rev 2 acpi0: sleep states S0 S3 S4 S5 acpi0: tables DSDT FACP APIC FPDT ECDT MCFG FIDT SSDT MSDM SSDT HPET UEFI SSDT LPIT WSMT SSDT SSDT DBGP DBG2 BGRT DMAR TPM2 acpi0: wakeup devices GLAN(S4) XHC_(S3) XDCI(S4) HDAS(S4) acpitimer0 at acpi0: 3579545 Hz, 24 bits acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz, 2594.75 MHz cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,SGX,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,SENSOR,ARAT cpu0: 256KB 64b/line 8-way L2 cache cpu0: smt 0, core 0, package 0 mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges cpu0: apic clock running at 24MHz cpu0: mwait min=64, max=64, C-substates=0.2.1.2.4.1.1.1, IBE cpu1 at mainbus0: apid 2 (application processor) cpu1: Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz, 2593.96 MHz cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,SGX,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,SENSOR,ARAT cpu1: 256KB 64b/line 8-way L2 cache cpu1: smt 0, core 1, package 0 cpu2 at mainbus0: apid 1 (application processor) cpu2: Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz, 2593.96 MHz cpu2: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,SGX,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,MPX,RDSEED,ADX,SMAP,CLFLUSHOPT,PT,SENSOR,ARAT cpu2: 256KB 64b/line 8-way L2 cache cpu2: smt 1, core 0, package 0 cpu3 at mainbus0: apid 3 (application processor) cpu3: Intel(R) Core(TM) i7-7500U CPU @ 2.70GHz, 2593.96 MHz cpu3: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CP
libfuse: signal handler doesn't cater for "Device busy" and other errors
The current libfuse signal handling assumes that the file system will always be unmounted by the child. One obvious case where this is not true is if the file system is busy. To replicate: 1. mount a fuse file system 2. cd anywhere on the file system 3. pkill -INT The result is a zombie child process and no more response to signals even if the file system is no longer busy. This patch ensures that the child always exits and that an error is printed to stdout if the file system cannot be unmounted. Tested with fuse-exfat and ntfs_3g. Suggestions for improvement are welcome. Index: fuse.c === RCS file: /cvs/src/lib/libfuse/fuse.c,v retrieving revision 1.34 diff -u -p -r1.34 fuse.c --- fuse.c 4 Nov 2017 13:17:18 - 1.34 +++ fuse.c 9 Nov 2017 08:41:11 - @@ -303,26 +303,40 @@ ifuse_get_signal(unused int num) { struct fuse *f; pid_t child; - int status; + int status, err; + + if (num == SIGCHLD) { + /* child fuse_unmount() completed, check error */ + while (waitpid(WAIT_ANY, &status, WNOHANG) == -1) { + if (errno != EINTR) + fprintf(stderr, "fuse: %s\n", strerror(errno)); + } + + if (WIFEXITED(status)) { + err = WEXITSTATUS(status); + if (err) + fprintf(stderr, "fuse: %s\n", strerror(err)); + } + + signal(SIGCHLD, SIG_DFL); + return; + } if (sigse != NULL) { + signal(SIGCHLD, ifuse_get_signal); child = fork(); if (child < 0) - return; + return ; - f = sigse->args; if (child == 0) { + /* try to unmount, file system may be busy */ + f = sigse->args; + errno = 0; fuse_unmount(f->fc->dir, f->fc); - sigse = NULL; - exit(0); + _exit(errno); } - fuse_loop(f); - while (waitpid(child, &status, 0) == -1) { - if (errno != EINTR) - break; - } } }
Re: TLS with static non-PIE binaries
On Thu, Nov 9, 2017 at 12:07 AM, Charles Collicutt wrote: > On Wed, Nov 08, 2017 at 10:58:09PM -0800, Philip Guenther wrote: > > @nobits means the section won't have filespace (where the initialization > > data is) included in the loaded data. It should work with @progbits > > instead. > > Wow, I suck at assembly. However, once again binutils saved me: > > $ readelf -SW test.o | grep .tdata > [ 4] .tdataPROGBITS 4c 04 00 > WAT 0 0 4 > Dang it, binutils keeps breaking all my pat answers! Totally unfair. > I'm afraid changing it to @progbits in the source doesn't make any > difference to the result. It still returns the correct thing when > dynamically linked but zero when statically linked. > More alignment fun. The problem will go away if your data has an alignment of 8 or a size that's a multiple of 8. I need to think about the right way to represent the necessary bitsat some time when I'm not falling asleep. Philip Guenther
Re: TLS with static non-PIE binaries
On Wed, Nov 08, 2017 at 10:58:09PM -0800, Philip Guenther wrote: > @nobits means the section won't have filespace (where the initialization > data is) included in the loaded data. It should work with @progbits > instead. Wow, I suck at assembly. However, once again binutils saved me: $ readelf -SW test.o | grep .tdata [ 4] .tdataPROGBITS 4c 04 00 WAT 0 0 4 I'm afraid changing it to @progbits in the source doesn't make any difference to the result. It still returns the correct thing when dynamically linked but zero when statically linked. -- Charles