Re: mg: extract exit status from pclose return value

[Scott Cheloha]

> On Aug 25, 2017, at 11:27 PM, Scott Cheloha  wrote:
> 
> compile_mode() currently just reports the value returned by
> pclose(3).  This is incorrect because pclose gives you
> whatever wait4(2) returned, which needs to be examined
> with the various W* macros in  to derive a proper
> exit status.
> 
> This patch checks how the popen'd process exited and chooses
> an exit status as the shell would.

bump and rebase

--
Scott Cheloha

Index: usr.bin/mg/grep.c
===
RCS file: /cvs/src/usr.bin/mg/grep.c,v
retrieving revision 1.45
diff -u -p -r1.45 grep.c
--- usr.bin/mg/grep.c   12 Oct 2017 14:12:00 -  1.45
+++ usr.bin/mg/grep.c   24 Oct 2017 02:59:15 -
@@ -2,8 +2,10 @@
 
 /* This file is in the public domain */
 
-#include 
 #include 
+#include 
+#include 
+
 #include 
 #include 
 #include 
@@ -180,7 +182,7 @@ compile_mode(const char *name, const cha
char*buf;
size_t   sz;
ssize_t  len;
-   int  ret, n;
+   int  n, ret, status;
char cwd[NFILEN], qcmd[NFILEN];
char timestr[NTIME];
time_t   t;
@@ -226,9 +228,10 @@ compile_mode(const char *name, const cha
t = time(NULL);
strftime(timestr, sizeof(timestr), "%a %b %e %T %Y", localtime());
addline(bp, "");
-   if (ret != 0)
-   addlinef(bp, "Command exited abnormally with code %d"
-   " at %s", ret, timestr);
+   status = WIFEXITED(ret) ? WEXITSTATUS(ret) : 128 + WTERMSIG(ret);
+   if (status != 0)
+   addlinef(bp, "Command exited abnormally with status %d"
+   " at %s", status, timestr);
else
addlinef(bp, "Command finished at %s", timestr);
 

dd: exit nonzero on receipt of SIGINT

[Scott Cheloha]

Hi,

Per this bit from POSIX on dd(1):

> For SIGINT, the dd utility shall interrupt its current processing,
> write status information to standard error, and exit as though
> terminated by SIGINT.  It shall take the standard action for all
> other signals; see [...].

I think we ought to exit nonzero when SIGINT'd.  FreeBSD, NetBSD,
Solaris, AIX, etc., all do, though they all have chosen their own
exit codes.

NetBSD actually goes so far as to restore the original signal handler
for SIGINT and then raises SIGINT again:

http://cvsweb.netbsd.org/bsdweb.cgi/src/bin/dd/misc.c.diff?r1=1.19=1.20_with_tag=MAIN=h

This is the most unique interpretation I found.  But my gut says that
this is a misreading of "exit as though terminated by SIGINT," and that
exiting with 128 + signo is closer to what was meant.

Thoughts?

--
Scott Cheloha

Index: bin/dd/misc.c
===
RCS file: /cvs/src/bin/dd/misc.c,v
retrieving revision 1.21
diff -u -p -r1.21 misc.c
--- bin/dd/misc.c   13 Aug 2017 02:06:42 -  1.21
+++ bin/dd/misc.c   24 Oct 2017 01:38:10 -
@@ -111,9 +111,8 @@ summaryx(int notused)
 }
 
 void
-terminate(int notused)
+terminate(int signo)
 {
-
summary();
-   _exit(0);
+   _exit(128 + signo);
 }

Re: refill msk(4) rx ring from a timeout when there's no mbufs

[David Gwynne]

theo pointed out the timeout should be cancelled when the interface
goes down. this adds a timeout_del in msk_stop.

some other drivers suffer this problem, ill have a look at them
once this is in.

On Mon, Oct 23, 2017 at 11:35:09AM +1000, David Gwynne wrote:
> if msk runs out of mbufs, the rx ring remains empty and there's
> nothing except an ifconfig down and up to get it going again.
> 
> this adds a timeout to refill the ring. it's largely copied from
> other drivers (vr in this case).
> 
> tests? ok?
> 
> Index: if_mskvar.h
> ===
> RCS file: /cvs/src/sys/dev/pci/if_mskvar.h,v
> retrieving revision 1.13
> diff -u -p -r1.13 if_mskvar.h
> --- if_mskvar.h   2 Jun 2017 01:47:36 -   1.13
> +++ if_mskvar.h   23 Oct 2017 01:32:04 -
> @@ -212,6 +212,7 @@ struct sk_if_softc {
>   int sk_pktlen;
>   int sk_link;
>   struct timeout  sk_tick_ch;
> + struct timeout  sk_tick_rx;
>   struct msk_chain_data   sk_cdata;
>   struct msk_ring_data*sk_rdata;
>   bus_dmamap_tsk_ring_map;
> Index: if_msk.c
> ===
> RCS file: /cvs/src/sys/dev/pci/if_msk.c,v
> retrieving revision 1.130
> diff -u -p -r1.130 if_msk.c
> --- if_msk.c  3 Oct 2017 15:37:58 -   1.130
> +++ if_msk.c  23 Oct 2017 01:32:04 -
> @@ -148,7 +148,7 @@ void msk_ifmedia_sts(struct ifnet *, str
>  int msk_newbuf(struct sk_if_softc *);
>  int msk_init_rx_ring(struct sk_if_softc *);
>  int msk_init_tx_ring(struct sk_if_softc *);
> -void msk_fill_rx_ring(struct sk_if_softc *);
> +void msk_fill_rx_ring(struct sk_if_softc *, int);
>  
>  int msk_miibus_readreg(struct device *, int, int);
>  void msk_miibus_writereg(struct device *, int, int, int);
> @@ -156,6 +156,7 @@ void msk_miibus_statchg(struct device *)
>  
>  void msk_iff(struct sk_if_softc *);
>  void msk_tick(void *);
> +void msk_fill_rx_tick(void *);
>  
>  #ifdef MSK_DEBUG
>  #define DPRINTF(x)   if (mskdebug) printf x
> @@ -428,7 +429,7 @@ msk_init_rx_ring(struct sk_if_softc *sc_
>   /* two ring entries per packet, so the effective ring size is halved */
>   if_rxr_init(_if->sk_cdata.sk_rx_ring, 2, MSK_RX_RING_CNT/2);
>  
> - msk_fill_rx_ring(sc_if);
> + msk_fill_rx_ring(sc_if, 0);
>   return (0);
>  }
>  
> @@ -1027,6 +1028,7 @@ msk_attach(struct device *parent, struct
>   ifmedia_set(_if->sk_mii.mii_media, IFM_ETHER|IFM_AUTO);
>  
>   timeout_set(_if->sk_tick_ch, msk_tick, sc_if);
> + timeout_set(_if->sk_tick_rx, msk_fill_rx_tick, sc_if);
>  
>   /*
>* Call MI attach routines.
> @@ -1779,7 +1781,7 @@ msk_txeof(struct sk_if_softc *sc_if)
>  }
>  
>  void
> -msk_fill_rx_ring(struct sk_if_softc *sc_if)
> +msk_fill_rx_ring(struct sk_if_softc *sc_if, int ticks)
>  {
>   u_int slots, used;
>  
> @@ -1792,6 +1794,20 @@ msk_fill_rx_ring(struct sk_if_softc *sc_
>   slots -= used;
>   }
>   if_rxr_put(_if->sk_cdata.sk_rx_ring, slots);
> + if (if_rxr_inuse(_if->sk_cdata.sk_rx_ring) == 0)
> + timeout_add(_if->sk_tick_rx, ticks);
> +}
> +
> +void
> +msk_fill_rx_tick(void *xsc_if)
> +{
> + struct sk_if_softc *sc_if = xsc_if;
> + int s;
> +
> + s = splnet();
> + if (if_rxr_inuse(_if->sk_cdata.sk_rx_ring) == 0)
> + msk_fill_rx_ring(sc_if, 1);
> + splx(s);
>  }
>  
>  void
> @@ -1902,12 +1918,12 @@ msk_intr(void *xsc)
>   CSR_WRITE_4(sc, SK_Y2_ICR, 2);
>  
>   if (rx[0]) {
> - msk_fill_rx_ring(sc_if0);
> + msk_fill_rx_ring(sc_if0, 0);
>   SK_IF_WRITE_2(sc_if0, 0, SK_RXQ1_Y2_PREF_PUTIDX,
>   sc_if0->sk_cdata.sk_rx_prod);
>   }
>   if (rx[1]) {
> - msk_fill_rx_ring(sc_if1);
> + msk_fill_rx_ring(sc_if1, 0);
>   SK_IF_WRITE_2(sc_if1, 0, SK_RXQ1_Y2_PREF_PUTIDX,
>   sc_if1->sk_cdata.sk_rx_prod);
>   }

Re: kqueue_scan() in parallel

[Alexander Bluhm]

On Wed, Oct 18, 2017 at 11:22:01AM +0200, Martin Pieuchot wrote:
> Diff below is the last version of my kqueue diff to allow grabbing the
> solock() and possibly sleeping, inside kqueue_scan().

I wonder why you don't call knote_release() when calling knote_drop().
Another thread could sleep in knote_acquire() and never get a
wakeup.  I think there should be a call to knote_release() from
knote_drop() just before freeing it.  Then other threads wake up
and can continue.

Why is knote_release() not a void function?

I did run all regress tests with your diff, everything passes.
Btw, I have added a link to my regress page where you can see the
diffs in /usr/src when I did not run the tests with a snapshot.
http://bluhm.genua.de/regress/results/regress.html

bluhm

Re: tftpd(8): diff for ip path rewrite

[Jan Klemkow]

On Sun, Oct 22, 2017 at 09:32:54PM +, Jeremie Courreges-Anglas wrote:
> On Sat, Oct 21 2017, Jan Klemkow  wrote:
> > On Fri, Oct 20, 2017 at 12:04:41PM +, Jeremie Courreges-Anglas wrote:
> >> On Fri, Oct 20 2017, Sebastien Marie  wrote:
> >> > On Thu, Oct 19, 2017 at 08:58:12PM +0200, Jan Klemkow wrote:
> >> >> +   char nfilename[PATH_MAX];
> >> >> +
> >> >> +   snprintf(nfilename, sizeof nfilename, "%s/%s",
> >> >> +   getip(>ss), filename);
> >> >
> >> > - filename has PATH_MAX length
> >> > - getip(>ss) could have NI_MAXHOST length
> >> 
> >> INET6_ADDRSTRLEN since getip() calls getnameinfo(NI_NUMERICHOST), but
> >> your point stands.
> >> 
> >> > so nfilename could be larger than PATH_MAX (sizeof nfilename).
> >> >
> >> > I assume the return of snprintf() need to be checked. if truncation
> >> > occured, a warning should be issued and nfilename discarded (just
> >> > calling tftp_open(client, filename)) ?
> >> 
> >> I think we should refuse the request if truncated.
> >
> > done
> >  
> >> >> +   if (access(nfilename, R_OK) == 0)
> >> >> +   tftp_open(client, nfilename);
> >> >> +   else
> >> >> +   tftp_open(client, filename);
> >> >> +   }
> >> 
> >> Here we look up the same file in both the client-specific subdirectory
> >> and the default directory.  Should we instead look only in the
> >> client-specific directory if the latter exist?
> >
> > Common files should be found in the default directory.  But, host
> > specific files could be overwritten if they exist in the subdirectory.
> 
> I think it would be better to perform those access tests in
> validate_access(); logic in a single place, and a less quirky handling
> of SEEDPATH.  Also the test done should probably depend on the type
> (read, write) of the request.  Retrying with the default directory may
> make sense in read mode, but maybe not in write (and -c, create) mode?
> 
> The updated diff below implements such semantics, but in
> validate_access().  While here,
> - improve diagnostic if both -i and -r are given; usage() doesn't show
>   the conflict
> - also test snprintf return value against -1, as spotted by semarie@
> 
> Maybe we should add a mention in the manpage that the client can
> "escape" its client-specific directory?  (ie /../192.0.2.1/file)
> 
> The logic is more complicated but I hope it's for good.

I successfully testes jca's diff in my setup and add two lines about
directory escaping to the manpage.

Thanks,
Jan

Index: tftpd.8
===
RCS file: /mount/openbsd/cvs/src/usr.sbin/tftpd/tftpd.8,v
retrieving revision 1.5
diff -u -p -r1.5 tftpd.8
--- tftpd.8 18 Jul 2015 05:32:56 -  1.5
+++ tftpd.8 23 Oct 2017 18:03:41 -
@@ -37,7 +37,7 @@
 .Nd DARPA Trivial File Transfer Protocol daemon
 .Sh SYNOPSIS
 .Nm tftpd
-.Op Fl 46cdv
+.Op Fl 46cdiv
 .Op Fl l Ar address
 .Op Fl p Ar port
 .Op Fl r Ar socket
@@ -100,6 +100,19 @@ If this option is specified,
 .Nm
 will run in the foreground and log
 the client IP, type of request, and filename to stderr.
+.It Fl i
+.Nm
+tries to rewrite the requested filename with a prefix of
+the client's IP address.
+If the rewritten path exists
+.Nm
+will serve this file.
+If not, it will serve the original filename.
+This option is no security feature.
+Clients are able to escape thier specific directory by
+paths like /../ or by changing their IP address.
+This option can not be combined with
+.Fl r .
 .It Fl l Ar address
 Listen on the specified address.
 By default
@@ -126,6 +139,8 @@ before the TFTP request will continue.
 By default
 .Nm
 does not use filename rewriting.
+This option can not be combined with
+.Fl i .
 .It Fl v
 Log the client IP, type of request, and filename.
 .It Ar directory
Index: tftpd.c
===
RCS file: /mount/openbsd/cvs/src/usr.sbin/tftpd/tftpd.c,v
retrieving revision 1.39
diff -u -p -r1.39 tftpd.c
--- tftpd.c 26 May 2017 17:38:46 -  1.39
+++ tftpd.c 23 Oct 2017 17:56:03 -
@@ -282,7 +282,7 @@ __dead void
 usage(void)
 {
extern char *__progname;
-   fprintf(stderr, "usage: %s [-46cdv] [-l address] [-p port] [-r socket]"
+   fprintf(stderr, "usage: %s [-46cdiv] [-l address] [-p port] [-r socket]"
" directory\n", __progname);
exit(1);
 }
@@ -290,6 +290,7 @@ usage(void)
 int  cancreate = 0;
 int  verbose = 0;
 int  debug = 0;
+int  iflag = 0;
 
 int
 main(int argc, char *argv[])
@@ -307,7 +308,7 @@ main(int argc, char *argv[])
int family = AF_UNSPEC;
int devnull = -1;
 
-   while ((c = getopt(argc, argv, "46cdl:p:r:v")) != -1) {
+   while ((c = getopt(argc, argv, "46cdil:p:r:v")) != -1) {
switch (c) {
case '4':
   

Re: [PATCH] etc/ksh.kshrc - unify command substitution

[Alexander Hall]

I'm OK with this.

/Alexander


On October 23, 2017 3:29:57 PM GMT+02:00, Raf Czlonka  
wrote:
>What say you?
>
>On Tue, Aug 29, 2017 at 08:44:43PM BST, Raf Czlonka wrote:
>> Ping.
>> 
>> Anyone?
>> 
>> On Sun, Jul 16, 2017 at 01:43:32PM BST, Raf Czlonka wrote:
>> > Hi all,
>> > 
>> > Further simplification - 'ps | grep' can be replaced by pgrep(1)
>> > and if-then-fi by &&.
>> > 
>> > > While there:
>> > > 
>> > > - remove ':' (null utility) from the very first line of the file
>-
>> > >   I *do* understand what it does but it doesn't seem like it's
>needed
>> > >   at all, unless I'm missing something (as is the case with some
>idioms)
>> > > [...]
>> > > 
>> > 
>> > As it transpired, this does indeed seem to be an old idiom denoting
>> > a Bourne shell script.
>> > 
>> > To quote rpe@: "I guess it's fine to remove the : line in 2017."
>> > 
>> > I agree.
>> > 
>> > Thanks again to Robert for all the feedback and suggestions.
>> > 
>> > Regards,
>> > 
>> > Raf
>> > 
>> > Index: etc/ksh.kshrc
>> > ===
>> > RCS file: /cvs/src/etc/ksh.kshrc,v
>> > retrieving revision 1.28
>> > diff -u -p -r1.28 ksh.kshrc
>> > --- etc/ksh.kshrc  15 Jul 2017 07:11:42 -  1.28
>> > +++ etc/ksh.kshrc  16 Jul 2017 11:49:55 -
>> > @@ -1,4 +1,3 @@
>> > -:
>> >  # $OpenBSD: ksh.kshrc,v 1.28 2017/07/15 07:11:42 tb Exp $
>> >  #
>> >  # NAME:
>> > @@ -74,9 +73,7 @@ case "$-" in
>> >xterm*)
>> >ILS='\033]1;'; ILE='\007'
>> >WLS='\033]2;'; WLE='\007'
>> > -  if ps -p $PPID -o command | grep -q telnet; then
>> > -  export TERM=xterms
>> > -  fi
>> > +  pgrep -qxs $PPID telnet && export TERM=xterms
>> >;;
>> >*)  ;;
>> >esac

Re: close cron sockets in child processes

[Jeremie Courreges-Anglas]

On Mon, Oct 23 2017, "Todd C. Miller"  wrote:
> On Mon, 23 Oct 2017 17:27:15 +0200, Jeremie Courreges-Anglas wrote:
>
>> Here it is.  Liberal use of O_CLOEXEC except for poke_daemon() which is
>> more obvious (but I can add it there too if you prefer).  cronSock
>> already uses SOCK_CLOEXEC.
>
> You can't set O_CLOEXEC for the fd in atrun.c as that will become
> the executed command's stdin.  The others are OK millert@

Gaah, yes indeed.  I should have used my existing diff instead of
re-doing it from scratch.  I'll check everything again before
committing, thanks.

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: close cron sockets in child processes

[Jeremie Courreges-Anglas]

On Mon, Oct 23 2017, Jeremie Courreges-Anglas  wrote:
> On Mon, Oct 23 2017, "Todd C. Miller"  wrote:
>> On Mon, 23 Oct 2017 09:05:48 +0200, Jeremie Courreges-Anglas wrote:
>>
>>> That doesn't work.  When here, we hold cronSock and dfd (should be 3 and
>>> 4).  We need dfd to get fd (should be 5), thus we can't just use
>>> "closefrom(3)".  The straightest way IMO is to close what we should
>>> close (including dfd), but this means making cronSock a global.
>>
>> OK millert@
>>
>>> I would also propose sprinkling more O_CLOEXEC magic but that can be in
>>> another diff.
>>
>> Sure.
>
> Here it is.  Liberal use of O_CLOEXEC except for poke_daemon() which is
> more obvious (but I can add it there too if you prefer).  cronSock
> already uses SOCK_CLOEXEC.
>
> ok?

Note: there may be others places where we could use close-on-exec mode
"e" for fopen, not sure about this yet.  Feel free to beat me to it. :)

>
> Index: atrun.c
> ===
> RCS file: /cvs/src/usr.sbin/cron/atrun.c,v
> retrieving revision 1.47
> diff -u -p -r1.47 atrun.c
> --- atrun.c   23 Oct 2017 15:15:22 -  1.47
> +++ atrun.c   23 Oct 2017 15:26:45 -
> @@ -83,7 +83,8 @@ scan_atjobs(at_db **db, struct timespec 
>   struct dirent *file;
>   struct stat sb;
>  
> - if ((dfd = open(_PATH_AT_SPOOL, O_RDONLY|O_DIRECTORY)) == -1) {
> + dfd = open(_PATH_AT_SPOOL, O_RDONLY|O_DIRECTORY|O_CLOEXEC);
> + if (dfd == -1) {
>   syslog(LOG_ERR, "(CRON) OPEN FAILED (%s)", _PATH_AT_SPOOL);
>   return (0);
>   }
> @@ -175,7 +176,8 @@ atrun(at_db *db, double batch_maxload, t
>   if (db == NULL)
>   return;
>  
> - if ((dfd = open(_PATH_AT_SPOOL, O_RDONLY|O_DIRECTORY)) == -1) {
> + dfd = open(_PATH_AT_SPOOL, O_RDONLY|O_DIRECTORY|O_CLOEXEC);
> + if (dfd == -1) {
>   syslog(LOG_ERR, "(CRON) OPEN FAILED (%s)", _PATH_AT_SPOOL);
>   return;
>   }
> @@ -262,7 +264,8 @@ run_job(const atjob *job, int dfd, const
>   char *nargv[2], *nenvp[1];
>  
>   /* Open the file and unlink it so we don't try running it again. */
> - if ((fd = openat(dfd, atfile, O_RDONLY|O_NONBLOCK|O_NOFOLLOW, 0)) < 0) {
> + fd = openat(dfd, atfile, O_RDONLY|O_NONBLOCK|O_NOFOLLOW|O_CLOEXEC, 0);
> + if (fd == -1) {
>   syslog(LOG_ERR, "(CRON) CAN'T OPEN (%s)", atfile);
>   return;
>   }
> Index: database.c
> ===
> RCS file: /cvs/src/usr.sbin/cron/database.c,v
> retrieving revision 1.35
> diff -u -p -r1.35 database.c
> --- database.c7 Jun 2017 23:36:43 -   1.35
> +++ database.c23 Oct 2017 15:26:45 -
> @@ -182,7 +182,8 @@ process_crontab(int dfd, const char *una
>   goto next_crontab;
>   }
>  
> - crontab_fd = openat(dfd, fname, O_RDONLY|O_NONBLOCK|O_NOFOLLOW);
> + crontab_fd = openat(dfd, fname,
> + O_RDONLY|O_NONBLOCK|O_NOFOLLOW|O_CLOEXEC);
>   if (crontab_fd < 0) {
>   /* crontab not accessible?
>*/

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: close cron sockets in child processes

[Todd C. Miller]

On Mon, 23 Oct 2017 17:27:15 +0200, Jeremie Courreges-Anglas wrote:

> Here it is.  Liberal use of O_CLOEXEC except for poke_daemon() which is
> more obvious (but I can add it there too if you prefer).  cronSock
> already uses SOCK_CLOEXEC.

You can't set O_CLOEXEC for the fd in atrun.c as that will become
the executed command's stdin.  The others are OK millert@

 - todd

Re: close cron sockets in child processes

[Jeremie Courreges-Anglas]

On Mon, Oct 23 2017, "Todd C. Miller"  wrote:
> On Mon, 23 Oct 2017 09:05:48 +0200, Jeremie Courreges-Anglas wrote:
>
>> That doesn't work.  When here, we hold cronSock and dfd (should be 3 and
>> 4).  We need dfd to get fd (should be 5), thus we can't just use
>> "closefrom(3)".  The straightest way IMO is to close what we should
>> close (including dfd), but this means making cronSock a global.
>
> OK millert@
>
>> I would also propose sprinkling more O_CLOEXEC magic but that can be in
>> another diff.
>
> Sure.

Here it is.  Liberal use of O_CLOEXEC except for poke_daemon() which is
more obvious (but I can add it there too if you prefer).  cronSock
already uses SOCK_CLOEXEC.

ok?


Index: atrun.c
===
RCS file: /cvs/src/usr.sbin/cron/atrun.c,v
retrieving revision 1.47
diff -u -p -r1.47 atrun.c
--- atrun.c 23 Oct 2017 15:15:22 -  1.47
+++ atrun.c 23 Oct 2017 15:26:45 -
@@ -83,7 +83,8 @@ scan_atjobs(at_db **db, struct timespec 
struct dirent *file;
struct stat sb;
 
-   if ((dfd = open(_PATH_AT_SPOOL, O_RDONLY|O_DIRECTORY)) == -1) {
+   dfd = open(_PATH_AT_SPOOL, O_RDONLY|O_DIRECTORY|O_CLOEXEC);
+   if (dfd == -1) {
syslog(LOG_ERR, "(CRON) OPEN FAILED (%s)", _PATH_AT_SPOOL);
return (0);
}
@@ -175,7 +176,8 @@ atrun(at_db *db, double batch_maxload, t
if (db == NULL)
return;
 
-   if ((dfd = open(_PATH_AT_SPOOL, O_RDONLY|O_DIRECTORY)) == -1) {
+   dfd = open(_PATH_AT_SPOOL, O_RDONLY|O_DIRECTORY|O_CLOEXEC);
+   if (dfd == -1) {
syslog(LOG_ERR, "(CRON) OPEN FAILED (%s)", _PATH_AT_SPOOL);
return;
}
@@ -262,7 +264,8 @@ run_job(const atjob *job, int dfd, const
char *nargv[2], *nenvp[1];
 
/* Open the file and unlink it so we don't try running it again. */
-   if ((fd = openat(dfd, atfile, O_RDONLY|O_NONBLOCK|O_NOFOLLOW, 0)) < 0) {
+   fd = openat(dfd, atfile, O_RDONLY|O_NONBLOCK|O_NOFOLLOW|O_CLOEXEC, 0);
+   if (fd == -1) {
syslog(LOG_ERR, "(CRON) CAN'T OPEN (%s)", atfile);
return;
}
Index: database.c
===
RCS file: /cvs/src/usr.sbin/cron/database.c,v
retrieving revision 1.35
diff -u -p -r1.35 database.c
--- database.c  7 Jun 2017 23:36:43 -   1.35
+++ database.c  23 Oct 2017 15:26:45 -
@@ -182,7 +182,8 @@ process_crontab(int dfd, const char *una
goto next_crontab;
}
 
-   crontab_fd = openat(dfd, fname, O_RDONLY|O_NONBLOCK|O_NOFOLLOW);
+   crontab_fd = openat(dfd, fname,
+   O_RDONLY|O_NONBLOCK|O_NOFOLLOW|O_CLOEXEC);
if (crontab_fd < 0) {
/* crontab not accessible?
 */

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE

Re: [PATCH] etc/ksh.kshrc - unify command substitution

[Raf Czlonka]

What say you?

On Tue, Aug 29, 2017 at 08:44:43PM BST, Raf Czlonka wrote:
> Ping.
> 
> Anyone?
> 
> On Sun, Jul 16, 2017 at 01:43:32PM BST, Raf Czlonka wrote:
> > Hi all,
> > 
> > Further simplification - 'ps | grep' can be replaced by pgrep(1)
> > and if-then-fi by &&.
> > 
> > > While there:
> > > 
> > > - remove ':' (null utility) from the very first line of the file -
> > >   I *do* understand what it does but it doesn't seem like it's needed
> > >   at all, unless I'm missing something (as is the case with some idioms)
> > > [...]
> > > 
> > 
> > As it transpired, this does indeed seem to be an old idiom denoting
> > a Bourne shell script.
> > 
> > To quote rpe@: "I guess it's fine to remove the : line in 2017."
> > 
> > I agree.
> > 
> > Thanks again to Robert for all the feedback and suggestions.
> > 
> > Regards,
> > 
> > Raf
> > 
> > Index: etc/ksh.kshrc
> > ===
> > RCS file: /cvs/src/etc/ksh.kshrc,v
> > retrieving revision 1.28
> > diff -u -p -r1.28 ksh.kshrc
> > --- etc/ksh.kshrc   15 Jul 2017 07:11:42 -  1.28
> > +++ etc/ksh.kshrc   16 Jul 2017 11:49:55 -
> > @@ -1,4 +1,3 @@
> > -:
> >  #  $OpenBSD: ksh.kshrc,v 1.28 2017/07/15 07:11:42 tb Exp $
> >  #
> >  # NAME:
> > @@ -74,9 +73,7 @@ case "$-" in
> > xterm*)
> > ILS='\033]1;'; ILE='\007'
> > WLS='\033]2;'; WLE='\007'
> > -   if ps -p $PPID -o command | grep -q telnet; then
> > -   export TERM=xterms
> > -   fi
> > +   pgrep -qxs $PPID telnet && export TERM=xterms
> > ;;
> > *)  ;;
> > esac

save_errno for SHA256File()

[Peter J. Philipp]

Hi,

I have a program that constantly stalls on reading /etc/spwd.db with 
SHA256File() (from sha2.h).  Here is the program flow:

>
sha256file: Operation not permitted
on file: /etc/spwd.db
2f6574632f737077642e6462
^C
beta$ stat /etc/spwd.db
1024 78977 -rw-r- 1 root _shadow 327856 57344 "Oct 23 14:58:27 2017" "Oct 
17 13:54:38 2017" "Oct 17 13:54:38 2017" 16384 112 0 /etc/spwd.db
beta$ id
uid=0(root) gid=0(wheel) groups=0(wheel), 2(kmem), 3(sys), 4(tty), 5(operator), 
20(staff), 31(guest)
<

I don't know what's up but my research led me to create a patch for this 
function, it basically completes there what was started before because
close() can rewrite errno afaik.

If anyone has a hint as to why my SHA256File() returns with NULL and sets errno
to 0 that would really interest me.  My program does no setuid or seteuid at
all!  If you'd like to see the program, I can provide that but I wanted
to put preference to the patch here.

Patch (against 6.2) below signature.

-peter

Index: helper.c
===
RCS file: /cvs/src/lib/libc/hash/helper.c,v
retrieving revision 1.16
diff -u -p -u -r1.16 helper.c
--- helper.c21 Sep 2016 04:38:57 -  1.16
+++ helper.c23 Oct 2017 13:06:46 -
@@ -71,13 +71,17 @@ HASHFileChunk(const char *filename, char
return (NULL);
if (len == 0) {
if (fstat(fd, ) == -1) {
+   save_errno = errno;
close(fd);
+   errno = save_errno;
return (NULL);
}
len = sb.st_size;
}
if (off > 0 && lseek(fd, off, SEEK_SET) < 0) {
+   save_errno = errno;
close(fd);
+   errno = save_errno;
return (NULL);
}
 

Re: close cron sockets in child processes

[Todd C. Miller]

On Mon, 23 Oct 2017 09:05:48 +0200, Jeremie Courreges-Anglas wrote:

> That doesn't work.  When here, we hold cronSock and dfd (should be 3 and
> 4).  We need dfd to get fd (should be 5), thus we can't just use
> "closefrom(3)".  The straightest way IMO is to close what we should
> close (including dfd), but this means making cronSock a global.

OK millert@

> I would also propose sprinkling more O_CLOEXEC magic but that can be in
> another diff.

Sure.

 - todd

Re: syslogd file system full

[Alexander Bluhm]

Anyone?

On Thu, Oct 05, 2017 at 07:19:29PM +0200, Alexander Bluhm wrote:
> Hi,
> 
> When /var/log/ is full, syslogd(8) stops writing to these files.
> It does this permanently so cleaning /var without SIGHUP to syslogd
> does not help.  Better retry, write an error message to other log
> hosts, and write a summary after it works again.
> 
> syslogd[52967]: write to file "/mnt/regress-syslogd/file.log": No space left 
> on device
> syslogd[52967]: dropped 36 messages to file "/mnt/regress-syslogd/file.log"
> 
> I am not sure whether we should also retry logging to a file if
> errno is EIO.  Can it ever recover?
> 
> ok?
> 
> bluhm
> 
> Index: usr.sbin/syslogd/syslogd.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/usr.sbin/syslogd/syslogd.c,v
> retrieving revision 1.252
> diff -u -p -r1.252 syslogd.c
> --- usr.sbin/syslogd/syslogd.c5 Oct 2017 16:34:59 -   1.252
> +++ usr.sbin/syslogd/syslogd.c5 Oct 2017 16:39:42 -
> @@ -158,7 +158,6 @@ struct filed {
>   struct tls  *f_ctx;
>   char*f_host;
>   int  f_reconnectwait;
> - int  f_dropped;
>   } f_forw;   /* forwarding address */
>   charf_fname[PATH_MAX];
>   struct {
> @@ -177,6 +176,7 @@ struct filed {
>   int f_prevcount;/* repetition cnt of prevline */
>   unsigned int f_repeatcount; /* number of "repeated" msgs */
>   int f_quick;/* abort when matched */
> + int f_dropped;  /* warn, dropped message */
>   time_t  f_lasterrtime;  /* last error was reported */
>  };
>  
> @@ -242,6 +242,7 @@ const char *ClientCertfile = NULL;
>  const char *ClientKeyfile = NULL;
>  const char *ServerCAfile = NULL;
>  int  tcpbuf_dropped = 0; /* count messages dropped from TCP or TLS */
> +int  file_dropped = 0;   /* messages dropped due to file system full */
>  int  init_dropped = 0;   /* messages dropped during initialization */
>  
>  #define CTL_READING_CMD  1
> @@ -1388,11 +1389,11 @@ tcp_writecb(struct bufferevent *bufev, v
>   log_debug("loghost \"%s\" successful write", f->f_un.f_forw.f_loghost);
>   f->f_un.f_forw.f_reconnectwait = 0;
>  
> - if (f->f_un.f_forw.f_dropped > 0 &&
> + if (f->f_dropped > 0 &&
>   EVBUFFER_LENGTH(f->f_un.f_forw.f_bufev->output) < MAX_TCPBUF) {
>   snprintf(ebuf, sizeof(ebuf), "to loghost \"%s\"",
>   f->f_un.f_forw.f_loghost);
> - dropped_warn(>f_un.f_forw.f_dropped, ebuf);
> + dropped_warn(>f_dropped, ebuf);
>   }
>  }
>  
> @@ -1443,7 +1444,7 @@ tcp_errorcb(struct bufferevent *bufev, s
>   evbuffer_drain(bufev->output, -1);
>   log_debug("loghost \"%s\" dropped partial message",
>   f->f_un.f_forw.f_loghost);
> - f->f_un.f_forw.f_dropped++;
> + f->f_dropped++;
>   }
>  
>   tcp_connect_retry(bufev, f);
> @@ -1894,6 +1895,7 @@ fprintlog(struct filed *f, int flags, ch
>   struct iovec *v;
>   int l, retryonce;
>   char line[LOG_MAXLINE + 1], repbuf[80], greetings[500];
> + char ebuf[ERRBUFSIZE];
>  
>   v = iov;
>   if (f->f_type == F_WALL) {
> @@ -2000,7 +2002,7 @@ fprintlog(struct filed *f, int flags, ch
>   if (EVBUFFER_LENGTH(f->f_un.f_forw.f_bufev->output) >=
>   MAX_TCPBUF) {
>   log_debug(" (dropped)");
> - f->f_un.f_forw.f_dropped++;
> + f->f_dropped++;
>   break;
>   }
>   /*
> @@ -2016,7 +2018,7 @@ fprintlog(struct filed *f, int flags, ch
>   IncludeHostname ? " " : "");
>   if (l < 0) {
>   log_debug(" (dropped snprintf)");
> - f->f_un.f_forw.f_dropped++;
> + f->f_dropped++;
>   break;
>   }
>   l = evbuffer_add_printf(f->f_un.f_forw.f_bufev->output,
> @@ -2028,7 +2030,7 @@ fprintlog(struct filed *f, int flags, ch
>   (char *)iov[4].iov_base);
>   if (l < 0) {
>   log_debug(" (dropped evbuffer_add_printf)");
> - f->f_un.f_forw.f_dropped++;
> + f->f_dropped++;
>   break;
>   }
>   bufferevent_enable(f->f_un.f_forw.f_bufev, EV_WRITE);
> @@ -2058,11 +2060,23 @@ fprintlog(struct filed *f, int flags, ch
>   if (writev(f->f_file, iov, 6) < 0) {
>   int e = errno;
>  
> + /* allow to recover from file system full */
> + if ((e == EIO || e == ENOSPC) && 

Re: umb(4): recover after temporary loss of packet service

[Stefan Sperling]

On Mon, Oct 23, 2017 at 11:25:37AM +0200, Gerhard Roth wrote:
> In case we have a temporary loss of connection in umb(4), the USB xfers
> may time-out. umb_txeof() should always check whether there are further
> mbufs in the if_snd queue; not only after successful transmits.
> 
> Also, aborting the xfer in case the watchdog timer triggers, can help
> to resume from hanging transmits.
> 
> 
> To test this fix, flood ping via umb(4) and then move somewhere where
> there's no reception. Ping will start spitting out "No buffer space
> available" errors after some time. Now go back to where there's
> reception. Without this fix, the ping errors will continue (because
> umb_start() isn't called anymore). With this fix, they'll go away.
> 
> 
> Gerhard

ok stsp@

This should help a lot on trains :)
 
> Index: if_umb.c
> ===
> RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 if_umb.c
> --- if_umb.c  20 Oct 2017 09:35:09 -  1.16
> +++ if_umb.c  23 Oct 2017 08:28:21 -
> @@ -896,7 +896,7 @@ umb_watchdog(struct ifnet *ifp)
>  
>   ifp->if_oerrors++;
>   printf("%s: watchdog timeout\n", DEVNAM(sc));
> - /* XXX FIXME: re-initialize device */
> + usbd_abort_pipe(sc->sc_tx_pipe);
>   return;
>  }
>  
> @@ -1845,10 +1845,9 @@ umb_txeof(struct usbd_xfer *xfer, void *
>   if (status == USBD_STALLED)
>   usbd_clear_endpoint_stall_async(sc->sc_tx_pipe);
>   }
> - } else {
> - if (IFQ_IS_EMPTY(>if_snd) == 0)
> - umb_start(ifp);
>   }
> + if (IFQ_IS_EMPTY(>if_snd) == 0)
> + umb_start(ifp);
>  
>   splx(s);
>  }
> 
> 

umb(4): recover after temporary loss of packet service

[Gerhard Roth]

In case we have a temporary loss of connection in umb(4), the USB xfers
may time-out. umb_txeof() should always check whether there are further
mbufs in the if_snd queue; not only after successful transmits.

Also, aborting the xfer in case the watchdog timer triggers, can help
to resume from hanging transmits.


To test this fix, flood ping via umb(4) and then move somewhere where
there's no reception. Ping will start spitting out "No buffer space
available" errors after some time. Now go back to where there's
reception. Without this fix, the ping errors will continue (because
umb_start() isn't called anymore). With this fix, they'll go away.


Gerhard


Index: if_umb.c
===
RCS file: /cvs/src/sys/dev/usb/if_umb.c,v
retrieving revision 1.16
diff -u -p -r1.16 if_umb.c
--- if_umb.c20 Oct 2017 09:35:09 -  1.16
+++ if_umb.c23 Oct 2017 08:28:21 -
@@ -896,7 +896,7 @@ umb_watchdog(struct ifnet *ifp)
 
ifp->if_oerrors++;
printf("%s: watchdog timeout\n", DEVNAM(sc));
-   /* XXX FIXME: re-initialize device */
+   usbd_abort_pipe(sc->sc_tx_pipe);
return;
 }
 
@@ -1845,10 +1845,9 @@ umb_txeof(struct usbd_xfer *xfer, void *
if (status == USBD_STALLED)
usbd_clear_endpoint_stall_async(sc->sc_tx_pipe);
}
-   } else {
-   if (IFQ_IS_EMPTY(>if_snd) == 0)
-   umb_start(ifp);
}
+   if (IFQ_IS_EMPTY(>if_snd) == 0)
+   umb_start(ifp);
 
splx(s);
 }

Re: close cron sockets in child processes

[Florian Riehm]

On 10/23/17 09:05, Jeremie Courreges-Anglas wrote:

On Fri, Oct 20 2017, "Todd C. Miller"  wrote:

On Fri, 20 Oct 2017 16:25:32 +0200, Florian Riehm wrote:


cron(8) opens /var/run/cron.sock for communication with crontab(1).
The forked cronjobs have the socked still open.
This prevents restarting cron while a job is running:
(CRON) DEATH (already running)

I think cron's children should not inherit sockets.


There is a similar issue with at jobs.  Here's a comprehensive diff.


Good catch.  The cron part ought to be committed now, but I think
atrun.c needs more love.

Good catch, indeed.
I have commited the cron part.




  - todd

Index: usr.sbin/cron/atrun.c
===
RCS file: /cvs/src/usr.sbin/cron/atrun.c,v
retrieving revision 1.46
diff -u -p -u -r1.46 atrun.c
--- usr.sbin/cron/atrun.c   8 Jun 2017 16:23:39 -   1.46
+++ usr.sbin/cron/atrun.c   20 Oct 2017 15:09:57 -
@@ -283,6 +283,9 @@ run_job(const atjob *job, int dfd, const
return;
}
  
+	/* close fds opened by the parent (e.g. cronSock) */

+   closefrom(3);
+


That doesn't work.  When here, we hold cronSock and dfd (should be 3 and
4).  We need dfd to get fd (should be 5), thus we can't just use
"closefrom(3)".  The straightest way IMO is to close what we should
close (including dfd), but this means making cronSock a global.

I would also propose sprinkling more O_CLOEXEC magic but that can be in
another diff.


I guess it is ok to make cronSock global. closefrom() is good if
you have obvious situations. In more complex scenarios like this, I
prefer close() since it shows more clearly what happens.



Thoughts?


Index: atrun.c
===
RCS file: /d/cvs/src/usr.sbin/cron/atrun.c,v
retrieving revision 1.46
diff -u -p -r1.46 atrun.c
--- atrun.c 8 Jun 2017 16:23:39 -   1.46
+++ atrun.c 23 Oct 2017 03:21:20 -
@@ -283,6 +283,10 @@ run_job(const atjob *job, int dfd, const
return;
}
  
+	/* Close fds opened by the parent. */

+   close(cronSock);
+   close(dfd);
+
/*
 * We don't want the main cron daemon to wait for our children--
 * we will do it ourselves via waitpid().
Index: cron.c
===
RCS file: /d/cvs/src/usr.sbin/cron/cron.c,v
retrieving revision 1.76
diff -u -p -r1.76 cron.c
--- cron.c  7 Jun 2017 23:36:43 -   1.76
+++ cron.c  23 Oct 2017 02:10:54 -
@@ -60,7 +60,6 @@ staticint open_socket(void);
  
  static	volatile sig_atomic_t	got_sigchld;

  statictime_t  timeRunning, virtualTime, clockTime;
-static int cronSock;
  staticlongGMToff;
  staticcron_db *database;
  staticat_db   *at_database;
@@ -68,6 +67,7 @@ staticdouble  batch_maxload = BATCH_MA
  staticint NoFork;
  statictime_t  StartTime;
gid_t   cron_gid;
+   int cronSock;
  
  static void

  usage(void)
Index: globals.h
===
RCS file: /d/cvs/src/usr.sbin/cron/globals.h,v
retrieving revision 1.14
diff -u -p -r1.14 globals.h
--- globals.h   7 Jun 2017 23:36:43 -   1.14
+++ globals.h   23 Oct 2017 02:03:18 -
@@ -18,5 +18,6 @@
   */
  
  extern gid_t	cron_gid;

+extern int cronSock;
  extern intLineNumber;
  extern char   *__progname;




ok friehm

Re: Enable TCP selective acknowledgements (SACK) on all kernels

[Job Snijders]

On Sun, Oct 22, 2017 at 04:04:30PM +0200, Mike Belopuhov wrote:
> > If this is as expected, OK job@
> 
> It's setting the option in my build here:
> 
> 15:55:20.336682 fe:e1:bb:d1:a2:f0 fe:e1:ba:d0:55:1e 0800 78: \
>   10.50.50.34.17078 > 10.50.50.1.80: S [tcp sum ok] 1313610867:1313610867(0) \
>   win 16384  3624645505 0> \
>   (DF) (ttl 64, id 25292, len 64)

I went back and tested again, but with a different packetsniffer. You
were right. I now question the tool I used to observe this traffic on a
middle box is giving me correct output.

I can confirm there is no difference observable with tcpdump between
miniroot ftp(1) and normal ftp(1). SACK is set in both contexts.

All good! :)

Kind regards,

Job

Re: close cron sockets in child processes

[Jeremie Courreges-Anglas]

On Fri, Oct 20 2017, "Todd C. Miller"  wrote:
> On Fri, 20 Oct 2017 16:25:32 +0200, Florian Riehm wrote:
>
>> cron(8) opens /var/run/cron.sock for communication with crontab(1).
>> The forked cronjobs have the socked still open.
>> This prevents restarting cron while a job is running:
>> (CRON) DEATH (already running)
>> 
>> I think cron's children should not inherit sockets.
>
> There is a similar issue with at jobs.  Here's a comprehensive diff.

Good catch.  The cron part ought to be committed now, but I think
atrun.c needs more love.

>  - todd
>
> Index: usr.sbin/cron/atrun.c
> ===
> RCS file: /cvs/src/usr.sbin/cron/atrun.c,v
> retrieving revision 1.46
> diff -u -p -u -r1.46 atrun.c
> --- usr.sbin/cron/atrun.c 8 Jun 2017 16:23:39 -   1.46
> +++ usr.sbin/cron/atrun.c 20 Oct 2017 15:09:57 -
> @@ -283,6 +283,9 @@ run_job(const atjob *job, int dfd, const
>   return;
>   }
>  
> + /* close fds opened by the parent (e.g. cronSock) */
> + closefrom(3);
> +

That doesn't work.  When here, we hold cronSock and dfd (should be 3 and
4).  We need dfd to get fd (should be 5), thus we can't just use
"closefrom(3)".  The straightest way IMO is to close what we should
close (including dfd), but this means making cronSock a global.

I would also propose sprinkling more O_CLOEXEC magic but that can be in
another diff.

Thoughts?


Index: atrun.c
===
RCS file: /d/cvs/src/usr.sbin/cron/atrun.c,v
retrieving revision 1.46
diff -u -p -r1.46 atrun.c
--- atrun.c 8 Jun 2017 16:23:39 -   1.46
+++ atrun.c 23 Oct 2017 03:21:20 -
@@ -283,6 +283,10 @@ run_job(const atjob *job, int dfd, const
return;
}
 
+   /* Close fds opened by the parent. */
+   close(cronSock);
+   close(dfd);
+
/*
 * We don't want the main cron daemon to wait for our children--
 * we will do it ourselves via waitpid().
Index: cron.c
===
RCS file: /d/cvs/src/usr.sbin/cron/cron.c,v
retrieving revision 1.76
diff -u -p -r1.76 cron.c
--- cron.c  7 Jun 2017 23:36:43 -   1.76
+++ cron.c  23 Oct 2017 02:10:54 -
@@ -60,7 +60,6 @@ staticint open_socket(void);
 
 static volatile sig_atomic_t   got_sigchld;
 static time_t  timeRunning, virtualTime, clockTime;
-static int cronSock;
 static longGMToff;
 static cron_db *database;
 static at_db   *at_database;
@@ -68,6 +67,7 @@ staticdouble  batch_maxload = BATCH_MA
 static int NoFork;
 static time_t  StartTime;
gid_t   cron_gid;
+   int cronSock;
 
 static void
 usage(void)
Index: globals.h
===
RCS file: /d/cvs/src/usr.sbin/cron/globals.h,v
retrieving revision 1.14
diff -u -p -r1.14 globals.h
--- globals.h   7 Jun 2017 23:36:43 -   1.14
+++ globals.h   23 Oct 2017 02:03:18 -
@@ -18,5 +18,6 @@
  */
 
 extern gid_t   cron_gid;
+extern int cronSock;
 extern int LineNumber;
 extern char*__progname;


-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE