Re: [patch] add show.c style flag descriptions to route(8)

2022-12-21 Thread Jason McIntyre
On Wed, Dec 21, 2022 at 05:15:35PM -0500, Paul Tagliamonte wrote:
> Good news, I've got my MUA sorted. Hopefully this fixes the issues with
> the CVS diffs in my reply body.
> 
> On Wed, Dec 21, 2022 at 10:09:24PM +, Jason McIntyre wrote:
> > i like the choice of "display" over "print out".
> > i don;t like the "related metadata" bit though.
> > 
> > what about just
> > 
> > Display the routing table.
> 
> done.
> 
> > you should start new sentences on new lines.
> 
> understood, done.
> 
> > i would not remove the Xr. i think if we go ahead with this, we can make
> > a separate commit where we point people to route(8) rather than
> > netstart(8), but i suspect many pages will both want references to both.
> 
> makes sense, done.
> 
> > except for that, diff seems fine.
> 
> updated patch for review attached; thank you for your (very fast!) review.
> On the off-chance I've dorked it up, i've attached it, but I think we
> should be sorted.
> 
>   paultag
> 
> -- 
> :wq
> 

hi.

so i committed your diff, with some tweaks:

- respaced the indent on the entire command list (width Fl -> Ds).
  it was too large.

- you missed an ".El" in your diff. that turned out to be quite
  fortunate, because trying to work out where it was made me find a bug
  associated with how we added the "nameserver" command. the result
  being that the text no longer squashes after "nameserver".

- i slightly reworded the text, since just plonking in "The flags column" seemed
  a bit isolated.

- i forgot it in my commit message, but i shortened the desciption for
  RTF_GATEWAY to keep all entries on a single line.

so what i actually committed to route(8) is shown below. i didn;t
include netstat(1) since i took that as-is.

next step is to examine Xr. will get round to that when i can.

thanks for the diff.
jmc

Index: sbin/route/route.8
===
RCS file: /cvs/src/sbin/route/route.8,v
retrieving revision 1.106
diff -u -p -r1.106 route.8
--- sbin/route/route.8  19 Nov 2022 19:23:37 -  1.106
+++ sbin/route/route.8  22 Dec 2022 07:17:26 -
@@ -92,7 +92,7 @@ instead of a real routing socket to test
 The
 .Nm
 utility provides the following simple commands:
-.Bl -tag -width Fl
+.Bl -tag -width Ds
 .It Xo
 .Nm route
 .Op Fl T Ar rtable
@@ -159,9 +159,6 @@ are shown.
 If a routing table is supplied with
 .Fl T ,
 only changes in that routing table will be displayed.
-.El
-.Pp
-.Bl -tag -width Fl -compact
 .It Xo
 .Ic route
 .Op Fl dtv
@@ -185,7 +182,6 @@ If no
 .Ar address
 argument is given, a request to remove the nameservers previously entered for
 the given interface is sent.
-.Pp
 .It Xo
 .Nm route
 .Op Fl nv
@@ -197,10 +193,7 @@ the given interface is sent.
 .Op Fl label Ar label
 .Op Fl priority Ar priority
 .Xc
-Print out the routing table, in a fashion similar to "netstat -r".
-The output is documented in more detail towards the end of the
-.Xr netstat 1
-manual.
+Display the routing table.
 .Pp
 If
 .Fl gateway
@@ -224,6 +217,34 @@ or
 .Cm bgp .
 If the priority is negative, then routes that do not match the numeric
 priority are shown.
+.Pp
+Within the output of
+.Cm show ,
+the "Flags" column indicates what flags are set on the route.
+The mapping between letters and flags is:
+.Bl -column "1" "RTF_BLACKHOLE" "Protocol specific routing flag #1."
+.It 1 Ta Dv RTF_PROTO1 Ta "Protocol specific routing flag #1."
+.It 2 Ta Dv RTF_PROTO2 Ta "Protocol specific routing flag #2."
+.It 3 Ta Dv RTF_PROTO3 Ta "Protocol specific routing flag #3."
+.It B Ta Dv RTF_BLACKHOLE Ta "Just discard pkts (during updates)."
+.It b Ta Dv RTF_BROADCAST Ta "Correspond to a local broadcast address."
+.It C Ta Dv RTF_CLONING Ta "Generate new routes on use."
+.It c Ta Dv RTF_CLONED Ta "Cloned routes (generated from RTF_CLONING)."
+.It D Ta Dv RTF_DYNAMIC Ta "Created dynamically (by redirect)."
+.It G Ta Dv RTF_GATEWAY Ta "Dest requires forwarding by intermediary."
+.It H Ta Dv RTF_HOST Ta "Host entry (net otherwise)."
+.It h Ta Dv RTF_CACHED Ta "Referenced by gateway route."
+.It L Ta Dv RTF_LLINFO Ta "Valid protocol to link address translation."
+.It l Ta Dv RTF_LOCAL Ta "Correspond to a local address."
+.It M Ta Dv RTF_MODIFIED Ta "Modified dynamically (by redirect)."
+.It m Ta Dv RTF_MULTICAST Ta "Correspond to a multicast address."
+.It n Ta Dv RTF_CONNECTED Ta "Interface route."
+.It P Ta Dv RTF_MPATH Ta "Multipath route."
+.It R Ta Dv RTF_REJECT Ta "Host or net unreachable."
+.It S Ta Dv RTF_STATIC Ta "Manually added."
+.It T Ta Dv RTF_MPLS Ta "MPLS route."
+.It U Ta Dv RTF_UP Ta "Route usable."
+.El
 .El
 .Pp
 .Bl -tag -width Fl -compact



Re: malloc and immutable

2022-12-21 Thread Otto Moerbeek
On Sun, Dec 18, 2022 at 08:10:48PM +0100, Otto Moerbeek wrote:

> Hi,
> 
> This is the latest verion of a diff I sent around previously, but now
> it's time this gets wider testing for real.
> 
> The main purpose is to rearrange malloc init in such a way that a few
> pages containing mallic internal data structures can be made
> immutable. In addtion to that, each pools is inited on-demand instead
> of always.
> 
> So please test! I did not get a lot of feedback on the earlier versions.

Many thanks to all who tested so far!

I do lacks test reports on some more exoctic platforms, these days
that is !(amd64 || arm64). Any takers?

-Otto

