Does openssl know about ROP?
I was reading some openssl source code, in particular the x86 assembly language files (which accelerate some crypto operations), and I find many cases where data tables are intentionally inserted into text (code) segments, and those tables include the byte value 0xC3. By intentional, I mean there's a comment, don't let me judge the tone of it: &set_label("AES_Td",64);# Yes! I keep it in the code segment! And then a little bit later (these macros expand to placing .long into the .text segment) &_data_word(0x50a7f451, 0x5365417e, 0xc3a4171a, 0x965e273a); ^^ It doesn't do it once. It does it nearly a hundred times. Here's another: &_data_word(0x02752fc3, 0x12f04c81, 0xa397468d, 0xc6f9d36b); ^^ Can I do anything dangerous with that byte? Does anyone on the internet know if a C3 byte in the code segment of a program is safe? I am very curious (if anyone knows..) I did not look carefully for other specific byte sequences in code which carry potential danger. Far be it from me to suggest that the security experts over there in OpenSSL land are unaware of modern exploitation methods! Very far from that, very very far. Surely I am wrong, all these bytes must be very very safe, these numerous 0xc3 bytes (and the bytes preceeding them) must have been reviewed repeatedly very carefully by the developers paid with the money they received after their heartbleed error -- to make sure these specific 0xc3 "instructions" (and the "instructions" precedeing them) are safe. Please let me know, I want to sleep better at night.
remove APM_IOC_NEXTEVENT mentions
The APM_IOC_NEXTEVENT ioctl was ripped out from i386 in 2001 in favor of a kqueue interface, but apm.4 on various arches still claim it's supported or may be in the future which is unlikely. https://github.com/openbsd/src/commit/1e8a3e096b6ef01dcdf4f881eac1b86b1bafe06d Index: share/man/man4/man4.amd64/apm.4 === RCS file: /cvs/src/share/man/man4/man4.amd64/apm.4,v retrieving revision 1.9 diff -u -p -u -p -r1.9 apm.4 --- share/man/man4/man4.amd64/apm.4 31 Mar 2022 17:27:21 - 1.9 +++ share/man/man4/man4.amd64/apm.4 30 Jan 2023 05:01:25 - @@ -103,44 +103,6 @@ The .Va minutes_left value contains the estimated number of minutes of battery life remaining. -.It Dv APM_IOC_NEXTEVENT -.Pq Li "struct apm_event_info" -The APM driver stores up to -.Dv APM_NEVENTS -events. -This was defined as 16 at the time this documentation was written. -If the event list is full when a new event is detected, the new event is lost. -.Dv APM_IOC_NEXTEVENT -ioctl returns the next event on the list or -.Er EAGAIN -if the event list is empty. -The format of the returned event is: -.Bd -literal -offset indent -struct apm_event_info { - u_int type; - u_int index; - u_int spare[8]; -}; -.Ed -where -.Va index -is a sequential count of events that can be used to check if any -events were lost and -.Va type -is one of: -.Bl -tag -width Ds -offset indent -compact -.It Dv APM_STANDBY_REQ -.It Dv APM_SUSPEND_REQ -.It Dv APM_NORMAL_RESUME -.It Dv APM_CRIT_RESUME -.It Dv APM_BATTERY_LOW -.It Dv APM_POWER_CHANGE -.It Dv APM_UPDATE_TIME -.It Dv APM_CRIT_SUSPEND_REQ -.It Dv APM_USER_STANDBY_REQ -.It Dv APM_USER_SUSPEND_REQ -.It Dv APM_SYS_STANDBY_RESUME -.El .It Dv APM_IOC_DEV_CTL .Pq Li "struct apm_ctl" Allows an application to directly set the Index: share/man/man4/man4.arm64/apm.4 === RCS file: /cvs/src/share/man/man4/man4.arm64/apm.4,v retrieving revision 1.5 diff -u -p -u -p -r1.5 apm.4 --- share/man/man4/man4.arm64/apm.4 31 Mar 2022 17:27:21 - 1.5 +++ share/man/man4/man4.arm64/apm.4 30 Jan 2023 05:01:25 - @@ -112,44 +112,6 @@ The .Va minutes_left value contains the estimated number of minutes of battery life remaining. -.It Dv APM_IOC_NEXTEVENT -.Pq Li "struct apm_event_info" -The APM driver stores up to -.Dv APM_NEVENTS -events. -This was defined as 16 at the time this documentation was written. -If the event list is full when a new event is detected, the new event is lost. -.Dv APM_IOC_NEXTEVENT -ioctl returns the next event on the list or -.Er EAGAIN -if the event list is empty. -The format of the returned event is: -.Bd -literal -offset indent -struct apm_event_info { - u_int type; - u_int index; - u_int spare[8]; -}; -.Ed -where -.Va index -is a sequential count of events that can be used to check if any -events were lost and -.Va type -is one of: -.Bl -tag -width Ds -offset indent -compact -.It Dv APM_STANDBY_REQ -.It Dv APM_SUSPEND_REQ -.It Dv APM_NORMAL_RESUME -.It Dv APM_CRIT_RESUME -.It Dv APM_BATTERY_LOW -.It Dv APM_POWER_CHANGE -.It Dv APM_UPDATE_TIME -.It Dv APM_CRIT_SUSPEND_REQ -.It Dv APM_USER_STANDBY_REQ -.It Dv APM_USER_SUSPEND_REQ -.It Dv APM_SYS_STANDBY_RESUME -.El .It Dv APM_IOC_DEV_CTL .Em NOT YET SUPPORTED on arm64 . .Pp Index: share/man/man4/man4.i386/apm.4 === RCS file: /cvs/src/share/man/man4/man4.i386/apm.4,v retrieving revision 1.35 diff -u -p -u -p -r1.35 apm.4 --- share/man/man4/man4.i386/apm.4 31 Mar 2022 17:27:21 - 1.35 +++ share/man/man4/man4.i386/apm.4 30 Jan 2023 05:01:25 - @@ -137,44 +137,6 @@ The .Va minutes_left value contains the estimated number of minutes of battery life remaining. -.It Dv APM_IOC_NEXTEVENT -.Pq Li "struct apm_event_info" -The APM driver stores up to -.Dv APM_NEVENTS -events. -This was defined as 16 at the time this documentation was written. -If the event list is full when a new event is detected, the new event is lost. -.Dv APM_IOC_NEXTEVENT -ioctl returns the next event on the list or -.Er EAGAIN -if the event list is empty. -The format of the returned event is: -.Bd -literal -offset indent -struct apm_event_info { - u_int type; - u_int index; - u_int spare[8]; -}; -.Ed -where -.Va index -is a sequential count of events that can be used to check if any -events were lost and -.Va type -is one of: -.Bl -tag -width Ds -offset indent -compact -.It Dv APM_STANDBY_REQ -.It Dv APM_SUSPEND_REQ -.It Dv APM_NORMAL_RESUME -.It Dv APM_CRIT_RESUME -.It Dv APM_BATTERY_LOW -.It Dv APM_POWER_CHANGE -.It Dv APM_UPDATE_TIME -.It Dv APM_CRIT_SUSPEND_REQ -.It Dv APM_USER_STANDBY_REQ -.It Dv APM_USER_SUSPEND_REQ -.It Dv APM_SYS_STANDBY_RESUME -.El .It Dv APM_IOC_DEV_CTL .Pq Li "struct apm_ctl" Allows an application to directly set the Index: share/man/man4/man4.loongson/apm.4 ===
Replace selwakeup() with KNOTE() in tun(4) and tap(4)
Replace selwakeup() with KNOTE() in tun(4) and tap(4). This patch makes the tun(4) and tap(4) event filters MP-safe. This is similar to the change that just got committed to pppac(4) and pppx(4). However, tun(4) and tap(4) can be destroyed abruptly, so klist_invalidate() has to be kept in tun_clone_destroy(). The selwakeup() call in tun_dev_close() can be removed. If the device is closed peacefully, the klists get cleared automatically and waiters notified before the close routine is invoked. On abrupt detach, klist_invalidate() in tun_clone_destroy() should clear any lingering knotes. OK? Index: net/if_tun.c === RCS file: src/sys/net/if_tun.c,v retrieving revision 1.237 diff -u -p -r1.237 if_tun.c --- net/if_tun.c2 Jul 2022 08:50:42 - 1.237 +++ net/if_tun.c30 Jan 2023 03:32:36 - @@ -47,13 +47,14 @@ #include #include #include -#include #include #include #include #include #include #include +#include +#include #include #include @@ -78,8 +79,9 @@ struct tun_softc { struct arpcom sc_ac; /* ethernet common data */ #define sc_if sc_ac.ac_if - struct selinfo sc_rsel;/* read select */ - struct selinfo sc_wsel;/* write select (not used) */ + struct mutexsc_mtx; + struct klistsc_rklist; /* knotes for read */ + struct klistsc_wklist; /* knotes for write (unused) */ SMR_LIST_ENTRY(tun_softc) sc_entry; /* all tunnel interfaces */ int sc_unit; @@ -125,22 +127,28 @@ int tun_init(struct tun_softc *); void tun_start(struct ifnet *); intfilt_tunread(struct knote *, long); intfilt_tunwrite(struct knote *, long); +intfilt_tunmodify(struct kevent *, struct knote *); +intfilt_tunprocess(struct knote *, struct kevent *); void filt_tunrdetach(struct knote *); void filt_tunwdetach(struct knote *); void tun_link_state(struct ifnet *, int); const struct filterops tunread_filtops = { - .f_flags= FILTEROP_ISFD, + .f_flags= FILTEROP_ISFD | FILTEROP_MPSAFE, .f_attach = NULL, .f_detach = filt_tunrdetach, .f_event= filt_tunread, + .f_modify = filt_tunmodify, + .f_process = filt_tunprocess, }; const struct filterops tunwrite_filtops = { - .f_flags= FILTEROP_ISFD, + .f_flags= FILTEROP_ISFD | FILTEROP_MPSAFE, .f_attach = NULL, .f_detach = filt_tunwdetach, .f_event= filt_tunwrite, + .f_modify = filt_tunmodify, + .f_process = filt_tunprocess, }; SMR_LIST_HEAD(tun_list, tun_softc); @@ -220,6 +228,9 @@ tun_create(struct if_clone *ifc, int uni ifp = &sc->sc_if; snprintf(ifp->if_xname, sizeof(ifp->if_xname), "%s%d", ifc->ifc_name, unit); + mtx_init(&sc->sc_mtx, IPL_NET); + klist_init_mutex(&sc->sc_rklist, &sc->sc_mtx); + klist_init_mutex(&sc->sc_wklist, &sc->sc_mtx); ifp->if_softc = sc; /* this is enough state for tun_dev_open to work with */ @@ -275,6 +286,8 @@ tun_create(struct if_clone *ifc, int uni return (0); exists: + klist_free(&sc->sc_rklist); + klist_free(&sc->sc_wklist); free(sc, M_DEVBUF, sizeof(*sc)); return (EEXIST); } @@ -284,7 +297,6 @@ tun_clone_destroy(struct ifnet *ifp) { struct tun_softc*sc = ifp->if_softc; dev_tdev; - int s; KERNEL_ASSERT_LOCKED(); @@ -314,10 +326,11 @@ tun_clone_destroy(struct ifnet *ifp) /* wait for device entrypoints to finish */ refcnt_finalize(&sc->sc_refs, "tundtor"); - s = splhigh(); - klist_invalidate(&sc->sc_rsel.si_note); - klist_invalidate(&sc->sc_wsel.si_note); - splx(s); + klist_invalidate(&sc->sc_rklist); + klist_invalidate(&sc->sc_wklist); + + klist_free(&sc->sc_rklist); + klist_free(&sc->sc_wklist); if (ISSET(sc->sc_flags, TUN_LAYER2)) ether_ifdetach(ifp); @@ -488,7 +501,6 @@ tun_dev_close(dev_t dev, struct proc *p) ifq_purge(&ifp->if_snd); CLR(sc->sc_flags, TUN_ASYNC); - selwakeup(&sc->sc_rsel); sigio_free(&sc->sc_sigio); if (!ISSET(sc->sc_flags, TUN_DEAD)) { @@ -654,7 +666,10 @@ tun_wakeup(struct tun_softc *sc) if (sc->sc_reading) wakeup(&sc->sc_if.if_snd); - selwakeup(&sc->sc_rsel); + mtx_enter(&sc->sc_mtx); + KNOTE(&sc->sc_rklist, 0); + mtx_leave(&sc->sc_mtx); + if (sc->sc_flags & TUN_ASYNC) pgsigio(&sc->sc_sigio, SIGIO, 0); } @@ -960,7 +975,6 @@ tun_dev_kqfilter(dev_t dev, struct knote struct ifnet*i
Re: Preferred TERM for pkg_add
Marc Espie wrote: > Actual patches to pkg_add (if need be) welcome > > The switch to not using full term length was actually done at Theo's > request. > > I don't remember the details, but Theo's rationale was quite > reasonable and made lots of sense. > > (specifically, using the full line length would be cool, but there > were lots of fringe cases where it would fuck the display up for > no reason). printing on the last character of the screen has nasty consequences on some terminal types, unless the entire input&output is fully controlled to ensure in input character doesn't become a canonical output character (ie. the way that a visual text or pager works). avoid it unless absolutely neccessay.
Re: xonly status
"Theo de Raadt" writes: ... removed the comprehensive summary of the effort. > The major application problems have been reasonably isolated, and after > 3 weeks they are mostly handled, or we know where the remaining problems > lie: > > - libcrypto assembly functions with incorrect data placement > - some ffi variations with the same problem > - managing v8's embedded code blob > - a few languages with very strange machine-dependent optimizations > > That is mostly managed in ports now, mostly by fixing the code, or if it > is too difficult or intentionally obscure the specific programs can be > linked --no-exec-only. Maybe someone from the ports team can reply to > this mail to say where things are at. GHC (Glasgow Haskell Compiler) is one of the languages with unusual machine code generation strategies. For now lang/ghc uses --no-exec-only. I'm exploring other options with upstream at https://gitlab.haskell.org/ghc/ghc/-/issues/22782 Thanks Greg
Re: Preferred TERM for pkg_add
Actual patches to pkg_add (if need be) welcome The switch to not using full term length was actually done at Theo's request. I don't remember the details, but Theo's rationale was quite reasonable and made lots of sense. (specifically, using the full line length would be cool, but there were lots of fringe cases where it would fuck the display up for no reason).
Re: Fix handling of ed addresses consisting only of separators
Hi, I've been using this patch for the past 6 months and haven't noticed any regressions. However, during this timespan I became aware of additional POSIX compatibility issues in OpenBSD ed: `3 2p` as well as addresses with interleaved `,` and `;` separators (e.g. `1,;`) are also evaluated incorrectly. These additional compatibility issues are unrelated to the issue fixed by the proposed patch. Is there a general interest in improving the POSIX compatibility of OpenBSD ed? If so, what would need to be done in order to get the proposed patch committed? Greetings, Sören Sören Tempel wrote: > PING. > > I have executed the ed tests (available in bin/ed/test) with this patch > applied and the proposed patch doesn't seem to introduce any new > regressions. > > If the patch needs to be revised further, please let me know. The only > thing I can think of right now is that it might make sense to rename the > `recur` variable to `sawseparator` or something along those lines. > > Greetings, > Sören > > Sören Tempel wrote: > > Hello, > > > > The POSIX specification of ed(1) includes a table in the rationale > > section which (among others) mandates the following address handling > > rules [1]: > > > > Address Addr1 Addr2 > > ,, $ $ > > ;; $ $ > > > > Unfortunately, OpenBSD does not correctly handle these two example > > addresses. As an example, consider the following `,,p` print command: > > > > printf "a\nfoo\nbar\nbaz\n.\n,,p\nQ\n" | ed > > > > This should only print the last line (as `,,p` should expand to `$,$p`) > > but instead prints all lines with OpenBSD ed. This seems to be a > > regression introduced by Jerome Frgagic in 2017 [2]. Prior to this > > change, `,,p` as well as `;;p` correctly printed the last line. The > > aforementioned change added a recursive invocation of next_addr() which > > does not update the local first variable (causing the second address > > separator to be mistaken for the first). The patch below fixes this by > > tracking recursive invocations of next_addr() via an additional function > > parameter. > > > > See also: https://austingroupbugs.net/view.php?id=1582 > > > > [1]: > > https://pubs.opengroup.org/onlinepubs/9699919799/utilities/ed.html#tag_20_38_18 > > [2]: > > https://github.com/openbsd/src/commit/b4c905bbf3932a50011ce3436b1e22acc742cfeb > > > > diff --git bin/ed/main.c bin/ed/main.c > > index 231d021ef19..ca1aeb102b3 100644 > > --- bin/ed/main.c > > +++ bin/ed/main.c > > @@ -64,7 +64,7 @@ void signal_hup(int); > > void signal_int(int); > > void handle_winch(int); > > > > -static int next_addr(void); > > +static int next_addr(int); > > static int check_addr_range(int, int); > > static int get_matching_node_addr(regex_t *, int); > > static char *get_filename(int); > > @@ -296,7 +296,7 @@ extract_addr_range(void) > > > > addr_cnt = 0; > > first_addr = second_addr = current_addr; > > - while ((addr = next_addr()) >= 0) { > > + while ((addr = next_addr(0)) >= 0) { > > addr_cnt++; > > first_addr = second_addr; > > second_addr = addr; > > @@ -328,7 +328,7 @@ extract_addr_range(void) > > > > /* next_addr: return the next line address in the command buffer */ > > static int > > -next_addr(void) > > +next_addr(int recur) > > { > > char *hd; > > int addr = current_addr; > > @@ -382,11 +382,15 @@ next_addr(void) > > case '%': > > case ',': > > case ';': > > - if (first) { > > + if (first && !recur) { > > ibufp++; > > addr_cnt++; > > second_addr = (c == ';') ? current_addr : 1; > > - if ((addr = next_addr()) < 0) > > + > > + // If the next address is omitted (e.g. "," or > > ";") or > > + // if it is also a separator (e.g. ",," or > > ";;") then > > + // return the last address (as per the omission > > rules). > > + if ((addr = next_addr(1)) < 0) > > addr = addr_last; > > break; > > }
Re: OpenBSD perl 5.36.0 - Call for Testing
One thing that's going to be very useful IMO is newer function bindings. Us perlers don't really care, matching $@ after the sub is cool. But having off-the-mill function "declarations" that look a lot like function declarations in other more common languages (e.g. sub f($a, $b) ) might help *a lot* in getting people to read perl code. Gonna try that one out for sure!
Re: xonly status
The two main test programs being used for xonly assessment, as attachments. They will eventually head towards src/regress but for now, people can play with them. #include #include #include #include #include #include #include #include #include #include #include int main(int argc, char *argv[]); void *s_ldso(void); void *s_mmap_xz(void); void *s_mmap_x(void); void *s_mmap_nrx(void); void *s_mmap_nwx(void); void *s_mmap_xnwx(void); struct readable { char *name; void *(*setup)(void); int isfn; void *addr; int uu, ku; int skip; } readables[] = { { "ld.so", s_ldso, 1, }, { "mmap xz", s_mmap_xz, 0, }, { "mmap x", s_mmap_x, 0, }, { "mmap nrx", s_mmap_nrx, 0, }, { "mmap nwx", s_mmap_nwx, 0, }, { "mmap xnwx", s_mmap_xnwx, 0, }, { "main", NULL, 1, &main }, { "libc unmapped?", NULL, 1, &wcstol }, { "libc mapped", NULL, 1, &mmap } }; jmp_buf fail; void sigsegv(int _unused) { longjmp(fail, 1); } void * s_mmap_xz(void) { return mmap(NULL, getpagesize(), PROT_EXEC, MAP_ANON | MAP_PRIVATE, -1, 0); /* no data written. tests read-fault of an unbacked exec-only page */ } void * s_mmap_x(void) { char *addr; addr = mmap(NULL, getpagesize(), PROT_READ | PROT_WRITE, MAP_ANON | MAP_PRIVATE, -1, 0); explicit_bzero(addr, getpagesize()); mprotect(addr, getpagesize(), PROT_EXEC); return addr; } void * s_mmap_nrx(void) { char *addr; addr = mmap(NULL, getpagesize(), PROT_NONE, MAP_ANON | MAP_PRIVATE, -1, 0); mprotect(addr, getpagesize(), PROT_READ | PROT_WRITE); explicit_bzero(addr, getpagesize()); mprotect(addr, getpagesize(), PROT_EXEC); return addr; } void * s_mmap_nwx(void) { char *addr; addr = mmap(NULL, getpagesize(), PROT_NONE, MAP_ANON | MAP_PRIVATE, -1, 0); mprotect(addr, getpagesize(), PROT_WRITE); explicit_bzero(addr, getpagesize()); mprotect(addr, getpagesize(), PROT_EXEC); return addr; } void * s_mmap_xnwx(void) { char *addr; addr = mmap(NULL, getpagesize(), PROT_EXEC, MAP_ANON | MAP_PRIVATE, -1, 0); mprotect(addr, getpagesize(), PROT_NONE); mprotect(addr, getpagesize(), PROT_WRITE); explicit_bzero(addr, getpagesize()); mprotect(addr, getpagesize(), PROT_EXEC); return addr; } void * s_ldso(void) { void *handle, *dlopenp; handle = dlopen("ld.so", RTLD_NOW); if (handle == NULL) *(volatile char *)0 = 0; dlopenp = dlsym(handle, "dlopen"); return dlopenp; } void s_table() { int i; for (i = 0; i < sizeof(readables)/sizeof(readables[0]); i++) { if (setjmp(fail) == 0) { if (readables[i].setup) readables[i].addr = readables[i].setup(); } else readables[i].skip = 1; #ifdef __hppa__ /* hppa ptable headers point at the instructions */ if (readables[i].isfn) readables[i].addr = (void *)*(u_int *) ((u_int)readables[i].addr & ~3); #endif } } int main(int argc, char *argv[]) { int p[2], i; signal(SIGSEGV, sigsegv); signal(SIGBUS, sigsegv); s_table(); for (i = 0; i < sizeof(readables)/sizeof(readables[0]); i++) { struct readable *r = &readables[i]; char c; if (r->skip) continue; pipe(p); fcntl(p[0], F_SETFL, O_NONBLOCK); if (write(p[1], r->addr, 1) == 1 && read(p[0], &c, 1) == 1) r->ku = 1; if (setjmp(fail) == 0) { volatile int x = *(int *)(r->addr); r->uu = 1; } close(p[0]); close(p[1]); } printf("%-16s userland kernel\n", ""); for (i = 0; i < sizeof(readables)/sizeof(readables[0]); i++) { struct readable *r = &readables[i]; if (r->skip) printf("%-16s %-10s %-10s\n", r->name, "skipped", "skipped"); else printf("%-16s %-10s %-10s\n", r->name, r->uu ? "readable" : "unreadable", r->ku ? "readable" : "unreadable"); } } #include #include #include #include #include #include #include #include #include #include #include #defineroundup(x, y) x)+((y)-1))/(y))*(y)) int pgsize; jmp_buf fail; void sigsegv(int _unused) { longjmp(fail, 1); } int callback(struct dl_phdr_info *info, size_t size, void *cookie) { Elf_Addr start, end, p; Elf_Off len; void *test; int i, count, total, col; /* Find all text segments */ for (i = 0; i < info->dlpi_phnum; i++) { if (info->dlpi_phdr[i].p_type == PT_LOAD && info->dlpi_phdr[i].p_flags & PF_X) { start = info->dlpi_addr + info->dlpi_phdr[i].p_vaddr; len = info->dlpi_phdr[i].p_memsz; end = start + len; printf("%s %llx-%llx (%llx, %lld pg) prot %s\n\t", info->dlpi_name, (uint64_t)start, (uint64_t)end, (uint64_t)len, ((uint64_t)len + pgsize) / pgsize, (info->dlpi_phdr[i].p_flags & PF_R) ? "RX" : "X"); test = malloc(pgsize); count = total = col = 0; /* attempt to read the entire text segment */ for (p = start; p < end; p += pgsize) { total++; if (setjmp(fail) == 0) { memcpy(test, (char *)p, pgsize); printf("y"); count++; } else { printf("n"); } if (++col % 64 == 0) printf("\n\t"); } printf("\n"); printf("\tread %d pages of
xonly status
We've made good progress in the xonly effort so here's a small summary. architectures crossed over completely arm64 - X bit without implied R in mmu riscv64 - X bit without implied R in mmu amd64 - using hardware 'PKU' feature powerpc64 - using feature similar to PKU hppa - using gateway feature architectures completed in the kernel, but needing ld.bfd work sparc64 - sun4u using split software TLB, sun4v cannot do this octeon - newer mips cpu have a read-inhibit bit in progress: macppc - powerpc G5 cpus, using a feature similar to PKU machines which cannot do this landisk sparc64 (sun4v) i386 alpha arm (32bit) macppc G4 cpus older mips64 amd64 cpu without PKU (but we have some ideas) A test program has been written which checks a variety of page mappings. Below, "userland" refers to the program directly accessing it's own memory. "kernel" refers to a program trying to write() the memory onto a pipe. It used to look like this: userland kernel ld.so readable readable mmap xz unreadable unreadable mmap xreadable readable mmap nrx readable readable mmap nwx readable readable mmap xnwx readable readable main readable readable libc unmapped?readable readable libc mapped readable readable On machines with fixes, it now looks like this: userland kernel ld.so unreadable unreadable mmap xz unreadable unreadable mmap xunreadable unreadable mmap nrx unreadable unreadable mmap nwx unreadable unreadable mmap xnwx unreadable unreadable main unreadable unreadable libc unmapped?unreadable unreadable libc mapped unreadable unreadable On machines lacking hardware enforcement ability, there is a diff coming which wraps the kernel copyin() and copyinstr() with some checks. 2 or 4 important code address spaces (in the main program, signal trampoline, ld.so, and libc.so) are added to a very short list, and checked ahead of time. This short list needs no locking because these address ranges are immutable (meaning, no address space changes can be made). This check is very quick and results in the following: userland kernel ld.so readable unreadable mmap xz unreadable unreadable mmap xreadable readable mmap nrx readable readable mmap nwx readable readable mmap xnwx readable readable main readable unreadable libc unmapped?readable unreadable libc mapped readable unreadable That blocks the classic "BROP" attack method of trying to write the text segments out a socket for offline gadget study. I want to bring attention to the "mmap xz" line. In the test program this is a new page mapped PROT_EXEC which has never been faulted. It contains no code, and no code is run there, so the first pagefault that happens against it is the testing PROT_READ page fault, and the higher-level VM code says no way -- this page can only be faulted with a PROT_EXEC request. That leads to a surprising behaviour, and another test program was written which tries to see what text pages may actually be read from userland. On a machine with proper hardware support, you cannot read any of the text pages. ./foo7b 445a7754960-445a7755190 (830, 1 pg) prot X n read 0 pages of 1 cannot read the whole /usr/lib/libc.so.96.5 4484b4139b0-4484b4b9c30 (a6280, 167 pg) prot X nnn read 0 pages of 167 cannot read the whole /usr/libexec/ld.so 4480810c000-44808117666 (b666, 12 pg) prot X read 0 pages of 12 cannot read the whole /usr/libexec/ld.so 4480810a000-4480810c000 (2000, 3 pg) prot RX nn read 0 pages of 2 cannot read the whole The surprise is what happens when this is tried on a machine which has no hardware enforcement, like i386 ./foo7b 1a67c670-1a67cea0 (830, 1 pg) prot RX n read 0 pages of 1 cannot read the whole /usr/lib/libc.so.96.5 d746400-d7daf70 (94b70, 149 pg) prot X yyynynnyyyyy yyyyynnynnyynnynnnyyynnyynyy yyyyn read 97 pages of 149 cannot read the whole /usr/libexec/ld.so 3683000-368d82f (a82f, 11 pg) prot X nyy read 10 pages of 11 cannot read the whole /usr/libexec/ld.so 3681000-3683000 (2000, 3 pg) prot RX nn read 0 pages of 2 cannot rea
Re: strptime.c
Unfortunately we cannot use strtonum(3) here since there may be non-digit characters following the number. So, strtoll(3) it is then. - todd Index: lib/libc/time/strptime.c === RCS file: /cvs/src/lib/libc/time/strptime.c,v retrieving revision 1.30 diff -u -p -u -r1.30 strptime.c --- lib/libc/time/strptime.c12 May 2019 12:49:52 - 1.30 +++ lib/libc/time/strptime.c29 Jan 2023 15:13:53 - @@ -29,8 +29,10 @@ */ #include +#include +#include #include -#include +#include #include #include @@ -72,8 +74,8 @@ static const int mon_lengths[2][MONSPERY { 31, 29, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31 } }; -static int _conv_num64(const unsigned char **, int64_t *, int64_t, int64_t); static int _conv_num(const unsigned char **, int *, int, int); +static int epoch_to_tm(const unsigned char **, struct tm *); static int leaps_thru_end_of(const int y); static char *_strptime(const char *, const char *, struct tm *, int); static const u_char *_find_string(const u_char *, int *, const char * const *, @@ -338,15 +340,10 @@ literal: if (!(_conv_num(&bp, &tm->tm_sec, 0, 60))) return (NULL); break; - case 's': /* Seconds since epoch */ - { - int64_t i64; - if (!(_conv_num64(&bp, &i64, 0, INT64_MAX))) - return (NULL); - if (!gmtime_r(&i64, tm)) - return (NULL); - fields = 0x; /* everything */ - } + case 's': /* Seconds since epoch. */ + if (!(epoch_to_tm(&bp, tm))) + return (NULL); + fields = 0x; /* everything */ break; case 'U': /* The week of year, beginning on sunday. */ case 'W': /* The week of year, beginning on monday. */ @@ -610,26 +607,27 @@ _conv_num(const unsigned char **buf, int } static int -_conv_num64(const unsigned char **buf, int64_t *dest, int64_t llim, int64_t ulim) +epoch_to_tm(const unsigned char **buf, struct tm *tm) { - int result = 0; - int64_t rulim = ulim; - - if (**buf < '0' || **buf > '9') - return (0); - - /* we use rulim to break out of the loop when we run out of digits */ - do { - result *= 10; - result += *(*buf)++ - '0'; - rulim /= 10; - } while ((result * 10 <= ulim) && rulim && **buf >= '0' && **buf <= '9'); - - if (result < llim || result > ulim) - return (0); - - *dest = result; - return (1); + int saved_errno = errno; + int ret = 0; + time_t secs; + char *ep; + + errno = 0; + secs = strtoll(*buf, &ep, 10); + if (*buf == (unsigned char *)ep) + goto done; + if (secs < 0 || + secs == LLONG_MAX && errno == ERANGE) + goto done; + if (localtime_r(&secs, tm) == NULL) + goto done; + ret = 1; +done: + *buf = ep; + errno = saved_errno; + return (ret); } static const u_char *
bgpd: remove ASDOT support
ASDOT started out as sort of a joke, but unfortunately gained some popularity in the 2010s with the rise of 4-byte ASNs and some people thinking "cute, I can write my longish number in a shorter exotic notation". Then, many operators came to realize that using a '.' (dot) in places which used to be simple integers is quite annoying (for example when regular expressions also are in play), and more fundamentally: why go through the trouble of using a complicated syntax when you can just write number itself? Perhaps time to bring ASDOT to the gardenshed? :-) Kind regards, Job Index: bgpd.conf.5 === RCS file: /cvs/src/usr.sbin/bgpd/bgpd.conf.5,v retrieving revision 1.230 diff -u -p -r1.230 bgpd.conf.5 --- bgpd.conf.5 24 Jan 2023 14:13:11 - 1.230 +++ bgpd.conf.5 29 Jan 2023 13:53:02 - @@ -127,17 +127,9 @@ for Latin America and the Caribbean for Europe, the Middle East, and parts of Asia .El .Pp -The AS numbers 64512 \(en 65534 are designated for private use. +The AS numbers 64496 \(en 65534 and 42 \(en 4294967294 are designated +for private use. The AS number 23456 is reserved and should not be used. -4-byte AS numbers may be specified in either the ASPLAIN format: -.Bd -literal -offset indent -AS 196618 -.Ed -.Pp -or in the older ASDOT format: -.Bd -literal -offset indent -AS 3.10 -.Ed .Pp .It Ic connect-retry Ar seconds Set the number of seconds to wait before attempting to re-open @@ -1991,7 +1983,7 @@ Communities are encoded as .Ar as-number : Ns Ar local . Four-octet encoding is used if the .Ar as-number -is bigger than 65535 or if the AS_DOT encoding is used. +is bigger than 65535. IPv4 Address Specific Extended Communities are encoded as .Ar IP : Ns Ar local . Opaque Extended Communities are encoded with a single numeric value. Index: parse.y === RCS file: /cvs/src/usr.sbin/bgpd/parse.y,v retrieving revision 1.440 diff -u -p -r1.440 parse.y --- parse.y 24 Jan 2023 14:13:11 - 1.440 +++ parse.y 29 Jan 2023 13:53:02 - @@ -242,7 +242,7 @@ typedef struct { %token NE LE GE XRANGE LONGER MAXLEN MAX %token STRING %token NUMBER -%typeasnumber as4number as4number_any optnumber +%typeasnumber as4number optnumber %typeespah family safi restart origincode nettype %typeyesno inout restricted expires enforce %typevalidity aspa_validity @@ -297,39 +297,7 @@ asnumber : NUMBER{ } } -as4number : STRING{ - const char *errstr; - char*dot; - uint32_t uvalh = 0, uval; - - if ((dot = strchr($1,'.')) != NULL) { - *dot++ = '\0'; - uvalh = strtonum($1, 0, USHRT_MAX, &errstr); - if (errstr) { - yyerror("number %s is %s", $1, errstr); - free($1); - YYERROR; - } - uval = strtonum(dot, 0, USHRT_MAX, &errstr); - if (errstr) { - yyerror("number %s is %s", dot, errstr); - free($1); - YYERROR; - } - free($1); - } else { - yyerror("AS %s is bad", $1); - free($1); - YYERROR; - } - if (uvalh == 0 && (uval == AS_TRANS || uval == 0)) { - yyerror("AS %u is reserved and may not be used", - uval); - YYERROR; - } - $$ = uval | (uvalh << 16); - } - | asnumber { +as4number : asnumber { if ($1 == AS_TRANS || $1 == 0) { yyerror("AS %u is reserved and may not be used", (uint32_t)$1); @@ -339,38 +307,6 @@ as4number : STRING{ } ; -as4number_any : STRING{ - const char *errstr; - char*dot; - uint32_t uvalh = 0, uval; - - if ((dot = strchr($1,'.')) != NULL) { - *dot++ = '\0'; - uvalh = strtonum($1, 0, USHRT_MAX, &errstr); -
npppd(8): remove "pipex" option
Hi, While switchind pppx(4) and pppac(4) from selwakeup() to KNOTE(9), I found npppd(8) doesn't create pppx interface with "pipex no" in npppd.conf, but successfully connects the client. So packets don't flow. However, the pppac(4) has no this problem, because corresponding pppac interface always created when npppd(8) opened device node. In fact, npppd(8) will not work with pppx(4) interfaces without pipex(4) support. Otherwise npppd(8) should create pppx(4) sessions with not pipex(4) specific PIPEXASESSION ioctl(2) command. I propose to remove "pipex" option from npppd(8). We already have "net.pipex.enable" sysctl MIB to control pipex behaviour. In the case then "net.pipex.enable" is set to 0, pipex(4) sessions will be always created, but the traffic will go outside pipex(4) layer. The "ifdef USE_NPPPD_PIPEX" left as is. If we decide to remove them, I will do this with the next diffs. Please note, we never have complains about the problem described above, so I doubt someone uses npppd(8) with "pipex no" in the npppd.conf(5). I tested both pppac(4) and pppx(4) cases with both "net.pipex.enable=1" and "net.pipex.enable=0". Index: usr.sbin/npppd/npppd/npppd.c === RCS file: /cvs/src/usr.sbin/npppd/npppd/npppd.c,v retrieving revision 1.53 diff -u -p -r1.53 npppd.c --- usr.sbin/npppd/npppd/npppd.c1 Jul 2022 09:57:24 - 1.53 +++ usr.sbin/npppd/npppd/npppd.c29 Jan 2023 11:04:30 - @@ -235,14 +235,12 @@ npppd_get_npppd() int npppd_init(npppd *_this, const char *config_file) { - int i, status = -1, value; + int i, status = -1; const char *pidpath0; FILE*pidfp = NULL; struct tunnconf *tunn; struct ipcpconf *ipcpconf; struct ipcpstat *ipcpstat; - int mib[] = { CTL_NET, PF_PIPEX, PIPEXCTL_ENABLE }; - size_t size; memset(_this, 0, sizeof(npppd)); #ifndefNO_ROUTE_FOR_POOLED_ADDRESS @@ -294,17 +292,6 @@ npppd_init(npppd *_this, const char *con if ((status = npppd_reload_config(_this)) != 0) return status; - TAILQ_FOREACH(tunn, &_this->conf.tunnconfs, entry) { - if (tunn->pipex) { - size = sizeof(value); - if (!sysctl(mib, nitems(mib), &value, &size, NULL, 0) - && value == 0) - log_printf(LOG_WARNING, - "pipex(4) is disabled by sysctl"); - break; - } - } - if ((_this->map_user_ppp = hash_create( (int (*) (const void *, const void *))strcmp, str_hash, NPPPD_USER_HASH_SIZ)) == NULL) { @@ -1052,7 +1039,6 @@ npppd_ppp_pipex_enable(npppd *_this, npp NPPPD_ASSERT(ppp != NULL); NPPPD_ASSERT(ppp->phy_context != NULL); - NPPPD_ASSERT(ppp->use_pipex != 0); pipex_setup_common(ppp, &req); Index: usr.sbin/npppd/npppd/npppd.conf.5 === RCS file: /cvs/src/usr.sbin/npppd/npppd/npppd.conf.5,v retrieving revision 1.30 diff -u -p -r1.30 npppd.conf.5 --- usr.sbin/npppd/npppd/npppd.conf.5 31 Mar 2022 17:27:30 - 1.30 +++ usr.sbin/npppd/npppd/npppd.conf.5 29 Jan 2023 11:04:30 - @@ -349,19 +349,6 @@ the address assigned by for the link. The default value is .Dq no . -.It Ic pipex Ar yes | no -Specify whether -.Xr npppd 8 -uses -.Xr pipex 4 . -The default is -.Dq yes . -The -.Xr sysctl 8 -variable -.Va net.pipex.enable -should also be enabled to use -.Xr pipex 4 . .It Ic debug-dump-pktin Ar protocol ... If this option is specified, .Xr npppd 8 Index: usr.sbin/npppd/npppd/npppd.h === RCS file: /cvs/src/usr.sbin/npppd/npppd/npppd.h,v retrieving revision 1.19 diff -u -p -r1.19 npppd.h --- usr.sbin/npppd/npppd/npppd.h12 Aug 2017 11:20:34 - 1.19 +++ usr.sbin/npppd/npppd/npppd.h29 Jan 2023 11:04:30 - @@ -133,8 +133,6 @@ struct tunnconf { bool ingress_filter; intcallnum_check; - bool pipex; - u_int debug_dump_pktin; u_int debug_dump_pktout; }; Index: usr.sbin/npppd/npppd/parse.y === RCS file: /cvs/src/usr.sbin/npppd/npppd/parse.y,v retrieving revision 1.25 diff -u -p -r1.25 parse.y --- usr.sbin/npppd/npppd/parse.y15 Oct 2021 15:01:28 - 1.25 +++ usr.sbin/npppd/npppd/parse.y29 Jan 2023 11:04:30 - @@ -125,7 +125,7 @@ typedef struct { %token L2TP_HELLO_INTERVAL L2TP_HELLO_TIMEOUT L2TP_ACCEPT_DIALIN %token MPPE MPPE_KEY_LENGTH MPPE_KEY_STATE %token IDLE_TIMEOUT TCP_M