Re: getent: prefer usernames over UIDs

2018-11-01 Thread Todd C. Miller
On Fri, 02 Nov 2018 00:45:06 +0100, Klemens Nanni wrote:

> On Thu, Nov 01, 2018 at 06:37:15PM -0400, Ted Unangst wrote:
> > yes, this is how it should be. please fix group lookup as well.
> Was in the queue for a separate diff, here it goes together.

Looks good.  OK millert@

 - todd



Re: wdc(4): remove unnecessary if/else block

2018-11-01 Thread Klemens Nanni
OK



Re: getent: prefer usernames over UIDs

2018-11-01 Thread Klemens Nanni
On Thu, Nov 01, 2018 at 06:37:15PM -0400, Ted Unangst wrote:
> yes, this is how it should be. please fix group lookup as well.
Was in the queue for a separate diff, here it goes together.

Index: getent.c
===
RCS file: /cvs/src/usr.bin/getent/getent.c,v
retrieving revision 1.20
diff -u -p -r1.20 getent.c
--- getent.c26 Sep 2018 16:39:19 -  1.20
+++ getent.c1 Nov 2018 23:43:40 -
@@ -201,11 +201,11 @@ group(int argc, char *argv[])
GROUPPRINT;
} else {
for (i = 2; i < argc; i++) {
-   gid = strtonum(argv[i], 0, GID_MAX, &err);
-   if (!err)
-   gr = getgrgid(gid);
-   else
-   gr = getgrnam(argv[i]);
+   if ((gr = getgrnam(argv[i])) == NULL) {
+   gid = strtonum(argv[i], 0, GID_MAX, &err);
+   if (err == NULL)
+   gr = getgrgid(gid);
+   }
if (gr != NULL)
GROUPPRINT;
else {
@@ -302,11 +302,11 @@ passwd(int argc, char *argv[])
PASSWDPRINT;
} else {
for (i = 2; i < argc; i++) {
-   uid = strtonum(argv[i], 0, UID_MAX, &err);
-   if (!err)
-   pw = getpwuid(uid);
-   else
-   pw = getpwnam(argv[i]);
+   if ((pw = getpwnam(argv[i])) == NULL) {
+   uid = strtonum(argv[i], 0, UID_MAX, &err);
+   if (err == NULL)
+   pw = getpwuid(uid);
+   }
if (pw != NULL)
PASSWDPRINT;
else {



Re: top: accept UIDs

2018-11-01 Thread Jason McIntyre
On Thu, Nov 01, 2018 at 10:27:18PM +0100, Klemens Nanni wrote:
> On Thu, Nov 01, 2018 at 09:20:10PM +, Jason McIntyre wrote:
> > while with the current behaviour it made sense to document it that way,
> > once you change it, it just sounds odd. it should really be wrapped into
> > the first sentence. sth like:
> > 
> > Show only those processes owned by username or UID
> > .Ar user .
> Thanks, even better as this is shorter yet more precise in both places.
> 
> I'll commit it this way tomorrow unless someone objects.
> 

sorry, i forgot the same text occurred in two places when i wrote.

i'm not against using the same text twice as such - it would be more
explicit. but note that you could make things less verbose by omitting
the changes to the second text. it would not be inconsistent - for
example, the description of -s has more info, and yet the reader can
understand that interactive "s" will work the same.

additionally, -u accepting a uid now is a big deal (because it is new
behaviour), but will soon become accepted. at that point, it will just
look odd to say that it can also be numeric.

still, the addition to interactive "u" is hardly substantial.

ok either way!

jmc

> Index: top.1
> ===
> RCS file: /cvs/src/usr.bin/top/top.1,v
> retrieving revision 1.69
> diff -u -p -r1.69 top.1
> --- top.1 25 Jul 2018 17:24:14 -  1.69
> +++ top.1 1 Nov 2018 21:25:08 -
> @@ -177,13 +177,11 @@ seconds.
>  The value may be fractional, to permit delays of less than 1 second.
>  The default delay between updates is 5 seconds.
>  .It Fl U Oo - Oc Ns Ar user
> -Show only those processes owned by
> +Show only those processes owned by username or UID
>  .Ar user .
>  The prefix
>  .Sq -
>  hides processes owned by that user.
> -This option currently only accepts usernames and does not understand
> -UID numbers.
>  .It Fl u
>  Do not take the time to map UID numbers to usernames.
>  Normally,
> @@ -362,7 +360,7 @@ Set the delay between screen updates to
>  .Ar time
>  seconds.
>  .It u Oo - Oc Ns Ar user
> -Show only those processes owned by
> +Show only those processes owned by username or UID
>  .Ar user .
>  .Sq u+
>  shows processes belonging to all users.
> Index: top.c
> ===
> RCS file: /cvs/src/usr.bin/top/top.c,v
> retrieving revision 1.95
> diff -u -p -r1.95 top.c
> --- top.c 1 Nov 2018 18:04:13 -   1.95
> +++ top.c 1 Nov 2018 21:26:52 -
> @@ -134,8 +134,10 @@ usage(void)
>  static int
>  filteruser(char buf[])
>  {
> + const char *errstr;
>   char *bufp = buf;
>   uid_t *uidp;
> + uid_t uid;
>  
>   if (bufp[0] == '-') {
>   bufp++;
> @@ -146,7 +148,16 @@ filteruser(char buf[])
>   ps.huid = (pid_t)-1;
>   }
>  
> - return uid_from_user(bufp, uidp);
> + if (uid_from_user(bufp, uidp) == 0)
> + return 0;
> +
> + uid = strtonum(bufp, 0, UID_MAX, &errstr);
> + if (errstr == NULL && user_from_uid(uid, 1) != NULL) {
> + *uidp = uid;
> + return 0;
> + }
> +
> + return -1;
>  }
>  
>  static int
> 



Re: getent: prefer usernames over UIDs

2018-11-01 Thread Ted Unangst
Klemens Nanni wrote:
> Prompted by tedu@'s recent reply, here's a fix for getent(1) to lookup
> keys as UIDs only if the username lookup fails to prevent clobbering
> numerical usernames.

yes, this is how it should be. please fix group lookup as well.



Re: top: accept UIDs

2018-11-01 Thread Ted Unangst
Klemens Nanni wrote:
> On Thu, Nov 01, 2018 at 02:56:38PM -0400, Ted Unangst wrote:
> > this looks to be in the wrong order. if 1000 is a username, that should be
> > matched first before numeric lookup.
> Preferring UIDs indeed clobbers usernames with the respective numerical
> username.
> 
> However, other parts in base such as getent(1) treat UIDs with higher
> priority and behave like that already:

yes, there are inconsistencies, but that just means there's more to fix. :)

i don't have the section handy, but i believe this is specced by posix.
usernames are first priority, then if no match, numeric match is attempted.

chown actually documents this:

 The owner may be either a numeric user ID or a user name.  If a user name
 is also a numeric user ID, the operand is used as a user name.  The group
 may be either a numeric group ID or a group name.  If a group name is
 also a numeric group ID, the operand is used as a group name.



Re: wdc(4): remove unnecessary if/else block

2018-11-01 Thread Stuart Henderson
On 2018/11/01 23:00, Frederic Cambus wrote:
> Hi tech@,
> 
> Remove unnecessary if/else block, both branches are identical. We can
> in fact use the ATA_DELAY macro directly.
> 
> Coverity CID 1453008.
> 
> Comments? OK?
> 
> Index: dev/ata/ata_wdc.c
> ===
> RCS file: /cvs/src/sys/dev/ata/ata_wdc.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 ata_wdc.c
> --- dev/ata/ata_wdc.c 30 Dec 2017 20:46:59 -  1.51
> +++ dev/ata/ata_wdc.c 1 Nov 2018 12:34:17 -
> @@ -240,7 +240,6 @@ _wdc_ata_bio_start(struct channel_softc 
>   u_int16_t cyl;
>   u_int8_t head, sect, cmd = 0;
>   int nblks;
> - int ata_delay;
>   int error, dma_flags = 0;
>  
>   WDCDEBUG_PRINT(("_wdc_ata_bio_start %s:%d:%d\n",
> @@ -283,10 +282,6 @@ _wdc_ata_bio_start(struct channel_softc 
>   if (ata_bio->flags & ATA_LBA48)
>   dma_flags |= WDC_DMA_LBA48;
>   }
> - if (ata_bio->flags & ATA_SINGLE)
> - ata_delay = ATA_DELAY;
> - else
> - ata_delay = ATA_DELAY;

There since r1.1 :-)

The indirection could possibly have been done to allow tweaking the
delay with ddb for debug, but I don't think that really matters for wdc
any more..

OK with me

>  again:
>   /*
>*
> @@ -345,7 +340,7 @@ again:
>   }
>   /* Initiate command */
>   wdc_set_drive(chp, xfer->drive);
> - if (wait_for_ready(chp, ata_delay) < 0)
> + if (wait_for_ready(chp, ATA_DELAY) < 0)
>   goto timeout;
>  
>   /* start the DMA channel (before) */
> @@ -391,7 +386,7 @@ again:
>   }
>   /* Initiate command! */
>   wdc_set_drive(chp, xfer->drive);
> - if (wait_for_ready(chp, ata_delay) < 0)
> + if (wait_for_ready(chp, ATA_DELAY) < 0)
>   goto timeout;
>   if (ata_bio->flags & ATA_LBA48) {
>   wdccommandext(chp, xfer->drive, cmd,
> @@ -410,7 +405,7 @@ again:
>   }
>   /* If this was a write and not using DMA, push the data. */
>   if ((ata_bio->flags & ATA_READ) == 0) {
> - if (wait_for_drq(chp, ata_delay) != 0) {
> + if (wait_for_drq(chp, ATA_DELAY) != 0) {
>   printf("%s:%d:%d: timeout waiting for DRQ, "
>   "st=0x%b, err=0x%02x\n",
>   chp->wdc->sc_dev.dv_xname, chp->channel,
> 



Re: pcidevs update for VIA HD Audio device

2018-11-01 Thread Mike Larkin
On Thu, Nov 01, 2018 at 11:07:21PM +0100, Frederic Cambus wrote:
> Hi tech@,
> 
> Add ID for a VIA HD Audio device found on my HP t5570 Thin Client.
> 
> While there, update URL for the vendor ID search engine.
> 
> Comments? OK?
> 

ok mlarkin

> Index: sys/dev/pci/pcidevs
> ===
> RCS file: /cvs/src/sys/dev/pci/pcidevs,v
> retrieving revision 1.1865
> diff -u -p -r1.1865 pcidevs
> --- sys/dev/pci/pcidevs   26 Oct 2018 05:53:58 -  1.1865
> +++ sys/dev/pci/pcidevs   31 Oct 2018 17:43:06 -
> @@ -39,7 +39,7 @@ $OpenBSD: pcidevs,v 1.1865 2018/10/26 05
>   *
>   * There is a Vendor ID search engine available at:
>   *
> - *   http://www.pcisig.com/membership/vid_search/
> + *   https://pcisig.com/membership/member-companies
>   */
>  
>  /*
> @@ -7623,7 +7623,7 @@ product VIATECH PM800_DRAM  0x3259  PM800 
>  product VIATECH KT880_3  0x3269  KT880 Host
>  product VIATECH K8HTB_3  0x3282  K8HTB Host
>  product VIATECH VT8251_ISA   0x3287  VT8251 ISA
> -product VIATECH HDA  0x3288  HD Audio
> +product VIATECH HDA_00x3288  HD Audio
>  product VIATECH CX700_4  0x3324  CX700 Host
>  product VIATECH P4M890_3 0x3327  P4M890 Host
>  product VIATECH K8M890_3 0x3336  K8M890 Host
> @@ -7706,6 +7706,7 @@ product VIATECH VT82C598AGP 0x8598  VT82C
>  product VIATECH VT82C601 0x8601  VT82C601 AGP
>  product VIATECH VT8605_AGP   0x8605  VT8605 AGP
>  product VIATECH VX900_IDE0x9001  VX900 IDE
> +product VIATECH HDA_10x9170  HD Audio
>  product VIATECH VX800_SDMMC  0x9530  VX800 SD/MMC
>  product VIATECH VX800_SDIO   0x95d0  VX800 SDIO
>  product VIATECH K8T890_PPB_A 0xa238  K8T890
> 



pcidevs update for VIA HD Audio device

2018-11-01 Thread Frederic Cambus
Hi tech@,

Add ID for a VIA HD Audio device found on my HP t5570 Thin Client.

While there, update URL for the vendor ID search engine.

Comments? OK?

Index: sys/dev/pci/pcidevs
===
RCS file: /cvs/src/sys/dev/pci/pcidevs,v
retrieving revision 1.1865
diff -u -p -r1.1865 pcidevs
--- sys/dev/pci/pcidevs 26 Oct 2018 05:53:58 -  1.1865
+++ sys/dev/pci/pcidevs 31 Oct 2018 17:43:06 -
@@ -39,7 +39,7 @@ $OpenBSD: pcidevs,v 1.1865 2018/10/26 05
  *
  * There is a Vendor ID search engine available at:
  *
- * http://www.pcisig.com/membership/vid_search/
+ * https://pcisig.com/membership/member-companies
  */
 
 /*
@@ -7623,7 +7623,7 @@ product VIATECH PM800_DRAM0x3259  PM800 
 product VIATECH KT880_30x3269  KT880 Host
 product VIATECH K8HTB_30x3282  K8HTB Host
 product VIATECH VT8251_ISA 0x3287  VT8251 ISA
-product VIATECH HDA0x3288  HD Audio
+product VIATECH HDA_0  0x3288  HD Audio
 product VIATECH CX700_40x3324  CX700 Host
 product VIATECH P4M890_3   0x3327  P4M890 Host
 product VIATECH K8M890_3   0x3336  K8M890 Host
@@ -7706,6 +7706,7 @@ product VIATECH VT82C598AGP   0x8598  VT82C
 product VIATECH VT82C601   0x8601  VT82C601 AGP
 product VIATECH VT8605_AGP 0x8605  VT8605 AGP
 product VIATECH VX900_IDE  0x9001  VX900 IDE
+product VIATECH HDA_1  0x9170  HD Audio
 product VIATECH VX800_SDMMC0x9530  VX800 SD/MMC
 product VIATECH VX800_SDIO 0x95d0  VX800 SDIO
 product VIATECH K8T890_PPB_A   0xa238  K8T890



wdc(4): remove unnecessary if/else block

2018-11-01 Thread Frederic Cambus
Hi tech@,

Remove unnecessary if/else block, both branches are identical. We can
in fact use the ATA_DELAY macro directly.

Coverity CID 1453008.

Comments? OK?

Index: dev/ata/ata_wdc.c
===
RCS file: /cvs/src/sys/dev/ata/ata_wdc.c,v
retrieving revision 1.51
diff -u -p -r1.51 ata_wdc.c
--- dev/ata/ata_wdc.c   30 Dec 2017 20:46:59 -  1.51
+++ dev/ata/ata_wdc.c   1 Nov 2018 12:34:17 -
@@ -240,7 +240,6 @@ _wdc_ata_bio_start(struct channel_softc 
u_int16_t cyl;
u_int8_t head, sect, cmd = 0;
int nblks;
-   int ata_delay;
int error, dma_flags = 0;
 
WDCDEBUG_PRINT(("_wdc_ata_bio_start %s:%d:%d\n",
@@ -283,10 +282,6 @@ _wdc_ata_bio_start(struct channel_softc 
if (ata_bio->flags & ATA_LBA48)
dma_flags |= WDC_DMA_LBA48;
}
-   if (ata_bio->flags & ATA_SINGLE)
-   ata_delay = ATA_DELAY;
-   else
-   ata_delay = ATA_DELAY;
 again:
/*
 *
@@ -345,7 +340,7 @@ again:
}
/* Initiate command */
wdc_set_drive(chp, xfer->drive);
-   if (wait_for_ready(chp, ata_delay) < 0)
+   if (wait_for_ready(chp, ATA_DELAY) < 0)
goto timeout;
 
/* start the DMA channel (before) */
@@ -391,7 +386,7 @@ again:
}
/* Initiate command! */
wdc_set_drive(chp, xfer->drive);
-   if (wait_for_ready(chp, ata_delay) < 0)
+   if (wait_for_ready(chp, ATA_DELAY) < 0)
goto timeout;
if (ata_bio->flags & ATA_LBA48) {
wdccommandext(chp, xfer->drive, cmd,
@@ -410,7 +405,7 @@ again:
}
/* If this was a write and not using DMA, push the data. */
if ((ata_bio->flags & ATA_READ) == 0) {
-   if (wait_for_drq(chp, ata_delay) != 0) {
+   if (wait_for_drq(chp, ATA_DELAY) != 0) {
printf("%s:%d:%d: timeout waiting for DRQ, "
"st=0x%b, err=0x%02x\n",
chp->wdc->sc_dev.dv_xname, chp->channel,



getent: prefer usernames over UIDs

2018-11-01 Thread Klemens Nanni
Prompted by tedu@'s recent reply, here's a fix for getent(1) to lookup
keys as UIDs only if the username lookup fails to prevent clobbering
numerical usernames.

$ getent passwd 1000 kn
kn:*:1000:1000:Klemens Nanni:/home/kn:/bin/ksh
kn:*:1000:1000:Klemens Nanni:/home/kn:/bin/ksh

$ ./obj/getent passwd 1000 kn
1000:*:1003:1003::/home/1000:/bin/ksh
kn:*:1000:1000:Klemens Nanni:/home/kn:/bin/ksh

OK?

Index: getent.c
===
RCS file: /cvs/src/usr.bin/getent/getent.c,v
retrieving revision 1.20
diff -u -p -r1.20 getent.c
--- getent.c26 Sep 2018 16:39:19 -  1.20
+++ getent.c1 Nov 2018 21:51:01 -
@@ -302,11 +302,11 @@ passwd(int argc, char *argv[])
PASSWDPRINT;
} else {
for (i = 2; i < argc; i++) {
-   uid = strtonum(argv[i], 0, UID_MAX, &err);
-   if (!err)
-   pw = getpwuid(uid);
-   else
-   pw = getpwnam(argv[i]);
+   if ((pw = getpwnam(argv[i])) == NULL) {
+   uid = strtonum(argv[i], 0, UID_MAX, &err);
+   if (err == NULL)
+   pw = getpwuid(uid);
+   }
if (pw != NULL)
PASSWDPRINT;
else {



Re: top: accept UIDs

2018-11-01 Thread Klemens Nanni
On Thu, Nov 01, 2018 at 09:20:10PM +, Jason McIntyre wrote:
> while with the current behaviour it made sense to document it that way,
> once you change it, it just sounds odd. it should really be wrapped into
> the first sentence. sth like:
> 
>   Show only those processes owned by username or UID
>   .Ar user .
Thanks, even better as this is shorter yet more precise in both places.

I'll commit it this way tomorrow unless someone objects.

Index: top.1
===
RCS file: /cvs/src/usr.bin/top/top.1,v
retrieving revision 1.69
diff -u -p -r1.69 top.1
--- top.1   25 Jul 2018 17:24:14 -  1.69
+++ top.1   1 Nov 2018 21:25:08 -
@@ -177,13 +177,11 @@ seconds.
 The value may be fractional, to permit delays of less than 1 second.
 The default delay between updates is 5 seconds.
 .It Fl U Oo - Oc Ns Ar user
-Show only those processes owned by
+Show only those processes owned by username or UID
 .Ar user .
 The prefix
 .Sq -
 hides processes owned by that user.
-This option currently only accepts usernames and does not understand
-UID numbers.
 .It Fl u
 Do not take the time to map UID numbers to usernames.
 Normally,
@@ -362,7 +360,7 @@ Set the delay between screen updates to
 .Ar time
 seconds.
 .It u Oo - Oc Ns Ar user
-Show only those processes owned by
+Show only those processes owned by username or UID
 .Ar user .
 .Sq u+
 shows processes belonging to all users.
Index: top.c
===
RCS file: /cvs/src/usr.bin/top/top.c,v
retrieving revision 1.95
diff -u -p -r1.95 top.c
--- top.c   1 Nov 2018 18:04:13 -   1.95
+++ top.c   1 Nov 2018 21:26:52 -
@@ -134,8 +134,10 @@ usage(void)
 static int
 filteruser(char buf[])
 {
+   const char *errstr;
char *bufp = buf;
uid_t *uidp;
+   uid_t uid;
 
if (bufp[0] == '-') {
bufp++;
@@ -146,7 +148,16 @@ filteruser(char buf[])
ps.huid = (pid_t)-1;
}
 
-   return uid_from_user(bufp, uidp);
+   if (uid_from_user(bufp, uidp) == 0)
+   return 0;
+
+   uid = strtonum(bufp, 0, UID_MAX, &errstr);
+   if (errstr == NULL && user_from_uid(uid, 1) != NULL) {
+   *uidp = uid;
+   return 0;
+   }
+
+   return -1;
 }
 
 static int



Re: top: accept UIDs

2018-11-01 Thread Jason McIntyre
On Thu, Nov 01, 2018 at 09:04:03PM +0100, Klemens Nanni wrote:
> On Thu, Nov 01, 2018 at 02:56:38PM -0400, Ted Unangst wrote:
> > this looks to be in the wrong order. if 1000 is a username, that should be
> > matched first before numeric lookup.
> Preferring UIDs indeed clobbers usernames with the respective numerical
> username.
> 
> However, other parts in base such as getent(1) treat UIDs with higher
> priority and behave like that already:
> 
>   $ doas useradd -- 1000
>   useradd: Warning: home directory `/home/1000' doesn't exist, and -m was 
> not specified
>   $ getent passwd 1000
>   kn:*:1000:1000:Klemens Nanni:/home/kn:/bin/ksh
>   $ getent passwd $(id -u 1000)
>   1000:*:1003:1003::/home/1000:/bin/ksh
> 
> Here's a diff that tries UIDs only if username lookup fails.
> 
> Index: top.1
> ===
> RCS file: /cvs/src/usr.bin/top/top.1,v
> retrieving revision 1.69
> diff -u -p -r1.69 top.1
> --- top.1 25 Jul 2018 17:24:14 -  1.69
> +++ top.1 1 Nov 2018 20:01:02 -
> @@ -182,8 +182,7 @@ Show only those processes owned by
>  The prefix
>  .Sq -
>  hides processes owned by that user.
> -This option currently only accepts usernames and does not understand
> -UID numbers.
> +This option accepts usernames and UID numbers.
>  .It Fl u
>  Do not take the time to map UID numbers to usernames.
>  Normally,

hi.

while with the current behaviour it made sense to document it that way,
once you change it, it just sounds odd. it should really be wrapped into
the first sentence. sth like:

Show only those processes owned by username or UID
.Ar user .

jmc



Re: top: accept UIDs

2018-11-01 Thread Todd C. Miller
On Thu, 01 Nov 2018 21:04:03 +0100, Klemens Nanni wrote:

> Here's a diff that tries UIDs only if username lookup fails.

OK millert@

 - todd



Re: top: accept UIDs

2018-11-01 Thread Klemens Nanni
On Thu, Nov 01, 2018 at 02:56:38PM -0400, Ted Unangst wrote:
> this looks to be in the wrong order. if 1000 is a username, that should be
> matched first before numeric lookup.
Preferring UIDs indeed clobbers usernames with the respective numerical
username.

However, other parts in base such as getent(1) treat UIDs with higher
priority and behave like that already:

$ doas useradd -- 1000
useradd: Warning: home directory `/home/1000' doesn't exist, and -m was 
not specified
$ getent passwd 1000
kn:*:1000:1000:Klemens Nanni:/home/kn:/bin/ksh
$ getent passwd $(id -u 1000)
1000:*:1003:1003::/home/1000:/bin/ksh

Here's a diff that tries UIDs only if username lookup fails.

Index: top.1
===
RCS file: /cvs/src/usr.bin/top/top.1,v
retrieving revision 1.69
diff -u -p -r1.69 top.1
--- top.1   25 Jul 2018 17:24:14 -  1.69
+++ top.1   1 Nov 2018 20:01:02 -
@@ -182,8 +182,7 @@ Show only those processes owned by
 The prefix
 .Sq -
 hides processes owned by that user.
-This option currently only accepts usernames and does not understand
-UID numbers.
+This option accepts usernames and UID numbers.
 .It Fl u
 Do not take the time to map UID numbers to usernames.
 Normally,
Index: top.c
===
RCS file: /cvs/src/usr.bin/top/top.c,v
retrieving revision 1.95
diff -u -p -r1.95 top.c
--- top.c   1 Nov 2018 18:04:13 -   1.95
+++ top.c   1 Nov 2018 20:01:02 -
@@ -134,8 +134,10 @@ usage(void)
 static int
 filteruser(char buf[])
 {
+   const char *errstr;
char *bufp = buf;
uid_t *uidp;
+   uid_t uid;
 
if (bufp[0] == '-') {
bufp++;
@@ -146,7 +148,16 @@ filteruser(char buf[])
ps.huid = (pid_t)-1;
}
 
-   return uid_from_user(bufp, uidp);
+   if (uid_from_user(bufp, uidp) == 0)
+   return 0;
+
+   uid = strtonum(bufp, 0, UID_MAX, &errstr);
+   if (errstr == NULL && user_from_uid(uid, 1) != NULL) {
+   *uidp = uid;
+   return 0;
+   }
+
+   return -1;
 }
 
 static int



Re: top: accept UIDs

2018-11-01 Thread Ted Unangst
Klemens Nanni wrote:
> This makes `top -U 0' and "u-1000" work.
> 
> Overview:
> 
> "kn" ok"10kn"ok if "10kn" exists else error
> "-kn"ok"-10kn"   ok if "10kn" exists else error
> "1000"   ok"--1000"  error, negative UID and "-1000" not valid
> "-1000"  ok"--10kn"  error, "-10kn" not valid
> 
> I wanted to drop the restriction from the manual page first, but this
> would actually remove information so shortly state what's accepted.
> 
> Feedback? OK?

this looks to be in the wrong order. if 1000 is a username, that should be
matched first before numeric lookup.



top: accept UIDs

2018-11-01 Thread Klemens Nanni
This makes `top -U 0' and "u-1000" work.

Overview:

"kn" ok"10kn"ok if "10kn" exists else error
"-kn"ok"-10kn"   ok if "10kn" exists else error
"1000"   ok"--1000"  error, negative UID and "-1000" not valid
"-1000"  ok"--10kn"  error, "-10kn" not valid

I wanted to drop the restriction from the manual page first, but this
would actually remove information so shortly state what's accepted.

Feedback? OK?

Index: top.1
===
RCS file: /cvs/src/usr.bin/top/top.1,v
retrieving revision 1.69
diff -u -p -r1.69 top.1
--- top.1   25 Jul 2018 17:24:14 -  1.69
+++ top.1   1 Nov 2018 18:28:29 -
@@ -182,8 +182,7 @@ Show only those processes owned by
 The prefix
 .Sq -
 hides processes owned by that user.
-This option currently only accepts usernames and does not understand
-UID numbers.
+This option accepts usernames and UID numbers.
 .It Fl u
 Do not take the time to map UID numbers to usernames.
 Normally,
Index: top.c
===
RCS file: /cvs/src/usr.bin/top/top.c,v
retrieving revision 1.95
diff -u -p -r1.95 top.c
--- top.c   1 Nov 2018 18:04:13 -   1.95
+++ top.c   1 Nov 2018 18:28:29 -
@@ -134,8 +134,10 @@ usage(void)
 static int
 filteruser(char buf[])
 {
+   const char *errstr;
char *bufp = buf;
uid_t *uidp;
+   uid_t uid;
 
if (bufp[0] == '-') {
bufp++;
@@ -144,6 +146,12 @@ filteruser(char buf[])
} else {
uidp = &ps.uid;
ps.huid = (pid_t)-1;
+   }
+
+   uid = strtonum(bufp, 0, UID_MAX, &errstr);
+   if (errstr == NULL && user_from_uid(uid, 1) != NULL) {
+   *uidp = uid;
+   return 0;
}
 
return uid_from_user(bufp, uidp);



Re: top: merge duplicate code into helper functions

2018-11-01 Thread Todd C. Miller
On Wed, 31 Oct 2018 23:38:58 +0100, Klemens Nanni wrote:

> New diff free of side-effects in filteruser(), rest unchanged.

OK millert@

 - todd



Re: bgpd, Adj-RIB-Out support

2018-11-01 Thread Sebastian Benoit
Claudio Jeker(cje...@diehard.n-r-g.com) on 2018.11.01 10:38:41 +0100:
> On Wed, Oct 31, 2018 at 11:55:51PM +0100, Sebastian Benoit wrote:
> > Denis Fondras(open...@ledeuns.net) on 2018.10.31 21:02:17 +0100:
> > > On Wed, Oct 31, 2018 at 04:24:49PM +0100, Claudio Jeker wrote:
> > > > This diff introduces a real Adj-RIB-Out. It is the minimal change to
> > > > introduce the new RIB. This removes the update_rib introduced before 6.4
> > > > lock because that is now replaced with a real RIB.
> > > > The code used by bgpctl show rib is now a fair amount easier since now 
> > > > one
> > > > RIB runner can be used for all cases.
> > > > path_update() is now returning if a prefix was not modified which is
> > > > needed in the update path to suppress unneeded updates.
> > > > The softreconfig out case becomes simpler because there is no more the
> > > > need to run with both filters and check results.
> > > > Last the shutdown code of the RDE had to be adjusted a bit to still work
> > > > with the Adj-RIB-Out.
> > > > 
> > > > This may cause memory consumption to go up but my testing has shown that
> > > > the result is actually not significant. Needs the commits from earlier
> > > > today to apply.
> > > 
> > > On my Route Server, it is quite a big change (in percentage).
> > > * Before :
> > > RDE memory statistics
> > >  11561 IPv4 unicast network entries using 452K of memory
> > >131 IPv6 unicast network entries using 7.2K of memory
> > >  23370 rib entries using 1.4M of memory
> > >  23517 prefix entries using 1.8M of memory
> > >   4894 BGP path attribute entries using 344K of memory
> > >and holding 23517 references
> > >   1720 BGP AS-PATH attribute entries using 76.4K of memory
> > >and holding 4894 references
> > >   1061 BGP attributes entries using 41.4K of memory
> > >and holding 10565 references
> > >   1060 BGP attributes using 25.6K of memory
> > > 101809 as-set elements in 58041 tables using 2.1M of memory
> > > 114429 prefix-set elments using 4.7M of memory
> > > RIB using 4.1M of memory
> > > Sets using 6.9M of memory
> > > 
> > > RDE hash statistics
> > > path hash: size 131072, 4894 entires
> > > min 0 max 3 avg/std-dev = 0.037/0.000
> > > aspath hash: size 131072, 1720 entires
> > > min 0 max 2 avg/std-dev = 0.013/0.000
> > > attr hash: size 16384, 1061 entires
> > > min 0 max 2 avg/std-dev = 0.065/0.000
> > > 
> > > * After:
> > > RDE memory statistics
> > >  11560 IPv4 unicast network entries using 452K of memory
> > >145 IPv6 unicast network entries using 7.9K of memory
> > >  34991 rib entries using 2.1M of memory
> > >  69844 prefix entries using 5.3M of memory
> > >   4929 BGP path attribute entries using 347K of memory
> > >and holding 69844 references
> > >   1727 BGP AS-PATH attribute entries using 76.6K of memory
> > >and holding 4929 references
> > >   1066 BGP attributes entries using 41.6K of memory
> > >and holding 10643 references
> > >   1065 BGP attributes using 25.6K of memory
> > > 101809 as-set elements in 58041 tables using 2.1M of memory
> > > 114429 prefix-set elments using 4.7M of memory
> > > RIB using 8.4M of memory
> > > Sets using 6.9M of memory
> > > 
> > > RDE hash statistics
> > > path hash: size 131072, 4929 entires
> > > min 0 max 3 avg/std-dev = 0.038/0.000
> > > aspath hash: size 131072, 1727 entires
> > > min 0 max 2 avg/std-dev = 0.013/0.000
> > > attr hash: size 16384, 1066 entires
> > > min 0 max 2 avg/std-dev = 0.065/0.000
> > > 
> > 
> > ok this is a more interesting router:
> > 
> > [benoit@border2:~]$ cat before  
> > RDE memory statistics
> > 715727 IPv4 unicast network entries using 27.3M of memory
> >  58953 IPv6 unicast network entries using 3.1M of memory
> >1549171 rib entries using 94.6M of memory
> >2897130 prefix entries using 265M of memory
> > 562423 BGP path attribute entries using 60.1M of memory
> > 149579 BGP AS-PATH attribute entries using 6.1M of memory,
> >and holding 562423 references
> >  37007 BGP attributes entries using 1.4M of memory
> >and holding 880668 references
> >  37006 BGP attributes using 912K of memory
> >  61964 as-set elements in 58064 tables using 950K of memory
> > 103150 prefix-set elments using 4.3M of memory
> > RIB using 459M of memory
> > Sets using 5.2M of memory
> > 
> > RDE hash statistics
> > path hash: size 131072, 562423 entires
> > min 0 max 20 avg/std-dev = 4.291/2.364
> > aspath hash: size 131072, 149579 entires
> > min 0 max 8 avg/std-dev = 1.141/0.835
> > attr hash: size 16384, 37007 entires
> > min 0 max 10 avg/std-dev = 2.259/1.378
> > 
> > [benoit@border2:~]$ cat now   

Xenocara: disable xdm-authorization-1 cleanly

2018-11-01 Thread Matthieu Herrb
Hi,

The XDM-AUTHORIZATION-1 protocol is currenly disabled in xenocara.
It makes sense: this auth protocol doesn't work with IPv6, is relying
on weak DES encryption and only useful with XDMP which isn't supported
by xenodm.

But it's currently only disabled as a side-effect of a buggy check in
xenodm. (If written correctly the test whould enable it...).
This buggy check is a left-over from xdm, and is useless for xenodm.

So let's remove it, and explicitely disable xdm-auth-1 in xserver.

ok ?

Index: app/xenodm/configure.ac
===
RCS file: /cvs/xenocara/app/xenodm/configure.ac,v
retrieving revision 1.9
diff -u -r1.9 configure.ac
--- app/xenodm/configure.ac 15 Jul 2018 09:05:11 -  1.9
+++ app/xenodm/configure.ac 1 Nov 2018 11:49:32 -
@@ -198,8 +198,6 @@
 XENODM_CFLAGS="$XENODM_CFLAGS $DMCP_CFLAGS $XLIB_CFLAGS $AUTH_CFLAGS 
$STATIC_GREETER_CFLAGS"
 XENODM_LIBS="$XENODM_LIBS $DMCP_LIBS"
 
-AC_CHECK_LIB(Xdmcp, XdmcpWrap, [xdmauth="yes"], [xdmauth="no"], [$DMCP_LIBS])
-
 AC_SUBST(XENODM_CFLAGS)
 AC_SUBST(XENODM_LIBS)
 
Index: xserver/Makefile.bsd-wrapper
===
RCS file: /cvs/xenocara/xserver/Makefile.bsd-wrapper,v
retrieving revision 1.67
diff -u -r1.67 Makefile.bsd-wrapper
--- xserver/Makefile.bsd-wrapper25 Oct 2018 21:55:18 -  1.67
+++ xserver/Makefile.bsd-wrapper1 Nov 2018 11:49:32 -
@@ -35,7 +35,7 @@
--with-module-dir=${LIBDIR}/modules ${GLX_OPTION} \
--disable-install-setuid --enable-privsep \
${KDRIVE_OPTION} \
-   --enable-xcsecurity \
+   --enable-xcsecurity --disable-xdm-auth-1 \
--without-fop --without-xmlto --without-xsltproc \
--disable-dmx ${NO_XORG_OPTION} \
--disable-unit-tests \

-- 
Matthieu Herrb



snmpctl segfault

2018-11-01 Thread Martijn van Duren
When experimenting with snmpd I found the following crash:
$ snmpctl snmp walk 127.0.0.1 oid 1   
Segmentation fault (core dumped)

The problem is a NULL dereference in ber_free_elements:
#0  0x0370920d24ca in ber_free_elements (root=0x0) at 
/usr/src/usr.sbin/snmpctl/../snmpd/ber.c:897
#1  0x0370920d4386 in ber_printf_elements (ber=0x0, fmt=0x370920c79b9 
"0}}") at /usr/src/usr.sbin/snmpctl/../snmpd/ber.c:645
#2  0x0370920dbd0d in snmpc_sendreq (sc=0x7f7bdcf0, type=1) at 
/usr/src/usr.sbin/snmpctl/snmpclient.c:413
#3  0x0370920dba93 in snmpc_request (sc=0x7f7bdcf0, type=1) at 
/usr/src/usr.sbin/snmpctl/snmpclient.c:217
#4  0x0370920dba38 in snmpc_run (sc=0x7f7bdcf0, action=WALK, 
oid=0x373171110e0 "1") at /usr/src/usr.sbin/snmpctl/snmpclient.c:207
#5  0x0370920db891 in snmpclient (res=0x3709211f4e8) at 
/usr/src/usr.sbin/snmpctl/snmpclient.c:175
#6  0x0370920dc8b4 in main (argc=5, argv=0x7f7be1a0) at 
/usr/src/usr.sbin/snmpctl/snmpctl.c:133

A simple NULL-check fixes the issue, but since I'm new to BER and snmp  
I'm not sure if this is the right approach or whether there's a logic-
fault somewhere else.

$ ./snmpctl snmp walk 127.0.0.1 oid 1
snmpctl: request failed: Invalid argument

martijn@

Index: ber.c
===
RCS file: /cvs/src/usr.sbin/snmpd/ber.c,v
retrieving revision 1.48
diff -u -p -r1.48 ber.c
--- ber.c   12 Aug 2018 22:04:09 -  1.48
+++ ber.c   1 Nov 2018 10:56:47 -
@@ -894,6 +894,8 @@ ber_free_element(struct ber_element *roo
 void
 ber_free_elements(struct ber_element *root)
 {
+   if (root == NULL)
+   return;
if (root->be_sub && (root->be_encoding == BER_TYPE_SEQUENCE ||
root->be_encoding == BER_TYPE_SET))
ber_free_elements(root->be_sub);



Re: bgpd, Adj-RIB-Out support

2018-11-01 Thread Claudio Jeker
On Wed, Oct 31, 2018 at 11:55:51PM +0100, Sebastian Benoit wrote:
> Denis Fondras(open...@ledeuns.net) on 2018.10.31 21:02:17 +0100:
> > On Wed, Oct 31, 2018 at 04:24:49PM +0100, Claudio Jeker wrote:
> > > This diff introduces a real Adj-RIB-Out. It is the minimal change to
> > > introduce the new RIB. This removes the update_rib introduced before 6.4
> > > lock because that is now replaced with a real RIB.
> > > The code used by bgpctl show rib is now a fair amount easier since now one
> > > RIB runner can be used for all cases.
> > > path_update() is now returning if a prefix was not modified which is
> > > needed in the update path to suppress unneeded updates.
> > > The softreconfig out case becomes simpler because there is no more the
> > > need to run with both filters and check results.
> > > Last the shutdown code of the RDE had to be adjusted a bit to still work
> > > with the Adj-RIB-Out.
> > > 
> > > This may cause memory consumption to go up but my testing has shown that
> > > the result is actually not significant. Needs the commits from earlier
> > > today to apply.
> > 
> > On my Route Server, it is quite a big change (in percentage).
> > * Before :
> > RDE memory statistics
> >  11561 IPv4 unicast network entries using 452K of memory
> >131 IPv6 unicast network entries using 7.2K of memory
> >  23370 rib entries using 1.4M of memory
> >  23517 prefix entries using 1.8M of memory
> >   4894 BGP path attribute entries using 344K of memory
> >and holding 23517 references
> >   1720 BGP AS-PATH attribute entries using 76.4K of memory
> >and holding 4894 references
> >   1061 BGP attributes entries using 41.4K of memory
> >and holding 10565 references
> >   1060 BGP attributes using 25.6K of memory
> > 101809 as-set elements in 58041 tables using 2.1M of memory
> > 114429 prefix-set elments using 4.7M of memory
> > RIB using 4.1M of memory
> > Sets using 6.9M of memory
> > 
> > RDE hash statistics
> > path hash: size 131072, 4894 entires
> > min 0 max 3 avg/std-dev = 0.037/0.000
> > aspath hash: size 131072, 1720 entires
> > min 0 max 2 avg/std-dev = 0.013/0.000
> > attr hash: size 16384, 1061 entires
> > min 0 max 2 avg/std-dev = 0.065/0.000
> > 
> > * After:
> > RDE memory statistics
> >  11560 IPv4 unicast network entries using 452K of memory
> >145 IPv6 unicast network entries using 7.9K of memory
> >  34991 rib entries using 2.1M of memory
> >  69844 prefix entries using 5.3M of memory
> >   4929 BGP path attribute entries using 347K of memory
> >and holding 69844 references
> >   1727 BGP AS-PATH attribute entries using 76.6K of memory
> >and holding 4929 references
> >   1066 BGP attributes entries using 41.6K of memory
> >and holding 10643 references
> >   1065 BGP attributes using 25.6K of memory
> > 101809 as-set elements in 58041 tables using 2.1M of memory
> > 114429 prefix-set elments using 4.7M of memory
> > RIB using 8.4M of memory
> > Sets using 6.9M of memory
> > 
> > RDE hash statistics
> > path hash: size 131072, 4929 entires
> > min 0 max 3 avg/std-dev = 0.038/0.000
> > aspath hash: size 131072, 1727 entires
> > min 0 max 2 avg/std-dev = 0.013/0.000
> > attr hash: size 16384, 1066 entires
> > min 0 max 2 avg/std-dev = 0.065/0.000
> > 
> 
> ok this is a more interesting router:
> 
> [benoit@border2:~]$ cat before  
> RDE memory statistics
> 715727 IPv4 unicast network entries using 27.3M of memory
>  58953 IPv6 unicast network entries using 3.1M of memory
>1549171 rib entries using 94.6M of memory
>2897130 prefix entries using 265M of memory
> 562423 BGP path attribute entries using 60.1M of memory
> 149579 BGP AS-PATH attribute entries using 6.1M of memory,
>and holding 562423 references
>  37007 BGP attributes entries using 1.4M of memory
>and holding 880668 references
>  37006 BGP attributes using 912K of memory
>  61964 as-set elements in 58064 tables using 950K of memory
> 103150 prefix-set elments using 4.3M of memory
> RIB using 459M of memory
> Sets using 5.2M of memory
> 
> RDE hash statistics
> path hash: size 131072, 562423 entires
> min 0 max 20 avg/std-dev = 4.291/2.364
> aspath hash: size 131072, 149579 entires
> min 0 max 8 avg/std-dev = 1.141/0.835
> attr hash: size 16384, 37007 entires
> min 0 max 10 avg/std-dev = 2.259/1.378
> 
> [benoit@border2:~]$ cat now   
>  
> RDE memory statistics
>2323535 unspec network entries using 0B of memory
> 716246 IPv6 unicast network entries using 38.3M of memory

This bgpctl is out of sync. So I don't trust the numbers too much.

Also check the ps alx or top output.

Re: bgpd, Adj-RIB-Out support

2018-11-01 Thread Denis Fondras
On Wed, Oct 31, 2018 at 10:02:05PM -0400, David Higgs wrote:
> On Wed, Oct 31, 2018 at 6:58 PM Sebastian Benoit  wrote:
> 
> 
> 
> On a phone, saw some typos and such, sorry no diff.
> 
> [benoit@border2:~]$ cat before
> > RDE memory statistics
> > 715727 IPv4 unicast network entries using 27.3M of memory
> >  58953 IPv6 unicast network entries using 3.1M of memory
> >1549171 rib entries using 94.6M of memory
> >2897130 prefix entries using 265M of memory
> > 562423 BGP path attribute entries using 60.1M of memory
> > 149579 BGP AS-PATH attribute entries using 6.1M of memory,
> >and holding 562423 references
> >  37007 BGP attributes entries using 1.4M of memory
> >and holding 880668 references
> 
> 
> Inconsistent comma usage above
> 

This one was fixed in a recent commit. It seems benno@ has not updated bgpctl :)


> 
> >  37006 BGP attributes using 912K of memory
> >  61964 as-set elements in 58064 tables using 950K of memory
> > 103150 prefix-set elments using 4.3M of memory
> 
> 
> elments => elements
> 
> RIB using 459M of memory
> > Sets using 5.2M of memory
> >
> > RDE hash statistics
> > path hash: size 131072, 562423 entires
> > min 0 max 20 avg/std-dev = 4.291/2.364
> > aspath hash: size 131072, 149579 entires
> > min 0 max 8 avg/std-dev = 1.141/0.835
> > attr hash: size 16384, 37007 entires
> > min 0 max 10 avg/std-dev = 2.259/1.378
> >
> 
> entires => entries
> 

Thanks for spotting.

> —david

Index: bgpctl.c
===
RCS file: /cvs/src/usr.sbin/bgpctl/bgpctl.c,v
retrieving revision 1.222
diff -u -p -r1.222 bgpctl.c
--- bgpctl.c31 Oct 2018 14:58:59 -  1.222
+++ bgpctl.c1 Nov 2018 08:17:21 -
@@ -1880,7 +1880,7 @@ show_rib_memory_msg(struct imsg *imsg)
printf("%10lld as-set elements in %lld tables using "
"%s of memory\n", stats.aset_nmemb, stats.aset_cnt,
fmt_mem(stats.aset_size));
-   printf("%10lld prefix-set elments using %s of memory\n",
+   printf("%10lld prefix-set elements using %s of memory\n",
stats.pset_cnt, fmt_mem(stats.pset_size));
printf("RIB using %s of memory\n", fmt_mem(pts +
stats.prefix_cnt * sizeof(struct prefix) +
@@ -1894,7 +1894,7 @@ show_rib_memory_msg(struct imsg *imsg)
break;
case IMSG_CTL_SHOW_RIB_HASH:
memcpy(&hash, imsg->data, sizeof(hash));
-   printf("\t%s: size %lld, %lld entires\n", hash.name, hash.num,
+   printf("\t%s: size %lld, %lld entries\n", hash.name, hash.num,
hash.sum);
avg = (double)hash.sum / (double)hash.num;
dev = sqrt(fmax(0, hash.sumq / hash.num - avg * avg));



parse.y: strndup() in cmdline_symset()

2018-11-01 Thread Michael Mikonos
Hello,

When I updated cmdline_symset() in parse.y in the following commit
I missed src/sbin/{iked,ipsecctl,pfctl}/parse.y. OK to update them?

https://marc.info/?l=openbsd-cvs&m=153631079505256&w=2


Index: iked/parse.y
===
RCS file: /cvs/src/sbin/iked/parse.y,v
retrieving revision 1.76
diff -u -p -u -r1.76 parse.y
--- iked/parse.y1 Nov 2018 00:18:44 -   1.76
+++ iked/parse.y1 Nov 2018 07:03:31 -
@@ -1660,17 +1660,13 @@ cmdline_symset(char *s)
 {
char*sym, *val;
int ret;
-   size_t  len;
 
if ((val = strrchr(s, '=')) == NULL)
return (-1);
 
-   len = strlen(s) - strlen(val) + 1;
-   if ((sym = malloc(len)) == NULL)
+   sym = strndup(s, val - s);
+   if (sym == NULL)
err(1, "%s", __func__);
-
-   strlcpy(sym, s, len);
-
ret = symset(sym, val + 1, 1);
free(sym);
 
Index: ipsecctl/parse.y
===
RCS file: /cvs/src/sbin/ipsecctl/parse.y,v
retrieving revision 1.174
diff -u -p -u -r1.174 parse.y
--- ipsecctl/parse.y1 Nov 2018 00:18:44 -   1.174
+++ ipsecctl/parse.y1 Nov 2018 07:03:32 -
@@ -1426,17 +1426,13 @@ cmdline_symset(char *s)
 {
char*sym, *val;
int ret;
-   size_t  len;
 
if ((val = strrchr(s, '=')) == NULL)
return (-1);
 
-   len = strlen(s) - strlen(val) + 1;
-   if ((sym = malloc(len)) == NULL)
+   sym = strndup(s, val - s);
+   if (sym == NULL)
err(1, "%s", __func__);
-
-   strlcpy(sym, s, len);
-
ret = symset(sym, val + 1, 1);
free(sym);
 
Index: pfctl/parse.y
===
RCS file: /cvs/src/sbin/pfctl/parse.y,v
retrieving revision 1.685
diff -u -p -u -r1.685 parse.y
--- pfctl/parse.y   1 Nov 2018 00:18:44 -   1.685
+++ pfctl/parse.y   1 Nov 2018 07:03:34 -
@@ -5564,11 +5564,9 @@ pfctl_cmdline_symset(char *s)
if ((val = strrchr(s, '=')) == NULL)
return (-1);
 
-   if ((sym = malloc(strlen(s) - strlen(val) + 1)) == NULL)
+   sym = strndup(s, val - s);  
+   if (sym == NULL);
err(1, "%s", __func__);
-
-   strlcpy(sym, s, strlen(s) - strlen(val) + 1);
-
ret = symset(sym, val + 1, 1);
free(sym);