> 
> Index: stdlib/malloc.c
> ===
> RCS file: /home/cvs/src/lib/libc/stdlib/malloc.c,v
> retrieving revision 1.275
> diff -u -p -r1.275 malloc.c
> --- stdlib/malloc.c   14 Oct 2022 04:38:39 -  1.275
> +++ stdlib/malloc.c   18 Dec 2022 18:57:16 -
> @@ -142,6 +142,7 @@ struct dir_info {
>   int malloc_junk;/* junk fill? */
>   int mmap_flag;  /* extra flag for mmap */
>   int mutex;
> + int malloc_mt;  /* multi-threaded mode? */
>   /* lists of free chunk info structs */
>   struct chunk_head chunk_info_list[MALLOC_MAXSHIFT + 1];
>   /* lists of chunks with free slots */
> @@ -181,8 +182,6 @@ struct dir_info {
>  #endif /* MALLOC_STATS */
>   u_int32_t canary2;
>  };
> -#define DIR_INFO_RSZ ((sizeof(struct dir_info) + MALLOC_PAGEMASK) & \
> - ~MALLOC_PAGEMASK)
>  
>  static void unmap(struct dir_info *d, void *p, size_t sz, size_t clear);
>  
> @@ -208,7 +207,6 @@ struct malloc_readonly {
>   /* Main bookkeeping information */
>   struct dir_info *malloc_pool[_MALLOC_MUTEXES];
>   u_int   malloc_mutexes; /* how much in actual use? */
> - int malloc_mt;  /* multi-threaded mode? */
>   int malloc_freecheck;   /* Extensive double free check */
>   int malloc_freeunmap;   /* mprotect free pages PROT_NONE? */
>   int def_malloc_junk;/* junk fill? */
> @@ -258,7 +256,7 @@ static void malloc_exit(void);
>  static inline void
>  _MALLOC_LEAVE(struct dir_info *d)
>  {
> - if (mopts.malloc_mt) {
> + if (d->malloc_mt) {
>   d->active--;
>   _MALLOC_UNLOCK(d->mutex);
>   }
> @@ -267,7 +265,7 @@ _MALLOC_LEAVE(struct dir_info *d)
>  static inline void
>  _MALLOC_ENTER(struct dir_info *d)
>  {
> - if (mopts.malloc_mt) {
> + if (d->malloc_mt) {
>   _MALLOC_LOCK(d->mutex);
>   d->active++;
>   }
> @@ -292,7 +290,7 @@ hash(void *p)
>  static inline struct dir_info *
>  getpool(void)
>  {
> - if (!mopts.malloc_mt)
> + if (mopts.malloc_pool[1] == NULL || !mopts.malloc_pool[1]->malloc_mt)
>   return mopts.malloc_pool[1];
>   else/* first one reserved for special pool */
>   return mopts.malloc_pool[1 + TIB_GET()->tib_tid %
> @@ -497,46 +495,22 @@ omalloc_init(void)
>  }
>  
>  static void
> -omalloc_poolinit(struct dir_info **dp, int mmap_flag)
> +omalloc_poolinit(struct dir_info *d, int mmap_flag)
>  {
> - char *p;
> - size_t d_avail, regioninfo_size;
> - struct dir_info *d;
>   int i, j;
>  
> - /*
> -  * Allocate dir_info with a guard page on either side. Also
> -  * randomise offset inside the page at which the dir_info
> -  * lies (subject to alignment by 1 << MALLOC_MINSHIFT)
> -  */
> - if ((p = MMAPNONE(DIR_INFO_RSZ + (MALLOC_PAGESIZE * 2), mmap_flag)) ==
> - MAP_FAILED)
> - wrterror(NULL, "malloc init mmap failed");
> - mprotect(p + MALLOC_PAGESIZE, DIR_INFO_RSZ, PROT_READ | PROT_WRITE);
> - d_avail = (DIR_INFO_RSZ - sizeof(*d)) >> MALLOC_MINSHIFT;
> - d = (struct dir_info *)(p + MALLOC_PAGESIZE +
> - (arc4random_uniform(d_avail) << MALLOC_MINSHIFT));
> -
> - rbytes_init(d);
> - d->regions_free = d->regions_total = MALLOC_INITIAL_REGIONS;
> - regioninfo_size = d->regions_total * sizeof(struct region_info);
> - d->r = MMAP(regioninfo_size, mmap_flag);
> - if (d->r == MAP_FAILED) {
> - d->regions_total = 0;
> - wrterror(NULL, "malloc init mmap failed");
> - }
> + d->r = NULL;
> + d->rbytesused = sizeof(d->rbytes);
> + d->regions_free = d->regions_total = 0;
>   for (i = 0; i <= MALLOC_MAXSHIFT; i++) {
>   LIST_INIT(>chunk_info_list[i]);
>   for (j = 0; j < MALLOC_CHUNK_LISTS; j++)
>   LIST_INIT(>chunk_dir[i][j]);
>   }
> - STATS_ADD(d->malloc_used, regioninfo_size + 3 * MALLOC_PAGESIZE);
>   d->mmap_flag = mmap_flag;
>   d->malloc_junk = mopts.def_malloc_junk;
>   d->canary1 = mopts.malloc_canary ^ 

Re: LLVM 15: mismatched bound errors

2022-12-21 Thread Theo de Raadt
Jeremie Courreges-Anglas  wrote:

> On Wed, Dec 21 2022, "Theo de Raadt"  wrote:
> >> But yeah, maybe we'll just flip the default option in LLVM and then
> >> we'll just not use that warning... at all?
> >
> > is this specific warning finding dangerous bugs?  is it finding a 
> > substantial
> > number of dangerous bugs?
> 
> No, but if we remove -Wdeprecated-non-prototype from -Wall, software
> built with clang -Wall on OpenBSD won't hit this warning and may later
> fail when built with clang -Wall on other systems.  *I* don't care that
> much, but I suspect that this is not the additional warning that will
> hurt us most...

well, i'm not sure it is bad if some other people get the opposite effect.

I don't think anyone disagrees with the move towards more strict C.

But the pain must be evenly distributed, and every maintainer must adopt
the practice on their own schedule, well mostly.  it is the maintainers
who must adopt the practices, one after the other, without workarounds
imposed by downstreams.

If we end up with 100 ports Makefiles containing these -W, even after
upstreams adapt to the new reality those -W options are going to stay
around and it will be 2038 before the last one is removed by someone
digging for bones



Re: LLVM 15: mismatched bound errors

2022-12-21 Thread Jeremie Courreges-Anglas
On Wed, Dec 21 2022, "Theo de Raadt"  wrote:
>> But yeah, maybe we'll just flip the default option in LLVM and then
>> we'll just not use that warning... at all?
>
> is this specific warning finding dangerous bugs?  is it finding a substantial
> number of dangerous bugs?

No, but if we remove -Wdeprecated-non-prototype from -Wall, software
built with clang -Wall on OpenBSD won't hit this warning and may later
fail when built with clang -Wall on other systems.  *I* don't care that
much, but I suspect that this is not the additional warning that will
hurt us most...

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



Re: LLVM 15: mismatched bound errors

2022-12-21 Thread Theo de Raadt
> So for base we don't really need to tweak the defaults, I'd
> rather see what happens in ports wrt -Wdeprecated-non-prototype, and
> I don't think it's the biggest offender.

If libz is hitting this, then I think the situation will be dire in
ports.

I think the clang developers are so busy chasing the future and are
accelerating being out of touch with reality in the trenches.



Re: LLVM 15: mismatched bound errors

2022-12-21 Thread Jeremie Courreges-Anglas
On Thu, Dec 22 2022, Patrick Wildt  wrote:
> Am Thu, Dec 22, 2022 at 01:57:37AM +0100 schrieb Jeremie Courreges-Anglas:
>> On Thu, Dec 22 2022, Theo Buehler  wrote:
>> > On Thu, Dec 22, 2022 at 11:39:41AM +1100, Jonathan Gray wrote:
>> >> On Thu, Dec 22, 2022 at 01:20:32AM +0100, Theo Buehler wrote:
>> >> > > Any concerns regarding the changes in libz?  It introduces diff to
>> >> > > upstream, but the recent commits seemed to indicate we have forked
>> >> > > anyway?
>> >> > 
>> >> > I've worked hard to keep the diff to upstream minimal. Why are these
>> >> > changes needed?
>> >> 
>> >> the warnings are from -Wdeprecated-non-prototype
>> >> 
>> >> https://github.com/madler/zlib/issues/633
>> >
>> > So, can't we fix this by adding -Wno-deprecated-non-prototype to the
>> > kernel build and wait until madler converts upstream?
>> 
>> That's what I initially proposed on my own wip branch:
>> 
>>   
>> https://github.com/jcourreges/openbsd-src/commit/50d8bd24dadc25aa7e985de898bccf92a60b72ee
>> 
>> -Wno-deprecated-non-prototype + -Wno-unknown-warning-option (the latter
>> option because current clang doesn't understand the former).  libz built
>> as part of libsa and the bootloaders also needs a similar tweak in MD
>> Makefiles:
>> 
>>   
>> https://github.com/jcourreges/openbsd-src/commit/2107b762420ef6ea863e349e5faea4254d44fdf9
>> 
>> The build of src/lib/libz doesn't use -Werror and thus doesn't error out.
>> 
>> The two commits above are obviously incomplete, other archs need similar
>> handling.  Apart from the fact that I authored them, I think they're
>> the sanest way to handle this libz "oddness".  Not just because I care
>> about tb's sanity.  :)
>
> I had hoped that we can resolve that without adding another set of -W
> flags everywhere, but I guess the libz churn isn't worth it.
>
> But yeah, maybe we'll just flip the default option in LLVM and then
> we'll just not use that warning... at all?

That's an approach we didn't use for llvm13.  Right now clang-local(1)
only lists -Wpointer-sign and -Waddress-of-packed-member as different
from upstream.  As far as base is
concerned, -Wno-deprecated-non-prototype can be introduced just for libz
(kernels + bootloaders) and then removed as soon as upstream makes the
jump.  So for base we don't really need to tweak the defaults, I'd
rather see what happens in ports wrt -Wdeprecated-non-prototype, and
I don't think it's the biggest offender.

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



Re: LLVM 15: mismatched bound errors

2022-12-21 Thread Theo de Raadt
> But yeah, maybe we'll just flip the default option in LLVM and then
> we'll just not use that warning... at all?

is this specific warning finding dangerous bugs?  is it finding a substantial
number of dangerous bugs?



Re: LLVM 15: mismatched bound errors

2022-12-21 Thread Jeremie Courreges-Anglas
On Tue, Dec 20 2022, Todd C. Miller  wrote:
> On Tue, 20 Dec 2022 23:44:08 +0100, Patrick Wildt wrote:
>
>> clang complains when the function is declared with a fixed array size in
>> a parameter while the prototype is unbounded, like this:
>>
>> /usr/src/sys/net/pf.c:4353:54: error: argument 'sns' of type 'struct pf_src_n
>> ode *[4]' with mismatched bound [-Werror,-Warray-parameter]
>> struct pf_rule_actions *act, struct pf_src_node *sns[PF_SN_MAX])
>>  ^
>> /usr/src/sys/net/pf.c:203:28: note: previously declared as 'struct pf_src_nod
>> e *[]' here
>> struct pf_src_node *[]);
>> ^
>> 1 error generated.
>>
>> We have a few of that, and was wondering what the better solution is.
>> clang apparently accepts using * instead of [].  The alternative would
>> be to hardcode the size in the prototype as well.  Here's a diff for
>> a three files for the first version, as example.
>
> Using * instead of [] is the saner approach.  Hard-coding the sizes
> into the prototype seems like a bad idea, even if clang is now smart
> enough to complain about a mismatch.

At first I went with this approach:

  
https://github.com/jcourreges/openbsd-src/commit/b24dd00848e30977d1aed7404afe9455aff0c9dc

but the full kernel diff to add the sizes to the declaration is short,
as the size is either hardcoded in the same file or hidden behind
a define in scope:

  
https://github.com/jcourreges/openbsd-src/commit/4862df383ccb8a8e03d5c11b4fb739b6a3a5a7c7

Sadly making the size available in the declaration doesn't seem to be
clang any smarter (yet?).  clang won't warn about passing the address of
array[10] to a function which access array[15] or so.

I don't care much about the direction we end up using, but specifying
the size in the declaration isn't insane.  We seldom pass a pointers to
a buffer without an accompanying buffer length.

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



Re: LLVM 15: mismatched bound errors

2022-12-21 Thread Patrick Wildt
Am Thu, Dec 22, 2022 at 01:57:37AM +0100 schrieb Jeremie Courreges-Anglas:
> On Thu, Dec 22 2022, Theo Buehler  wrote:
> > On Thu, Dec 22, 2022 at 11:39:41AM +1100, Jonathan Gray wrote:
> >> On Thu, Dec 22, 2022 at 01:20:32AM +0100, Theo Buehler wrote:
> >> > > Any concerns regarding the changes in libz?  It introduces diff to
> >> > > upstream, but the recent commits seemed to indicate we have forked
> >> > > anyway?
> >> > 
> >> > I've worked hard to keep the diff to upstream minimal. Why are these
> >> > changes needed?
> >> 
> >> the warnings are from -Wdeprecated-non-prototype
> >> 
> >> https://github.com/madler/zlib/issues/633
> >
> > So, can't we fix this by adding -Wno-deprecated-non-prototype to the
> > kernel build and wait until madler converts upstream?
> 
> That's what I initially proposed on my own wip branch:
> 
>   
> https://github.com/jcourreges/openbsd-src/commit/50d8bd24dadc25aa7e985de898bccf92a60b72ee
> 
> -Wno-deprecated-non-prototype + -Wno-unknown-warning-option (the latter
> option because current clang doesn't understand the former).  libz built
> as part of libsa and the bootloaders also needs a similar tweak in MD
> Makefiles:
> 
>   
> https://github.com/jcourreges/openbsd-src/commit/2107b762420ef6ea863e349e5faea4254d44fdf9
> 
> The build of src/lib/libz doesn't use -Werror and thus doesn't error out.
> 
> The two commits above are obviously incomplete, other archs need similar
> handling.  Apart from the fact that I authored them, I think they're
> the sanest way to handle this libz "oddness".  Not just because I care
> about tb's sanity.  :)

I had hoped that we can resolve that without adding another set of -W
flags everywhere, but I guess the libz churn isn't worth it.

But yeah, maybe we'll just flip the default option in LLVM and then
we'll just not use that warning... at all?



Re: LLVM 15: mismatched bound errors

2022-12-21 Thread Jeremie Courreges-Anglas
On Thu, Dec 22 2022, Theo Buehler  wrote:
> On Thu, Dec 22, 2022 at 11:39:41AM +1100, Jonathan Gray wrote:
>> On Thu, Dec 22, 2022 at 01:20:32AM +0100, Theo Buehler wrote:
>> > > Any concerns regarding the changes in libz?  It introduces diff to
>> > > upstream, but the recent commits seemed to indicate we have forked
>> > > anyway?
>> > 
>> > I've worked hard to keep the diff to upstream minimal. Why are these
>> > changes needed?
>> 
>> the warnings are from -Wdeprecated-non-prototype
>> 
>> https://github.com/madler/zlib/issues/633
>
> So, can't we fix this by adding -Wno-deprecated-non-prototype to the
> kernel build and wait until madler converts upstream?

That's what I initially proposed on my own wip branch:

  
https://github.com/jcourreges/openbsd-src/commit/50d8bd24dadc25aa7e985de898bccf92a60b72ee

-Wno-deprecated-non-prototype + -Wno-unknown-warning-option (the latter
option because current clang doesn't understand the former).  libz built
as part of libsa and the bootloaders also needs a similar tweak in MD
Makefiles:

  
https://github.com/jcourreges/openbsd-src/commit/2107b762420ef6ea863e349e5faea4254d44fdf9

The build of src/lib/libz doesn't use -Werror and thus doesn't error out.

The two commits above are obviously incomplete, other archs need similar
handling.  Apart from the fact that I authored them, I think they're
the sanest way to handle this libz "oddness".  Not just because I care
about tb's sanity.  :)

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



Re: LLVM 15: mismatched bound errors

2022-12-21 Thread Theo de Raadt
Theo Buehler  wrote:

> On Thu, Dec 22, 2022 at 11:39:41AM +1100, Jonathan Gray wrote:
> > On Thu, Dec 22, 2022 at 01:20:32AM +0100, Theo Buehler wrote:
> > > > Any concerns regarding the changes in libz?  It introduces diff to
> > > > upstream, but the recent commits seemed to indicate we have forked
> > > > anyway?
> > > 
> > > I've worked hard to keep the diff to upstream minimal. Why are these
> > > changes needed?
> > 
> > the warnings are from -Wdeprecated-non-prototype
> > 
> > https://github.com/madler/zlib/issues/633
> 
> So, can't we fix this by adding -Wno-deprecated-non-prototype to the
> kernel build and wait until madler converts upstream?

yes, very possible

or we could decide that clang is jumping the gun on disabling some legal
syntax, as well as trying to hunt down things they don't like but which
are not actually harmful, and flip the compiler default back.



Re: LLVM 15: mismatched bound errors

2022-12-21 Thread Theo Buehler
On Thu, Dec 22, 2022 at 11:39:41AM +1100, Jonathan Gray wrote:
> On Thu, Dec 22, 2022 at 01:20:32AM +0100, Theo Buehler wrote:
> > > Any concerns regarding the changes in libz?  It introduces diff to
> > > upstream, but the recent commits seemed to indicate we have forked
> > > anyway?
> > 
> > I've worked hard to keep the diff to upstream minimal. Why are these
> > changes needed?
> 
> the warnings are from -Wdeprecated-non-prototype
> 
> https://github.com/madler/zlib/issues/633

So, can't we fix this by adding -Wno-deprecated-non-prototype to the
kernel build and wait until madler converts upstream?



Re: LLVM 15: mismatched bound errors

2022-12-21 Thread Jonathan Gray
On Thu, Dec 22, 2022 at 01:20:32AM +0100, Theo Buehler wrote:
> > Any concerns regarding the changes in libz?  It introduces diff to
> > upstream, but the recent commits seemed to indicate we have forked
> > anyway?
> 
> I've worked hard to keep the diff to upstream minimal. Why are these
> changes needed?

the warnings are from -Wdeprecated-non-prototype

https://github.com/madler/zlib/issues/633



Re: LLVM 15: mismatched bound errors

2022-12-21 Thread Theo de Raadt
Theo Buehler  wrote:

> > Any concerns regarding the changes in libz?  It introduces diff to
> > upstream, but the recent commits seemed to indicate we have forked
> > anyway?
> 
> I've worked hard to keep the diff to upstream minimal. Why are these
> changes needed?

becauase clang believes they are the new custodians of the C language,
(while they spend all their time coding in other languages)



Re: LLVM 15: mismatched bound errors

2022-12-21 Thread Theo Buehler
> Any concerns regarding the changes in libz?  It introduces diff to
> upstream, but the recent commits seemed to indicate we have forked
> anyway?

I've worked hard to keep the diff to upstream minimal. Why are these
changes needed?



Re: LLVM 15: mismatched bound errors

2022-12-21 Thread Patrick Wildt
On Tue, Dec 20, 2022 at 05:48:41PM -0700, Todd C. Miller wrote:
> On Tue, 20 Dec 2022 23:44:08 +0100, Patrick Wildt wrote:
> 
> > clang complains when the function is declared with a fixed array size in
> > a parameter while the prototype is unbounded, like this:
> >
> > /usr/src/sys/net/pf.c:4353:54: error: argument 'sns' of type 'struct 
> > pf_src_n
> > ode *[4]' with mismatched bound [-Werror,-Warray-parameter]
> > struct pf_rule_actions *act, struct pf_src_node *sns[PF_SN_MAX])
> >  ^
> > /usr/src/sys/net/pf.c:203:28: note: previously declared as 'struct 
> > pf_src_nod
> > e *[]' here
> > struct pf_src_node *[]);
> > ^
> > 1 error generated.
> >
> > We have a few of that, and was wondering what the better solution is.
> > clang apparently accepts using * instead of [].  The alternative would
> > be to hardcode the size in the prototype as well.  Here's a diff for
> > a three files for the first version, as example.
> 
> Using * instead of [] is the saner approach.  Hard-coding the sizes
> into the prototype seems like a bad idea, even if clang is now smart
> enough to complain about a mismatch.
> 
>  - todd

So, here's the full diff that allows me to compile arm64 GENERIC.MP,
with radeondrm and amdgpu disabled.

Any concerns regarding the changes in libz?  It introduces diff to
upstream, but the recent commits seemed to indicate we have forked
anyway?

There's also an unrelated diff for iwx(4) in here, because of two
incidents of this:

In file included from /usr/src/sys/dev/pci/if_iwx.c:151:
/usr/src/sys/dev/pci/if_iwxreg.h:3626:2: error: field  within 'struct 
iwx_rx_mpdu_desc_v3' is less aligned than 'union 
iwx_rx_mpdu_desc_v3::(anonymous at /usr/src/sys/dev/pci/if_iwxreg.h:3626:2)' 
and is usually due to 'struct iwx_rx_mpdu_desc_v3' being packed, which can lead 
to unaligned accesses [-Werror,-Wunaligned-access]
union {
^

Cheers,
Patrick

diff --git a/sys/crypto/sha2.c b/sys/crypto/sha2.c
index f789ef3f55b..b2b79f286f3 100644
--- a/sys/crypto/sha2.c
+++ b/sys/crypto/sha2.c
@@ -470,7 +470,7 @@ SHA256Update(SHA2_CTX *context, const void *dataptr, size_t 
len)
 }
 
 void
-SHA256Final(u_int8_t digest[], SHA2_CTX *context)
+SHA256Final(u_int8_t *digest, SHA2_CTX *context)
 {
unsigned intusedspace;
 
@@ -795,7 +795,7 @@ SHA512Last(SHA2_CTX *context)
 }
 
 void
-SHA512Final(u_int8_t digest[], SHA2_CTX *context)
+SHA512Final(u_int8_t *digest, SHA2_CTX *context)
 {
 
SHA512Last(context);
@@ -834,7 +834,7 @@ SHA384Update(SHA2_CTX *context, const void *data, size_t 
len)
 }
 
 void
-SHA384Final(u_int8_t digest[], SHA2_CTX *context)
+SHA384Final(u_int8_t *digest, SHA2_CTX *context)
 {
 
SHA512Last(context);
diff --git a/sys/dev/ic/ar5008.c b/sys/dev/ic/ar5008.c
index cad0f142210..4c0126fd6d6 100644
--- a/sys/dev/ic/ar5008.c
+++ b/sys/dev/ic/ar5008.c
@@ -111,7 +111,7 @@ voidar5008_next_calib(struct athn_softc *);
 void   ar5008_calib_iq(struct athn_softc *);
 void   ar5008_calib_adc_gain(struct athn_softc *);
 void   ar5008_calib_adc_dc_off(struct athn_softc *);
-void   ar5008_write_txpower(struct athn_softc *, int16_t power[]);
+void   ar5008_write_txpower(struct athn_softc *, int16_t *);
 void   ar5008_set_viterbi_mask(struct athn_softc *, int);
 void   ar5008_hw_init(struct athn_softc *, struct ieee80211_channel *,
struct ieee80211_channel *);
@@ -119,9 +119,9 @@ uint8_t ar5008_get_vpd(uint8_t, const uint8_t *, const 
uint8_t *, int);
 void   ar5008_get_pdadcs(struct athn_softc *, uint8_t, struct athn_pier *,
struct athn_pier *, int, int, uint8_t, uint8_t *, uint8_t *);
 void   ar5008_get_lg_tpow(struct athn_softc *, struct ieee80211_channel *,
-   uint8_t, const struct ar_cal_target_power_leg *, int, uint8_t[]);
+   uint8_t, const struct ar_cal_target_power_leg *, int, uint8_t *);
 void   ar5008_get_ht_tpow(struct athn_softc *, struct ieee80211_channel *,
-   uint8_t, const struct ar_cal_target_power_ht *, int, uint8_t[]);
+   uint8_t, const struct ar_cal_target_power_ht *, int, uint8_t *);
 void   ar5008_set_noise_immunity_level(struct athn_softc *, int);
 void   ar5008_enable_ofdm_weak_signal(struct athn_softc *);
 void   ar5008_disable_ofdm_weak_signal(struct athn_softc *);
diff --git a/sys/dev/ic/ar9003.c b/sys/dev/ic/ar9003.c
index 565ea27c701..7ec3131f86b 100644
--- a/sys/dev/ic/ar9003.c
+++ b/sys/dev/ic/ar9003.c
@@ -114,7 +114,7 @@ int ar9003_init_calib(struct athn_softc *);
 void   ar9003_do_calib(struct athn_softc *);
 void   ar9003_next_calib(struct athn_softc *);
 void   ar9003_calib_iq(struct athn_softc *);
-intar9003_get_iq_corr(struct athn_softc *, int32_t[], int32_t[]);
+intar9003_get_iq_corr(struct athn_softc *, int32_t *, int32_t *);
 intar9003_calib_tx_iq(struct athn_softc *);
 void   ar9003_paprd_calib(struct athn_softc *, struct 

Introduce SBL_WAIT and SLB_NOINTR sblock() flags

2022-12-21 Thread Vitaliy Makkoveev
The first one replaces malloc(9) related M_NOWAIT and M_WAITOK flags we
use with sblock() to wait for lock if not immediately available.

The second one proposed to use if the lock awaiting should be
uninterruptible. It affects on sblock() behaviour for this call, instead
of SB_NOINTR flag which globally changes sblock() behaviour for this
socket buffer.

The SBL_NOINTR flag should be used for SB_NOINTR use cases like within
sorflush(). With standalone sblock() the similar sblock() calls will be
expanded to more places, and the SB_NOINTR flag operations around
sblock() calls are unwanted. At least, they are inconsistent because
concurrent sblock() threads could override this flag and change locking
behaviour for each others. Currently, sorflush() doesn't clear SB_NOINTR
flag but this looks incorrect, because after sorflush() call the
soreceive() path for this socket becomes uninterruptible. This is not
visible, because the socket is not used after shutdown(2) for send and
receive data.

Please note, the existing SB_NOINTR flag logic kept as is.

Also, we don't expose M_WAITOK/M_NOWAIT flags passed to sblock(), so
this diff doesn't breaks ABI.

Index: sys/kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.293
diff -u -p -r1.293 uipc_socket.c
--- sys/kern/uipc_socket.c  12 Dec 2022 08:30:22 -  1.293
+++ sys/kern/uipc_socket.c  21 Dec 2022 22:31:53 -
@@ -516,7 +516,7 @@ sodisconnect(struct socket *so)
 
 int m_getuio(struct mbuf **, int, long, struct uio *);
 
-#defineSBLOCKWAIT(f)   (((f) & MSG_DONTWAIT) ? M_NOWAIT : M_WAITOK)
+#defineSBLOCKWAIT(f)   (((f) & MSG_DONTWAIT) ? 0 : SBL_WAIT)
 /*
  * Send on a socket.
  * If send must go all at once and message is larger than
@@ -1209,9 +1209,8 @@ sorflush(struct socket *so)
const struct protosw *pr = so->so_proto;
int error;
 
-   sb->sb_flags |= SB_NOINTR;
-   error = sblock(so, sb, M_WAITOK);
-   /* with SB_NOINTR and M_WAITOK sblock() must not fail */
+   error = sblock(so, sb, SBL_WAIT | SBL_NOINTR);
+   /* with SBL_WAIT and SLB_NOINTR sblock() must not fail */
KASSERT(error == 0);
socantrcvmore(so);
m = sb->sb_mb;
@@ -1275,7 +1274,7 @@ sosplice(struct socket *so, int fd, off_
/* If no fd is given, unsplice by removing existing link. */
if (fd < 0) {
/* Lock receive buffer. */
-   if ((error = sblock(so, >so_rcv, M_WAITOK)) != 0) {
+   if ((error = sblock(so, >so_rcv, SBL_WAIT)) != 0) {
return (error);
}
if (so->so_sp->ssp_socket)
@@ -1308,10 +1307,10 @@ sosplice(struct socket *so, int fd, off_
}
 
/* Lock both receive and send buffer. */
-   if ((error = sblock(so, >so_rcv, M_WAITOK)) != 0) {
+   if ((error = sblock(so, >so_rcv, SBL_WAIT)) != 0) {
goto frele;
}
-   if ((error = sblock(so, >so_snd, M_WAITOK)) != 0) {
+   if ((error = sblock(so, >so_snd, SBL_WAIT)) != 0) {
sbunlock(so, >so_rcv);
goto frele;
}
Index: sys/kern/uipc_socket2.c
===
RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
retrieving revision 1.131
diff -u -p -r1.131 uipc_socket2.c
--- sys/kern/uipc_socket2.c 12 Dec 2022 08:30:22 -  1.131
+++ sys/kern/uipc_socket2.c 21 Dec 2022 22:31:54 -
@@ -494,9 +494,9 @@ sbwait(struct socket *so, struct sockbuf
 }
 
 int
-sblock(struct socket *so, struct sockbuf *sb, int wait)
+sblock(struct socket *so, struct sockbuf *sb, int flags)
 {
-   int error, prio = (sb->sb_flags & SB_NOINTR) ? PSOCK : PSOCK | PCATCH;
+   int error, prio = PSOCK;
 
soassertlocked(so);
 
@@ -504,8 +504,10 @@ sblock(struct socket *so, struct sockbuf
sb->sb_flags |= SB_LOCK;
return (0);
}
-   if (wait & M_NOWAIT)
+   if ((flags & SBL_WAIT) == 0)
return (EWOULDBLOCK);
+   if (!(flags & SBL_NOINTR || sb->sb_flags & SB_NOINTR))
+   prio |= PCATCH;
 
while (sb->sb_flags & SB_LOCK) {
sb->sb_flags |= SB_WANT;
Index: sys/sys/socketvar.h
===
RCS file: /cvs/src/sys/sys/socketvar.h,v
retrieving revision 1.114
diff -u -p -r1.114 socketvar.h
--- sys/sys/socketvar.h 12 Dec 2022 08:30:22 -  1.114
+++ sys/sys/socketvar.h 21 Dec 2022 22:31:54 -
@@ -265,6 +265,12 @@ sbfree(struct socket *so, struct sockbuf
 }
 
 /*
+ * Flags to sblock()
+ */
+#define SBL_WAIT   0x01/* Wait if lock not immidiately available. */
+#define SBL_NOINTR 0x02/* Enforce non-interruptible sleep. */
+
+/*
  * Set lock on sockbuf sb; sleep if lock is already held.
  * Unless SB_NOINTR is set on sockbuf, sleep is interruptible.
  * Returns 

Re: [patch] add show.c style flag descriptions to route(8)

2022-12-21 Thread Theo de Raadt
Jason McIntyre  wrote:

> i would not remove the Xr. i think if we go ahead with this, we can make
> a separate commit where we point people to route(8) rather than
> netstart(8), but i suspect many pages will both want references to both.

netstat, not netstart, heh

But yes it is true that some locations may want both.  There are probably
missing pieces in both comands...



Re: [patch] add show.c style flag descriptions to route(8)

2022-12-21 Thread Paul Tagliamonte
Good news, I've got my MUA sorted. Hopefully this fixes the issues with
the CVS diffs in my reply body.

On Wed, Dec 21, 2022 at 10:09:24PM +, Jason McIntyre wrote:
> i like the choice of "display" over "print out".
> i don;t like the "related metadata" bit though.
> 
> what about just
> 
>   Display the routing table.

done.

> you should start new sentences on new lines.

understood, done.

> i would not remove the Xr. i think if we go ahead with this, we can make
> a separate commit where we point people to route(8) rather than
> netstart(8), but i suspect many pages will both want references to both.

makes sense, done.

> except for that, diff seems fine.

updated patch for review attached; thank you for your (very fast!) review.
On the off-chance I've dorked it up, i've attached it, but I think we
should be sorted.

  paultag

-- 
:wq

Index: sbin/route/route.8
===
RCS file: /cvs/src/sbin/route/route.8,v
retrieving revision 1.106
diff -u -p -r1.106 route.8
--- sbin/route/route.8  19 Nov 2022 19:23:37 -  1.106
+++ sbin/route/route.8  21 Dec 2022 22:12:12 -
@@ -197,10 +197,7 @@ the given interface is sent.
 .Op Fl label Ar label
 .Op Fl priority Ar priority
 .Xc
-Print out the routing table, in a fashion similar to "netstat -r".
-The output is documented in more detail towards the end of the
-.Xr netstat 1
-manual.
+Display the routing table.
 .Pp
 If
 .Fl gateway
@@ -224,6 +221,31 @@ or
 .Cm bgp .
 If the priority is negative, then routes that do not match the numeric
 priority are shown.
+.Pp
+The "Flags" column indicates what flags are set on the route.
+The mapping between letters and flags is:
+.Bl -column "1" "RTF_BLACKHOLE" "Protocol specific routing flag #1."
+.It 1 Ta Dv RTF_PROTO1 Ta "Protocol specific routing flag #1."
+.It 2 Ta Dv RTF_PROTO2 Ta "Protocol specific routing flag #2."
+.It 3 Ta Dv RTF_PROTO3 Ta "Protocol specific routing flag #3."
+.It B Ta Dv RTF_BLACKHOLE Ta "Just discard pkts (during updates)."
+.It b Ta Dv RTF_BROADCAST Ta "Correspond to a local broadcast address."
+.It C Ta Dv RTF_CLONING Ta "Generate new routes on use."
+.It c Ta Dv RTF_CLONED Ta "Cloned routes (generated from RTF_CLONING)."
+.It D Ta Dv RTF_DYNAMIC Ta "Created dynamically (by redirect)."
+.It G Ta Dv RTF_GATEWAY Ta "Destination requires forwarding by intermediary."
+.It H Ta Dv RTF_HOST Ta "Host entry (net otherwise)."
+.It h Ta Dv RTF_CACHED Ta "Referenced by gateway route."
+.It L Ta Dv RTF_LLINFO Ta "Valid protocol to link address translation."
+.It l Ta Dv RTF_LOCAL Ta "Correspond to a local address."
+.It M Ta Dv RTF_MODIFIED Ta "Modified dynamically (by redirect)."
+.It m Ta Dv RTF_MULTICAST Ta "Correspond to a multicast address."
+.It n Ta Dv RTF_CONNECTED Ta "Interface route."
+.It P Ta Dv RTF_MPATH Ta "Multipath route."
+.It R Ta Dv RTF_REJECT Ta "Host or net unreachable."
+.It S Ta Dv RTF_STATIC Ta "Manually added."
+.It T Ta Dv RTF_MPLS Ta "MPLS route."
+.It U Ta Dv RTF_UP Ta "Route usable."
 .El
 .Pp
 .Bl -tag -width Fl -compact
Index: usr.bin/netstat/netstat.1
===
RCS file: /cvs/src/usr.bin/netstat/netstat.1,v
retrieving revision 1.95
diff -u -p -r1.95 netstat.1
--- usr.bin/netstat/netstat.1   8 Sep 2022 13:18:47 -   1.95
+++ usr.bin/netstat/netstat.1   21 Dec 2022 22:12:13 -
@@ -343,31 +343,6 @@ and
 .Xr route 4
 manual pages.
 .Pp
-The mapping between letters and flags is:
-.Bl -column "1" "RTF_BLACKHOLE" "Protocol specific routing flag #1."
-.It 1 Ta Dv RTF_PROTO1 Ta "Protocol specific routing flag #1."
-.It 2 Ta Dv RTF_PROTO2 Ta "Protocol specific routing flag #2."
-.It 3 Ta Dv RTF_PROTO3 Ta "Protocol specific routing flag #3."
-.It B Ta Dv RTF_BLACKHOLE Ta "Just discard pkts (during updates)."
-.It b Ta Dv RTF_BROADCAST Ta "Correspond to a local broadcast address."
-.It C Ta Dv RTF_CLONING Ta "Generate new routes on use."
-.It c Ta Dv RTF_CLONED Ta "Cloned routes (generated from RTF_CLONING)."
-.It D Ta Dv RTF_DYNAMIC Ta "Created dynamically (by redirect)."
-.It G Ta Dv RTF_GATEWAY Ta "Destination requires forwarding by intermediary."
-.It H Ta Dv RTF_HOST Ta "Host entry (net otherwise)."
-.It h Ta Dv RTF_CACHED Ta "Referenced by gateway route."
-.It L Ta Dv RTF_LLINFO Ta "Valid protocol to link address translation."
-.It l Ta Dv RTF_LOCAL Ta "Correspond to a local address."
-.It M Ta Dv RTF_MODIFIED Ta "Modified dynamically (by redirect)."
-.It m Ta Dv RTF_MULTICAST Ta "Correspond to a multicast address."
-.It n Ta Dv RTF_CONNECTED Ta "Interface route."
-.It P Ta Dv RTF_MPATH Ta "Multipath route."
-.It R Ta Dv RTF_REJECT Ta "Host or net unreachable."
-.It S Ta Dv RTF_STATIC Ta "Manually added."
-.It T Ta Dv RTF_MPLS Ta "MPLS route."
-.It U Ta Dv RTF_UP Ta "Route usable."
-.El
-.Pp
 Direct routes are created for each interface attached to the local host;
 the gateway field for such entries shows the address of the outgoing 

Re: [patch] add show.c style flag descriptions to route(8)

2022-12-21 Thread Jason McIntyre
On Wed, Dec 21, 2022 at 12:41:42PM -0500, Paul R. Tagliamonte wrote:
> > So I think route.8 is where we should try to have complete documentation,
> > and once that is done we should change Xr's and other documentation to
> > point at route.8 instead of netstat.8
> 
> In an effort to have my interactions on this list wind up being helpful
> rather than more work for overworked maintainers, attached is a
> revised patch attempting to do so.
> 
> This removes the "Flags" description from the netstat.1 manpage
> Directly above it is an instruction to check out the route(8/4)
> pages. The Flags table was moved to route(8).
> 
> While doing this, I noticed in the exact place I was going to make a
> change to add the flags, lo and behold I found a reference to
> netlink(1). I feel quite dumb about that one; if I had read the
> manpage better, I'd have found that link and avoided all this.
> 
> That being said, I do genuinely believe moving the table to
> route is a bit more expected, so; here it is!
> 
>paultag
> --
> :wq
> 
> Index: sbin/route/route.8
> ===
> RCS file: /cvs/src/sbin/route/route.8,v
> retrieving revision 1.106
> diff -u -p -r1.106 route.8
> --- sbin/route/route.8 19 Nov 2022 19:23:37 - 1.106
> +++ sbin/route/route.8 21 Dec 2022 17:36:17 -
> @@ -197,10 +197,7 @@ the given interface is sent.
>  .Op Fl label Ar label
>  .Op Fl priority Ar priority
>  .Xc
> -Print out the routing table, in a fashion similar to "netstat -r".
> -The output is documented in more detail towards the end of the
> -.Xr netstat 1
> -manual.
> +Display all entries from the routing table, along with related metadata.

i like the choice of "display" over "print out".
i don;t like the "related metadata" bit though.

what about just

Display the routing table.

>  .Pp
>  If
>  .Fl gateway
> @@ -224,6 +221,31 @@ or
>  .Cm bgp .
>  If the priority is negative, then routes that do not match the numeric
>  priority are shown.
> +.Pp
> +The "Flags" column indicates what flags are set on the route. The mapping
> +between letters and flags is:

you should start new sentences on new lines.

> +.Bl -column "1" "RTF_BLACKHOLE" "Protocol specific routing flag #1."
> +.It 1 Ta Dv RTF_PROTO1 Ta "Protocol specific routing flag #1."
> +.It 2 Ta Dv RTF_PROTO2 Ta "Protocol specific routing flag #2."
> +.It 3 Ta Dv RTF_PROTO3 Ta "Protocol specific routing flag #3."
> +.It B Ta Dv RTF_BLACKHOLE Ta "Just discard pkts (during updates)."
> +.It b Ta Dv RTF_BROADCAST Ta "Correspond to a local broadcast address."
> +.It C Ta Dv RTF_CLONING Ta "Generate new routes on use."
> +.It c Ta Dv RTF_CLONED Ta "Cloned routes (generated from RTF_CLONING)."
> +.It D Ta Dv RTF_DYNAMIC Ta "Created dynamically (by redirect)."
> +.It G Ta Dv RTF_GATEWAY Ta "Destination requires forwarding by intermediary."
> +.It H Ta Dv RTF_HOST Ta "Host entry (net otherwise)."
> +.It h Ta Dv RTF_CACHED Ta "Referenced by gateway route."
> +.It L Ta Dv RTF_LLINFO Ta "Valid protocol to link address translation."
> +.It l Ta Dv RTF_LOCAL Ta "Correspond to a local address."
> +.It M Ta Dv RTF_MODIFIED Ta "Modified dynamically (by redirect)."
> +.It m Ta Dv RTF_MULTICAST Ta "Correspond to a multicast address."
> +.It n Ta Dv RTF_CONNECTED Ta "Interface route."
> +.It P Ta Dv RTF_MPATH Ta "Multipath route."
> +.It R Ta Dv RTF_REJECT Ta "Host or net unreachable."
> +.It S Ta Dv RTF_STATIC Ta "Manually added."
> +.It T Ta Dv RTF_MPLS Ta "MPLS route."
> +.It U Ta Dv RTF_UP Ta "Route usable."
>  .El
>  .Pp
>  .Bl -tag -width Fl -compact
> @@ -635,7 +657,6 @@ to create the new entry.
>  .El
>  .Sh SEE ALSO
>  .Xr id 1 ,
> -.Xr netstat 1 ,

i would not remove the Xr. i think if we go ahead with this, we can make
a separate commit where we point people to route(8) rather than
netstart(8), but i suspect many pages will both want references to both.

>  .Xr gethostbyname 3 ,
>  .Xr netintro 4 ,
>  .Xr route 4 ,
> Index: usr.bin/netstat/netstat.1
> ===
> RCS file: /cvs/src/usr.bin/netstat/netstat.1,v
> retrieving revision 1.95
> diff -u -p -r1.95 netstat.1
> --- usr.bin/netstat/netstat.1 8 Sep 2022 13:18:47 - 1.95
> +++ usr.bin/netstat/netstat.1 21 Dec 2022 17:36:18 -
> @@ -343,31 +343,6 @@ and
>  .Xr route 4
>  manual pages.
>  .Pp
> -The mapping between letters and flags is:
> -.Bl -column "1" "RTF_BLACKHOLE" "Protocol specific routing flag #1."
> -.It 1 Ta Dv RTF_PROTO1 Ta "Protocol specific routing flag #1."
> -.It 2 Ta Dv RTF_PROTO2 Ta "Protocol specific routing flag #2."
> -.It 3 Ta Dv RTF_PROTO3 Ta "Protocol specific routing flag #3."
> -.It B Ta Dv RTF_BLACKHOLE Ta "Just discard pkts (during updates)."
> -.It b Ta Dv RTF_BROADCAST Ta "Correspond to a local broadcast address."
> -.It C Ta Dv RTF_CLONING Ta "Generate new routes on use."
> -.It c Ta Dv RTF_CLONED Ta "Cloned routes (generated from RTF_CLONING)."
> -.It D Ta Dv RTF_DYNAMIC Ta 

Re: pvbus: pass M_ZERO properly

2022-12-21 Thread Michael Knudsen
On Wed, Dec 21, 2022 at 01:08:14PM +0100, Claudio Jeker wrote:
> In general I think finding such errors quickly would be nice.
> Is there a way to make this a compile time error? I guess that requires
> some ugly macro magic.

I agree a compile time check would be better but I don't have
a good idea of how to achieve it.  Perhaps type and flags could
be made to be different types.

There also is a tradeoff in terms of added work and future
effort in porting stuff from other BSDs.

> Also depending on KMEMSTAT to trigger the error seems not the best design.

I agree.  I went with this change because it was the smallest
change with no added runtime overhead that worked in GENERIC.

-m.

-- 
Rumour is information distilled so finely that it can filter
through anything.
-- (Terry Pratchett, Feet of Clay)



Re: [patch] add show.c style flag descriptions to route(8)

2022-12-21 Thread Paul R. Tagliamonte
> So I think route.8 is where we should try to have complete documentation,
> and once that is done we should change Xr's and other documentation to
> point at route.8 instead of netstat.8

In an effort to have my interactions on this list wind up being helpful
rather than more work for overworked maintainers, attached is a
revised patch attempting to do so.

This removes the "Flags" description from the netstat.1 manpage
Directly above it is an instruction to check out the route(8/4)
pages. The Flags table was moved to route(8).

While doing this, I noticed in the exact place I was going to make a
change to add the flags, lo and behold I found a reference to
netlink(1). I feel quite dumb about that one; if I had read the
manpage better, I'd have found that link and avoided all this.

That being said, I do genuinely believe moving the table to
route is a bit more expected, so; here it is!

   paultag
--
:wq

Index: sbin/route/route.8
===
RCS file: /cvs/src/sbin/route/route.8,v
retrieving revision 1.106
diff -u -p -r1.106 route.8
--- sbin/route/route.8 19 Nov 2022 19:23:37 - 1.106
+++ sbin/route/route.8 21 Dec 2022 17:36:17 -
@@ -197,10 +197,7 @@ the given interface is sent.
 .Op Fl label Ar label
 .Op Fl priority Ar priority
 .Xc
-Print out the routing table, in a fashion similar to "netstat -r".
-The output is documented in more detail towards the end of the
-.Xr netstat 1
-manual.
+Display all entries from the routing table, along with related metadata.
 .Pp
 If
 .Fl gateway
@@ -224,6 +221,31 @@ or
 .Cm bgp .
 If the priority is negative, then routes that do not match the numeric
 priority are shown.
+.Pp
+The "Flags" column indicates what flags are set on the route. The mapping
+between letters and flags is:
+.Bl -column "1" "RTF_BLACKHOLE" "Protocol specific routing flag #1."
+.It 1 Ta Dv RTF_PROTO1 Ta "Protocol specific routing flag #1."
+.It 2 Ta Dv RTF_PROTO2 Ta "Protocol specific routing flag #2."
+.It 3 Ta Dv RTF_PROTO3 Ta "Protocol specific routing flag #3."
+.It B Ta Dv RTF_BLACKHOLE Ta "Just discard pkts (during updates)."
+.It b Ta Dv RTF_BROADCAST Ta "Correspond to a local broadcast address."
+.It C Ta Dv RTF_CLONING Ta "Generate new routes on use."
+.It c Ta Dv RTF_CLONED Ta "Cloned routes (generated from RTF_CLONING)."
+.It D Ta Dv RTF_DYNAMIC Ta "Created dynamically (by redirect)."
+.It G Ta Dv RTF_GATEWAY Ta "Destination requires forwarding by intermediary."
+.It H Ta Dv RTF_HOST Ta "Host entry (net otherwise)."
+.It h Ta Dv RTF_CACHED Ta "Referenced by gateway route."
+.It L Ta Dv RTF_LLINFO Ta "Valid protocol to link address translation."
+.It l Ta Dv RTF_LOCAL Ta "Correspond to a local address."
+.It M Ta Dv RTF_MODIFIED Ta "Modified dynamically (by redirect)."
+.It m Ta Dv RTF_MULTICAST Ta "Correspond to a multicast address."
+.It n Ta Dv RTF_CONNECTED Ta "Interface route."
+.It P Ta Dv RTF_MPATH Ta "Multipath route."
+.It R Ta Dv RTF_REJECT Ta "Host or net unreachable."
+.It S Ta Dv RTF_STATIC Ta "Manually added."
+.It T Ta Dv RTF_MPLS Ta "MPLS route."
+.It U Ta Dv RTF_UP Ta "Route usable."
 .El
 .Pp
 .Bl -tag -width Fl -compact
@@ -635,7 +657,6 @@ to create the new entry.
 .El
 .Sh SEE ALSO
 .Xr id 1 ,
-.Xr netstat 1 ,
 .Xr gethostbyname 3 ,
 .Xr netintro 4 ,
 .Xr route 4 ,
Index: usr.bin/netstat/netstat.1
===
RCS file: /cvs/src/usr.bin/netstat/netstat.1,v
retrieving revision 1.95
diff -u -p -r1.95 netstat.1
--- usr.bin/netstat/netstat.1 8 Sep 2022 13:18:47 - 1.95
+++ usr.bin/netstat/netstat.1 21 Dec 2022 17:36:18 -
@@ -343,31 +343,6 @@ and
 .Xr route 4
 manual pages.
 .Pp
-The mapping between letters and flags is:
-.Bl -column "1" "RTF_BLACKHOLE" "Protocol specific routing flag #1."
-.It 1 Ta Dv RTF_PROTO1 Ta "Protocol specific routing flag #1."
-.It 2 Ta Dv RTF_PROTO2 Ta "Protocol specific routing flag #2."
-.It 3 Ta Dv RTF_PROTO3 Ta "Protocol specific routing flag #3."
-.It B Ta Dv RTF_BLACKHOLE Ta "Just discard pkts (during updates)."
-.It b Ta Dv RTF_BROADCAST Ta "Correspond to a local broadcast address."
-.It C Ta Dv RTF_CLONING Ta "Generate new routes on use."
-.It c Ta Dv RTF_CLONED Ta "Cloned routes (generated from RTF_CLONING)."
-.It D Ta Dv RTF_DYNAMIC Ta "Created dynamically (by redirect)."
-.It G Ta Dv RTF_GATEWAY Ta "Destination requires forwarding by intermediary."
-.It H Ta Dv RTF_HOST Ta "Host entry (net otherwise)."
-.It h Ta Dv RTF_CACHED Ta "Referenced by gateway route."
-.It L Ta Dv RTF_LLINFO Ta "Valid protocol to link address translation."
-.It l Ta Dv RTF_LOCAL Ta "Correspond to a local address."
-.It M Ta Dv RTF_MODIFIED Ta "Modified dynamically (by redirect)."
-.It m Ta Dv RTF_MULTICAST Ta "Correspond to a multicast address."
-.It n Ta Dv RTF_CONNECTED Ta "Interface route."
-.It P Ta Dv RTF_MPATH Ta "Multipath route."
-.It R Ta Dv RTF_REJECT Ta "Host or net unreachable."
-.It S Ta Dv RTF_STATIC Ta "Manually added."

Re: [patch] add show.c style flag descriptions to route(8)

2022-12-21 Thread Theo de Raadt
Claudio Jeker  wrote:

> In the old world only netstat could show the routing table. I think this
> is still the case in FreeBSD for example. We added route show at some
> point but the documentation was not shared between the manpages.
> I agree that it is annoying to open up the netstat man page to find the
> flags shown by route show. For this we added:
> 
>Print out the routing table, in a fashion similar to
>  "netstat -r".  The output is documented in more detail
>  towards the end of the netstat(1) manual.
> 
> To the route manpage when describing route show. Not sure if that is
> enough or if we should duplicate tables (whith the usual sync problem).

This is all true.

So netstat in those days was more of a "kvm reader" program, and as such
racy.  Nowadays both route & netstat programs's subcommands are a mix of
sysctl readers and route socket askers/listeners, and thus they have
better atomicity or at least other types of truth.

The sub-command extensions in route are better designed, mostly because
they are newer and were built in an era where kernels maintained more than
a handful of routes.  I think we want to lean people towards using route,
instead of netstat.

So I think route.8 is where we should try to have complete documentation,
and once that is done we should change Xr's and other documentation to 
point at route.8 instead of netstat.8



Re: [patch] add show.c style flag descriptions to route(8)

2022-12-21 Thread Claudio Jeker
On Wed, Dec 21, 2022 at 04:48:38PM +, Jason McIntyre wrote:
> On Wed, Dec 21, 2022 at 10:43:18AM -0500, Paul R. Tagliamonte wrote:
> > On Tue, Dec 20, 2022 at 8:27 PM Paul R. Tagliamonte  
> > wrote:
> > >
> > > Heyya tech@,
> > >
> > > Please keep me on cc, I'm not subscribed
> > 
> > Please keep me on cc. The last message wasn't sent to me, so my In-Reply-To
> > is going to be wrong. I'm not subscribed to tech@.
> 
> yep, sorry, i didn;t spot that note 
> 
> > From the web:
> > > some of the relevant flags are already documented in route(8) ("Routes
> > > have associated flags...). the entire list is documented in route(4),
> > > but you have to explicitly ask for it (man 4 route)
> > 
> > AFAICT none of these documents which flag "h" maps to, for example.
> > 
> 
> no, but netstat(8) does.
> 
> > > and again the flags with detail in netstat(8) ("The mapping between
> > > letters and flags is...").
> > 
> > I will admit I'm not smart enough to think to check netstat(8) when
> > looking at route(8) output, but that's a fair point.
> > 
> > I understand netstat.8 documenting flags defined in usr.bin/netstat/show.c,
> > but is the review here that we should instruct users in route.8 to look up
> > the flags coming from sbin/route/show.c in netstat.8 which documents
> > usr.bin/netstat/show.c, not sbin/route/show.c ?
> > 
> > I am very sympathetic to the argument that duplicating documentation
> > is bad, and can result in maintenance burden or out of date docs, but
> > surely people would be more likely to update a manpage in the same
> > directory as the file?
> > 
> > I'm OK with this going NOTABUG WONTFIX; I did find the right mappings,
> > but I just had to go to the source repo to find it, so I guess
> > selfishly I've got
> > the knowledge I needed. I was just trying to fix a doc bug when I had state
> > in memory, since I know I'd appreciate that as a fellow distro maintainer.
> > 
> >   paultag
> > 
> 
> i don;t know why the text is where it is. maybe some of the network
> people can say whether the placement makes sense or not.

In the old world only netstat could show the routing table. I think this
is still the case in FreeBSD for example. We added route show at some
point but the documentation was not shared between the manpages.
I agree that it is annoying to open up the netstat man page to find the
flags shown by route show. For this we added:

 Print out the routing table, in a fashion similar to
 "netstat -r".  The output is documented in more detail
 towards the end of the netstat(1) manual.

To the route manpage when describing route show. Not sure if that is
enough or if we should duplicate tables (whith the usual sync problem).

-- 
:wq Claudio



Re: [patch] add show.c style flag descriptions to route(8)

2022-12-21 Thread Jason McIntyre
On Wed, Dec 21, 2022 at 10:43:18AM -0500, Paul R. Tagliamonte wrote:
> On Tue, Dec 20, 2022 at 8:27 PM Paul R. Tagliamonte  wrote:
> >
> > Heyya tech@,
> >
> > Please keep me on cc, I'm not subscribed
> 
> Please keep me on cc. The last message wasn't sent to me, so my In-Reply-To
> is going to be wrong. I'm not subscribed to tech@.

yep, sorry, i didn;t spot that note 

> From the web:
> > some of the relevant flags are already documented in route(8) ("Routes
> > have associated flags...). the entire list is documented in route(4),
> > but you have to explicitly ask for it (man 4 route)
> 
> AFAICT none of these documents which flag "h" maps to, for example.
> 

no, but netstat(8) does.

> > and again the flags with detail in netstat(8) ("The mapping between
> > letters and flags is...").
> 
> I will admit I'm not smart enough to think to check netstat(8) when
> looking at route(8) output, but that's a fair point.
> 
> I understand netstat.8 documenting flags defined in usr.bin/netstat/show.c,
> but is the review here that we should instruct users in route.8 to look up
> the flags coming from sbin/route/show.c in netstat.8 which documents
> usr.bin/netstat/show.c, not sbin/route/show.c ?
> 
> I am very sympathetic to the argument that duplicating documentation
> is bad, and can result in maintenance burden or out of date docs, but
> surely people would be more likely to update a manpage in the same
> directory as the file?
> 
> I'm OK with this going NOTABUG WONTFIX; I did find the right mappings,
> but I just had to go to the source repo to find it, so I guess
> selfishly I've got
> the knowledge I needed. I was just trying to fix a doc bug when I had state
> in memory, since I know I'd appreciate that as a fellow distro maintainer.
> 
>   paultag
> 

i don;t know why the text is where it is. maybe some of the network
people can say whether the placement makes sense or not.

jmc



Re: BROP mitigation

2022-12-21 Thread Theo de Raadt
Theo de Raadt  wrote:

> A few weeks ago a conversation about retguard (a diff is probably
> coming) caused me re-consider & re-read the BROP paper
> 
>   https://www.scs.stanford.edu/brop/bittau-brop.pdf

new version of the diff.  The small piece of locking has been improved
by using a private mutex, and the check function has been renamed a bit.

It is important to check things like coredump (this contains a workaround),
and ptrace, and other special cases.

As seen in subr_log.c, there will be other naked copyin() which need
xonly() checks before them.  I made an attempt to protect *all* copyin,
by changing copyin() to do this itself.  but it lacks the proc pointer so I
tried curproc, but this doesn't seem to work right, doesn't feel right
either.  And furthermore I found there are copyin that occur inside map locks
so that seems like a step too far.

Index: sys/systm.h
===
RCS file: /cvs/src/sys/sys/systm.h,v
retrieving revision 1.159
diff -u -p -u -r1.159 systm.h
--- sys/systm.h 3 Sep 2022 15:29:44 -   1.159
+++ sys/systm.h 21 Dec 2022 07:17:32 -
@@ -211,6 +211,8 @@ int copyin(const void *, void *, size_t)
 intcopyout(const void *, void *, size_t);
 intcopyin32(const uint32_t *, uint32_t *);
 
+intxonly(struct proc *, const void *, size_t);
+
 void   random_start(int);
 void   enqueue_randomness(unsigned int);
 void   suspend_randomness(void);
Index: kern/exec_subr.c
===
RCS file: /cvs/src/sys/kern/exec_subr.c,v
retrieving revision 1.64
diff -u -p -u -r1.64 exec_subr.c
--- kern/exec_subr.c5 Dec 2022 23:18:37 -   1.64
+++ kern/exec_subr.c21 Dec 2022 06:58:19 -
@@ -215,6 +215,10 @@ vmcmd_map_pagedvn(struct proc *p, struct
if (cmd->ev_flags & VMCMD_IMMUTABLE)
uvm_map_immutable(>p_vmspace->vm_map, cmd->ev_addr,
round_page(cmd->ev_addr + cmd->ev_len), 1);
+   if ((flags & UVM_FLAG_SYSCALL) ||
+   ((cmd->ev_flags & VMCMD_IMMUTABLE) && (cmd->ev_prot & 
PROT_EXEC)))
+   uvm_map_xonly(>p_vmspace->vm_map,
+   cmd->ev_addr, round_page(cmd->ev_addr + 
cmd->ev_len));
}
 
return (error);
Index: kern/kern_sig.c
===
RCS file: /cvs/src/sys/kern/kern_sig.c,v
retrieving revision 1.301
diff -u -p -u -r1.301 kern_sig.c
--- kern/kern_sig.c 16 Oct 2022 16:27:02 -  1.301
+++ kern/kern_sig.c 21 Dec 2022 06:58:19 -
@@ -1642,6 +1642,9 @@ coredump(struct proc *p)
 
atomic_setbits_int(>ps_flags, PS_COREDUMP);
 
+   /* disable xonly checks, so we can write out text sections if needed */
+   p->p_vmspace->vm_map.xonly_count = 0;
+
/* Don't dump if will exceed file size limit. */
if (USPACE + ptoa(vm->vm_dsize + vm->vm_ssize) >= lim_cur(RLIMIT_CORE))
return (EFBIG);
Index: kern/kern_subr.c
===
RCS file: /cvs/src/sys/kern/kern_subr.c,v
retrieving revision 1.51
diff -u -p -u -r1.51 kern_subr.c
--- kern/kern_subr.c14 Aug 2022 01:58:27 -  1.51
+++ kern/kern_subr.c21 Dec 2022 15:50:34 -
@@ -78,8 +78,12 @@ uiomove(void *cp, size_t n, struct uio *
sched_pause(preempt);
if (uio->uio_rw == UIO_READ)
error = copyout(cp, iov->iov_base, cnt);
-   else
-   error = copyin(iov->iov_base, cp, cnt);
+   else {
+   error = xonly(uio->uio_procp,
+   iov->iov_base, cnt);
+   if (error == 0)
+   error = copyin(iov->iov_base, cp, cnt);
+   }
if (error)
return (error);
break;
Index: kern/subr_log.c
===
RCS file: /cvs/src/sys/kern/subr_log.c,v
retrieving revision 1.75
diff -u -p -u -r1.75 subr_log.c
--- kern/subr_log.c 2 Jul 2022 08:50:42 -   1.75
+++ kern/subr_log.c 21 Dec 2022 15:16:41 -
@@ -644,6 +644,8 @@ dosendsyslog(struct proc *p, const char 
 */
len = MIN(nbyte, sizeof(pri));
if (sflg == UIO_USERSPACE) {
+   if ((error = xonly(p, buf, len)))
+   return (error);
if ((error = copyin(buf, pri, len)))
return (error);
} else
Index: uvm/uvm_map.c
===
RCS file: /cvs/src/sys/uvm/uvm_map.c,v
retrieving revision 1.305
diff -u -p -u -r1.305 

Re: [patch] add show.c style flag descriptions to route(8)

2022-12-21 Thread Paul R. Tagliamonte
On Tue, Dec 20, 2022 at 8:27 PM Paul R. Tagliamonte  wrote:
>
> Heyya tech@,
>
> Please keep me on cc, I'm not subscribed

Please keep me on cc. The last message wasn't sent to me, so my In-Reply-To
is going to be wrong. I'm not subscribed to tech@.

>From the web:
> some of the relevant flags are already documented in route(8) ("Routes
> have associated flags...). the entire list is documented in route(4),
> but you have to explicitly ask for it (man 4 route)

AFAICT none of these documents which flag "h" maps to, for example.

> and again the flags with detail in netstat(8) ("The mapping between
> letters and flags is...").

I will admit I'm not smart enough to think to check netstat(8) when
looking at route(8) output, but that's a fair point.

I understand netstat.8 documenting flags defined in usr.bin/netstat/show.c,
but is the review here that we should instruct users in route.8 to look up
the flags coming from sbin/route/show.c in netstat.8 which documents
usr.bin/netstat/show.c, not sbin/route/show.c ?

I am very sympathetic to the argument that duplicating documentation
is bad, and can result in maintenance burden or out of date docs, but
surely people would be more likely to update a manpage in the same
directory as the file?

I'm OK with this going NOTABUG WONTFIX; I did find the right mappings,
but I just had to go to the source repo to find it, so I guess
selfishly I've got
the knowledge I needed. I was just trying to fix a doc bug when I had state
in memory, since I know I'd appreciate that as a fellow distro maintainer.

  paultag



Re: pvbus: pass M_ZERO properly

2022-12-21 Thread Claudio Jeker
On Tue, Dec 13, 2022 at 12:33:03AM +0100, Michael Knudsen wrote:
> We can detect this class of error by redefining the flags to
> use the high bits instead of the low ones.  This way the malloc()
> KMEMSTAT value check triggers because 0x1000 is greater than
> the maximum type value.
> 
> I tested the following, and it boots fine, and when I moved
> M_ZERO to the type field in acpibtn the allocation panicked.
> 
> (Of course, having different prefixes for type and flags would
> help but that is another matter.)

In general I think finding such errors quickly would be nice.
Is there a way to make this a compile time error? I guess that requires
some ugly macro magic.
Also depending on KMEMSTAT to trigger the error seems not the best design.

> Index: sys/malloc.h
> ===
> RCS file: /cvs/src/sys/sys/malloc.h,v
> retrieving revision 1.122
> diff -u -p -r1.122 malloc.h
> --- sys/malloc.h  3 Feb 2022 17:18:22 -   1.122
> +++ sys/malloc.h  12 Dec 2022 23:24:23 -
> @@ -53,11 +53,13 @@
>  
>  /*
>   * flags to malloc
> + * Uses high bits to trigger 'type' limit check
> + * if misused.
>   */
> -#define  M_WAITOK0x0001
> -#define  M_NOWAIT0x0002
> -#define  M_CANFAIL   0x0004
> -#define  M_ZERO  0x0008
> +#define  M_WAITOK0x1000
> +#define  M_NOWAIT0x2000
> +#define  M_CANFAIL   0x4000
> +#define  M_ZERO  0x8000
>  
>  /*
>   * Types of memory to be allocated
> 
> -- 
> Rumour is information distilled so finely that it can filter
> through anything.
> -- (Terry Pratchett, Feet of Clay)
> 

-- 
:wq Claudio