Re: Fix vi(1) - recovery is a lie

[trondd]

Vi(1) recovery is broken.  At best, you lose your new work.  At the worst
you can even lose saved work on disk.  Pushing again to get something in
place to help the situation.

I've been running this for a long time on amd64 and arm64.  Besides my
testing, I've also experienced real power failures without data lose or a
crash during recovery.

Tim.

trondd  wrote:

> Any other dev interested in fixing this?  Feedback, suggestions, review?
> 
> Tim.
> 
> Andrew Hewus Fresh  wrote:
> 
> > In my quick test, this works a lot better than what we have now.  At
> > least I get back more of the file I was working on.  I also haven't been
> > able to reproduce the annoying segfault and half the original file
> > disappearing, although that was infrequent before so could just be luck.
> > 
> > User experience with this patch is improved in my opinion, I'd like to
> > see it go in, so OK afresh1@
> > 
> > On Sat, Oct 09, 2021 at 08:26:13PM -0400, trondd wrote:
> > > This is a new attempt at fixing vi(1) recovery by actually writing
> > > to the recovery file.  Previously I restored the SIGALRM method that
> > > was deleted in the 90's but wondered if that was still the best way
> > > to handle this.  Checking and syncing to the recovery every 2 minutes
> > > seems arbitrary and overly cautious.
> > > 
> > > This attempt takes it to the other direction.  I'm writing each
> > > change to the recovery file immediately after the in-memory database
> > > is modified.  Though, I can see that this might have a noticeable
> > > impact on slower file systems.
> > > 
> > > VIM takes a sort of hybrid approach and writes to the backup every
> > > 200 characters or after 4 seconds of idle time.  Perhaps this is the
> > > best method to not get too far behind while also not hammering the
> > > filesystem with quick edits.
> > > 
> > > For now, I'm sticking to the naive approach for review.  The diff is
> > > smaller than using SIGALRM and more straight forward and I'd like to
> > > hear what method might make sense to improve the process.  This code
> > > would probably be the basis for other improvements.
> > > 
> > > Below is my original explanation of the problem with vi(1)'s
> > > recovery.
> > > 
> > > This is a reference to the older SIGALRM diff (I have a version
> > > updated to use the atomic signal flags if we want to look at the
> > > SIGALRM process instead).
> > > https://marc.info/?l=openbsd-tech=162940011614049
> > > 
> > > Tim.
> > > 
> > > -
> > > 
> > > While investigating an occasional crash when recovering a file with 'vi 
> > > -r'
> > > after a power failure, I noticed that the recovery files are actually 
> > > never
> > > updated during an editing session.  The recovery files are created upon
> > > initial modification of the file which saves the state of the file at the
> > > time of the edit.  You can work on a file for as long as you want and even
> > > write it to disk but the recovery file is never updated.  If the session 
> > > is
> > > then lost due to power failure or a SIGKILL and you attempt to recover 
> > > with
> > > -r, you'll be presented with the contents of the file from that first 
> > > edit.
> > > It won't contain unsaved changes nor even any changes manually written to
> > > disk to the original file.  Accepting the recovered version would lose all
> > > of your work.
> > > 
> > > Reading the vi docs, man page, and source comments in the OpenBSD tree, 
> > > they
> > > all mention the use of SIGALRM to periodically save changes to the 
> > > recovery
> > > file.  However, the code never sets up a handler or captures SIGALRM.  It
> > > only ever updates the recovery file on a SIGTERM but then also exits, I
> > > guess to cover the case of an inadvertent clean system shutdown.
> > > 
> > > I dug through an nvi source repository[0] that seemed to cover it's entire
> > > history and it seems this functionality was lost somewhere around 1994 
> > > and I
> > > don't see it having been replaced by anything else.  Our version seems to 
> > > be
> > > from 1996 and editors/nvi in ports still lacks code to update the recovery
> > > file, as well.
> > > 
> > > What I've done is re-implement periodic updates to the recovery file using
> > > SIGALRM and a timer like the original implementation but rewritten a bit 
> 

Re: Fix vi(1) recovery - new method

[trondd]

Any other dev interested in fixing this?  Feedback, suggestions, review?

Tim.

Andrew Hewus Fresh  wrote:

> In my quick test, this works a lot better than what we have now.  At
> least I get back more of the file I was working on.  I also haven't been
> able to reproduce the annoying segfault and half the original file
> disappearing, although that was infrequent before so could just be luck.
> 
> User experience with this patch is improved in my opinion, I'd like to
> see it go in, so OK afresh1@
> 
> On Sat, Oct 09, 2021 at 08:26:13PM -0400, trondd wrote:
> > This is a new attempt at fixing vi(1) recovery by actually writing
> > to the recovery file.  Previously I restored the SIGALRM method that
> > was deleted in the 90's but wondered if that was still the best way
> > to handle this.  Checking and syncing to the recovery every 2 minutes
> > seems arbitrary and overly cautious.
> > 
> > This attempt takes it to the other direction.  I'm writing each
> > change to the recovery file immediately after the in-memory database
> > is modified.  Though, I can see that this might have a noticeable
> > impact on slower file systems.
> > 
> > VIM takes a sort of hybrid approach and writes to the backup every
> > 200 characters or after 4 seconds of idle time.  Perhaps this is the
> > best method to not get too far behind while also not hammering the
> > filesystem with quick edits.
> > 
> > For now, I'm sticking to the naive approach for review.  The diff is
> > smaller than using SIGALRM and more straight forward and I'd like to
> > hear what method might make sense to improve the process.  This code
> > would probably be the basis for other improvements.
> > 
> > Below is my original explanation of the problem with vi(1)'s
> > recovery.
> > 
> > This is a reference to the older SIGALRM diff (I have a version
> > updated to use the atomic signal flags if we want to look at the
> > SIGALRM process instead).
> > https://marc.info/?l=openbsd-tech=162940011614049
> > 
> > Tim.
> > 
> > -
> > 
> > While investigating an occasional crash when recovering a file with 'vi -r'
> > after a power failure, I noticed that the recovery files are actually never
> > updated during an editing session.  The recovery files are created upon
> > initial modification of the file which saves the state of the file at the
> > time of the edit.  You can work on a file for as long as you want and even
> > write it to disk but the recovery file is never updated.  If the session is
> > then lost due to power failure or a SIGKILL and you attempt to recover with
> > -r, you'll be presented with the contents of the file from that first edit.
> > It won't contain unsaved changes nor even any changes manually written to
> > disk to the original file.  Accepting the recovered version would lose all
> > of your work.
> > 
> > Reading the vi docs, man page, and source comments in the OpenBSD tree, they
> > all mention the use of SIGALRM to periodically save changes to the recovery
> > file.  However, the code never sets up a handler or captures SIGALRM.  It
> > only ever updates the recovery file on a SIGTERM but then also exits, I
> > guess to cover the case of an inadvertent clean system shutdown.
> > 
> > I dug through an nvi source repository[0] that seemed to cover it's entire
> > history and it seems this functionality was lost somewhere around 1994 and I
> > don't see it having been replaced by anything else.  Our version seems to be
> > from 1996 and editors/nvi in ports still lacks code to update the recovery
> > file, as well.
> > 
> > What I've done is re-implement periodic updates to the recovery file using
> > SIGALRM and a timer like the original implementation but rewritten a bit to
> > fit the newer source file layout and event handling.  That keeps the 
> > recovery
> > updated every 2 minutes.  Then it seemed silly to be able to write changes 
> > to
> > the original file and if a crash happens before the next SIGALRM, recovery
> > would try to roll you back to before those saved changes.  So I've also 
> > added
> > a call to sync the recovery file if you explicitly write changes to disk.  I
> > don't think the recovery system should try to punish you for actively saving
> > your work even if it is only at most 2 minutes worth.
> > 
> > Comments or feedback?  I'm unsure I've covered all caveats with this code or
> > if there are vi/ex usecases where it won't work correctly.  For testing, 
> > I've
> > covered my usage and several scenarios I could contrive but

Re: Fix vi(1) recovery - new method

[trondd]

Any feedback, direction, or suggestions?  I'd like to see something
get in as the current situation not only doesn't recover unsaved work
but it sets a user up to potentially lose saved work, too.

Tim.


trondd  wrote:

> This is a new attempt at fixing vi(1) recovery by actually writing
> to the recovery file.  Previously I restored the SIGALRM method that
> was deleted in the 90's but wondered if that was still the best way
> to handle this.  Checking and syncing to the recovery every 2 minutes
> seems arbitrary and overly cautious.
> 
> This attempt takes it to the other direction.  I'm writing each
> change to the recovery file immediately after the in-memory database
> is modified.  Though, I can see that this might have a noticeable
> impact on slower file systems.
> 
> VIM takes a sort of hybrid approach and writes to the backup every
> 200 characters or after 4 seconds of idle time.  Perhaps this is the
> best method to not get too far behind while also not hammering the
> filesystem with quick edits.
> 
> For now, I'm sticking to the naive approach for review.  The diff is
> smaller than using SIGALRM and more straight forward and I'd like to
> hear what method might make sense to improve the process.  This code
> would probably be the basis for other improvements.
> 
> Below is my original explanation of the problem with vi(1)'s
> recovery.
> 
> This is a reference to the older SIGALRM diff (I have a version
> updated to use the atomic signal flags if we want to look at the
> SIGALRM process instead).
> https://marc.info/?l=openbsd-tech=162940011614049
> 
> Tim.
> 
> -
> 
> While investigating an occasional crash when recovering a file with 'vi -r'
> after a power failure, I noticed that the recovery files are actually never
> updated during an editing session.  The recovery files are created upon
> initial modification of the file which saves the state of the file at the
> time of the edit.  You can work on a file for as long as you want and even
> write it to disk but the recovery file is never updated.  If the session is
> then lost due to power failure or a SIGKILL and you attempt to recover with
> -r, you'll be presented with the contents of the file from that first edit.
> It won't contain unsaved changes nor even any changes manually written to
> disk to the original file.  Accepting the recovered version would lose all
> of your work.
> 
> Reading the vi docs, man page, and source comments in the OpenBSD tree, they
> all mention the use of SIGALRM to periodically save changes to the recovery
> file.  However, the code never sets up a handler or captures SIGALRM.  It
> only ever updates the recovery file on a SIGTERM but then also exits, I
> guess to cover the case of an inadvertent clean system shutdown.
> 
> I dug through an nvi source repository[0] that seemed to cover it's entire
> history and it seems this functionality was lost somewhere around 1994 and I
> don't see it having been replaced by anything else.  Our version seems to be
> from 1996 and editors/nvi in ports still lacks code to update the recovery
> file, as well.
> 
> What I've done is re-implement periodic updates to the recovery file using
> SIGALRM and a timer like the original implementation but rewritten a bit to
> fit the newer source file layout and event handling.  That keeps the recovery
> updated every 2 minutes.  Then it seemed silly to be able to write changes to
> the original file and if a crash happens before the next SIGALRM, recovery
> would try to roll you back to before those saved changes.  So I've also added
> a call to sync the recovery file if you explicitly write changes to disk.  I
> don't think the recovery system should try to punish you for actively saving
> your work even if it is only at most 2 minutes worth.
> 
> Comments or feedback?  I'm unsure I've covered all caveats with this code or
> if there are vi/ex usecases where it won't work correctly.  For testing, I've
> covered my usage and several scenarios I could contrive but I don't regularly
> use ex, for example, or change many options from the default.  I've been
> running with this code for a week.  And I suppose there must be a reason no
> one has noticed or cared about this for over 20 years.  Everyone else uses
> vim, I guess?
> 
> Tim.
> 
> [0] https://repo.or.cz/nvi.git
> 
> -
> 

Index: common/exf.h
===
RCS file: /cvs/src/usr.bin/vi/common/exf.h,v
retrieving revision 1.5
diff -u -p -r1.5 exf.h
--- common/exf.h24 Apr 2015 21:48:31 -  1.5
+++ common/exf.h9 Oct 2021 22:40:17 -
@@ -58,7 +58,8 @@ struct _exf {
 #defineF_RCV_NORM  0x020   /* Don't delete recovery files. 

Re: update xterm to version 369

[TronDD]

On Tue Oct 12, 2021 at 2:08 PM EDT, Matthieu Herrb wrote:
> Hi,
>
> The patch below updates xterm to version 369. Please test and report
> failures especially if you rely on some obscure feature...
>
> Changes:
>
> Patch #369 - 2021/09/21
>
> * modify run-tic.sh to work around bug in development version of
> ncurses which was packaged in FreeBSD ports.
> * remove ifdef's for OPT_COLOR_RES and OPT_COLOR_RES2.
> * improve performance over slow connections (report by Harald
> Dunkel).
> * update cursor if restoring mode for DECTCEM.
> * modify CharWidth macro to ensure that the shortcut for Latin-1 is
> only applied when UTF-8 is not enabled, to fix a bug in handling
> soft-hyphen from patch #334 changes (patch by Martijn van Duren).
> * improve terminfo:
> + fill-in function-keys in terminfo which are not Sun/HP
> keyboards using xterm+nopcfkeys building-block.
> + add kbeg to xterm+keypad to accommodate termcap applications
> + add smglp and smgrp to vt420+lrmm, to provide useful data for
> the "tabs" +m option
> * support shift-tab in Sun, HP and SCO keyboards.
> * document some legacy features in ctlseqs.ms (prompted by discussion
> with Jimmy Aguilar Mena "Ergus").
> * add "trim" option to cdXtraScroll and tiXtraScroll.
> * remove support for non-fifo save-lines configuration.
> * extend cdXtraScroll to check if the cursor is at the upper-left of
> the scrolling region when the erasure is for the remainder of the
> screen versus the whole screen (prompted by discussion with J g
> Breitbart).
> * add workaround for broken pcre2 package in Debian 10.
> * change screen-refresh call used for DECCARA and DECRARA to ensure
> that trailing blanks which are part of the rectangle are repainted
> (report/analysis by Dennis Filder).
> * when resetting the terminal, ensure that the cursor shape also is
> reset, e.g., if DECSCUSR has been used to modify the cursor shape
> for an xterm which was started with the underlined cursor option
> (report/analysis by Luis Javier Merino).
> * prevent DECSCUSR from blinking the cursor if the cursorBlink
> resource is "never" (report by Vladimir D Seleznev).
> * invert the sense of DECSDM, to correspond with VT382 manuals (lsix
> #41).
> * update tables in wcwidth.c based on Unicode 14.0.0
>
> Patch #368 - 2021/06/07
>
> * add DefaultOff option to RenderFont resource, as part of the
> session-management feature.
> * add auto-scroll-lock feature (patch by Stelios Bounanos).
> * update the window-size information returned via TIOCGWINSZ when
> rows/columns are unchanged but the font-size changes (report by
> Nick Black).
> * improve session-management feature by saving/restoring the font
> settings.
> * update config.guess, config.sub
>

Looks good for me on AMD64 with my usual usecases.  Japanese text display
and input.  Curses applications.  Font sizes.

Tim.

Fix vi(1) recovery - new method

[trondd]

This is a new attempt at fixing vi(1) recovery by actually writing
to the recovery file.  Previously I restored the SIGALRM method that
was deleted in the 90's but wondered if that was still the best way
to handle this.  Checking and syncing to the recovery every 2 minutes
seems arbitrary and overly cautious.

This attempt takes it to the other direction.  I'm writing each
change to the recovery file immediately after the in-memory database
is modified.  Though, I can see that this might have a noticeable
impact on slower file systems.

VIM takes a sort of hybrid approach and writes to the backup every
200 characters or after 4 seconds of idle time.  Perhaps this is the
best method to not get too far behind while also not hammering the
filesystem with quick edits.

For now, I'm sticking to the naive approach for review.  The diff is
smaller than using SIGALRM and more straight forward and I'd like to
hear what method might make sense to improve the process.  This code
would probably be the basis for other improvements.

Below is my original explanation of the problem with vi(1)'s
recovery.

This is a reference to the older SIGALRM diff (I have a version
updated to use the atomic signal flags if we want to look at the
SIGALRM process instead).
https://marc.info/?l=openbsd-tech=162940011614049

Tim.

-

While investigating an occasional crash when recovering a file with 'vi -r'
after a power failure, I noticed that the recovery files are actually never
updated during an editing session.  The recovery files are created upon
initial modification of the file which saves the state of the file at the
time of the edit.  You can work on a file for as long as you want and even
write it to disk but the recovery file is never updated.  If the session is
then lost due to power failure or a SIGKILL and you attempt to recover with
-r, you'll be presented with the contents of the file from that first edit.
It won't contain unsaved changes nor even any changes manually written to
disk to the original file.  Accepting the recovered version would lose all
of your work.

Reading the vi docs, man page, and source comments in the OpenBSD tree, they
all mention the use of SIGALRM to periodically save changes to the recovery
file.  However, the code never sets up a handler or captures SIGALRM.  It
only ever updates the recovery file on a SIGTERM but then also exits, I
guess to cover the case of an inadvertent clean system shutdown.

I dug through an nvi source repository[0] that seemed to cover it's entire
history and it seems this functionality was lost somewhere around 1994 and I
don't see it having been replaced by anything else.  Our version seems to be
from 1996 and editors/nvi in ports still lacks code to update the recovery
file, as well.

What I've done is re-implement periodic updates to the recovery file using
SIGALRM and a timer like the original implementation but rewritten a bit to
fit the newer source file layout and event handling.  That keeps the recovery
updated every 2 minutes.  Then it seemed silly to be able to write changes to
the original file and if a crash happens before the next SIGALRM, recovery
would try to roll you back to before those saved changes.  So I've also added
a call to sync the recovery file if you explicitly write changes to disk.  I
don't think the recovery system should try to punish you for actively saving
your work even if it is only at most 2 minutes worth.

Comments or feedback?  I'm unsure I've covered all caveats with this code or
if there are vi/ex usecases where it won't work correctly.  For testing, I've
covered my usage and several scenarios I could contrive but I don't regularly
use ex, for example, or change many options from the default.  I've been
running with this code for a week.  And I suppose there must be a reason no
one has noticed or cared about this for over 20 years.  Everyone else uses
vim, I guess?

Tim.

[0] https://repo.or.cz/nvi.git

-


Index: common/exf.h
===
RCS file: /cvs/src/usr.bin/vi/common/exf.h,v
retrieving revision 1.5
diff -u -p -r1.5 exf.h
--- common/exf.h24 Apr 2015 21:48:31 -  1.5
+++ common/exf.h9 Oct 2021 22:40:17 -
@@ -58,7 +58,8 @@ struct _exf {
 #defineF_RCV_NORM  0x020   /* Don't delete recovery files. 
*/
 #defineF_RCV_ON0x040   /* Recovery is possible. */
 #defineF_UNDO  0x080   /* No change since last undo. */
-   u_int8_t flags;
+#defineF_RCV_SYNC  0x100   /* Recovery file sync needed. */
+   u_int16_t flags;
 };
 
 /* Flags to db_get(). */
Index: common/line.c
===
RCS file: /cvs/src/usr.bin/vi/common/line.c,v
retrieving revision 1.15
diff -u -p -r1.15 line.c
--- common/line.c   6 Jan 2016 22:28:52 -   1.15
+++ common/line.c   9 Oct 2021 22:40:17 -
@@ -218,7 

Re: Atomic signal flags for vi(1)

[trondd]

Ingo Schwarze  wrote:

> Hi,
> 
> Ingo Schwarze wrote on Wed, Sep 01, 2021 at 04:38:51PM +0200:
> 
> > Note that the h_hup() and h_term() signal handlers are still unsafe
> > after this commit because they also set the "killersig" (how fitting!)
> > field in a global struct.
> 
> I like it when fixing two bugs only amounts to minus: minus 14 LOC,
> 1 function, 1 global variable, 2 automatic variables, 1 struct member,
> 9 pointer dereferences, 1 #define, and minus 1 #undef.
> 
> The argument that only one single GS exists applies unchanged.
> 
> OK?
>   Ingo

Great.  This is the direction I was going to go in with it.  I hadn't
thought of collapsing h_hup an h_term, though.

This makes sense as I understand the signal/event handling and a quick
test through the signals shows no difference in behavior.

Should h_term() and cl_sigterm be named something more general to
indicate that they also handle SIGHUP?  Or is it good enough since it
includes all the signals that 'term'inate vi?  It's not hard to follow
as-is. 

Tim. (not @ :P )


> 
> 
> Index: cl/cl.h
> ===
> RCS file: /cvs/src/usr.bin/vi/cl/cl.h,v
> retrieving revision 1.11
> diff -u -p -r1.11 cl.h
> --- cl/cl.h   1 Sep 2021 14:28:15 -   1.11
> +++ cl/cl.h   1 Sep 2021 17:15:34 -
> @@ -11,7 +11,6 @@
>   *   @(#)cl.h10.19 (Berkeley) 9/24/96
>   */
>  
> -extern   volatile sig_atomic_t cl_sighup;
>  extern   volatile sig_atomic_t cl_sigint;
>  extern   volatile sig_atomic_t cl_sigterm;
>  extern   volatile sig_atomic_t cl_sigwinch;
> @@ -31,7 +30,6 @@ typedef struct _cl_private {
>   char*rmso, *smso;   /* Inverse video terminal strings. */
>   char*smcup, *rmcup; /* Terminal start/stop strings. */
>  
> - int  killersig; /* Killer signal. */
>  #define  INDX_HUP0
>  #define  INDX_INT1
>  #define  INDX_TERM   2
> Index: cl/cl_funcs.c
> ===
> RCS file: /cvs/src/usr.bin/vi/cl/cl_funcs.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 cl_funcs.c
> --- cl/cl_funcs.c 1 Sep 2021 14:28:15 -   1.21
> +++ cl/cl_funcs.c 1 Sep 2021 17:15:34 -
> @@ -437,7 +437,7 @@ cl_refresh(SCR *sp, int repaint)
>* If we received a killer signal, we're done, there's no point
>* in refreshing the screen.
>*/
> - if (clp->killersig)
> + if (cl_sigterm)
>   return (0);
>  
>   /*
> @@ -582,7 +582,7 @@ cl_suspend(SCR *sp, int *allowedp)
>* unchanged.  In addition, the terminal has already been reset
>* correctly, so leave it alone.
>*/
> - if (clp->killersig) {
> + if (cl_sigterm) {
>   F_CLR(clp, CL_SCR_EX_INIT | CL_SCR_VI_INIT);
>   return (0);
>   }
> Index: cl/cl_main.c
> ===
> RCS file: /cvs/src/usr.bin/vi/cl/cl_main.c,v
> retrieving revision 1.34
> diff -u -p -r1.34 cl_main.c
> --- cl/cl_main.c  1 Sep 2021 14:28:15 -   1.34
> +++ cl/cl_main.c  1 Sep 2021 17:15:34 -
> @@ -33,7 +33,6 @@
>  
>  GS *__global_list;   /* GLOBAL: List of screens. */
>  
> -volatile sig_atomic_t cl_sighup;
>  volatile sig_atomic_t cl_sigint;
>  volatile sig_atomic_t cl_sigterm;
>  volatile sig_atomic_t cl_sigwinch;
> @@ -120,9 +119,9 @@ main(int argc, char *argv[])
>   }
>  
>   /* If a killer signal arrived, pretend we just got it. */
> - if (clp->killersig) {
> - (void)signal(clp->killersig, SIG_DFL);
> - (void)kill(getpid(), clp->killersig);
> + if (cl_sigterm) {
> + (void)signal(cl_sigterm, SIG_DFL);
> + (void)kill(getpid(), cl_sigterm);
>   /* NOTREACHED */
>   }
>  
> @@ -215,17 +214,6 @@ term_init(char *ttype)
>   }
>  }
>  
> -#define  GLOBAL_CLP \
> - CL_PRIVATE *clp = GCLP(__global_list);
> -static void
> -h_hup(int signo)
> -{
> - GLOBAL_CLP;
> -
> - cl_sighup = 1;
> - clp->killersig = SIGHUP;
> -}
> -
>  static void
>  h_int(int signo)
>  {
> @@ -235,10 +223,7 @@ h_int(int signo)
>  static void
>  h_term(int signo)
>  {
> - GLOBAL_CLP;
> -
> - cl_sigterm = 1;
> - clp->killersig = SIGTERM;
> + cl_sigterm = signo;
>  }
>  
>  static void
> @@ -246,7 +231,6 @@ h_winch(int signo)
>  {
>   cl_sigwinch = 1;
>  }
> -#undef   GLOBAL_CLP
>  
>  /*
>   * sig_init --
> @@ -261,20 +245,19 @@ sig_init(GS *gp, SCR *sp)
>  
>   clp = GCLP(gp);
>  
> - cl_sighup = 0;
>   cl_sigint = 0;
>   cl_sigterm = 0;
>   cl_sigwinch = 0;
>  
>   if (sp == NULL) {
> - if (setsig(SIGHUP, >oact[INDX_HUP], h_hup) ||
> + if (setsig(SIGHUP, >oact[INDX_HUP], h_term) ||
>   setsig(SIGINT, >oact[INDX_INT], h_int) ||
>   setsig(SIGTERM, >oact[INDX_TERM], h_term) ||
> 

Atomic signal flags for vi(1)

[trondd]

"Theo de Raadt"  wrote:

> +h_alrm(int signo)
> +{
> +   GLOBAL_CLP;
> +
> +   F_SET(clp, CL_SIGALRM);
> 
> F_SET is |=, which is not atomic.
> 
> This is unsafe.  Safe signal handlers need to make single stores to
> atomic-sized variables, which tend to be int-sized, easier to declare
> as sig_atomic_t.  Most often these are global scope with the following
> type:
> 
>   volatile sig_atomic_t variable
> 
> I can see you copied an existing practice.  Sadly all the other
> signal handlers are also broken in the same way.
> 
> The flag bits should be replaced with seperate sig_atomic_t variables.

Ok.  Took a swing at converting the signal handling to sig_atomic_t flags.
No additional changes added other than removing a redundant check of the
flags in cl_read.c.  Seemed to be a pretty straight-forward conversion and
I haven't found any change in behavior.

Tim.


Index: cl/cl.h
===
RCS file: /cvs/src/usr.bin/vi/cl/cl.h,v
retrieving revision 1.10
diff -u -p -r1.10 cl.h
--- cl/cl.h 27 May 2016 09:18:11 -  1.10
+++ cl/cl.h 24 Aug 2021 23:34:27 -
@@ -11,6 +11,11 @@
  * @(#)cl.h10.19 (Berkeley) 9/24/96
  */
 
+extern volatile sig_atomic_t cl_sighup;
+extern volatile sig_atomic_t cl_sigint;
+extern volatile sig_atomic_t cl_sigterm;
+extern volatile sig_atomic_t cl_sigwinch;
+
 typedef struct _cl_private {
CHAR_T   ibuf[256]; /* Input keys. */
 
@@ -45,11 +50,7 @@ typedef struct _cl_private {
 #defineCL_RENAME_OK0x0004  /* User wants the windows renamed. */
 #defineCL_SCR_EX_INIT  0x0008  /* Ex screen initialized. */
 #defineCL_SCR_VI_INIT  0x0010  /* Vi screen initialized. */
-#defineCL_SIGHUP   0x0020  /* SIGHUP arrived. */
-#defineCL_SIGINT   0x0040  /* SIGINT arrived. */
-#defineCL_SIGTERM  0x0080  /* SIGTERM arrived. */
-#defineCL_SIGWINCH 0x0100  /* SIGWINCH arrived. */
-#defineCL_STDIN_TTY0x0200  /* Talking to a terminal. */
+#defineCL_STDIN_TTY0x0020  /* Talking to a terminal. */
u_int32_t flags;
 } CL_PRIVATE;
 
Index: cl/cl_funcs.c
===
RCS file: /cvs/src/usr.bin/vi/cl/cl_funcs.c,v
retrieving revision 1.20
diff -u -p -r1.20 cl_funcs.c
--- cl/cl_funcs.c   27 May 2016 09:18:11 -  1.20
+++ cl/cl_funcs.c   24 Aug 2021 23:34:27 -
@@ -601,7 +601,7 @@ cl_suspend(SCR *sp, int *allowedp)
if (cl_ssize(sp, 1, NULL, NULL, ))
return (1);
if (changed)
-   F_SET(CLP(sp), CL_SIGWINCH);
+   cl_sigwinch = 1;
 
return (0);
 }
Index: cl/cl_main.c
===
RCS file: /cvs/src/usr.bin/vi/cl/cl_main.c,v
retrieving revision 1.33
diff -u -p -r1.33 cl_main.c
--- cl/cl_main.c5 May 2016 20:36:41 -   1.33
+++ cl/cl_main.c24 Aug 2021 23:34:27 -
@@ -33,6 +33,11 @@
 
 GS *__global_list; /* GLOBAL: List of screens. */
 
+volatile sig_atomic_t cl_sighup;
+volatile sig_atomic_t cl_sigint;
+volatile sig_atomic_t cl_sigterm;
+volatile sig_atomic_t cl_sigwinch;
+
 static void   cl_func_std(GS *);
 static CL_PRIVATE *cl_init(GS *);
 static GS*gs_init(void);
@@ -217,16 +222,14 @@ h_hup(int signo)
 {
GLOBAL_CLP;
 
-   F_SET(clp, CL_SIGHUP);
+   cl_sighup = 1;
clp->killersig = SIGHUP;
 }
 
 static void
 h_int(int signo)
 {
-   GLOBAL_CLP;
-
-   F_SET(clp, CL_SIGINT);
+   cl_sigint = 1;
 }
 
 static void
@@ -234,16 +237,14 @@ h_term(int signo)
 {
GLOBAL_CLP;
 
-   F_SET(clp, CL_SIGTERM);
+   cl_sigterm = 1;
clp->killersig = SIGTERM;
 }
 
 static void
 h_winch(int signo)
 {
-   GLOBAL_CLP;
-
-   F_SET(clp, CL_SIGWINCH);
+   cl_sigwinch = 1;
 }
 #undef GLOBAL_CLP
 
@@ -259,6 +260,11 @@ sig_init(GS *gp, SCR *sp)
CL_PRIVATE *clp;
 
clp = GCLP(gp);
+
+   cl_sighup = 0;
+   cl_sigint = 0;
+   cl_sigterm = 0;
+   cl_sigwinch = 0;
 
if (sp == NULL) {
if (setsig(SIGHUP, >oact[INDX_HUP], h_hup) ||
Index: cl/cl_read.c
===
RCS file: /cvs/src/usr.bin/vi/cl/cl_read.c,v
retrieving revision 1.21
diff -u -p -r1.21 cl_read.c
--- cl/cl_read.c27 May 2016 09:18:11 -  1.21
+++ cl/cl_read.c24 Aug 2021 23:34:27 -
@@ -54,34 +54,32 @@ cl_event(SCR *sp, EVENT *evp, u_int32_t 
 * so that we just keep returning them until the editor dies.
 */
clp = CLP(sp);
-retest:if (LF_ISSET(EC_INTERRUPT) || F_ISSET(clp, CL_SIGINT)) {
-   if (F_ISSET(clp, CL_SIGINT)) {
-   F_CLR(clp, CL_SIGINT);
+retest:if (LF_ISSET(EC_INTERRUPT) || cl_sigint) {
+   if (cl_sigint) {
+   cl_sigint 

Re: Improve vi(1) recovery

[trondd]

I've switched this to using alarm(3) instead of setitimer(2) which is a
little simpler in the code but most of the changes are just vi event
handling boilderplate.  There is very little new functional code.

Original investigation write-up follows.  Have had quite a bit of user
feedback and testing in support of fixing this.

Tim.

-

While investigating an occasional crash when recovering a file with 'vi -r'
after a power failure, I noticed that the recovery files are actually never
updated during an editing session.  The recovery files are created upon
initial modification of the file which saves the state of the file at the
time of the edit.  You can work on a file for as long as you want and even
write it to disk but the recovery file is never updated.  If the session is
then lost due to power failure or a SIGKILL and you attempt to recover with
-r, you'll be presented with the contents of the file from that first edit.
It won't contain unsaved changes nor even any changes manually written to
disk to the original file.  Accepting the recovered version would lose all
of your work.

Reading the vi docs, man page, and source comments in the OpenBSD tree, they
all mention the use of SIGALRM to periodically save changes to the recovery
file.  However, the code never sets up a handler or captures SIGALRM.  It
only ever updates the recovery file on a SIGTERM but then also exits, I
guess to cover the case of an inadvertent clean system shutdown.

I dug through an nvi source repository[0] that seemed to cover it's entire
history and it seems this functionality was lost somewhere around 1994 and I
don't see it having been replaced by anything else.  Our version seems to be
from 1996 and editors/nvi in ports still lacks code to update the recovery
file, as well.

What I've done is re-implement periodic updates to the recovery file using
SIGALRM and a timer like the original implementation but rewritten a bit to
fit the newer source file layout and event handling.  That keeps the recovery
updated every 2 minutes.  Then it seemed silly to be able to write changes to
the original file and if a crash happens before the next SIGALRM, recovery
would try to roll you back to before those saved changes.  So I've also added
a call to sync the recovery file if you explicitly write changes to disk.  I
don't think the recovery system should try to punish you for actively saving
your work even if it is only at most 2 minutes worth.

Comments or feedback?  I'm unsure I've covered all caveats with this code or
if there are vi/ex usecases where it won't work correctly.  For testing, I've
covered my usage and several scenarios I could contrive but I don't regularly
use ex, for example, or change many options from the default.  I've been
running with this code for a week.  And I suppose there must be a reason no
one has noticed or cared about this for over 20 years.  Everyone else uses
vim, I guess?

Tim.

[0] https://repo.or.cz/nvi.git


Index: cl/cl.h
===
RCS file: /cvs/src/usr.bin/vi/cl/cl.h,v
retrieving revision 1.10
diff -u -p -r1.10 cl.h
--- cl/cl.h 27 May 2016 09:18:11 -  1.10
+++ cl/cl.h 19 Aug 2021 19:04:01 -
@@ -30,8 +30,9 @@ typedef struct _cl_private {
 #defineINDX_HUP0
 #defineINDX_INT1
 #defineINDX_TERM   2
-#defineINDX_WINCH  3
-#defineINDX_MAX4   /* Original signal information. */
+#defineINDX_ALRM   3
+#defineINDX_WINCH  4
+#defineINDX_MAX5   /* Original signal information. */
struct sigaction oact[INDX_MAX];
 
enum {  /* Tty group write mode. */
@@ -48,8 +49,9 @@ typedef struct _cl_private {
 #defineCL_SIGHUP   0x0020  /* SIGHUP arrived. */
 #defineCL_SIGINT   0x0040  /* SIGINT arrived. */
 #defineCL_SIGTERM  0x0080  /* SIGTERM arrived. */
-#defineCL_SIGWINCH 0x0100  /* SIGWINCH arrived. */
-#defineCL_STDIN_TTY0x0200  /* Talking to a terminal. */
+#defineCL_SIGALRM  0x0100  /* SIGALRM arrived. */
+#defineCL_SIGWINCH 0x0200  /* SIGWINCH arrived. */
+#defineCL_STDIN_TTY0x0400  /* Talking to a terminal. */
u_int32_t flags;
 } CL_PRIVATE;
 
Index: cl/cl_main.c
===
RCS file: /cvs/src/usr.bin/vi/cl/cl_main.c,v
retrieving revision 1.33
diff -u -p -r1.33 cl_main.c
--- cl/cl_main.c5 May 2016 20:36:41 -   1.33
+++ cl/cl_main.c19 Aug 2021 19:04:01 -
@@ -239,6 +239,14 @@ h_term(int signo)
 }
 
 static void
+h_alrm(int signo)
+{
+   GLOBAL_CLP;
+
+   F_SET(clp, CL_SIGALRM);
+}
+
+static void
 h_winch(int signo)
 {
GLOBAL_CLP;
@@ -264,6 +272,7 @@ sig_init(GS *gp, SCR *sp)
if (setsig(SIGHUP, >oact[INDX_HUP], h_hup) ||
setsig(SIGINT, >oact[INDX_INT], 

Re: Improve vi(1) recovery

[trondd]

trondd  wrote:

> trondd  wrote:
> 
> > trondd  wrote:
> > 
> > > 
> > > While investigating an occasional crash when recovering a file with 'vi 
> > > -r'
> > > after a power failure, I noticed that the recovery files are actually 
> > > never
> > > updated during an editing session.  The recovery files are created upon
> > > initial modification of the file which saves the state of the file at the
> > > time of the edit.  You can work on a file for as long as you want and even
> > > write it to disk but the recovery file is never updated.  If the session 
> > > is
> > > then lost due to power failure or a SIGKILL and you attempt to recover 
> > > with
> > > -r, you'll be presented with the contents of the file from that first 
> > > edit.
> > > It won't contain unsaved changes nor even any changes manually written to
> > > disk to the original file.  Accepting the recovered version would lose all
> > > of your work.
> > > 
> > > Reading the vi docs, man page, and source comments in the OpenBSD tree, 
> > > they
> > > all mention the use of SIGALRM to periodically save changes to the 
> > > recovery
> > > file.  However, the code never sets up a handler or captures SIGALRM.  It
> > > only ever updates the recovery file on a SIGTERM but then also exists, I
> > > guess to cover the case of an inadvertent clean system shutdown.
> > > 
> > > I dug through an nvi source repository[0] that seemed to cover it's entire
> > > history and it seems this functionality was lost somewhere around 1994 
> > > and I
> > > don't see it having been replaced by anything else.  Our version seems to 
> > > be
> > > from 1996 and editors/nvi in ports still lacks code to update the recovery
> > > file, as well.
> > > 
> > > What I've done is re-implement periodic updates to the recovery file using
> > > SIGALRM and a timer like the original implementation but rewritten a bit 
> > > to
> > > fit the newer source file layout and event handling.  That keeps the 
> > > recovery
> > > updated every 2 minutes.  Then it seemed silly to be able to write 
> > > changes to
> > > the original file and if a crash happens before the next SIGALRM, recovery
> > > would try to roll you back to before those saved changes.  So I've also 
> > > added
> > > a call to sync the recovery file if you explicitly write changes to disk. 
> > >  I
> > > don't think the recovery system should try to punish you for actively 
> > > saving
> > > your work even if it is only at most 2 minutes worth.
> > > 
> > > Comments or feedback?  I'm unsure I've covered all caveats with this code 
> > > or
> > > if there are vi/ex usecases where it won't work correctly.  For testing, 
> > > I've
> > > covered my usage and several scenarios I could contrive but I don't 
> > > regularly
> > > use ex, for example, or change many options from the default.  I've been
> > > running with this code for a week.  And I suppose there must be a reason 
> > > no
> > > one has noticed or cared about this for over 20 years.  Everyone else uses
> > > vim, I guess?
> > > 
> > > Tim.
> > > 
> > > [0] https://repo.or.cz/nvi.git
> > > 
> > 
> > Got positive testing from one user.  Anyone else interested?  Or any other
> > feedback?
> > 
> > Tim.
> > 
> 
> Throwing this hat back into the ring post-release.  Still only gotten user
> feedback so far.
> 
> Tim.
> 

Gotten more positive user feedback and tests.  Any developers interested in
this is some form or another?

Diff regenerated after expandtab change, which didn't really move anything.

Tim.


Index: cl/cl.h
===
RCS file: /cvs/src/usr.bin/vi/cl/cl.h,v
retrieving revision 1.10
diff -u -p -r1.10 cl.h
--- cl/cl.h 27 May 2016 09:18:11 -  1.10
+++ cl/cl.h 9 May 2021 20:03:21 -
@@ -30,8 +30,9 @@ typedef struct _cl_private {
 #defineINDX_HUP0
 #defineINDX_INT1
 #defineINDX_TERM   2
-#defineINDX_WINCH  3
-#defineINDX_MAX4   /* Original signal information. */
+#defineINDX_ALRM   3
+#defineINDX_WINCH  4
+#defineINDX_MAX5   /* Original signal information. */
struct sigaction oact[INDX_MAX];
 
enum {  /* Tty group write mode. */
@@ -48,8 +49,9 @@ typedef struct _c

Attempt to fix phantasia(6) game files

[trondd]

Phantasia was one of the games broken by removal of setgid.  But it has
another problem that led me to start rummaging around in the code.

The games set in the releases includes pre-generated world files which get
installed when you upgrade the OS which overwrite any existing game files.
So upgrading will delete the scoreboard and your active characters.

I've been able to play Phantasia by adding my user to the games group which
allows access to the files in /var/games/phantasia and the game plays fine
with multiple users.  I am not sure if that is a recommended solution to
accessing the shared files.  Past discussion indicated desire to remove
such shared files altogether.

The attached diff is just a conversation starter.  There are a couple of
possible directions to go in.  What I have here is removal of the pre-
generated world files from the games set to stop overwriting an existing
game world and instead including Phantasia's setup utility that generates
those files for the game.  A user can then run the setup to start playing
or reset an existing world as desired.

Some alternative options are to move the games files into $HOME or go a
similar route as hack(6) and allow user defined directories to be
configured to remove the /var/games shared directory usage.  A simple
solution could be to continue to generate the files as part of the release
but into /usr/share/games and let users copy them into place.

Regardless of how we want to deal with the shared files, I want to at least
stop deleting an existing game world and characters during an upgrade.

Again, this is so I can get feedback on direction.  I haven't tested the
release packaging or installation related changes yet but hopefully it
demonstrates what I'm talking about.

Tim.


Index: games/phantasia/Makefile
===
RCS file: /cvs/src/games/phantasia/Makefile,v
retrieving revision 1.18
diff -u -p -r1.18 Makefile
--- games/phantasia/Makefile24 Nov 2015 03:10:10 -  1.18
+++ games/phantasia/Makefile7 May 2021 23:44:30 -
@@ -17,15 +17,15 @@ phantglobs.o.bld: phantglobs.c
${HOSTCC} -c ${CFLAGS} -o ${.TARGET} ${.CURDIR}/phantglobs.c
 
 setup: phantglobs.o.bld setup.o monsters.asc ${DPADD} 
-   ${HOSTCC} ${CFLAGS} ${LDFLAGS} ${LDSTATIC} -o ${.TARGET} \
+   ${HOSTCC} ${CFLAGS} ${LDFLAGS} ${LDSTATIC} -o phantasia-${.TARGET} \
  phantglobs.o.bld setup.o ${LDADD}
 
 phantasia.6: phantasia.6tbl
cp ${.ALLSRC} ${.TARGET}
 
-beforeinstall: 
-   ./setup -m ${.CURDIR}/monsters.asc
-   chown root:games ${DESTDIR}/var/games/phantasia/*
+beforeinstall:
+   ${INSTALL} ${INSTALL_COPY} -o ${BINOWN} -g ${BINGRP} -m 444 
${.CURDIR}/monsters.asc \
+   ${DESTDIR}/usr/share/games/phantasia
 
 # Make Phantasia map.  Change the map commands reflect your installation.
 # PLOTDEVICE is used for plotting the map.  Change as appropriate.
Index: games/phantasia/misc.c
===
RCS file: /cvs/src/games/phantasia/misc.c,v
retrieving revision 1.21
diff -u -p -r1.21 misc.c
--- games/phantasia/misc.c  10 Jan 2016 13:35:10 -  1.21
+++ games/phantasia/misc.c  7 May 2021 23:44:31 -
@@ -1411,7 +1411,7 @@ error(char *whichfile)
cleanup(FALSE);
 
warn("%s", whichfile);
-   fprintf(stderr, "Please run 'setup' to determine the problem.\n");
+   fprintf(stderr, "Please run 'phantasia-setup' to determine the 
problem.\n");
exit(1);
 }
 /**/
Index: games/phantasia/setup.c
===
RCS file: /cvs/src/games/phantasia/setup.c,v
retrieving revision 1.20
diff -u -p -r1.20 setup.c
--- games/phantasia/setup.c 28 Jun 2019 13:32:52 -  1.20
+++ games/phantasia/setup.c 7 May 2021 23:44:31 -
@@ -5,6 +5,7 @@
  * setup.c - set up all files for Phantasia
  */
 #include 
+#include 
 
 #include 
 #include 
@@ -65,7 +66,7 @@ static char *files[] = {  /* all files t
NULL,
 };
 
-char *monsterfile="monsters.asc";
+char *monsterfile="/usr/share/games/phantasia/monsters.asc";
 
 int
 main(int argc, char *argv[])
@@ -77,6 +78,11 @@ main(int argc, char *argv[])
int ch;
char path[PATH_MAX], *prefix;
 
+   if (geteuid()) {
+   fprintf(stderr, "Error:  needs to be run as root\n");
+   return(1);
+   }
+
while ((ch = getopt(argc, argv, "hm:")) != -1)
switch(ch) {
case 'm':
@@ -89,17 +95,21 @@ main(int argc, char *argv[])
argc -= optind;
argv += optind;
 
+umask(0002);   /* owner and group read/write and read for 
world for te directory */
+
+/* create data directory */
+if(mkdir("/var/games/phantasia", 0775) && errno != EEXIST) 
+   Error("Cannot create %s.\n", "/var/games/phantasia");
+
 umask(0117);   /* only owner can 

Copy/paste fix in phantasia(6)

[trondd]

Looks like we picked up some extra loopty loops during an update from
NetBSD back in 1998.

I sanity checked against NetBSD to make sure this matches their
source.

Tim.


Index: io.c
===
RCS file: /cvs/src/games/phantasia/io.c,v
retrieving revision 1.9
diff -u -p -r1.9 io.c
--- io.c10 Jan 2016 13:35:09 -  1.9
+++ io.c29 Apr 2021 00:20:18 -
@@ -324,7 +324,6 @@ getanswer(char *choices, bool def)
alarm(0);   /* make sure alarm is off */
 
for (loop = 3; loop; --loop)
-   for (loop = 3; loop; --loop)
/* try for 3 times */
{
if (setjmp(Timeoenv) != 0)

Re: re-enable A-MSDU support with iwm(4) and iwx(4) fixed

[trondd]

Stefan Sperling  wrote:

> This patch adds A-MSDU rx offloading support for both iwm(4) and iwx(4)
> and re-enables net80211's software A-MSDU Rx support for all 11n drivers.
> 
> Meaning iwn(4) and athn(4) will also be receiving A-MSDUs again.
> This feature has been turned off since July 2019:
> https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net80211/ieee80211_input.c#rev1.207
> The root cause of the problem I saw at that time was related to dlg's Rx
> queue back-pressure mechanism. Once we figured this out, all wireless drivers
> were fixed to call if_input() only once per interrupt so this is no longer
> an issue.
> 
> For a very brief period we tried to enable A-MSDUs again in -current but
> we found out that iwm/iwx needed additional work for the new devices which
> received support while A-MSDU was disabled in-tree:
> https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/net80211/ieee80211_input.c#rev1.226
> The patch below completes the missing bits to make A-MSDUs work on these
> new devices.
> 
> My previous iwm-only patch and related test reports are here:
> https://marc.info/?t=16170390711=1=2
> The iwm changes have already been extensively tested.
> For iwm, this new patch only adds an additional range check:
> @@ -4640,7 +4640,8 @@ iwm_rx_reorder(struct iwm_softc *sc, struct mbuf 
> *m, i
>  
>   baid = (reorder_data & IWM_RX_MPDU_REORDER_BAID_MASK) >>
>   IWM_RX_MPDU_REORDER_BAID_SHIFT;
> - if (baid == IWM_RX_REORDER_DATA_INVALID_BAID)
> + if (baid == IWM_RX_REORDER_DATA_INVALID_BAID ||
> + baid >= nitems(sc->sc_rxba_data))
>   return 0;
>  
>   rxba = >sc_rxba_data[baid];
> 
> 
> I have tested on the following devices:
> 
> iwm0 at pci2 dev 0 function 0 "Intel Dual Band Wireless-AC 8265" rev 0x78, msi
> iwm0: hw rev 0x230, fw ver 34.0.1, address 7c:11:xx:xx:xx:xx
> 
> iwx0 at pci5 dev 0 function 0 "Intel Wi-Fi 6 AX200" rev 0x1a, msix
> iwx0: hw rev 0x340, fw ver 48.1335886879.0, address d0:ab:xx:xx:xx:xx
> 
> iwx0 at pci0 dev 20 function 3 "Intel Wi-Fi 6 AX201" rev 0x00, msix
> iwx0: hw rev 0x350, fw ver 48.1335886879.0, address 0c:7a:xx:xx:xx:xx
> 
> ok?
> 

No regressions under normal use and a little bit of stress testing.

iwm0 at pci2 dev 0 function 0 "Intel AC 7265" rev 0x59, msi
iwm0: hw rev 0x210, fw ver 17.3216344376.0, address dc:53:xx:xx:xx:xx

Tim.

Re: Improve vi(1) recovery

[trondd]

trondd  wrote:

> trondd  wrote:
> 
> > 
> > While investigating an occasional crash when recovering a file with 'vi -r'
> > after a power failure, I noticed that the recovery files are actually never
> > updated during an editing session.  The recovery files are created upon
> > initial modification of the file which saves the state of the file at the
> > time of the edit.  You can work on a file for as long as you want and even
> > write it to disk but the recovery file is never updated.  If the session is
> > then lost due to power failure or a SIGKILL and you attempt to recover with
> > -r, you'll be presented with the contents of the file from that first edit.
> > It won't contain unsaved changes nor even any changes manually written to
> > disk to the original file.  Accepting the recovered version would lose all
> > of your work.
> > 
> > Reading the vi docs, man page, and source comments in the OpenBSD tree, they
> > all mention the use of SIGALRM to periodically save changes to the recovery
> > file.  However, the code never sets up a handler or captures SIGALRM.  It
> > only ever updates the recovery file on a SIGTERM but then also exists, I
> > guess to cover the case of an inadvertent clean system shutdown.
> > 
> > I dug through an nvi source repository[0] that seemed to cover it's entire
> > history and it seems this functionality was lost somewhere around 1994 and I
> > don't see it having been replaced by anything else.  Our version seems to be
> > from 1996 and editors/nvi in ports still lacks code to update the recovery
> > file, as well.
> > 
> > What I've done is re-implement periodic updates to the recovery file using
> > SIGALRM and a timer like the original implementation but rewritten a bit to
> > fit the newer source file layout and event handling.  That keeps the 
> > recovery
> > updated every 2 minutes.  Then it seemed silly to be able to write changes 
> > to
> > the original file and if a crash happens before the next SIGALRM, recovery
> > would try to roll you back to before those saved changes.  So I've also 
> > added
> > a call to sync the recovery file if you explicitly write changes to disk.  I
> > don't think the recovery system should try to punish you for actively saving
> > your work even if it is only at most 2 minutes worth.
> > 
> > Comments or feedback?  I'm unsure I've covered all caveats with this code or
> > if there are vi/ex usecases where it won't work correctly.  For testing, 
> > I've
> > covered my usage and several scenarios I could contrive but I don't 
> > regularly
> > use ex, for example, or change many options from the default.  I've been
> > running with this code for a week.  And I suppose there must be a reason no
> > one has noticed or cared about this for over 20 years.  Everyone else uses
> > vim, I guess?
> > 
> > Tim.
> > 
> > [0] https://repo.or.cz/nvi.git
> > 
> 
> Got positive testing from one user.  Anyone else interested?  Or any other
> feedback?
> 
> Tim.
> 

Throwing this hat back into the ring post-release.  Still only gotten user
feedback so far.

Tim.


Index: cl/cl.h
===
RCS file: /cvs/src/usr.bin/vi/cl/cl.h,v
retrieving revision 1.10
diff -u -p -r1.10 cl.h
--- cl/cl.h 27 May 2016 09:18:11 -  1.10
+++ cl/cl.h 26 Dec 2020 17:47:32 -
@@ -30,8 +30,9 @@ typedef struct _cl_private {
 #defineINDX_HUP0
 #defineINDX_INT1
 #defineINDX_TERM   2
-#defineINDX_WINCH  3
-#defineINDX_MAX4   /* Original signal information. */
+#defineINDX_ALRM   3
+#defineINDX_WINCH  4
+#defineINDX_MAX5   /* Original signal information. */
struct sigaction oact[INDX_MAX];
 
enum {  /* Tty group write mode. */
@@ -48,8 +49,9 @@ typedef struct _cl_private {
 #defineCL_SIGHUP   0x0020  /* SIGHUP arrived. */
 #defineCL_SIGINT   0x0040  /* SIGINT arrived. */
 #defineCL_SIGTERM  0x0080  /* SIGTERM arrived. */
-#defineCL_SIGWINCH 0x0100  /* SIGWINCH arrived. */
-#defineCL_STDIN_TTY0x0200  /* Talking to a terminal. */
+#defineCL_SIGALRM  0x0100  /* SIGALRM arrived. */
+#defineCL_SIGWINCH 0x0200  /* SIGWINCH arrived. */
+#defineCL_STDIN_TTY0x0400  /* Talking to a terminal. */
u_int32_t flags;
 } CL_PRIVATE;
 
Index: cl/cl_main.c
===
RCS file: /cvs/src/usr.bin/vi/cl/cl_main.c,v
retrieving revision 1.33
diff -u -p -r1.33 cl_main.c
--- cl/cl_main

Re: update xterm to version 367

[trondd]

Matthieu Herrb  wrote:

> On Sat, Mar 27, 2021 at 10:22:15AM +0100, Matthieu Herrb wrote:
> > Patch #367 - 2021/03/26
> > 
> > ok ? comments ?
> 
> Hi,
> 
> I could use some actual test results. I'd like to commit the update.
> 
> -- 
> Matthieu Herrb

Sorry, had this in mind to test and hadn't gotten to it.  I've run
it through my usual paces on amd64 and aarch64.  Font sizes,
Japanese character display, tmux with split frames.

On amd64, also Japanese text entry, I ran around a bit in
games/cataclysm-dda in text mode, and ran a few of the vttest
scripts.

No regresions noticed.

Tim.

Re: iwm(4) A-MSDU support

[trondd]

Stefan Sperling  wrote:

> This patch attempts to add support for receiving A-MSDUs to iwm(4).
> If you are using iwm(4) then please run with this patch and let me
> know if it causes regressions. Thanks!
> 
> ACHTUNG: This patch breaks iwx(4)! Don't use it there! For this reason,
> the patch can neither be committed nor be put into snapshots!!!
> 
> Our net80211 stack de-aggregates A-MSDUs in software. This works fine with
> iwm 7k and 8k devices. However, iwm 9k devices de-aggregate A-MSDUs in
> hardware and net80211 is currently unable to handle this.
> 
> Our current iwm 9k code only works because long ago I disabled reception
> of A-MSDUs for all devices. Unfortunately, the only way to get A-MSDUs
> working on 9k hardware is to add a bunch of driver-specific code which
> handles de-aggregation. Otherwise, net80211 will drop frames which appear
> to arrive out of order, or appear as duplicates even though they were
> simply part of the same A-MSDU and thus share a sequence number.
> The driver can re-order frames correctly based on information provided
> by firmware. I'd rather implement this than letting these devices miss
> out on A-MSDUs because the Rx speed advantage can be significant, around
> 80/90 Mbps (but this will very much depend on the AP).
> 
> If these A-* acronyms don't make sense and you'd like to know more, here
> is a fairly good explanation: https://mrncciew.com/2013/04/11/a-mpdu-a-msdu/
> Note that we care about the nested case, which is also mentioned in this
> article but not explained in detail. It's simply an A-MSDU stashed inside
> an A-MPDU. If an AP can do 11AC, then it should support this.
> 
> iwx(4) hardware has the same problem.
> If this patch works fine on iwm(4) then I can port the changes to iwx(4),
> do another round of testing, and eventually commit to both drivers at
> the same time.
> 
> Some of the changes below are based on code from the Linux iwlwifi driver.
> I am not entirely happy with some of its style. But the code should be in
> good enough shape to be tested.

No regression noticed here for several days under my normal workload.

iwm0 at pci2 dev 0 function 0 "Intel Dual Band Wireless AC 7265" rev 0x59, msi
iwm0: hw rev 0x210, fw ver 17.3216344376.0, address dc:53:60:4a:b1:ea

Tim.

Re: Improve vi(1) recovery

[trondd]

trondd  wrote:

> 
> While investigating an occasional crash when recovering a file with 'vi -r'
> after a power failure, I noticed that the recovery files are actually never
> updated during an editing session.  The recovery files are created upon
> initial modification of the file which saves the state of the file at the
> time of the edit.  You can work on a file for as long as you want and even
> write it to disk but the recovery file is never updated.  If the session is
> then lost due to power failure or a SIGKILL and you attempt to recover with
> -r, you'll be presented with the contents of the file from that first edit.
> It won't contain unsaved changes nor even any changes manually written to
> disk to the original file.  Accepting the recovered version would lose all
> of your work.
> 
> Reading the vi docs, man page, and source comments in the OpenBSD tree, they
> all mention the use of SIGALRM to periodically save changes to the recovery
> file.  However, the code never sets up a handler or captures SIGALRM.  It
> only ever updates the recovery file on a SIGTERM but then also exists, I
> guess to cover the case of an inadvertent clean system shutdown.
> 
> I dug through an nvi source repository[0] that seemed to cover it's entire
> history and it seems this functionality was lost somewhere around 1994 and I
> don't see it having been replaced by anything else.  Our version seems to be
> from 1996 and editors/nvi in ports still lacks code to update the recovery
> file, as well.
> 
> What I've done is re-implement periodic updates to the recovery file using
> SIGALRM and a timer like the original implementation but rewritten a bit to
> fit the newer source file layout and event handling.  That keeps the recovery
> updated every 2 minutes.  Then it seemed silly to be able to write changes to
> the original file and if a crash happens before the next SIGALRM, recovery
> would try to roll you back to before those saved changes.  So I've also added
> a call to sync the recovery file if you explicitly write changes to disk.  I
> don't think the recovery system should try to punish you for actively saving
> your work even if it is only at most 2 minutes worth.
> 
> Comments or feedback?  I'm unsure I've covered all caveats with this code or
> if there are vi/ex usecases where it won't work correctly.  For testing, I've
> covered my usage and several scenarios I could contrive but I don't regularly
> use ex, for example, or change many options from the default.  I've been
> running with this code for a week.  And I suppose there must be a reason no
> one has noticed or cared about this for over 20 years.  Everyone else uses
> vim, I guess?
> 
> Tim.
> 
> [0] https://repo.or.cz/nvi.git
> 

Got positive testing from one user.  Anyone else interested?  Or any other
feedback?

Tim.


Index: cl/cl.h
===
RCS file: /cvs/src/usr.bin/vi/cl/cl.h,v
retrieving revision 1.10
diff -u -p -r1.10 cl.h
--- cl/cl.h 27 May 2016 09:18:11 -  1.10
+++ cl/cl.h 26 Dec 2020 17:47:32 -
@@ -30,8 +30,9 @@ typedef struct _cl_private {
 #defineINDX_HUP0
 #defineINDX_INT1
 #defineINDX_TERM   2
-#defineINDX_WINCH  3
-#defineINDX_MAX4   /* Original signal information. */
+#defineINDX_ALRM   3
+#defineINDX_WINCH  4
+#defineINDX_MAX5   /* Original signal information. */
struct sigaction oact[INDX_MAX];
 
enum {  /* Tty group write mode. */
@@ -48,8 +49,9 @@ typedef struct _cl_private {
 #defineCL_SIGHUP   0x0020  /* SIGHUP arrived. */
 #defineCL_SIGINT   0x0040  /* SIGINT arrived. */
 #defineCL_SIGTERM  0x0080  /* SIGTERM arrived. */
-#defineCL_SIGWINCH 0x0100  /* SIGWINCH arrived. */
-#defineCL_STDIN_TTY0x0200  /* Talking to a terminal. */
+#defineCL_SIGALRM  0x0100  /* SIGALRM arrived. */
+#defineCL_SIGWINCH 0x0200  /* SIGWINCH arrived. */
+#defineCL_STDIN_TTY0x0400  /* Talking to a terminal. */
u_int32_t flags;
 } CL_PRIVATE;
 
Index: cl/cl_main.c
===
RCS file: /cvs/src/usr.bin/vi/cl/cl_main.c,v
retrieving revision 1.33
diff -u -p -r1.33 cl_main.c
--- cl/cl_main.c5 May 2016 20:36:41 -   1.33
+++ cl/cl_main.c26 Dec 2020 17:47:32 -
@@ -239,6 +239,14 @@ h_term(int signo)
 }
 
 static void
+h_alrm(int signo)
+{
+   GLOBAL_CLP;
+
+   F_SET(clp, CL_SIGALRM);
+}
+
+static void
 h_winch(int signo)
 {
GLOBAL_CLP;
@@ -264,6 +272,7 @@ sig_init(GS *gp, SCR *sp)
if (setsig(SIGHUP, >oact[INDX_HUP], h_hup) ||
setsig(SIGINT, >oact

Re: [patch] update xterm to version 363

[trondd]

On Fri, January 1, 2021 11:29 am, Matthieu Herrb wrote:
> Hi,
>
> the attached patch updates xterm to version 363 (from 351)
>
> Changelog can be found here:
> https://invisible-island.net/xterm/xterm.log.html
>
> To test, apply the patch in ${XSRCDIR}/app/xterm and rebuild xterm :
>
> cd ${XSRCDIR}/app/xterm
> gzcat /path/to/xterm-363.diff.gz | patch -p0 -E
> make obj
> make
> doas make install
>
> and restart your terminals.
>
> Testing and comments welcome. Especially from those who use advanced /
> specific xterm options or features.
>
>
> --
> Matthieu Herrb
>

Some quick testing today.  Compiled on aarch64 and amd64.  I enable sixel
graphics which, while not supported in the OBSD config, is something
unique.

Played with font sizes, colors and ASCII graphics on BBSes, a game of
hack(6), multiple screens and split screens with vi(1), Japanese input and
display.  Also playing around with the vttest scripts.

Haven't noticed any differences from the previous version.  Did most stuff
on amd64, I'll go back to aarch64 tomorrow and I'll keep using it and
trying different things that I can think of.

Tim.

Improve vi(1) recovery

[trondd]

While investigating an occasional crash when recovering a file with 'vi -r'
after a power failure, I noticed that the recovery files are actually never
updated during an editing session.  The recovery files are created upon
initial modification of the file which saves the state of the file at the
time of the edit.  You can work on a file for as long as you want and even
write it to disk but the recovery file is never updated.  If the session is
then lost due to power failure or a SIGKILL and you attempt to recover with
-r, you'll be presented with the contents of the file from that first edit.
It won't contain unsaved changes nor even any changes manually written to
disk to the original file.  Accepting the recovered version would lose all
of your work.

Reading the vi docs, man page, and source comments in the OpenBSD tree, they
all mention the use of SIGALRM to periodically save changes to the recovery
file.  However, the code never sets up a handler or captures SIGALRM.  It
only ever updates the recovery file on a SIGTERM but then also exists, I
guess to cover the case of an inadvertent clean system shutdown.

I dug through an nvi source repository[0] that seemed to cover it's entire
history and it seems this functionality was lost somewhere around 1994 and I
don't see it having been replaced by anything else.  Our version seems to be
from 1996 and editors/nvi in ports still lacks code to update the recovery
file, as well.

What I've done is re-implement periodic updates to the recovery file using
SIGALRM and a timer like the original implementation but rewritten a bit to
fit the newer source file layout and event handling.  That keeps the recovery
updated every 2 minutes.  Then it seemed silly to be able to write changes to
the original file and if a crash happens before the next SIGALRM, recovery
would try to roll you back to before those saved changes.  So I've also added
a call to sync the recovery file if you explicitly write changes to disk.  I
don't think the recovery system should try to punish you for actively saving
your work even if it is only at most 2 minutes worth.

Comments or feedback?  I'm unsure I've covered all caveats with this code or
if there are vi/ex usecases where it won't work correctly.  For testing, I've
covered my usage and several scenarios I could contrive but I don't regularly
use ex, for example, or change many options from the default.  I've been
running with this code for a week.  And I suppose there must be a reason no
one has noticed or cared about this for over 20 years.  Everyone else uses
vim, I guess?

Tim.

[0] https://repo.or.cz/nvi.git


Index: cl/cl.h
===
RCS file: /cvs/src/usr.bin/vi/cl/cl.h,v
retrieving revision 1.10
diff -u -p -r1.10 cl.h
--- cl/cl.h 27 May 2016 09:18:11 -  1.10
+++ cl/cl.h 26 Dec 2020 17:47:32 -
@@ -30,8 +30,9 @@ typedef struct _cl_private {
 #defineINDX_HUP0
 #defineINDX_INT1
 #defineINDX_TERM   2
-#defineINDX_WINCH  3
-#defineINDX_MAX4   /* Original signal information. */
+#defineINDX_ALRM   3
+#defineINDX_WINCH  4
+#defineINDX_MAX5   /* Original signal information. */
struct sigaction oact[INDX_MAX];
 
enum {  /* Tty group write mode. */
@@ -48,8 +49,9 @@ typedef struct _cl_private {
 #defineCL_SIGHUP   0x0020  /* SIGHUP arrived. */
 #defineCL_SIGINT   0x0040  /* SIGINT arrived. */
 #defineCL_SIGTERM  0x0080  /* SIGTERM arrived. */
-#defineCL_SIGWINCH 0x0100  /* SIGWINCH arrived. */
-#defineCL_STDIN_TTY0x0200  /* Talking to a terminal. */
+#defineCL_SIGALRM  0x0100  /* SIGALRM arrived. */
+#defineCL_SIGWINCH 0x0200  /* SIGWINCH arrived. */
+#defineCL_STDIN_TTY0x0400  /* Talking to a terminal. */
u_int32_t flags;
 } CL_PRIVATE;
 
Index: cl/cl_main.c
===
RCS file: /cvs/src/usr.bin/vi/cl/cl_main.c,v
retrieving revision 1.33
diff -u -p -r1.33 cl_main.c
--- cl/cl_main.c5 May 2016 20:36:41 -   1.33
+++ cl/cl_main.c26 Dec 2020 17:47:32 -
@@ -239,6 +239,14 @@ h_term(int signo)
 }
 
 static void
+h_alrm(int signo)
+{
+   GLOBAL_CLP;
+
+   F_SET(clp, CL_SIGALRM);
+}
+
+static void
 h_winch(int signo)
 {
GLOBAL_CLP;
@@ -264,6 +272,7 @@ sig_init(GS *gp, SCR *sp)
if (setsig(SIGHUP, >oact[INDX_HUP], h_hup) ||
setsig(SIGINT, >oact[INDX_INT], h_int) ||
setsig(SIGTERM, >oact[INDX_TERM], h_term) ||
+   setsig(SIGALRM, >oact[INDX_ALRM], h_alrm) ||
setsig(SIGWINCH, >oact[INDX_WINCH], h_winch)
)
err(1, NULL);
@@ -271,6 +280,7 @@ sig_init(GS *gp, SCR *sp)
if 

Re: $pexp in re.subr(8)

[trondd]

On Thu, August 6, 2020 9:12 pm, Thomas Levine wrote:
> The present patch changes the rc.subr(8) manual page to match
> the implementation.
>
> The current manual page for rc.subr(8) says that $pexp is "A regular
> expression to be passed to pgrep(1) in order to find the desired process
> or to be passed to pkill(1) to stop it."
>
> The file /etc/rc.d/rc.subr currently uses $pexp only as a fixed
> expression, with the -xf flags.
>
>   > grep pexp /etc/rc.d/rc.subr
>   pexp=${pexp}
>   multicast nfs_server pexp pf pkg_scripts shlib_dirs spamd_black
> pgrep -T "${daemon_rtable}" -q -xf "${pexp}"
> pkill -HUP -T "${daemon_rtable}" -xf "${pexp}"
> pkill -T "${daemon_rtable}" -xf "${pexp}"
>   # make sure pexp matches the process (i.e. doesn't include the quotes)
>   pexp="$(eval echo ${daemon}${daemon_flags:+ ${daemon_flags}})"
>
> The -xf flags are explained in the pgrep(1) and pkill(1) man page.
>
> -x  Require an exact match of the process name, or argument
> list if -f is given.  The default is to match any substring.
>
> The present patch replaces instances of "regular expression" with
> "fixed expression".
>

I'm not sure what you mean by a "fixed expression".  A string?  But reread
the pgrep/pkill manpage.  They take a pattern as an operand which is
described to be an extended regular expression.

This is true.  Even with -x.

Try it.

Re: sysupgrade(8) and http_proxy

[trondd]

Steffen Nurpmeso  wrote:

> trondd wrote in <49f29107642e86c17283b0582a9f09f4.squir...@mail.kagu-tsu\
> chi.com>:
>  |On Sun, November 3, 2019 12:02 pm, trondd wrote:
>  |> On Sun, November 3, 2019 6:27 am, Florian Obser wrote:
>  |>> On Sun, Nov 03, 2019 at 12:21:59PM +0100, Antoine Jacoutot wrote:
>  |>>> On Sun, Nov 03, 2019 at 12:16:56PM +0100, Florian Obser wrote:
>  ...
>  |I've tested the diff and it works as expected in my environment.  I don't
>  |need a username and password for proxy access but it populates the
>  |rc.firsttime file fine.
>  |
>  |The quote() function is actually pretty simple.
>  |
>  |quote() (
>  |# Since this is a subshell we won't pollute the calling namespace.
>  |for _a; do
>  |# alias string to Q, does escaping and quoting
>  |alias Q=$_a;
>  |# set variable back to value of alias
>  |_a=$(alias Q);
>  |# print variable, chopping off alias definition
>  |#   no newline, don't substitute the escape sequences
>  |#   we made above
>  |print -rn -- " ${_a#Q=}"
>  |done | sed '1s/ //'
>  |echo
>  |)
> 
> I felt a bit undecided from your first mail on, maybe also because
> of your mailer, but wanted to post the all-compatible all-shell
> quote of and from Robert Elz here.
> 
>   #@ Round trip quote strings in POSIX shell.  E.g.,
>   #@set -- x 'a \ b' "foo'" "\\'b\\a\\r\\" Aä
>   #@printf "%s: <%s><%s><%s><%s><%s>\n" "$#" "${1}" "${2}" "${3}" "$4" 
> "$5"
>   #@saved_parameters=`quote_rndtrip "$@"`
>   #@eval "set -- $saved_parameters"
>   #@printf "%s: <%s><%s><%s><%s><%s>\n" "$#" "${1}" "${2}" "${3}" "$4" 
> "$5"
>   #
>   # 2017 Robert Elz (kre).
> 
> ...
> 
>   # Though slower use a subshell version instead of properly restoring $IFS
>   # and flags, as elder shells may not be able to properly restore flags via
>   # "set +o" as later standardized in POSIX, and it seems overkill to handle
>   # all possible forms of output "set +o" may or may not actually generate.
>   quote__rndtrip() (
>  case "$1" in
>  *\'*) ;;
>  *) printf "'%s'" "$1"; return 0;;
>  esac
>  a="$1" s= e=
>  while case "$a" in
> \'*)  a=${a#?}; s="${s}'";;
> *\')  a=${a%?}; e="${e}'";;
> '')   printf "${s}${e}"; exit 0;;
> *) false;;
> esac
>  do
> continue
>  done
>  IFS=\'
>  set -f
>  set -- $a
>  r="${1}"
>   shift
>  for a
>  do
> r="${r}'\\''${a}"
>  done
>  printf "${s}'%s'${e}" "${r}"
>  exit 0
>   )
> 
>   quote_rndtrip() (
>  j=
>  for i
>  do
> [ -n "$j" ] && printf ' '
> j=' '
> quote__rndtrip "$i"
>  done
>   )
> 
>   quote_string() (
>  j=
>  for i
>  do
> [ -n "$j" ] && printf '\\ '
> j=' '
> quote__rndtrip "$i"
>  done
>   )
> 
> 
>  |$ export "test=fancy ' stuff #and not a comment"
>  |$ ./quote.ksh
>  |$ cat test.out
>  |
>  |export 'http_proxy=fancy '\'' stuff #and not a comment'
>  |
>  |$ export "test=even
>  |> this works #"
>  |$ ./quote.ksh
>  |$ cat test.out
>  |
>  |export 'http_proxy=even
>  |this works #'
> 
> Of course, if in install.sub there is already your quote function,
> that surely is preferred.
> 
> Good night,
> 
> --steffen
> |
> |Der Kragenbaer,The moon bear,
> |der holt sich munter   he cheerfully and one by one
> |einen nach dem anderen runter  wa.ks himself off
> |(By Robert Gernhardt)

To clarify, it's not my quote function.  It's been in install.sub since
2010 [0] and was used when http_proxy was added there [1].  I figured it
was a good idea so pulled it in for sysupgrade as well.

[0] 
http://cvsweb.openbsd.org/src/distrib/miniroot/install.sub?rev=1.628=text/x-cvsweb-markup
[1] 
http://cvsweb.openbsd.org/src/distrib/miniroot/install.sub?rev=1.1058=text/x-cvsweb-markup

Tim.

Re: sysupgrade(8) and http_proxy

[trondd]

On Sun, November 3, 2019 12:02 pm, trondd wrote:
> On Sun, November 3, 2019 6:27 am, Florian Obser wrote:
>> On Sun, Nov 03, 2019 at 12:21:59PM +0100, Antoine Jacoutot wrote:
>>> On Sun, Nov 03, 2019 at 12:16:56PM +0100, Florian Obser wrote:
>>> > I like it, if someone who is fluent in ksh line noise could please
>>> > verify and commit, that would be awesome, thanks.
>>>
>>> Why not let the installer handle this? It already has code for it.
>>> sysupgrade ony needs to create the proper auto_upgrade.conf containing
>>> the
>>> answer to the proxy question.
>>
>> Right, that is better.
>>
>
> Unless I'm missing something, the installer only asks for a proxy when you
> select 'http' for the sets for a network install which sysupgrade is
> designed to not do.
>
> This would mean changing the installer to ask for a proxy regardless of
> install method.  Which could make sense since the execution of tools in
> rc.firsttime requires internet access regardless of set installation
> method.
>
> I don't know that much of a change is desired, though.
>
> Tim.
>

I've tested the diff and it works as expected in my environment.  I don't
need a username and password for proxy access but it populates the
rc.firsttime file fine.

The quote() function is actually pretty simple.

quote() (
# Since this is a subshell we won't pollute the calling namespace.
for _a; do
# alias string to Q, does escaping and quoting
alias Q=$_a;
# set variable back to value of alias
_a=$(alias Q);
# print variable, chopping off alias definition
#   no newline, don't substitute the escape sequences
#   we made above
print -rn -- " ${_a#Q=}"
done | sed '1s/ //'
echo
)

$ export "test=fancy ' stuff #and not a comment"
$ ./quote.ksh
$ cat test.out

export 'http_proxy=fancy '\'' stuff #and not a comment'

$ export "test=even
> this works #"
$ ./quote.ksh
$ cat test.out

export 'http_proxy=even
this works #'


Tim.


>>>
>>>
>>> >
>>> > On Fri, Nov 01, 2019 at 09:37:04PM -0400, trondd wrote:
>>> > > Anthony Coulter  wrote:
>>> > >
>>> > > > Hello @tech,
>>> > > >
>>> > > > When I manually upgrade OpenBSD using bsd.rd, I have to set
>>> http_proxy
>>> > > > to fetch the file sets. When I reboot after installing, fw_update
>>> > > > succeeds because theinstall script was clever enough to export
>>> > > > http_proxy in /etc/rc.firsttime.
>>> > > >
>>> > > > Unfortunately sysupgrade(8) does not remember that http_proxy was
>>> set
>>> > > > when it fetched the file sets, and so when I run sysupgrade I
>>> have
>>> to
>>> > > > either wait for fw_update to manually time out on first reboot,
>>> or
>>> kill
>>> > > > it manually with ^C.
>>> > > >
>>> > > > Adding the line:
>>> > > >
>>> > > > [ ${http_proxy} ] && echo "export http_proxy=${http_proxy}"
>>> >>/etc/rc.firsttime
>>> > > >
>>> > > > to a spot near the bottom of /usr/sbin/sysupgrade fixes my
>>> fw_update
>>> > > > problem, at least until the upgrade restores all of my files to
>>> their
>>> > > > stock defaults. Is this something we could integrate into the
>>> official
>>> > > > repository?
>>> > > >
>>> > > > Thanks and regards,
>>> > > > Anthony Coulter
>>> > >
>>> > > Here it is in patch form in case there is interest.  This also
>>> pulls
>>> in the
>>> > > quote function from install.sub to make sure a proxy username or
>>> password is
>>> > > quoted properly.  I haven't tested this since I could only use it
>>> at
>>> work.
>>> > >
>>> > > Tim.
>>> > >
>>> > >
>>> > > Index: sysupgrade.sh
>>> > > ===
>>> > > RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
>>> > > retrieving revision 1.29
>>> > > diff -u -p -r1.29 sysupgrade.sh
>>> > > --- sysupgrade.sh   26 Oct 2019 04:04:20 -  1.29
>>> > > +++ sysupgrade.sh  

Re: sysupgrade(8) and http_proxy

[trondd]

On Sun, November 3, 2019 6:27 am, Florian Obser wrote:
> On Sun, Nov 03, 2019 at 12:21:59PM +0100, Antoine Jacoutot wrote:
>> On Sun, Nov 03, 2019 at 12:16:56PM +0100, Florian Obser wrote:
>> > I like it, if someone who is fluent in ksh line noise could please
>> > verify and commit, that would be awesome, thanks.
>>
>> Why not let the installer handle this? It already has code for it.
>> sysupgrade ony needs to create the proper auto_upgrade.conf containing
>> the
>> answer to the proxy question.
>
> Right, that is better.
>

Unless I'm missing something, the installer only asks for a proxy when you
select 'http' for the sets for a network install which sysupgrade is
designed to not do.

This would mean changing the installer to ask for a proxy regardless of
install method.  Which could make sense since the execution of tools in
rc.firsttime requires internet access regardless of set installation
method.

I don't know that much of a change is desired, though.

Tim.

>>
>>
>> >
>> > On Fri, Nov 01, 2019 at 09:37:04PM -0400, trondd wrote:
>> > > Anthony Coulter  wrote:
>> > >
>> > > > Hello @tech,
>> > > >
>> > > > When I manually upgrade OpenBSD using bsd.rd, I have to set
>> http_proxy
>> > > > to fetch the file sets. When I reboot after installing, fw_update
>> > > > succeeds because theinstall script was clever enough to export
>> > > > http_proxy in /etc/rc.firsttime.
>> > > >
>> > > > Unfortunately sysupgrade(8) does not remember that http_proxy was
>> set
>> > > > when it fetched the file sets, and so when I run sysupgrade I have
>> to
>> > > > either wait for fw_update to manually time out on first reboot, or
>> kill
>> > > > it manually with ^C.
>> > > >
>> > > > Adding the line:
>> > > >
>> > > > [ ${http_proxy} ] && echo "export http_proxy=${http_proxy}"
>> >>/etc/rc.firsttime
>> > > >
>> > > > to a spot near the bottom of /usr/sbin/sysupgrade fixes my
>> fw_update
>> > > > problem, at least until the upgrade restores all of my files to
>> their
>> > > > stock defaults. Is this something we could integrate into the
>> official
>> > > > repository?
>> > > >
>> > > > Thanks and regards,
>> > > > Anthony Coulter
>> > >
>> > > Here it is in patch form in case there is interest.  This also pulls
>> in the
>> > > quote function from install.sub to make sure a proxy username or
>> password is
>> > > quoted properly.  I haven't tested this since I could only use it at
>> work.
>> > >
>> > > Tim.
>> > >
>> > >
>> > > Index: sysupgrade.sh
>> > > ===
>> > > RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
>> > > retrieving revision 1.29
>> > > diff -u -p -r1.29 sysupgrade.sh
>> > > --- sysupgrade.sh26 Oct 2019 04:04:20 -  1.29
>> > > +++ sysupgrade.sh2 Nov 2019 00:39:05 -
>> > > @@ -73,6 +73,16 @@ rmel() {
>> > >  echo -n "$_c"
>> > >  }
>> > >
>> > > +# Prints the supplied parameters properly escaped for future sh/ksh
>> parsing.
>> > > +# Quotes are added if needed, so you should not do that yourself.
>> > > +quote() (
>> > > +# Since this is a subshell we won't pollute the calling 
>> > > namespace.
>> > > +for _a; do
>> > > +alias Q=$_a; _a=$(alias Q); print -rn -- " ${_a#Q=}"
>> > > +done | sed '1s/ //'
>> > > +echo
>> > > +)
>> > > +
>> > >  RELEASE=false
>> > >  SNAP=false
>> > >  FORCE=false
>> > > @@ -199,6 +209,9 @@ if ! ${KEEP}; then
>> > >  rm -f /home/_sysupgrade/{${CLEAN}}
>> > >  __EOT
>> > >  fi
>> > > +
>> > > +[[ -n $http_proxy ]] &&
>> > > +quote export "http_proxy=$http_proxy" >>/etc/rc.firsttime
>> > >
>> > >  install -F -m 700 bsd.rd /bsd.upgrade
>> > >  sync
>> > >
>> >
>> > --
>> > I'm not entirely sure you are real.
>> >
>>
>> --
>> Antoine
>>
>
> --
> I'm not entirely sure you are real.
>

Re: sysupgrade(8) and http_proxy

[trondd]

Anthony Coulter  wrote:

> Hello @tech,
> 
> When I manually upgrade OpenBSD using bsd.rd, I have to set http_proxy
> to fetch the file sets. When I reboot after installing, fw_update
> succeeds because theinstall script was clever enough to export
> http_proxy in /etc/rc.firsttime.
> 
> Unfortunately sysupgrade(8) does not remember that http_proxy was set
> when it fetched the file sets, and so when I run sysupgrade I have to
> either wait for fw_update to manually time out on first reboot, or kill
> it manually with ^C.
> 
> Adding the line:
> 
> [ ${http_proxy} ] && echo "export http_proxy=${http_proxy}" 
> >>/etc/rc.firsttime
> 
> to a spot near the bottom of /usr/sbin/sysupgrade fixes my fw_update
> problem, at least until the upgrade restores all of my files to their
> stock defaults. Is this something we could integrate into the official
> repository?
> 
> Thanks and regards,
> Anthony Coulter

Here it is in patch form in case there is interest.  This also pulls in the
quote function from install.sub to make sure a proxy username or password is
quoted properly.  I haven't tested this since I could only use it at work.

Tim.


Index: sysupgrade.sh
===
RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
retrieving revision 1.29
diff -u -p -r1.29 sysupgrade.sh
--- sysupgrade.sh   26 Oct 2019 04:04:20 -  1.29
+++ sysupgrade.sh   2 Nov 2019 00:39:05 -
@@ -73,6 +73,16 @@ rmel() {
echo -n "$_c"
 }
 
+# Prints the supplied parameters properly escaped for future sh/ksh parsing.
+# Quotes are added if needed, so you should not do that yourself.
+quote() (
+   # Since this is a subshell we won't pollute the calling namespace.
+   for _a; do
+   alias Q=$_a; _a=$(alias Q); print -rn -- " ${_a#Q=}"
+   done | sed '1s/ //'
+   echo
+)
+
 RELEASE=false
 SNAP=false
 FORCE=false
@@ -199,6 +209,9 @@ if ! ${KEEP}; then
 rm -f /home/_sysupgrade/{${CLEAN}}
 __EOT
 fi
+
+[[ -n $http_proxy ]] &&
+   quote export "http_proxy=$http_proxy" >>/etc/rc.firsttime
 
 install -F -m 700 bsd.rd /bsd.upgrade
 sync

Re: pledge xenodm

[trondd]

On Sat, November 3, 2018 7:16 am, Ricardo Mestre wrote:
> prodded by deraadt@, here's a rebased diff on xenocara's source root
directory,
> usually /usr/xenocara.

I'm using DisplayManager*autoLogin in xenodm-config and starting xenodm as
desired (not at boot) and it logs me in the first time but after ending
the X session, when xenodm tries to reset it gets a:

xenodm[63122]: pledge "getpw", syscall 33


Turns out, though, this is kind of a bogus use case.  I'm trying to exit
X.  Having it log me right back in again is silly.  I like that it kills
xenodm.  That's actually what I want. :D

Tim.

Re: Fix descriptions of smtps vs smtp+tls in smtpd.conf.5

[trondd]

On Thu, October 25, 2018 2:24 am, Raf Czlonka wrote:
> On Thu, Oct 25, 2018 at 07:11:47AM BST, Gilles Chehade wrote:
>>
>> smtpd will _always_ display a 'starttls' log line when the TLS channel
>> starts,
>> disregarding if TLS was started at connect time (smtps) or within the
>> protocol
>> (smtp+tls, or even smtp since it does opportunistic tls).
>>
>
> I guess this is the confusing bit - seeing 'starttls' in the log
> file and thinking 'STARTTLS', i.e. the "TLS upgrade".
>
> R.
>

Yes, I mistakenly assumed that where it didn't log "starttls" it wasn't
using STARTTLS and therefore using TLS and where it did log "starttls"
meant it was using STARTTLS.  Silly me. :P

Unfortunatly I also didn't know which my ISP was actually using since
before, with secure:// it would try both and always sent mail.  I should
have figured out what I was dealing with first.

Fix descriptions of smtps vs smtp+tls in smtpd.conf.5

[trondd]

Unless I'm confused, it seems the description of the smarthosts smtps and
smtp+tls are revered in the smtpd.conf man page.

My log seemed to back this up.  When using smtp+tls, which the man page said
uses STARTTLS but seems to actually use TLS which my ISP does not:

Oct 21 21:42:58 ember smtpd[41596]: ca9dba5e7f80e6ca mta connecting 
address=smtp+tls://68.87.20.6:465 host=omta-ch2.sys.comcast.net
Oct 21 21:42:58 ember smtpd[41596]: ca9dba5e7f80e6ca mta connected
Oct 21 21:43:59 ember smtpd[41596]: ca9dba5e7f80e6ca mta error 
reason=Connection closed unexpectedly


And with smtps, which the man page said uses TLS, logs show STARTTLS:

Oct 21 22:02:06 ember smtpd[66745]: a9193b70dbc40df0 mta connecting 
address=smtps://68.87.20.6:465 host=omta-ch2.sys.comcast.net
Oct 21 22:02:06 ember smtpd[66745]: a9193b70dbc40df0 mta connected
Oct 21 22:02:06 ember smtpd[66745]: a9193b70dbc40df0 mta starttls 
ciphers=version=TLSv1.2, cipher=ECDHE-RSA-AES256-GCM-SHA384, bits=256
Oct 21 22:02:06 ember smtpd[66745]: smtp-out: Server certificate verification 
succeeded on session a9193b70dbc40df0


A diff to swap the descriptions and reorder to group STARTLS and TLS smarthosts
together.

Tim.


Index: smtpd.conf.5
===
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.conf.5,v
retrieving revision 1.206
diff -u -p -r1.206 smtpd.conf.5
--- smtpd.conf.58 Oct 2018 06:10:17 -   1.206
+++ smtpd.conf.522 Oct 2018 23:52:25 -
@@ -244,14 +244,14 @@ The following protocols are available:
 .It smtp
 Normal SMTP session with opportunistic STARTTLS
 (the default).
-.It smtp+tls
+.It smtps
 Normal SMTP session with mandatory STARTTLS.
+.It smtp+tls
+SMTP session with forced TLS on connection.
 .It smtp+notls
 Plain text SMTP session without TLS.
 .It lmtp
 LMTP session.
-.It smtps
-SMTP session with forced TLS on connection.
 .El
 .Pp
 The

Re: httpd socket leak - Re: httpd ranges are not liked by freshclam

[trondd]

On Thu, April 5, 2018 2:59 am, Florian Obser wrote:
>
> this fixes it for me, instead of goto done we could also set
> clt->clt_done = 1; but at this point it means the same, I think...
>
> diff --git server_http.c server_http.c
> index 414e297f082..53e78b62f31 100644
> --- server_http.c
> +++ server_http.c
> @@ -701,6 +701,8 @@ server_read_httprange(struct bufferevent *bev, void
> *arg)
>   r->range_index++;
>   break;
>   case TOREAD_HTTP_NONE:
> + goto done;
> + break;
>   case 0:
>   break;
>   }
>
>
> --
> I'm not entirely sure you are real.
>

Yes. This seems to be all that's needed.  Freshclam is happy, and my curl
tests work as expected.  I did an install from my internal mirror,
pkg_add-ed a couple things, and pkg_info-ed a couple other things.

Thanks for digging in with me.  This would have taken me a while to figure
out.

Tim.

Re: httpd ranges are not liked by freshclam

[trondd]

Bringing this back up from the depths.  I kept rolling back to older httpd
code and forgetting about this :(

I still see this issue in 6.3  A new packet cap look the same.

On Tue, April 18, 2017 4:23 pm, trondd wrote:
> On Tue, April 18, 2017 3:46 pm, Reyk Floeter wrote:
>>
>>> Am 18.04.2017 um 20:53 schrieb trondd <tro...@kagu-tsuchi.com>:
>>>
>>> I have an OpenBSD httpd(8) web server hosting security/clamav main.cvd
>>> and
>>> daily.cvd files.  Upon upgrading to 6.1, freshclam can no longer
>>> successfully fetch the cvd files.
>>>
>>> Freshclam does a request for the first 512 bytes of the files to check
>>> the
>>> dates in their header.  Then pulls the rest of the file if needed.  It
>>> looks like it pulls the *whole* file again.  It doesn't pick up where
>>> it
>>> left off.
>>>
>>> With httpd from 6.0, fully patched, this was working fine.  Whith 6.1,
>>> freshclam would request the 512 chunk, then timeout with
>>> "nonblock_recv:
>>> recv timing out (30 secs)".
>>>
>>> Knowing there were a couple of changes to ranges in httpd, I started
>>> rolling things back.  I took out the pipelining fix:
>>> http://marc.info/?l=openbsd-cvs=148607400902939=2
>>>
>>> Which didn't help.  Then I also took out the range rewrite:
>>> http://marc.info/?l=openbsd-cvs=148587359420912=2
>>>
>>> And bingo.  Freshclam happily pulled it's now much out of date daily
>>> database. :)
>>>
>>> I don't know if freshclam is doing something wacky here or if it's
>>> httpd.
>>> It does return the requested byte range, and I was able to pull a range
>>> with curl as well.  I don't know another test case for this off hand.
>>>
>>
>> Do you have any more details like request/response HTTP headers with old
>> and new code?
>>
>> Reyk
>>
>
> Yes.  Hopefully these attach properly.  Only have access to web mail from
> here so scream at me if all you get is garbage and I can resend properly
> later.
>
> ASCII output from the tcpdump showing success and failure cases.  I have
> the full binary pcaps if needed.  Comparing quickly, I see 6.1 sends the
> Partial Content response header in a seperate packet from the content.
> Previous code didn't do that.
>
> Tim.

Re: regression in pflog output

[trondd]

On Thu, February 8, 2018 11:24 am, Alexandr Nedvedicky wrote:
> Hello,
>
> Matthias Pitzl discovered a regression introduced by my earlier commit
> [1].
> Matthias has noticed the pflogd output changes for ruleset here:
> 8<---8<---8<--8<
> block out log quick from any to 1.1.1.1
> block out log quick from any to 1.1.1.2
> anchor log_test {
>   block out log quick from any to 2.1.1.1
>   block out log quick from any to 2.1.1.2
> }
> block out log quick from any to 3.1.1.1
> block out log quick from any to 3.1.1.2
> 8<---8<---8<--8<
>
> pinging addresses used in rules above Matthias noticed the rule numbers
> and anchors in log are incorrect:
>
> Feb  7 16:34:47 sys pf: rule 0/(match) [usid 0, pid 95203] block out
> ... > 1.1.1.1
> Feb  7 16:34:48 sys pf: rule 1/(match) [usid 0, pid 95203] block out
> ... > 1.1.1.2
> Feb  7 16:34:50 sys pf: rule 2.log_test.0s/(match) [uid 0, pid 95203]
> block out on ... > 2.1.1.1
> Feb  7 16:34:52 sys pf: rule 2.log_test.1s/(match) [uid 0, pid 95203]
> block out on ... > 2.1.1.2
> Feb  7 16:34:55 sys pf: rule 2/(match) [usid 0, pid 95203] block out
> on ... > 3.1.1.1
> Feb  7 16:34:57 sys pf: rule 2/(match) [uid 0, pid 95203] block out on
> ... > 3.1.1.2
>
> in output above the entries for 3.1.1.1 and 3.1.1.2 have a wrong rule
> number.
> we should see rule number 3 for 3.1.1.1 and 4 for 3.1.1.2
>
> With joint effort we could identify two problems in my earlier change.
>
> 1) pf_match_rule() must remember anchor rule and its ruleset
>   kept in ctx, before it updates ctx for descent:
>
> 3689 ctx->a = r; /* remember
> anchor */
> 3690 ctx->aruleset = ruleset;/* and
> its ruleset */
> 3691 if (pf_step_into_anchor(ctx, r) !=
> PF_TEST_OK)
> 3692 break;
>
>   once pf_step_into_anchor() returns and we are supposed to continue
>   with rule evaluation, we are better to restore ctx->a and ctx->aruleset,
>   which match our nesting level.
>
> 2) PFLOG_PACKET() called from pf_test_rule():
>
> 3789 #if NPFLOG > 0
> 3790 if (r->log)
> 3791 PFLOG_PACKET(pd, ctx.reason, r, ctx.a, ruleset,
> NULL);
> 3792 if (ctx.act.log & PF_LOG_MATCHES)
> 3793 pf_log_matches(pd, r, ctx.a, ruleset,
> );
> 3794 #endif  /* NPFLOG > 0 */
>
> uses anchor kept in ctx, instead of local variable, which holds anchor
> for
> matching rule.
>
>
> OK?
>
> thanks and
> regards
> sasha
>
> [1]
> https://github.com/openbsd/src/commit/e236f0fa7b23e94c7258b2055ec8e7c9804957c7#diff-9517dfce4e8db974781a4536fd38cfc1
>

I ran into this as well and can at least confirm that this fixes it.

Tim.


> 8<---8<---8<--8<
> diff --git a/sys/net/pf.c b/sys/net/pf.c
> index 51a91114c74..75d4e7158c2 100644
> --- a/sys/net/pf.c
> +++ b/sys/net/pf.c
> @@ -3108,9 +3108,9 @@ pf_step_into_anchor(struct pf_test_ctx *ctx, struct
> pf_rule *r)
>   rv = pf_match_rule(ctx, >ruleset);
>   if ((rv == PF_TEST_QUICK) || (rv == PF_TEST_FAIL)) {
>   /*
> -  * we either hit a rule qith quick action
> +  * we either hit a rule with quick action
>* (more likely), or hit some runtime
> -  * error (e.g. pool_get() faillure).
> +  * error (e.g. pool_get() failure).
>*/
>   break;
>   }
> @@ -3497,6 +3497,8 @@ enum pf_test_status
>  pf_match_rule(struct pf_test_ctx *ctx, struct pf_ruleset *ruleset)
>  {
>   struct pf_rule  *r;
> + struct pf_rule  *save_a;
> + struct pf_ruleset   *save_aruleset;
>
>   r = TAILQ_FIRST(ruleset->rules.active.ptr);
>   while (r != NULL) {
> @@ -3682,11 +3684,18 @@ pf_match_rule(struct pf_test_ctx *ctx, struct
> pf_ruleset *ruleset)
>   break;
>   }
>   } else {
> + save_a = ctx->a;
> + save_aruleset = ctx->aruleset;
>   ctx->a = r; /* remember anchor */
>   ctx->aruleset = ruleset;/* and its ruleset */
> - if (pf_step_into_anchor(ctx, r) != PF_TEST_OK) {
> + /*
> +  * Note: we don't need to restore if we are not going
> +  * to continue with ruleset evaluation.
> +  */
> + if (pf_step_into_anchor(ctx, r) != PF_TEST_OK)
>   break;
> - 

Re: Fix for vi(1) manpage Visual command

[trondd]

On Sat, February 3, 2018 4:28 pm, Ingo Schwarze wrote:
>
>> a good way to do it without adding verbiage would be to not document it!
>> if it really is a crappy quirk, let's just leave it out then.
>
> I actually like that idea, it makes the long list of EX COMMANDS
> a bit simpler, making the relevant stuff easier to find.
>
> Of course, we still have to say how to get out of split screen
> mode when you stumble into it unintentionally - which esily
> happens because the colon requires the shift key on many keyboards,
> so accidentally hitting ":N" instead of ":n" is not uncommon.
>
> If people like the idea, i'll also fix up :exusage (unless people
> want :exusage deleted, which i would of course prefer).
>
> OK?
>   Ingo
>

Well this is kind of a bummer.  I agree the documentation is inconsistent
and confusing, but once I got past reading the man page, I was so excited
to discover that I could get split screens in base vi without having to
install vim with its extra baggage.  I'm sad that a feature I use
constantly will now become undocumented (if not removed) and hidden from
other's who might find it valuable.

Not to discourage your work.  I'm not the one maintaining vi, and I can go
crawling back to ports for nvi if I need to.

Tim.

Fix for vi(1) manpage Visual command

[trondd]

The manpage for vi(1) has a small error for the :Visual/:visual command.  The
'V' can be capital or lowercase, followed by an 'i' and optionally 'sual'. But
the manpage shows the command as [Vi]i[sual] instead of [Vv]i[sual].

The usage text in vi confirms the correct syntax (as does the roff
documentation).

usr.bin/vi/ex/ex_cmd.c - line 386:

/* C_VISUAL_VI */
{"visual",  ex_edit,E_NEWSCREEN,
"f1o",
"[Vv]i[sual][!] [+cmd] [file]",
"edit another file (from vi mode only)"},

Tim.


Index: docs/USD.doc/vi.man/vi.1
===
RCS file: /cvs/src/usr.bin/vi/docs/USD.doc/vi.man/vi.1,v
retrieving revision 1.74
diff -u -p -r1.74 vi.1
--- docs/USD.doc/vi.man/vi.122 Aug 2017 20:27:18 -  1.74
+++ docs/USD.doc/vi.man/vi.13 Feb 2018 02:02:38 -
@@ -2200,7 +2200,7 @@ Enter
 .Nm vi .
 .Pp
 .It Xo
-.Op Cm Vi Ns
+.Op Cm Vv Ns
 .Cm i Ns Op Cm sual Ns
 .Op Cm !\&
 .Op Ar +cmd

Re: Propagate http_proxy from installer to rc.firsttime

[trondd]

trondd <tro...@kagu-tsuchi.com> wrote:

> If you need to use a proxy to fetch the base sets, you'll likely need
> it when you reboot in order for fw_update and syspatch to work.  It's
> helpful for fw_update especially as it has a painfully long timeout.
> 
> If the http_proxy was set, simply pass it on to rc.firsttime so you
> have it a reboot, too.
> 
> Tim.
> 

Reposting with updated patch.  I didn't get a "no thanks" and I think
this makes sense, so putting it out there again.

Tim.


Index: install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.1052
diff -u -p -r1.1052 install.sub
--- install.sub 22 Dec 2017 19:41:49 -  1.1052
+++ install.sub 23 Dec 2017 23:18:52 -
@@ -2718,6 +2718,10 @@ finish_up() {
[[ $MODE == upgrade ]] &&
echo "/usr/sbin/sysmerge -b" >>/mnt/etc/rc.sysmerge
 
+   # If a proxy was needed to fetch the sets, use it for fw_update and 
syspatch
+   [[ -n $http_proxy ]] &&
+   echo "export http_proxy=$http_proxy" >>/mnt/etc/rc.firsttime
+
# Ensure that fw_update is run on reboot.
echo "/usr/sbin/fw_update -v" >>/mnt/etc/rc.firsttime
 

Propagate http_proxy from installer to rc.firsttime

[trondd]

If you need to use a proxy to fetch the base sets, you'll likely need
it when you reboot in order for fw_update and syspatch to work.  It's
helpful for fw_update especially as it has a painfully long timeout.

If the http_proxy was set, simply pass it on to rc.firsttime so you
have it a reboot, too.

Tim.

Index: install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.1045
diff -u -p -r1.1045 install.sub
--- install.sub 1 Dec 2017 18:19:30 -   1.1045
+++ install.sub 7 Dec 2017 00:38:52 -
@@ -2724,6 +2724,10 @@ finish_up() {
[[ $MODE == upgrade ]] &&
echo "/usr/sbin/sysmerge -b" >>/mnt/etc/rc.sysmerge
 
+   # If a proxy was needed to fetch the sets, use it for fw_update and 
syspatch
+   [[ -n $http_proxy ]] &&
+   echo "export http_proxy=$http_proxy" >>/mnt/etc/rc.firsttime
+
# Ensure that fw_update is run on reboot.
echo "/usr/sbin/fw_update -v" >>/mnt/etc/rc.firsttime
 

Re: Wrong rule number in pflog with anchors

[trondd]

"trondd" <tro...@kagu-tsuchi.com> wrote:

> If you have an anchor in your pf ruleset, a packet that matches a rule
> with a log directive will reflect the rule number of the last anchor
> definition instead of the rule that caused the logging.
> 
> My first rule in pf.conf is 'block log (all) all'.  In 6.1, packets
> matching the block rule will show rule 1 as the matching rule.  Since 6.2
> and in current (not sure when during 6.2's development this started) the
> same blocked packet will show the rule number of the last anchor in the
> ruleset as the matching rule.
> 

I found that this was introduced in R1.1024 of pf.c which makes sense given
that the commit reworks anchor stacks.

A simplified pf.conf to demonstrate what I am seeing:

set skip on lo
block log all
pass out proto { udp tcp } to any port { ssh http https domain }
anchor "test"

Tim.

> 
> This is what I expact, and do get when no anchor is defined:
> 
> root@portabsd:~$ pfctl -sr -R1
> block return log (all) all
> 
> root@portabsd:~$ tcpdump -nettti pflog0 action block
> tcpdump: WARNING: snaplen raised from 116 to 160
> tcpdump: listening on pflog0, link-type PFLOG
> Oct 11 20:43:58.834603 rule 1/(match) block in on iwm0: 192.168.1.3.5353 >
> 224.0.0.251.5353: 0 [17q][|domain]
> Oct 11 20:43:58.837980 rule 1/(match) block in on iwm0:
> fe80::8c2:5295:cd0e:f5e4.5353 > ff02::fb.5353: 0 [17q][|domain] [flowlabel
> 0x84d6b]
> Oct 11 20:44:18.233207 rule 1/(match) block in on iwm0: 192.168.1.3.52286
> > 192.168.1.15.445: S 176378676:176378676(0) win 65535  1460,nop,wscale 5,nop,nop,timestamp 2314135130 0,[|tcp]> (DF) [tos 0x10] ^C
> 3 packets received by filter
> 0 packets dropped by kernel
> 
> 
> Add a bogus 'anchor "test"' to the bottom of pf.conf and reload.  Hit the
> system with blockable traffic again:
> 
> root@portabsd:~$ tcpdump -nettti pflog0 action block
> tcpdump: WARNING: snaplen raised from 116 to 160
> tcpdump: listening on pflog0, link-type PFLOG
> Oct 11 20:44:50.038509 rule 43/(match) block in on iwm0: 192.168.1.3.52289
> > 192.168.1.15.445: SWE 3438533119:3438533119(0) win 65535  1460,nop,wscale 5,nop,nop,timestamp 2314166871 0,[|tcp]> (DF) [tos 0x10] ^C
> 1 packets received by filter
> 0 packets dropped by kernel
> 
> root@portabsd:~$ pfctl -sr -R1
> block return log (all) all
> 
> root@portabsd:~$ pfctl -sr -R 43
> anchor "test" all
> 
> 
> My cleaned up pf.conf used in the above reproductions:
> 
> wan_services = "{ http https pop3s imaps smtps whois 11371 ssh 53589 8008 }"
> set skip on { lo enc }
> match in all scrub (no-df random-id reassemble tcp)
> set block-policy return
> block log (all) all
> antispoof quick for egress
> vm_net = "{ 10.10.10.0/24 }"
> match out on egress inet from $vm_net to any nat-to (egress:0)
> pass in quick on vether0 from $vm_net to any
> pass out quick proto { tcp udp } to 192.168.1.1 port 53
> pass out quick proto tcp to any port { 6667 6697 } user irc
> block out quick proto { udp tcp } user irc
> pass out quick proto tcp to any port $wan_services
> pass out quick proto { udp } to any port 123
> pass quick proto udp to any port { 67 68 }
> pass out quick proto icmp all
> pass quick inet proto icmp all icmp-type unreach code needfrag
> pass out quick proto udp to port 33433 >< 33626
> block in quick from 192.168.1.1 to 224.0.0.1
> vpn_dest = "{ xxx.xxx.xxx.xxx xxx.xxx.xxx.xxx }"
> pass in on egress proto esp from $vpn_dest to (self)
> pass out on egress proto esp from (self) to $vpn_dest
> pass in on egress proto udp from $vpn_dest to (self) port { isakmp
> ipsec-nat-t }
> pass out on egress proto udp from (self) to $vpn_dest port { isakmp
> ipsec-nat-t }
> pass in log quick proto tcp from 192.168.1.0/24 to (self) port ssh pass
> quick on egress proto tcp to any port 22000
> anchor "test"

Wrong rule number in pflog with anchors

[trondd]

If you have an anchor in your pf ruleset, a packet that matches a rule
with a log directive will reflect the rule number of the last anchor
definition instead of the rule that caused the logging.

My first rule in pf.conf is 'block log (all) all'.  In 6.1, packets
matching the block rule will show rule 1 as the matching rule.  Since 6.2
and in current (not sure when during 6.2's development this started) the
same blocked packet will show the rule number of the last anchor in the
ruleset as the matching rule.


This is what I expact, and do get when no anchor is defined:

root@portabsd:~$ pfctl -sr -R1
block return log (all) all

root@portabsd:~$ tcpdump -nettti pflog0 action block
tcpdump: WARNING: snaplen raised from 116 to 160
tcpdump: listening on pflog0, link-type PFLOG
Oct 11 20:43:58.834603 rule 1/(match) block in on iwm0: 192.168.1.3.5353 >
224.0.0.251.5353: 0 [17q][|domain]
Oct 11 20:43:58.837980 rule 1/(match) block in on iwm0:
fe80::8c2:5295:cd0e:f5e4.5353 > ff02::fb.5353: 0 [17q][|domain] [flowlabel
0x84d6b]
Oct 11 20:44:18.233207 rule 1/(match) block in on iwm0: 192.168.1.3.52286
> 192.168.1.15.445: S 176378676:176378676(0) win 65535  (DF) [tos 0x10] ^C
3 packets received by filter
0 packets dropped by kernel


Add a bogus 'anchor "test"' to the bottom of pf.conf and reload.  Hit the
system with blockable traffic again:

root@portabsd:~$ tcpdump -nettti pflog0 action block
tcpdump: WARNING: snaplen raised from 116 to 160
tcpdump: listening on pflog0, link-type PFLOG
Oct 11 20:44:50.038509 rule 43/(match) block in on iwm0: 192.168.1.3.52289
> 192.168.1.15.445: SWE 3438533119:3438533119(0) win 65535  (DF) [tos 0x10] ^C
1 packets received by filter
0 packets dropped by kernel

root@portabsd:~$ pfctl -sr -R1
block return log (all) all

root@portabsd:~$ pfctl -sr -R 43
anchor "test" all


My cleaned up pf.conf used in the above reproductions:

wan_services = "{ http https pop3s imaps smtps whois 11371 ssh 53589 8008 }"
set skip on { lo enc }
match in all scrub (no-df random-id reassemble tcp)
set block-policy return
block log (all) all
antispoof quick for egress
vm_net = "{ 10.10.10.0/24 }"
match out on egress inet from $vm_net to any nat-to (egress:0)
pass in quick on vether0 from $vm_net to any
pass out quick proto { tcp udp } to 192.168.1.1 port 53
pass out quick proto tcp to any port { 6667 6697 } user irc
block out quick proto { udp tcp } user irc
pass out quick proto tcp to any port $wan_services
pass out quick proto { udp } to any port 123
pass quick proto udp to any port { 67 68 }
pass out quick proto icmp all
pass quick inet proto icmp all icmp-type unreach code needfrag
pass out quick proto udp to port 33433 >< 33626
block in quick from 192.168.1.1 to 224.0.0.1
vpn_dest = "{ xxx.xxx.xxx.xxx xxx.xxx.xxx.xxx }"
pass in on egress proto esp from $vpn_dest to (self)
pass out on egress proto esp from (self) to $vpn_dest
pass in on egress proto udp from $vpn_dest to (self) port { isakmp
ipsec-nat-t }
pass out on egress proto udp from (self) to $vpn_dest port { isakmp
ipsec-nat-t }
pass in log quick proto tcp from 192.168.1.0/24 to (self) port ssh pass
quick on egress proto tcp to any port 22000
anchor "test"

Re: RTM_DESYNC when starting network

[trondd]

On Thu, October 12, 2017 10:36 am, Martin Pieuchot wrote:
> On 12/10/17(Thu) 10:33, trondd wrote:
>> Just updated my -current VM hosted in VMware Fusion.  Upon reboot, I got
>> an em0: RTM_DESYNC and the boot process hung.  After a couple forced
>> reboots, I figured out that if I "Disconnect Network Adapter" then
>> reconnect it, booting will continue with the DHCP exchange and all seems
>> to work fine.
>
> This has just been fixed in -current.  Please compile a new kernel or
> wait for the next snapshot.
>

Confirmed.  Thanks.

Tim.

RTM_DESYNC when starting network

[trondd]

Just updated my -current VM hosted in VMware Fusion.  Upon reboot, I got
an em0: RTM_DESYNC and the boot process hung.  After a couple forced
reboots, I figured out that if I "Disconnect Network Adapter" then
reconnect it, booting will continue with the DHCP exchange and all seems
to work fine.

Tim.


>From dmesg -s:

starting network
em0: RTM_DESYNC  <-- Hangs here until network disconnect/reconnect
em0: DHCPREQUEST to 255.255.255.255
em0: DHCPACK from 172.25.177.1 (00:10:db:ff:10:02)
em0: bound to 172.25.177.119 -- renewal in 129600 seconds
reordering libraries: done.


Full dmesg:

OpenBSD 6.2-current (GENERIC.MP) #143: Thu Oct 12 01:07:17 MDT 2017
dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 2130640896 (2031MB)
avail mem = 2059272192 (1963MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.4 @ 0xe0010 (242 entries)
bios0: vendor Phoenix Technologies LTD version "6.00" date 07/02/2015
bios0: VMware, Inc. VMware Virtual Platform
acpi0 at bios0: rev 2
acpi0: sleep states S0 S1 S4 S5
acpi0: tables DSDT FACP BOOT APIC MCFG SRAT HPET WAET
acpi0: wakeup devices PCI0(S3) USB_(S1) P2P0(S3) S1F0(S3) S2F0(S3)
S8F0(S3) S16F(S3) S18F(S3) S22F(S3) S23F(S3) S24F(S3) S25F(S3) PE40(S3)
S1F0(S3) PE50(S3) S1F0(S3) [...]
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-3720QM CPU @ 2.60GHz, 2593.76 MHz
cpu0:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,MMX,FXSR,SSE,SSE2,SS,SSE3,PCLMUL,SSSE3,CX16,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,RDTSCP,LONG,LAHF,PERF,ITSC,FSGSBASE,SMEP,ERMS,SENSOR,ARAT
cpu0: 256KB 64b/line 8-way L2 cache
acpitimer0: recalibrated TSC frequency -1 Hz
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
cpu0: apic clock running at 65MHz
cpu1 at mainbus0: apid 2 (application processor)
cpu1: Intel(R) Core(TM) i7-3720QM CPU @ 2.60GHz, 2593.16 MHz
cpu1:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,MMX,FXSR,SSE,SSE2,SS,SSE3,PCLMUL,SSSE3,CX16,PCID,SSE4.1,SSE4.2,x2APIC,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,RDTSCP,LONG,LAHF,PERF,ITSC,FSGSBASE,SMEP,ERMS,SENSOR,ARAT
cpu1: 256KB 64b/line 8-way L2 cache
cpu1: smt 0, core 0, package 2
ioapic0 at mainbus0: apid 1 pa 0xfec0, version 11, 24 pins
acpimcfg0 at acpi0 addr 0xf000, bus 0-127
acpihpet0 at acpi0: 14318179 Hz
acpihpet0: recalibrated TSC frequency 2593859033 Hz
acpiprt0 at acpi0: bus 0 (PCI0)
acpicpu0 at acpi0: C1(@1 halt!)
acpicpu1 at acpi0: C1(@1 halt!)
"PNP0001" at acpi0 not configured
"VMW0003" at acpi0 not configured
"PNP0A05" at acpi0 not configured
acpiac0 at acpi0: AC unit online
pvbus0 at mainbus0: VMware
vmt0 at pvbus0
pci0 at mainbus0 bus 0
pchb0 at pci0 dev 0 function 0 "Intel 82443BX AGP" rev 0x01
ppb0 at pci0 dev 1 function 0 "Intel 82443BX AGP" rev 0x01
pci1 at ppb0 bus 1
pcib0 at pci0 dev 7 function 0 "Intel 82371AB PIIX4 ISA" rev 0x08
pciide0 at pci0 dev 7 function 1 "Intel 82371AB IDE" rev 0x01: DMA,
channel 0 configured to compatibility, channel 1 configured to
compatibility
wd0 at pciide0 channel 0 drive 0: 
wd0: 64-sector PIO, LBA, 20480MB, 41943040 sectors
wd0(pciide0:0:0): using PIO mode 4, Ultra-DMA mode 2
atapiscsi0 at pciide0 channel 1 drive 0
scsibus1 at atapiscsi0: 2 targets
cd0 at scsibus1 targ 0 lun 0:  ATAPI
5/cdrom removable
cd0(pciide0:1:0): using PIO mode 4, Ultra-DMA mode 2
piixpm0 at pci0 dev 7 function 3 "Intel 82371AB Power" rev 0x08: SMBus
disabled
"VMware VMCI" rev 0x10 at pci0 dev 7 function 7 not configured
vga1 at pci0 dev 15 function 0 "VMware SVGA II" rev 0x00
wsdisplay0 at vga1 mux 1: console (80x25, vt100 emulation)
wsdisplay0: screen 1-5 added (80x25, vt100 emulation)
ppb1 at pci0 dev 17 function 0 "VMware PCI" rev 0x02
pci2 at ppb1 bus 2
em0 at pci2 dev 0 function 0 "Intel 82545EM" rev 0x01: apic 1 int 18,
address 00:0c:29:18:ce:28
eap0 at pci2 dev 1 function 0 "Ensoniq AudioPCI97" rev 0x02: apic 1 int 19
ac97: codec id 0x43525913 (Cirrus Logic CS4297A rev 3)
audio0 at eap0
midi0 at eap0: 
ppb2 at pci0 dev 21 function 0 "VMware PCIE" rev 0x01
pci3 at ppb2 bus 3
ppb3 at pci0 dev 21 function 1 "VMware PCIE" rev 0x01
pci4 at ppb3 bus 4
ppb4 at pci0 dev 21 function 2 "VMware PCIE" rev 0x01
pci5 at ppb4 bus 5
ppb5 at pci0 dev 21 function 3 "VMware PCIE" rev 0x01
pci6 at ppb5 bus 6
ppb6 at pci0 dev 21 function 4 "VMware PCIE" rev 0x01
pci7 at ppb6 bus 7
ppb7 at pci0 dev 21 function 5 "VMware PCIE" rev 0x01
pci8 at ppb7 bus 8
ppb8 at pci0 dev 21 function 6 "VMware PCIE" rev 0x01
pci9 at ppb8 bus 9
ppb9 at pci0 dev 21 function 7 "VMware PCIE" rev 0x01
pci10 at ppb9 bus 10
ppb10 at pci0 dev 22 function 0 "VMware PCIE" rev 0x01
pci11 at ppb10 bus 11
ppb11 at pci0 dev 22 function 1 "VMware PCIE" rev 0x01
pci12 at ppb11 bus 12

[PATCH] Syspatch not clearing needed free space counter

[trondd]

Stuart Henderson  wrote:

> Ah, perhaps the change to disk behaviour wasn't reflected in calculations 
> then..

I got it figured out.

In the checkfs function, the 'eval $(stat...)' command stores a list of disk
devices and creates a variable named for each device to store the size of the
files in the patch to be installed there.  It uses :+ to accumulate the sizes of
multiple files.  But since ksh creates variales as global by default, these are
not cleared between patches and :+ takes an existing value if the variable 
already
exists.  So the value stored in these variables will continue to accumulate.

A simple solution seems to be to mark these variables local.  Ksh will clean 
them
up between calls to checkfs.

Tim.


Index: syspatch.sh
===
RCS file: /cvs/src/usr.sbin/syspatch/syspatch.sh,v
retrieving revision 1.120
diff -u -p -r1.120 syspatch.sh
--- syspatch.sh 2 Aug 2017 05:58:29 -   1.120
+++ syspatch.sh 3 Aug 2017 21:59:13 -
@@ -91,7 +91,7 @@ checkfs()
# - nonexistent files (i.e. syspatch is installing new files)
# - broken interpolation due to bogus devices like remote filesystems
eval $(cd / &&
-   stat -qf "_dev=\"\${_dev} %Sd\" %Sd=\"\${%Sd:+\${%Sd}\+}%Uz\"" \
+   stat -qf "_dev=\"\${_dev} %Sd\"; local 
%Sd=\"\${%Sd:+\${%Sd}\+}%Uz\"" \
${_files}) 2>/dev/null || _ret=$?
set -e
[[ ${_ret} == 127 ]] && sp_err "Remote filesystem, aborting" 

Re: Syspatch not clearing needed free space counter

[trondd]

On Thu, August 3, 2017 1:09 pm, Stuart Henderson wrote:
> On 2017/08/03 09:55, trondd wrote:
>> I noticed, when applying multiple patches, I would eventually be told I
>> was out of space on /.  There was about 60M free which is plenty for a
>> kernel and a copy.  Rerunning syspatch would allow it to continue for a
>> another handful of patches.
>>
>> Mucking about in checkfs(), it seems that the value of the space needed
>> for a partition is not cleared and accumulates for each patch.  Afer a
>> few
>> kernel patches, the number grows 10M at a time and I'd eventually be
>> told
>> I didn't have enough space.
>
> This was fixed (22 July), you can copy the syspatch script from a -current
> system to a 6.1. With 6.1's syspatch, unless you're very tight on space,
> you can keep re-running it and it should eventually make it through.
>

I believe the clean ups were for data on disk.  This problem seems to be
with the variable value in memory.  My free disk space isn't changing,
just the free space syspatch thinks it needs.

Also I am using the latest syspatch from CVS.

# $OpenBSD: syspatch.sh,v 1.120 2017/08/02 05:58:29 ajacoutot Exp $

Syspatch not clearing needed free space counter

[trondd]

I noticed, when applying multiple patches, I would eventually be told I
was out of space on /.  There was about 60M free which is plenty for a
kernel and a copy.  Rerunning syspatch would allow it to continue for a
another handful of patches.

Mucking about in checkfs(), it seems that the value of the space needed
for a partition is not cleared and accumulates for each patch.  Afer a few
kernel patches, the number grows 10M at a time and I'd eventually be told
I didn't have enough space.

I honestly have no idea how the magic works that _d can be the disk device
name in the for loop (line 99) and then be the space required in $((_d))
(line 103) so I don't know how to fix this.  However, if at the top of
checkfs you set _d to "sda0" and echo $((_d)), you'll see the old value is
still there.

_d="sd0a"
echo "magic = $((_d))"

Below is my debugging output.  You can see "magic" has a value when output
at the top of checkfs and _sz keeps accumulating.  I do an ls and a df
when it errors out.

Sorry it's a mishmash of my debugging instead of a patch...  Hopefully
it's obvious to someone who knows ksh well enough.

Tim.

Get/Verify syspatch61-001_dhcpd.tgz 100%
|| 71730  
00:00
Installing patch 001_dhcpd
init: d = , parens d = 0, _df = , sz =
magic = 0
dev list =  sd0f sd0f
d = sd0f, parens d = 176846
d = sd0f, df = 539134, sz = 172
Get/Verify syspatch61-002_vmmfpu.tgz 100%
|***|  9377 KB   
00:02
Installing patch 002_vmmfpu
init: d = , parens d = 0, _df = , sz =
magic = 0
dev list =  sd0a sd0a sd0f sd0f sd0f sd0f sd0f sd0f
d = sd0a, parens d = 21503878
d = sd0a, df = 69752, sz = 20999
d = sd0f, parens d = 236338
d = sd0f, df = 539134, sz = 230
Get/Verify syspatch61-003_libress... 100%
|***| 11391 KB   
00:02
Installing patch 003_libressl
s = -s /^bsd.mp$//
init: d = , parens d = 0, _df = , sz =
magic = 21503878
dev list =  sd0a sd0a sd0f sd0f sd0f sd0f sd0f sd0f sd0f
d = sd0a, parens d = 23928486
d = sd0a, df = 59288, sz = 23367
d = sd0f, parens d = 35585944
d = sd0f, df = 539132, sz = 34751
Get/Verify syspatch61-004_softrai... 100%
|***|  9356 KB   
00:02
Installing patch 004_softraid_concat
init: d = , parens d = 0, _df = , sz =
magic = 23928486
dev list =  sd0a
d = sd0a, parens d = 34612636
d = sd0a, df = 59288, sz = 33801
Get/Verify syspatch61-005_pf_src_... 100%
|***|  9355 KB   
00:02
Installing patch 005_pf_src_tracking
init: d = , parens d = 0, _df = , sz =
magic = 34612636
dev list =  sd0a
d = sd0a, parens d = 45296786
d = sd0a, df = 59288, sz = 44235
Get/Verify syspatch61-006_libssl.tgz 100%
|***|  2276 KB   
00:00
Installing patch 006_libssl
init: d = , parens d = 0, _df = , sz =
magic = 45296786
dev list =  sd0f sd0f sd0f
d = sd0f, parens d = 42837381
d = sd0f, df = 539132, sz = 41833
Get/Verify syspatch61-007_freetyp... 100%
|***|   732 KB   
00:00
Installing patch 007_freetype
init: d = , parens d = 0, _df = , sz =
magic = 45296786
dev list =  sd0g sd0g
d = sd0g, parens d = 1696747
d = sd0g, df = 336564, sz = 1656
Get/Verify syspatch61-008_exec_su... 100%
|***|  9356 KB   
00:01
Installing patch 008_exec_subr
init: d = , parens d = 0, _df = , sz =
magic = 45296786
dev list =  sd0a
d = sd0a, parens d = 55980936
d = sd0a, df = 59288, sz = 54668
Get/Verify syspatch61-009_icmp_op... 100%
|***|  9356 KB   
00:02
Installing patch 009_icmp_opts
init: d = , parens d = 0, _df = , sz =
magic = 55980936
dev list =  sd0a
d = sd0a, parens d = 5086
d = sd0a, df = 59288, sz = 65102
Filesystem SizeUsed   Avail Capacity  Mounted on
/dev/sd0a  129M   64.6M   57.9M53%/
/dev/sd0i  3.7G   62.0K3.5G 0%/home
/dev/sd0d  198M   19.4M168M10%/tmp
/dev/sd0f  935M362M527M41%/usr
/dev/sd0g  532M177M329M35%/usr/X11R6
/dev/sd0h  2.1G   38.3M1.9G 2%/usr/local
/dev/sd0e  209M   39.8M159M20%/var
/dev/sd1a 19.7G2.0K   18.7G 0%/var/postgresql
total 81648
drwxr-xr-x  14 root  wheel  512B Aug  3 13:20 .
drwxr-xr-x  14 root  wheel  512B Aug  3 13:20 ..
-rw-r--r--   1 root  wheel  578B Apr  1 19:38 .cshrc
-rw-r--r--   1 root  wheel  468B Apr  1 19:38 .profile
drwxr-xr-x   2 root  wheel  512B Apr  1 19:38 altroot
drwxr-xr-x   2 root  wheel  1.0K Apr  1 19:38 bin
-rw-r--r--   1 root  wheel 84.6K May 10 11:05 boot
-rwxr-xr-x   1 root  wheel 10.2M May 18 20:16 bsd
-rw-r--r--   1 root  wheel  9.0M May 10 11:00 bsd.rd
-rw-r--r--   1 root  wheel 10.2M May 10 

Re: httpd ranges are not liked by freshclam

[trondd]

On Tue, April 18, 2017 3:46 pm, Reyk Floeter wrote:
>
>> Am 18.04.2017 um 20:53 schrieb trondd <tro...@kagu-tsuchi.com>:
>>
>> I have an OpenBSD httpd(8) web server hosting security/clamav main.cvd
>> and
>> daily.cvd files.  Upon upgrading to 6.1, freshclam can no longer
>> successfully fetch the cvd files.
>>
>> Freshclam does a request for the first 512 bytes of the files to check
>> the
>> dates in their header.  Then pulls the rest of the file if needed.  It
>> looks like it pulls the *whole* file again.  It doesn't pick up where it
>> left off.
>>
>> With httpd from 6.0, fully patched, this was working fine.  Whith 6.1,
>> freshclam would request the 512 chunk, then timeout with "nonblock_recv:
>> recv timing out (30 secs)".
>>
>> Knowing there were a couple of changes to ranges in httpd, I started
>> rolling things back.  I took out the pipelining fix:
>> http://marc.info/?l=openbsd-cvs=148607400902939=2
>>
>> Which didn't help.  Then I also took out the range rewrite:
>> http://marc.info/?l=openbsd-cvs=148587359420912=2
>>
>> And bingo.  Freshclam happily pulled it's now much out of date daily
>> database. :)
>>
>> I don't know if freshclam is doing something wacky here or if it's
>> httpd.
>> It does return the requested byte range, and I was able to pull a range
>> with curl as well.  I don't know another test case for this off hand.
>>
>
> Do you have any more details like request/response HTTP headers with old
> and new code?
>
> Reyk
>

Yes.  Hopefully these attach properly.  Only have access to web mail from
here so scream at me if all you get is garbage and I can resend properly
later.

ASCII output from the tcpdump showing success and failure cases.  I have
the full binary pcaps if needed.  Comparing quickly, I see 6.1 sends the
Partial Content response header in a seperate packet from the content. 
Previous code didn't do that.

Tim.Apr 18 14:46:43.193474 00:50:56:a2:e3:59 00:50:56:53:53:06 0800 74: 
172.25.87.253.36732 > 172.25.87.91.80: S [tcp sum ok] 3129109532:3129109532(0) 
win 29200  (DF) (ttl 64, 
id 24030, len 60)
.PVSS..PV..Y..E..<].@.@..R..W...W[.|.P..l...r.12.
..1.
Apr 18 14:46:43.375068 00:50:56:53:53:06 00:50:56:a2:e3:59 0800 78: 
172.25.87.91.80 > 172.25.87.253.36732: S [tcp sum ok] 4257186703:4257186703(0) 
ack 3129109533 win 16384  (ttl 64, id 51915, len 64)
.PV..Y.PVSS...E..@@..a..W[..W..P.|..l...@..6.
..C...1.
Apr 18 14:46:43.375489 00:50:56:a2:e3:59 00:50:56:53:53:06 0800 66: 
172.25.87.253.36732 > 172.25.87.91.80: . [tcp sum ok] 1:1(0) ack 1 win 229 
<nop,nop,timestamp 3036951213 4072752100> (DF) (ttl 64, id 24031, len 52)
.PVSS..PV..Y..E..4].@.@..Y..W...W[.|.P..l..8.
..2...C.
Apr 18 14:46:43.375506 00:50:56:a2:e3:59 00:50:56:53:53:06 0800 285: 
172.25.87.253.36732 > 172.25.87.91.80: P [tcp sum ok] 1:220(219) ack 1 win 229 
<nop,nop,timestamp 3036951213 4072752100> (DF) (ttl 64, id 24032, len 271)
.PVSS..PV..Y..E...].@.@..}..W...W[.|.P..l.wy.
..2...C.GET /main.cld HTTP/1.0
Host: obsd-build.llan.ll.mit.edu
User-Agent: ClamAV/0.99.2 (OS: linux-gnu, ARCH: x86_64, CPU: x86_64)
Connection: close
Range: bytes=0-511
If-Modified-Since: Wed, 16 Mar 2016 23:17:06 GMT


Apr 18 14:46:43.376135 00:50:56:53:53:06 00:50:56:a2:e3:59 0800 644: 
172.25.87.91.80 > 172.25.87.253.36732: P [bad tcp cksum 9f4! -> d979] 
1:579(578) ack 220 win 271 <nop,nop,timestamp 4072752100 3036951213> (ttl 64, 
id 54869, len 630)
.PV..Y.PVSS...E..v.U..@.W[..W..P.|..l.  ..
..C...2.HTTP/1.0 404 Not Found
Date: Tue, 18 Apr 2017 18:46:43 GMT
Server: OpenBSD httpd
Connection: close
Content-Type: text/html
Content-Length: 427





404 Not Found
<!--
body { background-color: white; color: black; font-family: 'Comic Sans MS', 
'Chalkboard SE', 'Comic Neue', sans-serif; }
hr { border: 0; border-bottom: 1px dashed; }

-->


404 Not Found

OpenBSD httpd



Apr 18 14:46:43.376185 00:50:56:53:53:06 00:50:56:a2:e3:59 0800 66: 
172.25.87.91.80 > 172.25.87.253.36732: F [bad tcp cksum 7b2! -> 14f0] 
579:579(0) ack 220 win 271 <nop,nop,timestamp 4072752100 3036951213> (ttl 64, 
id 45714, len 52)
.PV..Y.PVSS...E..4@.W[..W..P.|..l
..C...2.
Apr 18 14:46:43.376729 00:50:56:a2:e3:59 00:50:56:53:53:06 0800 66: 
172.25.87.253.36732 > 172.25.87.91.80: . [tcp sum ok] 220:220(0) ack 579 win 
238 <nop,nop,timestamp 3036951214 4072752100> (DF) (ttl 64, id 24033, len 52)
.PVSS..PV..Y..E..4].@.@..W..W...W[.|.P..l
..2...C.
Apr 18 14:46:43.376738 00:50:56:a2:e3:59 00:50:56:53:53:06 0800 66: 
172.25.87.253.36732 > 172.25.87.91.80: F [tcp sum ok] 220:220(0) ack 580 win 
238 <nop,nop,timestamp 3036951214 4072752100> (

httpd ranges are not liked by freshclam

[trondd]

I have an OpenBSD httpd(8) web server hosting security/clamav main.cvd and
daily.cvd files.  Upon upgrading to 6.1, freshclam can no longer
successfully fetch the cvd files.

Freshclam does a request for the first 512 bytes of the files to check the
dates in their header.  Then pulls the rest of the file if needed.  It
looks like it pulls the *whole* file again.  It doesn't pick up where it
left off.

With httpd from 6.0, fully patched, this was working fine.  Whith 6.1,
freshclam would request the 512 chunk, then timeout with "nonblock_recv:
recv timing out (30 secs)".

Knowing there were a couple of changes to ranges in httpd, I started
rolling things back.  I took out the pipelining fix:
http://marc.info/?l=openbsd-cvs=148607400902939=2

Which didn't help.  Then I also took out the range rewrite:
http://marc.info/?l=openbsd-cvs=148587359420912=2

And bingo.  Freshclam happily pulled it's now much out of date daily
database. :)

I don't know if freshclam is doing something wacky here or if it's httpd. 
It does return the requested byte range, and I was able to pull a range
with curl as well.  I don't know another test case for this off hand.

Tim.

Re: Add a "random" target to bsd.regress.mk

[trondd]

On Sat, March 18, 2017 6:42 pm, Scott Cheloha wrote:
> In general, a test can put your system into a state that allows a
> subsequent test to pass when it would have otherwise failed.
>

>
> Any takers?  Thoughts?
>

If such a bug is revealed, how does someone rerun the tests in the same
order to reproduce the bug or verify when it's fixed?

"Manually" is tough if you don't know which previous test of many may have
caused the problematic system state.

Actually, rereading what I quoted, I see you're concerned with unmasking
false positives, which can be manually rerun in isolation to reproduce. 
My thought still stands for the flip-side where a test fails due to a
previous test.

Tim.

Allow device/fifo creation with zipped archives

[trondd]

As brought up on misc@ pax doesn't allow creation of devices or fifos without
the p flag, however this is only when the archive is not compressed.  If you
compress the archive, you can create them upon decompression/unarchiving.  
Since dpath was added to allow creation of devices in the pledge call for the
non-compression code path, I am assuming it was meant to be added to the
compresson code path as well.

Tim.


Index: pax.c
===
RCS file: /cvs/src/bin/pax/pax.c,v
retrieving revision 1.44
diff -u -p -r1.44 pax.c
--- pax.c   16 Dec 2015 01:39:11 -  1.44
+++ pax.c   23 Jun 2016 00:40:55 -
@@ -267,7 +267,7 @@ main(int argc, char **argv)
 
/* Copy mode, or no gzip -- don't need to fork/exec. */
if (gzip_program == NULL || act == COPY) {
-   if (pledge("stdio rpath wpath fattr cpath getpw ioctl",
+   if (pledge("stdio rpath wpath dpath fattr cpath getpw 
ioctl",
NULL) == -1)
err(1, "pledge");
}

[PATCH] www - Missing body tags and footer color

[trondd]

Add some missing body tags using the standard colors.
Also set the background color in the footer table cell on the main page.

Tim.

Index: crypto.html
===
RCS file: /cvs/www/crypto.html,v
retrieving revision 1.149
diff -u -p -r1.149 crypto.html
--- crypto.html 24 Apr 2016 20:08:48 -  1.149
+++ crypto.html 18 May 2016 01:22:46 -
@@ -9,6 +9,8 @@
 http://www.openbsd.org/crypto.html;>
 
 
+
+
 
 
 OpenBSD
Index: hackathons.html
===
RCS file: /cvs/www/hackathons.html,v
retrieving revision 1.97
diff -u -p -r1.97 hackathons.html
--- hackathons.html 25 Apr 2016 09:09:55 -  1.97
+++ hackathons.html 18 May 2016 01:22:46 -
@@ -11,6 +11,8 @@
 http://www.openbsd.org/hackathons.html;>
 
 
+
+
 
 
 OpenBSD
Index: index.html
===
RCS file: /cvs/www/index.html,v
retrieving revision 1.698
diff -u -p -r1.698 index.html
--- index.html  9 May 2016 20:19:23 -   1.698
+++ index.html  18 May 2016 01:22:46 -
@@ -84,7 +84,7 @@
   

-
+
   
   Associated projects:
   http://www.openssh.com;>OpenSSH,
Index: lyrics.html
===
RCS file: /cvs/www/lyrics.html,v
retrieving revision 1.170
diff -u -p -r1.170 lyrics.html
--- lyrics.html 24 Apr 2016 20:08:49 -  1.170
+++ lyrics.html 18 May 2016 01:22:46 -
@@ -12,6 +12,8 @@
 http://www.openbsd.org/lyrics.html;>
 
 
+
+
 
 
 OpenBSD
Index: papers/index.html
===
RCS file: /cvs/www/papers/index.html,v
retrieving revision 1.254
diff -u -p -r1.254 index.html
--- papers/index.html   23 Apr 2016 03:26:00 -  1.254
+++ papers/index.html   18 May 2016 01:22:46 -
@@ -13,6 +13,8 @@
 UPDATE events.html WHEN ADDING TALKS HERE!
 -->
 
+
+
 
 
 OpenBSD

Re: sendbug subject

[trondd]

On Sun, May 15, 2016 1:22 pm, Juan Francisco Cantero Hurtado wrote:
> On Sun, May 15, 2016 at 06:43:16PM +0200, Jeremie Courreges-Anglas wrote:
>> "Ted Unangst"  writes:
>>
>> > i'm tired of seeing bug reports with no subject. i also get a fair bit
>> of spam
>> > with no subject and i am easily confused. something is better than
>> nothing.
>>
>> I fear that after that change all bug reports will only have [bug
>> report] as Subject.  Something that wouldn't catch the eye of people
>> that might be able to understand and fix the problem.
>

Why not make Subject a required field?  Might want to also add a comment
there like Category and Synopsis have.

Tim.


Index: sendbug.c
===
RCS file: /cvs/src/usr.bin/sendbug/sendbug.c,v
retrieving revision 1.74
diff -u -p -r1.74 sendbug.c
--- sendbug.c   17 Mar 2016 19:40:43 -  1.74
+++ sendbug.c   15 May 2016 21:10:00 -
@@ -513,7 +513,7 @@ checkfile(const char *pathname)
 {
FILE *fp;
size_t len;
-   int category = 0, synopsis = 0;
+   int category = 0, synopsis = 0, subject = 0;
char *buf;

if ((fp = fopen(pathname, "r")) == NULL) {
@@ -525,9 +525,11 @@ checkfile(const char *pathname)
category = 1;
else if (matchline(">Synopsis:", buf, len))
synopsis = 1;
+   else if (matchline("Subject:", buf, len))
+   subject = 1;
}
fclose(fp);
-   return (category && synopsis);
+   return (category && synopsis && subject);
 }

 void

[PATCH] vmm(4) manpage missed the vmctl rename

[trondd]

Update manpage reference from vmmctl(8) to vmctl(8)

Tim.


Index: vmm.4
===
RCS file: /cvs/src/share/man/man4/man4.amd64/vmm.4,v
retrieving revision 1.3
diff -u -p -r1.3 vmm.4
--- vmm.4   13 Nov 2015 07:55:37 -  1.3
+++ vmm.4   6 Dec 2015 16:27:41 -
@@ -72,7 +72,7 @@ For more information, consult the CPU ve
 .Xr intro 4 ,
 .Xr virtio 4 ,
 .Xr vmd 8 ,
-.Xr vmmctl 8
+.Xr vmctl 8
 .Sh HISTORY
 The
 .Nm

smtpd needs to pledge unix for lmtp socket delivery

[trondd]

smtpd needs to pledge unix to support delivery_backend_lmtp to a socket.

Config example:
accept for domain "mydomain.com" alias  deliver to lmtp
"/var/dovecot/lmtp"

Tim.


Index: smtpd.c
===
RCS file: /cvs/src/usr.sbin/smtpd/smtpd.c,v
retrieving revision 1.249
diff -u -p -r1.249 smtpd.c
--- smtpd.c 17 Oct 2015 04:36:10 -  1.249
+++ smtpd.c 17 Oct 2015 16:53:46 -
@@ -690,7 +690,7 @@ main(int argc, char *argv[])

purge_task();

-   if (pledge("stdio rpath wpath cpath flock tmppath getpw sendfd
proc exec id",
+   if (pledge("stdio rpath wpath cpath flock tmppath getpw sendfd
proc exec id unix",
NULL) == -1)
err(1, "pledge");

Re: smtpd needs to pledge unix for lmtp socket delivery

[trondd]

On Sat, October 17, 2015 12:59 pm, trondd wrote:
>
> smtpd needs to pledge unix to support delivery_backend_lmtp to a socket.
>

And now that I am getting email, I see this was fixed just minutes after I
had updated my cvs checkout. :)

Carry on.

Tim.

Re: Another lock(1) pledge tweak

[trondd]

Whoops.  I meant lock(1) in the subject.  I guess making a patch put the
word patch into my head.

On Thu, October 15, 2015 9:25 pm, trondd wrote:
> Is it safer to drop the recently added proc and exec pledges if the
> arguments are not chosen which need them?
>
> Index: lock.c
> ===
> RCS file: /cvs/src/usr.bin/lock/lock.c,v
> retrieving revision 1.32
> diff -u -p -r1.32 lock.c
> --- lock.c  15 Oct 2015 02:35:04 -  1.32
> +++ lock.c  16 Oct 2015 01:22:46 -
> @@ -148,6 +148,8 @@ main(int argc, char *argv[])
> strftime(date, sizeof(date), "%c", timp);
>
> if (!usemine) {
> +   if (pledge("stdio rpath wpath getpw tty", NULL) == -1)
> +   err(1, "pledge");
> /* get key and check again */
> if (!readpassphrase("Key: ", s, sizeof(s), RPP_ECHO_OFF)
> ||
> *s == '\0')
>

Another patch(1) pledge tweak

[trondd]

Is it safer to drop the recently added proc and exec pledges if the
arguments are not chosen which need them?

Index: lock.c
===
RCS file: /cvs/src/usr.bin/lock/lock.c,v
retrieving revision 1.32
diff -u -p -r1.32 lock.c
--- lock.c  15 Oct 2015 02:35:04 -  1.32
+++ lock.c  16 Oct 2015 01:22:46 -
@@ -148,6 +148,8 @@ main(int argc, char *argv[])
strftime(date, sizeof(date), "%c", timp);

if (!usemine) {
+   if (pledge("stdio rpath wpath getpw tty", NULL) == -1)
+   err(1, "pledge");
/* get key and check again */
if (!readpassphrase("Key: ", s, sizeof(s), RPP_ECHO_OFF) ||
*s == '\0')

Re: Changes to network memory allocation/reporting?

[trondd]

On Thu, September 3, 2015 6:35 am, Martin Pieuchot wrote:
>
> This is a side effect of the *8 pool change.  Diff below fixes it, ok?
>

I can confirm the patch applies and fixes the numbers.  Can't speak to the
accuracy of the math, though.

Tim.

Changes to network memory allocation/reporting?

[trondd]

I just noticed on my -current systems, memory reporting from netstat -m 
seems to show that memory is overcommited.


$ netstat -m
535 mbufs in use:
289 mbufs allocated to data
8 mbufs allocated to packet headers
238 mbufs allocated to socket names and addresses
171/288/6144 mbuf 2048 byte clusters in use (current/peak/max)
0/8/6144 mbuf 4096 byte clusters in use (current/peak/max)
0/8/6144 mbuf 8192 byte clusters in use (current/peak/max)
0/14/6146 mbuf 9216 byte clusters in use (current/peak/max)
0/10/6150 mbuf 12288 byte clusters in use (current/peak/max)
0/8/6144 mbuf 16384 byte clusters in use (current/peak/max)
0/8/6144 mbuf 65536 byte clusters in use (current/peak/max)
272 Kbytes allocated to network (174% in use)   <---
0 requests for memory denied
0 requests for memory delayed
0 calls to protocol drain routines

I see this on all -current systems, but not on 5.7-stable.  I didn't see 
this on my system that was last updated to -current July 26.
It does change the allocated Kbytes eventually but I saw it as high as 
280% and rarely below 100%.


Tim.


Dmesg for the example system (this one happens to be a VM running 
snapshots, also seen on hardware running from source):


OpenBSD 5.8-current (GENERIC.MP) #1267: Mon Aug 24 15:30:23 MDT 2015
dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 1056833536 (1007MB)
avail mem = 1020964864 (973MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.8 @ 0xf0c70 (10 entries)
bios0: vendor SeaBIOS version 
"rel-1.7.5.1-0-g8936dbb-20141113_115728-nilsson.home.kraxel.org" date 
04/01/2014

bios0: QEMU Standard PC (i440FX + PIIX, 1996)
acpi0 at bios0: rev 0
acpi0: sleep states S3 S4 S5
acpi0: tables DSDT FACP SSDT APIC HPET
acpi0: wakeup devices
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Common KVM processor, 2660.39 MHz
cpu0: 
FPU,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SSE3,CX16,x2APIC,HV,NXE,LONG,LAHF
cpu0: 64KB 64b/line 2-way I-cache, 64KB 64b/line 2-way D-cache, 512KB 
64b/line 16-way L2 cache

cpu0: ITLB 255 4KB entries direct-mapped, 255 4MB entries direct-mapped
cpu0: DTLB 255 4KB entries direct-mapped, 255 4MB entries direct-mapped
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
cpu0: apic clock running at 1000MHz
cpu1 at mainbus0: apid 1 (application processor)
cpu1: Common KVM processor, 2659.90 MHz
cpu1: 
FPU,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SSE3,CX16,x2APIC,HV,NXE,LONG,LAHF
cpu1: 64KB 64b/line 2-way I-cache, 64KB 64b/line 2-way D-cache, 512KB 
64b/line 16-way L2 cache

cpu1: ITLB 255 4KB entries direct-mapped, 255 4MB entries direct-mapped
cpu1: DTLB 255 4KB entries direct-mapped, 255 4MB entries direct-mapped
cpu1: smt 0, core 0, package 1
ioapic0 at mainbus0: apid 0 pa 0xfec0, version 11, 24 pins
acpihpet0 at acpi0: 1 Hz
acpiprt0 at acpi0: bus 0 (PCI0)
acpicpu0 at acpi0: C1(@1 halt!)
acpicpu1 at acpi0: C1(@1 halt!)
pvbus0 at mainbus0: KVM
pci0 at mainbus0 bus 0
pchb0 at pci0 dev 0 function 0 "Intel 82441FX" rev 0x02
pcib0 at pci0 dev 1 function 0 "Intel 82371SB ISA" rev 0x00
pciide0 at pci0 dev 1 function 1 "Intel 82371SB IDE" rev 0x00: DMA, 
channel 0 wired to compatibility, channel 1 wired to compatibility

pciide0: channel 0 disabled (no drives)
atapiscsi0 at pciide0 channel 1 drive 0
scsibus1 at atapiscsi0: 2 targets
cd0 at scsibus1 targ 0 lun 0:  ATAPI 5/cdrom 
removable

cd0(pciide0:1:0): using PIO mode 4, DMA mode 2
uhci0 at pci0 dev 1 function 2 "Intel 82371SB USB" rev 0x01: apic 0 int 
11
piixpm0 at pci0 dev 1 function 3 "Intel 82371AB Power" rev 0x03: apic 0 
int 9

iic0 at piixpm0
vga1 at pci0 dev 2 function 0 "Cirrus Logic CL-GD5446" rev 0x00
wsdisplay0 at vga1 mux 1: console (80x25, vt100 emulation)
wsdisplay0: screen 1-5 added (80x25, vt100 emulation)
virtio0 at pci0 dev 3 function 0 "Qumranet Virtio Memory" rev 0x00: 
Virtio Memory Balloon Device

viomb0 at virtio0
virtio0: apic 0 int 11
virtio1 at pci0 dev 10 function 0 "Qumranet Virtio Storage" rev 0x00: 
Virtio Block Device

vioblk0 at virtio1
scsibus2 at vioblk0: 2 targets
sd0 at scsibus2 targ 0 lun 0:  SCSI3 0/direct 
fixed

sd0: 32768MB, 512 bytes/sector, 67108864 sectors
virtio1: apic 0 int 10
em0 at pci0 dev 18 function 0 "Intel 82540EM" rev 0x03: apic 0 int 10, 
address 00:50:56:53:53:01
em1 at pci0 dev 19 function 0 "Intel 82540EM" rev 0x03: apic 0 int 11, 
address ee:b2:ba:5d:50:ab

isa0 at pcib0
isadma0 at isa0
fdc0 at isa0 port 0x3f0/6 irq 6 drq 2
fd0 at fdc0 drive 1: density unknown
pckbc0 at isa0 port 0x60/5 irq 1 irq 12
pckbd0 at pckbc0 (kbd slot)
wskbd0 at pckbd0: console keyboard, using wsdisplay0
pms0 at pckbc0 (aux slot)
wsmouse0 at pms0 mux 0
pcppi0 

Re: Using tame() in userland

[trondd]

On 2015-08-29 06:05, Steven McDonald wrote:


I think chmod fits in the cannot be tamed category. tame(2) says of
chmod(2) and friends:

Setuid/setgid bits do not work, nor can the user or group be
changed on a file.

This breaks 'chmod u+s'.


I ran into this when building Xenocara.

Also upon reboot (after building a tamed userland) mkdir is getting 
killed with syscall 15 right after clearing /tmp


Seems to be in Setup_X_sockets making a directory with a specific mode.

Tim.

Re: doas -s as a login shell

[trondd]

On Sun, August 9, 2015 11:24 pm, Philip Guenther wrote:

 If you're asking for sudo's -i option, sorry, we're out of option
 space in doas.  sudo is over there -- ports


 Philip Guenther


That's fair.  It's easy enough to work around.

Tim.

doas -s as a login shell

[trondd]

Was it a choice to not have 'doas -s' launch the shell as a login shell? 
 Doing so reloads profiles preserving aliases and prompt variables.


If a user is allowed to run the shell, the user can source the profile 
anyway, so this is just a convenience.  Is there a security risk I'm 
missing?


Tim.


Index: doas.c
===
RCS file: /cvs/src/usr.bin/doas/doas.c,v
retrieving revision 1.34
diff -u -p -r1.34 doas.c
--- doas.c  3 Aug 2015 15:31:05 -   1.34
+++ doas.c  10 Aug 2015 01:07:15 -
@@ -317,7 +317,7 @@ main(int argc, char **argv, char **envp)
const char *safepath = /bin:/sbin:/usr/bin:/usr/sbin:
/usr/local/bin:/usr/local/sbin;
const char *confpath = NULL;
-   char *shargv[] = { NULL, NULL };
+   char *shargv[] = { NULL, NULL , NULL };
char *sh;
const char *cmd;
char cmdline[LINE_MAX];
@@ -379,8 +379,9 @@ main(int argc, char **argv, char **envp)
shargv[0] = pw-pw_shell;
else
shargv[0] = sh;
+   shargv[1] = -l;
argv = shargv;
-   argc = 1;
+   argc = 2;
}

if (confpath) {

Re: [patch] Remove archaic manual sizing from dump(8)

[trondd]

On 2015-07-24 19:15, Michael McConville wrote:

I removed it because I wasn't sure whether the project liked placebo
legacy flags. Should I replace it?


For reference: When -U was removed it was removed.

http://marc.info/?t=14280712873r=1w=2

Tim.

[PATCH] Slight clarification in iked.conf(5)

[trondd]

The from and to values for iked seemed backwords to me and the man page 
example description didn't clarify it.


Make it a little more specific which end the example is talking about.

Tim.


Index: iked.conf.5
===
RCS file: /cvs/src/sbin/iked/iked.conf.5,v
retrieving revision 1.38
diff -u -p -r1.38 iked.conf.5
--- iked.conf.5 28 Feb 2015 21:51:57 -  1.38
+++ iked.conf.5 5 Jul 2015 00:17:49 -
@@ -831,7 +831,7 @@ or the non-standard Curve25519.
 Please note that the EC2N groups are considered as insecure and only
 provided for backwards compatibility.
 .Sh EXAMPLES
-The first example is intended for clients connecting to
+The first example is intended for a server with clients connecting to
 .Xr iked 8
 as an IPsec gateway, or IKEv2 responder, using mutual public key
 authentication and additional challenge-based EAP-MSCHAPv2 password

Re: httpd rewrites with Lua's pattern matching

[trondd]

On Sun, June 21, 2015 10:01 am, Reyk Floeter wrote:

 location match '^/page/(%d+)' {
 block return 302 /index.cgi?page=%1'
 }


So I was playing with the below config, then figured out it's not coded to
capture on 'server match'.  I want to redirect anything I get to use https
and add the FQDN without having to care which domain they are trying to
get to.

A simplified config:

server match '^(.*)$' {
listen on em0 port 80
block return 301 'https://%1.my.fqdn.com/'
}

Is there a way to do it?  Any reason to not capture on 'server match'?

Tim.

Re: httpd rewrites with Lua's pattern matching

[trondd]

On Tue, June 23, 2015 11:28 am, Reyk Floeter wrote:
 It is just not done yet.  As I said, we're improving the interface.
 But this doesn't affect the initial implementation itself.

 Reyk


Ok, thanks.  I think I have a 'location match' use case I can play with, too.

Tim.

Re: chroot: add -c to use login class with -u

[trondd]

On Mon, May 18, 2015 6:30 pm, Todd C. Miller wrote:
 Currently, chroot -u doesn't use the settings in /etc/login.conf.

Nice.  I was missing this option.


 Open questions:

 1) Should this just be default behavior with -u?  Are there cases
when you would *not* want to set the priority and resouce limits
based on the target user when one is specified?

When I was setting up an application in a chroot, my expectation was that
-u would apply that user's class.  When I noticed that it didn't, I looked
for a -c to set the class, which didn't exist.  So I would vote for the
class being set by default and -c to override it.  I believe this is what
su does...?

Tim.

[PATCH] relayd.conf man page confusion

[trondd]

Fix a contradiction in the relayd.conf man page in the Protocols, tls 
section.  The definition of TLS client and server is the opposite of 
what is stated in the forward and listen on descriptions, and the 
TLS Relays section.


Tim.

Index: relayd.conf.5
===
RCS file: /cvs/src/usr.sbin/relayd/relayd.conf.5,v
retrieving revision 1.161
diff -u -p -r1.161 relayd.conf.5
--- relayd.conf.5   9 Mar 2015 17:20:38 -   1.161
+++ relayd.conf.5   5 May 2015 00:21:24 -
@@ -905,12 +905,12 @@ are true:
 .Pp
 .Bl -bullet -compact -offset indent
 .It
-TLS client mode is enabled by the
+TLS server mode is enabled by the
 .Ic listen
 directive:
 .Ic listen on ... tls .
 .It
-TLS server mode and divert lookups are enabled by the
+TLS client mode and divert lookups are enabled by the
 .Ic forward
 directive:
 .Ic forward with tls to destination .

Mention pkg-readmes in FAQ

[trondd]

Seems like I see a lot of people who don't know about pkg-readmes and
it was a long time before I knew about them, too. Note their existence
in the package/ports FAQ.

Tim.

===
RCS file: /cvs/www/faq/faq15.html,v
retrieving revision 1.105
diff -u -p -r1.105 faq15.html
--- faq15.html  27 Feb 2015 09:16:26 -  1.105
+++ faq15.html  25 Apr 2015 15:18:01 -
@@ -370,6 +370,9 @@ scaled. Below is the relevant section fr
 /pre/blockquote

 p
+Additionally, some packages provide configuration and other information
+in a file located in i/usr/local/share/docs/pkg-readmes/i.
+p
 Let us now continue with an example of a package which has 
dependencies:


 blockquotepre

Re: libtls documentation

[trondd]

libtls?  What are you trying to do with it?  There are several
examples in the OpenBSD source code (relayd, ftp), but if you are
compiling for linux, maybe you aren't an obsd user with the code
handy.

I incorporated it into links+ (links2):
https://github.com/trondd555/links-plus/blob/master/connect.c

It's pretty easy to use, assuming it does what you would have done
with direct openssl calls.

Tim.

Re: Flag to set from address in mail(1)

[trondd]

trondd tro...@gmail.com wrote:

 Nathanael Rensen nathan...@list.polymorpheus.com wrote:
 
  On Wed, 07 Jan 2015 14:31:13 -0700, Todd C. Miller wrote:
  
   Here's a version that does not rely on non-standard sendmail -t
   behavior.
   
- todd
  
  Much better. Works well for me, thanks.
  
  Nathanael
  
 
 Awesome.  Working here, too.  Command line and interactive.
 
 Tim.

Was this going to be commited?

Tim.

Re: Flag to set from address in mail(1)

[trondd]

Nathanael Rensen nathan...@list.polymorpheus.com wrote:

 On Wed, 07 Jan 2015 14:31:13 -0700, Todd C. Miller wrote:
 
  Here's a version that does not rely on non-standard sendmail -t
  behavior.
  
   - todd
 
 Much better. Works well for me, thanks.
 
 Nathanael
 

Awesome.  Working here, too.  Command line and interactive.

Tim.

Re: Flag to set from address in mail(1)

[trondd]

Sorry, my fault. Try the diff below.

Nathanael


Yup, this works!  Now I can filter by sender in smptd to use the right 
SMTP server.

Thanks.
Tim.

Re: Flag to set from address in mail(1)

[trondd]

 
 I like this better. But I still want the set from=XXX in .mailrc and
 of course the manpage.
 

I would like to have this option.  The diff doesn't work, however. If 
you reply to a message, it messes up the header and replyall will crash.

 set from=tro...@gmail.com
 set
...
fromtro...@gmail.com
...
 r 294
From: Replyall
To: ...@x.xxx
Subject: Re: Hi

~x
 R 294
Bus error (core dumped)

Patch for mail(1) man page

[trondd]

Just a small patch to document the '=' command.

Tim.

Index: mail.1
===
RCS file: /cvs/src/usr.bin/mail/mail.1,v
retrieving revision 1.70
diff -u -p -r1.70 mail.1
--- mail.1  16 Dec 2014 18:37:17 -  1.70
+++ mail.1  3 Jan 2015 18:42:15 -
@@ -385,6 +385,8 @@ argument
 goes to the
 .Ar n Ns th
 previous message and prints it.
+.It Ic \=
+Prints the currently selected message number.
 .It Ic \?
 Prints a brief summary of commands.
 .It Ic \!

Re: httpd: redirect to https, or www, or non-www

[trondd]

On Tue, Dec 23, 2014 at 12:44 PM, Carlin Bingham c...@viennan.net wrote:

 ngninx and apache support url rewriting, letting you redirect from
 arbitrary urls with pattern matching. In my experience the primary
 uses for this are to redirect from http to https or to remove/add www
 in the hostname, so I thought it might be useful to have options making
 these specific uses possible and simple to do.

 This adds an enforce option, where enforce https redirects non-http
 to https, enforce www redirects from example.org to www.example.org 
 http://www.example.org,
 and enforce no www redirects from www.example.org 
 http://www.example.org to example.org.

 Would appreciate any feedback or suggestions.

 Thanks


As someone who admins a number of web services with redirects to enforce
SSL, and consistent hostnames:

I feel like enforce https is ok, as it's a protocol and the options are
pretty limited and require differences in configurations so you know what
you want to support.  However enforce www seems too specific.  What if I
have www1.server.com or ww2.server.com or anything.server.com?  I would
want a more general approach to hostname/domain name rewriting.

Tim.

Re: NTP

[trondd]

On Fri, Dec 19, 2014 at 8:22 PM, Theo de Raadt dera...@cvs.openbsd.org
wrote:

 whereas ntp.org's codebase is reportedly 100,000 lines of
 unknown or largely unused code


That made me curious.  Is it that bloated?

$ for i in $(find . -name *.[ch]); do cat $i  allcode; done
$ egrep -v '[:blank:]*/?\*' allcode | grep -v ^ *$ | wc -l
  192870

This is ntp-4.2.8  A rough estimate but close enough if we are comparing to
a know solution that is 5000.

Keep up the good work.
Tim.