Re: reorder_kernel: suport ro /usr like library_aslr does

2022-11-14 Thread Theo de Raadt
Klemens Nanni  wrote:

> Reading /etc/rc I was under the impression that read-only /usr is indeed
> a scenario we support, since reorder_libs() already does what I propose,
> only in a more complicated way:
> 
>   revision 1.481
>   date: 2016/05/26 14:59:48;  author: rpe;  state: Exp;  lines: +32 -7;
>   - rename rebuildlibs() to reorder_libs()
>   - move the info message inside the function
>   - skip reordering if /usr/lib is on a nfs mounted filesystem
>   - temporarily remount rw if /usr/lib is on a ro ffs file-system
> 
>   OK deraadt
> 
> Did OpenBSD's stance on read-only /usr change between then and now?

The OpenBSD stance never changed.

If the configuration isn't created by the install script, you are on
your own.  Maybe one developer put some words in about ffs, but I am sure
the idea was to protect NFS diskless which cannot be mounted rw.

I do not see the point behind diff after diff trying to add scripting
for bizzare configurations that basically none of our users configure.
Those admins are fully aware that when they make weird configuration changes,
they are on their own, and they have to maintain their own diffs.

I do not agree with this continual pushiness you have to find weird
special cases and propose scripting changes to deal with it.

OpenBSD's prime principle is SIMPLICITY, even when it means there are
limited operations for operation, and you are going way out of bounds.

Take a step back.



Re: reorder_kernel: suport ro /usr like library_aslr does

2022-11-14 Thread Klemens Nanni
On Mon, Nov 14, 2022 at 03:15:53PM -0700, Theo de Raadt wrote:
> Readonly /usr is not a supported or recomended configuration.
> 
> This is adding a lot of scripting that we don't everyone to run.
> 
> I disagree strongly with this direction of OpenBSD having undocumented
> (undocumentable?) little behaviours that allow root to configure their
> machine in novel non-default ways and it will still work because there
> piles of of trashy shell scripts which cope with the weird situations,
> which under 1% of users will use.
> 
> I disagree with this flexiblity being a strength, I think it is very
> fragile when we encourage users to do bizzare things to their machines
> which they (also) will not include in future bug reports.

Reading /etc/rc I was under the impression that read-only /usr is indeed
a scenario we support, since reorder_libs() already does what I propose,
only in a more complicated way:

revision 1.481
date: 2016/05/26 14:59:48;  author: rpe;  state: Exp;  lines: +32 -7;
- rename rebuildlibs() to reorder_libs()
- move the info message inside the function
- skip reordering if /usr/lib is on a nfs mounted filesystem
- temporarily remount rw if /usr/lib is on a ro ffs file-system

OK deraadt

Did OpenBSD's stance on read-only /usr change between then and now?

Or does the earlier running /etc/rc take care of some read-only /usr
scenario (I am not aware of) which is not relevant for reorder_kernel?



Re: reorder_kernel: suport ro /usr like library_aslr does

2022-11-14 Thread Theo de Raadt
Readonly /usr is not a supported or recomended configuration.

This is adding a lot of scripting that we don't everyone to run.

I disagree strongly with this direction of OpenBSD having undocumented
(undocumentable?) little behaviours that allow root to configure their
machine in novel non-default ways and it will still work because there
piles of of trashy shell scripts which cope with the weird situations,
which under 1% of users will use.

I disagree with this flexiblity being a strength, I think it is very
fragile when we encourage users to do bizzare things to their machines
which they (also) will not include in future bug reports.

Klemens Nanni  wrote:

> On Tue, Nov 08, 2022 at 11:10:19AM +, Klemens Nanni wrote:
> > More read-only filesystems mean less fsck during boot after crashes.
> > Especially on crappy machines like the Pinebook Poop, I keep /usr
> > read-only and run with this diff so I still get a relinked kernel.
> > 
> > rc's reorder_libs() already does the same remount dance, but for
> > multiple directories/filesystems.
> > 
> > My diff works as expected with read-write and read-only /usr as well as
> > interrupting /usr/libexec/reorder_kernel runs with ^C, i.e. a failed run
> > will correctly mount /usr ro again if it was ro before the run.
> > 
> > Feedback? OK?
> 
> Ping.
> 
> Index: libexec/reorder_kernel/reorder_kernel.sh
> ===
> RCS file: /cvs/src/libexec/reorder_kernel/reorder_kernel.sh,v
> retrieving revision 1.13
> diff -u -p -r1.13 reorder_kernel.sh
> --- libexec/reorder_kernel/reorder_kernel.sh  7 Nov 2022 15:55:56 -   
> 1.13
> +++ libexec/reorder_kernel/reorder_kernel.sh  14 Nov 2022 19:33:25 -
> @@ -27,13 +27,32 @@ LOGFILE=$KERNEL_DIR/$KERNEL/relink.log
>  PROGNAME=${0##*/}
>  SHA256=/var/db/kernel.SHA256
>  
> -# Silently skip if on a NFS mounted filesystem.
> -df -t nonfs $KERNEL_DIR >/dev/null 2>&1
> +# Silently skip if on NFS, otherwise remount the filesystem read-write.
> +DEV=$(df -t ffs $KERNEL_DIR 2>/dev/null | awk 'NR == 2 { print $1 }')
> +MP=$(mount | grep ^$DEV)
> +RO=false
> +if [[ $MP == *read-only* ]]; then
> + MP=${MP%% *}
> + mount -u -w $MP
> + RO=true
> +fi
> +
> +restore_mount() {
> + if $RO; then
> + # Close the logfile to unbusy $MP by switching back to console.
> + exec 1>/dev/console
> + exec 2>&1
> + mount -u -r $MP
> + fi
> +}
>  
>  # Install trap handlers to inform about success or failure via syslog.
>  ERRMSG='failed'
> -trap 'trap - EXIT; logger -st $PROGNAME "$ERRMSG" >/dev/console 2>&1' ERR
> -trap 'logger -t $PROGNAME "kernel relinking done"' EXIT
> +trap 'trap - EXIT
> + logger -st $PROGNAME "$ERRMSG" 2>/dev/console
> + restore_mount' ERR
> +trap 'logger -t $PROGNAME "kernel relinking done"
> + restore_mount' EXIT
>  
>  # Create kernel compile dir and redirect stdout/stderr to a logfile.
>  mkdir -m 700 -p $KERNEL_DIR/$KERNEL
> 



Re: xenodm: save ~/.xesssion to ~/.xsession.old

2022-11-14 Thread Theo de Raadt
Klemens Nanni  wrote:

> On Mon, Nov 14, 2022 at 10:33:44AM -0700, Theo de Raadt wrote:
> > Stuart Henderson  wrote:
> > 
> > > > Index: app/xenodm//config/Xsession.in
> > > > ===
> > > > RCS file: /cvs/xenocara/app/xenodm/config/Xsession.in,v
> > > > retrieving revision 1.2
> > > > diff -u -p -r1.2 Xsession.in
> > > > --- app/xenodm//config/Xsession.in  1 Jul 2022 20:42:06 -   
> > > > 1.2
> > > > +++ app/xenodm//config/Xsession.in  14 Nov 2022 16:47:03 -
> > > > @@ -7,6 +7,7 @@ exec_prefix="@exec_prefix@"
> > > >  # redirect errors to a file in user's home directory if we can
> > > >  
> > > >  errfile="$HOME/.xsession-errors"
> > > > +cp -f "$errfile" "$errfile.old" 2> /dev/null
> > > 
> > > Those files can get pretty big. mv is probably a better idea than cp.
> > 
> > And consider the target being a symbolic link, both file and directory
> 
> `mv -f 2>/dev/null' should be good enough for all of this, no?
> It's a silent one-shot:  if it works, great.  If not, no .old backup and
> the file gets truncated as usual.

ln -s /tmp ~/.xsession-errors.old

Your plan is half baked.






Re: Push kernel lock into pru_control()

2022-11-14 Thread Vitaliy Makkoveev
> On 10 Nov 2022, at 13:54, Klemens Nanni  wrote:
> 
> Purely mechanical, then in6_control() and in_control() can be pushed
> further individually.
> 
> Feedback? OK?

SS_PRIV is immutable, no reason to check it with kernel lock held.

> ---
> sys/kern/sys_socket.c | 2 --
> sys/netinet/in.c  | 4 
> sys/netinet6/in6.c| 4 
> 3 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/sys/kern/sys_socket.c b/sys/kern/sys_socket.c
> index b07119b71cd..e42dd948006 100644
> --- a/sys/kern/sys_socket.c
> +++ b/sys/kern/sys_socket.c
> @@ -134,9 +134,7 @@ soo_ioctl(struct file *fp, u_long cmd, caddr_t data, 
> struct proc *p)
>   }
>   if (IOCGROUP(cmd) == 'r')
>   return (EOPNOTSUPP);
> - KERNEL_LOCK();
>   error = pru_control(so, cmd, data, NULL);
> - KERNEL_UNLOCK();
>   break;
>   }
> 
> diff --git a/sys/netinet/in.c b/sys/netinet/in.c
> index cd8289d2e89..990aaf84c8a 100644
> --- a/sys/netinet/in.c
> +++ b/sys/netinet/in.c
> @@ -202,6 +202,8 @@ in_control(struct socket *so, u_long cmd, caddr_t data, 
> struct ifnet *ifp)
>   int privileged;
>   int error;
> 
> + KERNEL_LOCK();
> +
>   privileged = 0;
>   if ((so->so_state & SS_PRIV) != 0)
>   privileged++;
> @@ -218,6 +220,8 @@ in_control(struct socket *so, u_long cmd, caddr_t data, 
> struct ifnet *ifp)
>   break;
>   }
> 
> + KERNEL_UNLOCK();
> +
>   return error;
> }
> 
> diff --git a/sys/netinet6/in6.c b/sys/netinet6/in6.c
> index 900456fff1a..a51ca2fa5a4 100644
> --- a/sys/netinet6/in6.c
> +++ b/sys/netinet6/in6.c
> @@ -199,6 +199,8 @@ in6_control(struct socket *so, u_long cmd, caddr_t data, 
> struct ifnet *ifp)
>   int privileged;
>   int error;
> 
> + KERNEL_LOCK();
> +
>   privileged = 0;
>   if ((so->so_state & SS_PRIV) != 0)
>   privileged++;
> @@ -215,6 +217,8 @@ in6_control(struct socket *so, u_long cmd, caddr_t data, 
> struct ifnet *ifp)
>   break;
>   }
> 
> + KERNEL_UNLOCK();
> +
>   return error;
> }
> 
> -- 
> 2.38.1
> 



Re: export {b,r}ootduid as sysctl, installer/sysupgrade improvements

2022-11-14 Thread Theo de Raadt
Mark Kettenis  wrote:

> > From: "Theo de Raadt" 
> > Date: Mon, 14 Nov 2022 10:02:40 -0700
> > 
> > An OpenBSD machine only has one OpenBSD install.
> 
> I have to disagree here.  Not everyone has a pile of test machines
> lying around.  

Are you going to work on the install script?

And then, after that, on the feature requests for sysupgrade?

> Well, that is the real question: will this increase complexity?

It will increase complexity.

> We
> currently have code that makes what I'd describe as an "educated
> guess" at what is the OpenBSD root disk of a machine.  If we can
> replace that with something that finds the disk based on its DUID,
> that would make things more robust and might even decrease complexity
> in the installer.

I don't believe either of you.

I believe that is_rootdisk() will be replaced with 30 lines of kernel
code, 10 lines in distrib/special/sysctl/sysctl.c, and 5 lines of
shell script ..

AND AFTER THAT IS DONE, a bunch of extensions will be proposed for
various other system install / update features, which will then increase
complexity.



And I claim that will happen because this is not proposing 1 new sysctl,
it is proposing 2 of them, without explaining why there needs to be 2.
I think the explanation for that is "undisclosed goals".







Re: export {b,r}ootduid as sysctl, installer/sysupgrade improvements

2022-11-14 Thread Mark Kettenis
> From: "Theo de Raadt" 
> Date: Mon, 14 Nov 2022 10:02:40 -0700
> 
> An OpenBSD machine only has one OpenBSD install.

I have to disagree here.  Not everyone has a pile of test machines
lying around.  

> As soon as we leave that model, and allow other setup models, perhaps you
> think there will be two or three potential configurations that people setup.
> 
> I don't think so, I think it will keep being extended by people who do
> more and more scewed up bizzare configurations, all of which (obviously, to
> you) will need to be supported, and everything gets more complicated.
> 
> I think we should say "STOP". Now.  And not start down that roadmap.
> 
> You built something for a testlab.  Your conclusion is that it should
> work for everyone.  I simply cannot come to the same conclusion,
> because it requires complexity, but instead I think we should embrace
> simplicity even if it limits choice.

Well, that is the real question: will this increase complexity?  We
currently have code that makes what I'd describe as an "educated
guess" at what is the OpenBSD root disk of a machine.  If we can
replace that with something that finds the disk based on its DUID,
that would make things more robust and might even decrease complexity
in the installer.

That said, I don't immediately see how this would work and how the
sysctl's would help.  So it would be good if Klemens showed us the
complete picture.


> Klemens Nanni  wrote:
> 
> > On Mon, Nov 14, 2022 at 07:49:11AM -0700, Theo de Raadt wrote:
> > > Klemens Nanni  wrote:
> > > 
> > > > This is because the installer always considers the first root disk it
> > > > finds as the one to upgrade, which is certainly not what I intend or
> > > > expect when booting/upgrading the softraid installation on sd1-3.
> > > 
> > > What does
> > > 
> > >  first root disk
> > > 
> > > Mean?
> > 
> > One machine with two phsyical disks, say one NVMe and regular SSD.
> > Both disks contain a standalone OpenBSD installation.
> > 
> > I consider each of them a root disk.
> > 
> > > 
> > > There is only one root disk.  The root disk is the one that actually 
> > > contains
> > > the / that is mounted.
> > > 
> > > It is this one:
> > > 
> > > root on sd0a (fb786f6b01042b30.a) swap on sd0b dump on sd0b
> > > 
> > > You cannot change this.  If you use the bootloader to tell the kernel do
> > > do something else, then I argue that sysupgrade and the installer should
> > > punish you unless you *manually tell it that every time*
> > 
> > I don't set anything in /etc/boot.conf or the boot> prompt.
> > 
> > I select the disk to boot from in the UEFI boot manager.
> > 
> > > 
> > > The install script knows what the root filesystem is using very simple
> > > heuristics, but by creating two new sysctl, I am afraid you will enrich
> > > this ability to support bizzare configurations that did not work, and
> > > I argue *should never work*.
> > > 
> > > > It is probably not that common to have multiple installations/root disks
> > > > in one machine, but it isn't "weird" to me, either.
> > > 
> > > What?  It is not weird
> > > 
> > > I think this is unsupported bullshit.
> > > 
> > > Why do we need the install script to support this configuration you
> > > created?  Why do we need to encourage other people to have such
> > > configurations?  When they create such a configuration, and find the
> > > tooling can support it, won't they go and do even stranger things, then
> > > find the tooling doesn't support that even-stranger setup, and then
> > > you'll come back adding support for increasingly strange setups, and
> > > eventually we are going to end up with a large userbase *not using* a
> > > single root filesystem?
> > > 
> > > > Overwriting the wrong system during an upgrade because the installer
> > > > makes too big of an assumption about the first disk is weird to me.
> > > > 
> > > > 
> > > > I can post console logs later showing how the installer picks the wrong
> > > > disk, if you want.
> > > 
> > > There is only one possible root filesystem.
> > > 
> > > If you created multiple root filesystems, you have created a mess and
> > > why is it wrong for me to argue you need to experience pain for the
> > > decisions that led you there?
> > > 
> > > I do not think you are being honest about the reason for these extensions.
> > > 
> > 
> > I'm pretty honest, but apparently not precise enough.
> > 
> > Now that all the softraid/installboot diffs landed and the dust has
> > settled, let me iterate over and test my setup again to make sure I'm
> > not tripping over my own mistakes.
> > 
> > Then I can come back with a clear update or reproducer.
> > 
> 



Re: xenodm: save ~/.xesssion to ~/.xsession.old

2022-11-14 Thread Klemens Nanni
On Mon, Nov 14, 2022 at 10:33:44AM -0700, Theo de Raadt wrote:
> Stuart Henderson  wrote:
> 
> > > Index: app/xenodm//config/Xsession.in
> > > ===
> > > RCS file: /cvs/xenocara/app/xenodm/config/Xsession.in,v
> > > retrieving revision 1.2
> > > diff -u -p -r1.2 Xsession.in
> > > --- app/xenodm//config/Xsession.in1 Jul 2022 20:42:06 -   
> > > 1.2
> > > +++ app/xenodm//config/Xsession.in14 Nov 2022 16:47:03 -
> > > @@ -7,6 +7,7 @@ exec_prefix="@exec_prefix@"
> > >  # redirect errors to a file in user's home directory if we can
> > >  
> > >  errfile="$HOME/.xsession-errors"
> > > +cp -f "$errfile" "$errfile.old" 2> /dev/null
> > 
> > Those files can get pretty big. mv is probably a better idea than cp.
> 
> And consider the target being a symbolic link, both file and directory

`mv -f 2>/dev/null' should be good enough for all of this, no?
It's a silent one-shot:  if it works, great.  If not, no .old backup and
the file gets truncated as usual.


Index: app/xenodm/config/Xsession.in
===
RCS file: /cvs/xenocara/app/xenodm/config/Xsession.in,v
retrieving revision 1.2
diff -u -p -r1.2 Xsession.in
--- app/xenodm/config/Xsession.in   1 Jul 2022 20:42:06 -   1.2
+++ app/xenodm/config/Xsession.in   14 Nov 2022 19:39:57 -
@@ -7,6 +7,7 @@ exec_prefix="@exec_prefix@"
 # redirect errors to a file in user's home directory if we can
 
 errfile="$HOME/.xsession-errors"
+mv -f "$errfile" "$errfile.old"
 if ( umask 077 && cp /dev/null "$errfile" 2> /dev/null )
 then
exec > "$errfile" 2>&1



Re: reorder_kernel: suport ro /usr like library_aslr does

2022-11-14 Thread Klemens Nanni
On Tue, Nov 08, 2022 at 11:10:19AM +, Klemens Nanni wrote:
> More read-only filesystems mean less fsck during boot after crashes.
> Especially on crappy machines like the Pinebook Poop, I keep /usr
> read-only and run with this diff so I still get a relinked kernel.
> 
> rc's reorder_libs() already does the same remount dance, but for
> multiple directories/filesystems.
> 
> My diff works as expected with read-write and read-only /usr as well as
> interrupting /usr/libexec/reorder_kernel runs with ^C, i.e. a failed run
> will correctly mount /usr ro again if it was ro before the run.
> 
> Feedback? OK?

Ping.

Index: libexec/reorder_kernel/reorder_kernel.sh
===
RCS file: /cvs/src/libexec/reorder_kernel/reorder_kernel.sh,v
retrieving revision 1.13
diff -u -p -r1.13 reorder_kernel.sh
--- libexec/reorder_kernel/reorder_kernel.sh7 Nov 2022 15:55:56 -   
1.13
+++ libexec/reorder_kernel/reorder_kernel.sh14 Nov 2022 19:33:25 -
@@ -27,13 +27,32 @@ LOGFILE=$KERNEL_DIR/$KERNEL/relink.log
 PROGNAME=${0##*/}
 SHA256=/var/db/kernel.SHA256
 
-# Silently skip if on a NFS mounted filesystem.
-df -t nonfs $KERNEL_DIR >/dev/null 2>&1
+# Silently skip if on NFS, otherwise remount the filesystem read-write.
+DEV=$(df -t ffs $KERNEL_DIR 2>/dev/null | awk 'NR == 2 { print $1 }')
+MP=$(mount | grep ^$DEV)
+RO=false
+if [[ $MP == *read-only* ]]; then
+   MP=${MP%% *}
+   mount -u -w $MP
+   RO=true
+fi
+
+restore_mount() {
+   if $RO; then
+   # Close the logfile to unbusy $MP by switching back to console.
+   exec 1>/dev/console
+   exec 2>&1
+   mount -u -r $MP
+   fi
+}
 
 # Install trap handlers to inform about success or failure via syslog.
 ERRMSG='failed'
-trap 'trap - EXIT; logger -st $PROGNAME "$ERRMSG" >/dev/console 2>&1' ERR
-trap 'logger -t $PROGNAME "kernel relinking done"' EXIT
+trap 'trap - EXIT
+   logger -st $PROGNAME "$ERRMSG" 2>/dev/console
+   restore_mount' ERR
+trap 'logger -t $PROGNAME "kernel relinking done"
+   restore_mount' EXIT
 
 # Create kernel compile dir and redirect stdout/stderr to a logfile.
 mkdir -m 700 -p $KERNEL_DIR/$KERNEL



Re: xenodm: save ~/.xesssion to ~/.xsession.old

2022-11-14 Thread Theo de Raadt
Stuart Henderson  wrote:

> > Index: app/xenodm//config/Xsession.in
> > ===
> > RCS file: /cvs/xenocara/app/xenodm/config/Xsession.in,v
> > retrieving revision 1.2
> > diff -u -p -r1.2 Xsession.in
> > --- app/xenodm//config/Xsession.in  1 Jul 2022 20:42:06 -   1.2
> > +++ app/xenodm//config/Xsession.in  14 Nov 2022 16:47:03 -
> > @@ -7,6 +7,7 @@ exec_prefix="@exec_prefix@"
> >  # redirect errors to a file in user's home directory if we can
> >  
> >  errfile="$HOME/.xsession-errors"
> > +cp -f "$errfile" "$errfile.old" 2> /dev/null
> 
> Those files can get pretty big. mv is probably a better idea than cp.

And consider the target being a symbolic link, both file and directory




Re: xenodm: save ~/.xesssion to ~/.xsession.old

2022-11-14 Thread Stuart Henderson
On 2022/11/14 16:50, Klemens Nanni wrote:
> X segfaulted when I opened a window, Xorg.log.old only showed the
> address without anything specific, no core dump was created and
> xenodm automatically restarted.
> 
> After I logged in I checked ~/.xsession for possible indications, but
> that file gets truncated on login.
> 
> I've restarted xenodm again, so the old Xorg.log.old containing the
> generic "X segfaulted at addr 0x86..." is also gone.
> 
> Woul it make sense to save an old copy in analogy to Xorg.log.old
> so crashes like this have a higher chance of being hunted down?
> 
> I haven't extensively tested this patch, but relogging into X now leaves
> ~/.xesssion-errors.old with logs from the previous session behind.
> 
> Index: app/xenodm//config/Xsession.in
> ===
> RCS file: /cvs/xenocara/app/xenodm/config/Xsession.in,v
> retrieving revision 1.2
> diff -u -p -r1.2 Xsession.in
> --- app/xenodm//config/Xsession.in1 Jul 2022 20:42:06 -   1.2
> +++ app/xenodm//config/Xsession.in14 Nov 2022 16:47:03 -
> @@ -7,6 +7,7 @@ exec_prefix="@exec_prefix@"
>  # redirect errors to a file in user's home directory if we can
>  
>  errfile="$HOME/.xsession-errors"
> +cp -f "$errfile" "$errfile.old" 2> /dev/null

Those files can get pretty big. mv is probably a better idea than cp.

>  if ( umask 077 && cp /dev/null "$errfile" 2> /dev/null )
>  then
>   exec > "$errfile" 2>&1
> 



Re: xenodm: save ~/.xesssion to ~/.xsession.old

2022-11-14 Thread Aaron Bieber


Klemens Nanni  writes:

> X segfaulted when I opened a window, Xorg.log.old only showed the
> address without anything specific, no core dump was created and
> xenodm automatically restarted.
>
> After I logged in I checked ~/.xsession for possible indications, but
> that file gets truncated on login.
>
> I've restarted xenodm again, so the old Xorg.log.old containing the
> generic "X segfaulted at addr 0x86..." is also gone.
>
> Woul it make sense to save an old copy in analogy to Xorg.log.old
> so crashes like this have a higher chance of being hunted down?
>
> I haven't extensively tested this patch, but relogging into X now leaves
> ~/.xesssion-errors.old with logs from the previous session behind.
>

Tested, works as expected! This would have helped me semi-recently!

OK abieber@ fwiw :D

> Index: app/xenodm//config/Xsession.in
> ===
> RCS file: /cvs/xenocara/app/xenodm/config/Xsession.in,v
> retrieving revision 1.2
> diff -u -p -r1.2 Xsession.in
> --- app/xenodm//config/Xsession.in1 Jul 2022 20:42:06 -   1.2
> +++ app/xenodm//config/Xsession.in14 Nov 2022 16:47:03 -
> @@ -7,6 +7,7 @@ exec_prefix="@exec_prefix@"
>  # redirect errors to a file in user's home directory if we can
>  
>  errfile="$HOME/.xsession-errors"
> +cp -f "$errfile" "$errfile.old" 2> /dev/null
>  if ( umask 077 && cp /dev/null "$errfile" 2> /dev/null )
>  then
>   exec > "$errfile" 2>&1



Re: mips64, loongson, octeon: switch to clockintr(9)

2022-11-14 Thread Visa Hankala
On Sun, Nov 06, 2022 at 07:48:09PM +, Scott Cheloha wrote:
> This patch switches loongson and octeon to clockintr(9).
> 
> It has survived several release builds and upgrades from the resulting
> bsd.rd images on my ER-4.  The ER-4 doesn't have enough RAM to crunch a
> parallel release build.  It chokes on some of the larger LLVM modules.
> 
> visa@ reports it survived a partial build on a loongson machine (he
> skipped LLVM).  I believe he is also testing this on a package
> building machine, too.
> 
> Testing on beefier octeon machines would help demonstrate this is
> stable.  My ER-4 only has USB2.0, which slows things down.

So far, this patch has worked fine on the mips64 package build machines.

> Notes:
> 
> - octeon and loongson machines now have a randomized statclock().
> 
> - This patch merely disables the loongson glxclk.  If the device has
>   no other use we can fully remove the driver in a separate patch.

Lets keep the driver for now. The disabling is fine for the time being.

> @@ -324,6 +324,10 @@ cpu_initclocks(void)
>   tc_init(_timecounter);
>   }
>  
> + stathz = hz;
> + profhz = stathz * 10;
> + clockintr_init(CL_RNDSTAT);

I think this clockintr_init() should be in cp0_startclock(). This would
let other clock drivers do their own adjusting of the hz variables
before clockintr initialization. With this fixed,

OK visa@



Re: export {b,r}ootduid as sysctl, installer/sysupgrade improvements

2022-11-14 Thread Theo de Raadt
An OpenBSD machine only has one OpenBSD install.

As soon as we leave that model, and allow other setup models, perhaps you
think there will be two or three potential configurations that people setup.

I don't think so, I think it will keep being extended by people who do
more and more scewed up bizzare configurations, all of which (obviously, to
you) will need to be supported, and everything gets more complicated.

I think we should say "STOP". Now.  And not start down that roadmap.


How many Windows configurations can I put onto a PC?
Can I put two Andriod configurations onto my phone?


You built something for a testlab.  Your conclusion is that it should
work for everyone.  I simply cannot come to the same conclusion,
because it requires complexity, but instead I think we should embrace
simplicity even if it limits choice.

Klemens Nanni  wrote:

> On Mon, Nov 14, 2022 at 07:49:11AM -0700, Theo de Raadt wrote:
> > Klemens Nanni  wrote:
> > 
> > > This is because the installer always considers the first root disk it
> > > finds as the one to upgrade, which is certainly not what I intend or
> > > expect when booting/upgrading the softraid installation on sd1-3.
> > 
> > What does
> > 
> >  first root disk
> > 
> > Mean?
> 
> One machine with two phsyical disks, say one NVMe and regular SSD.
> Both disks contain a standalone OpenBSD installation.
> 
> I consider each of them a root disk.
> 
> > 
> > There is only one root disk.  The root disk is the one that actually 
> > contains
> > the / that is mounted.
> > 
> > It is this one:
> > 
> > root on sd0a (fb786f6b01042b30.a) swap on sd0b dump on sd0b
> > 
> > You cannot change this.  If you use the bootloader to tell the kernel do
> > do something else, then I argue that sysupgrade and the installer should
> > punish you unless you *manually tell it that every time*
> 
> I don't set anything in /etc/boot.conf or the boot> prompt.
> 
> I select the disk to boot from in the UEFI boot manager.
> 
> > 
> > The install script knows what the root filesystem is using very simple
> > heuristics, but by creating two new sysctl, I am afraid you will enrich
> > this ability to support bizzare configurations that did not work, and
> > I argue *should never work*.
> > 
> > > It is probably not that common to have multiple installations/root disks
> > > in one machine, but it isn't "weird" to me, either.
> > 
> > What?  It is not weird
> > 
> > I think this is unsupported bullshit.
> > 
> > Why do we need the install script to support this configuration you
> > created?  Why do we need to encourage other people to have such
> > configurations?  When they create such a configuration, and find the
> > tooling can support it, won't they go and do even stranger things, then
> > find the tooling doesn't support that even-stranger setup, and then
> > you'll come back adding support for increasingly strange setups, and
> > eventually we are going to end up with a large userbase *not using* a
> > single root filesystem?
> > 
> > > Overwriting the wrong system during an upgrade because the installer
> > > makes too big of an assumption about the first disk is weird to me.
> > > 
> > > 
> > > I can post console logs later showing how the installer picks the wrong
> > > disk, if you want.
> > 
> > There is only one possible root filesystem.
> > 
> > If you created multiple root filesystems, you have created a mess and
> > why is it wrong for me to argue you need to experience pain for the
> > decisions that led you there?
> > 
> > I do not think you are being honest about the reason for these extensions.
> > 
> 
> I'm pretty honest, but apparently not precise enough.
> 
> Now that all the softraid/installboot diffs landed and the dust has
> settled, let me iterate over and test my setup again to make sure I'm
> not tripping over my own mistakes.
> 
> Then I can come back with a clear update or reproducer.
> 



Re: export {b,r}ootduid as sysctl, installer/sysupgrade improvements

2022-11-14 Thread Klemens Nanni
On Mon, Nov 14, 2022 at 07:49:11AM -0700, Theo de Raadt wrote:
> Klemens Nanni  wrote:
> 
> > This is because the installer always considers the first root disk it
> > finds as the one to upgrade, which is certainly not what I intend or
> > expect when booting/upgrading the softraid installation on sd1-3.
> 
> What does
> 
>  first root disk
> 
> Mean?

One machine with two phsyical disks, say one NVMe and regular SSD.
Both disks contain a standalone OpenBSD installation.

I consider each of them a root disk.

> 
> There is only one root disk.  The root disk is the one that actually contains
> the / that is mounted.
> 
> It is this one:
> 
> root on sd0a (fb786f6b01042b30.a) swap on sd0b dump on sd0b
> 
> You cannot change this.  If you use the bootloader to tell the kernel do
> do something else, then I argue that sysupgrade and the installer should
> punish you unless you *manually tell it that every time*

I don't set anything in /etc/boot.conf or the boot> prompt.

I select the disk to boot from in the UEFI boot manager.

> 
> The install script knows what the root filesystem is using very simple
> heuristics, but by creating two new sysctl, I am afraid you will enrich
> this ability to support bizzare configurations that did not work, and
> I argue *should never work*.
> 
> > It is probably not that common to have multiple installations/root disks
> > in one machine, but it isn't "weird" to me, either.
> 
> What?  It is not weird
> 
> I think this is unsupported bullshit.
> 
> Why do we need the install script to support this configuration you
> created?  Why do we need to encourage other people to have such
> configurations?  When they create such a configuration, and find the
> tooling can support it, won't they go and do even stranger things, then
> find the tooling doesn't support that even-stranger setup, and then
> you'll come back adding support for increasingly strange setups, and
> eventually we are going to end up with a large userbase *not using* a
> single root filesystem?
> 
> > Overwriting the wrong system during an upgrade because the installer
> > makes too big of an assumption about the first disk is weird to me.
> > 
> > 
> > I can post console logs later showing how the installer picks the wrong
> > disk, if you want.
> 
> There is only one possible root filesystem.
> 
> If you created multiple root filesystems, you have created a mess and
> why is it wrong for me to argue you need to experience pain for the
> decisions that led you there?
> 
> I do not think you are being honest about the reason for these extensions.
> 

I'm pretty honest, but apparently not precise enough.

Now that all the softraid/installboot diffs landed and the dust has
settled, let me iterate over and test my setup again to make sure I'm
not tripping over my own mistakes.

Then I can come back with a clear update or reproducer.



xenodm: save ~/.xesssion to ~/.xsession.old

2022-11-14 Thread Klemens Nanni
X segfaulted when I opened a window, Xorg.log.old only showed the
address without anything specific, no core dump was created and
xenodm automatically restarted.

After I logged in I checked ~/.xsession for possible indications, but
that file gets truncated on login.

I've restarted xenodm again, so the old Xorg.log.old containing the
generic "X segfaulted at addr 0x86..." is also gone.

Woul it make sense to save an old copy in analogy to Xorg.log.old
so crashes like this have a higher chance of being hunted down?

I haven't extensively tested this patch, but relogging into X now leaves
~/.xesssion-errors.old with logs from the previous session behind.

Index: app/xenodm//config/Xsession.in
===
RCS file: /cvs/xenocara/app/xenodm/config/Xsession.in,v
retrieving revision 1.2
diff -u -p -r1.2 Xsession.in
--- app/xenodm//config/Xsession.in  1 Jul 2022 20:42:06 -   1.2
+++ app/xenodm//config/Xsession.in  14 Nov 2022 16:47:03 -
@@ -7,6 +7,7 @@ exec_prefix="@exec_prefix@"
 # redirect errors to a file in user's home directory if we can
 
 errfile="$HOME/.xsession-errors"
+cp -f "$errfile" "$errfile.old" 2> /dev/null
 if ( umask 077 && cp /dev/null "$errfile" 2> /dev/null )
 then
exec > "$errfile" 2>&1



Netbooting Sparc64

2022-11-14 Thread Andrew Grillet
Does anyone know what could cause this ...

SC Alert: Host System has Reset
\

Sun Fire T200, No Keyboard
Copyright (c) 1998, 2010, Oracle and/or its affiliates. All rights reserved.
OpenBoot 4.30.4.b, 16256 MB memory available, Serial #82196894.
Ethernet address 0:14:4f:e6:39:9e, Host ID: 84e6399e.



Bad magic number in disk label
Can't open disk label package
ERROR: boot-read fail
Boot device: net  File and args:
1000 Mbps full duplex  Link up
Requesting Internet Address for 0:14:4f:e6:39:9e
Requesting Internet Address for 0:14:4f:e6:39:9e
Requesting Internet Address for 0:14:4f:e6:39:9e
>> OpenBSD BOOT 1.22
1000 Mbps full duplex  Link up
Using BOOTPARAMS protocol: ip address: 192.168.2.100bootparamd: 'whoami'
call failed
Using BOOTP protocol: ip address: 192.168.2.100, hostname: elk, netmask:
255.255.255.0, gateway: 192.168.2.254
root addr=192.168.2.15 path=/install/
Trying bsd...
1000 Mbps full duplex  Link up
Using BOOTPARAMS protocol: ip address: 192.168.2.100bootparamd: 'whoami'
call failed
Using BOOTP protocol: ip address: 192.168.2.100, hostname: elk, netmask:
255.255.255.0, gateway: 192.168.2.254
root addr=192.168.2.15 path=/install/
1000 Mbps full duplex  Link up
Using BOOTPARAMS protocol: ip address: 192.168.2.100bootparamd: 'whoami'
call failed
Using BOOTP protocol: ip address: 192.168.2.100, hostname: elk, netmask:
255.255.255.0, gateway: 192.168.2.254
root addr=192.168.2.15 path=/install/
Booting /pci@780/pci@0/pci@1/network@0/bsd
8492368@0x100+2736@0x1819550+196552@0x1c0+3997752@0x1c2ffc8
symbols @ 0xfec42400 470285+165+565944+375574 start=0x100
[ using 1413000 bytes of bsd ELF symbol table ]
console is /virtual-devices@100/console@1
Copyright (c) 1982, 1986, 1989, 1991, 1993
The Regents of the University of California.  All rights reserved.
Copyright (c) 1995-2018 OpenBSD. All rights reserved.
https://www.OpenBSD.org

OpenBSD 6.3 (GENERIC) #480: Sat Mar 24 22:11:46 MDT 2018
dera...@sparc64.openbsd.org:/usr/src/sys/arch/sparc64/compile/GENERIC
real mem = 17045651456 (16256MB)
avail mem = 16729997312 (15954MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root: Sun Fire T200
cpu0 at mainbus0: SUNW,UltraSPARC-T1 (rev 0.0) @ 1000 MHz
"SUNW,UltraSPARC-T1" at mainbus0 not configured
[repeated for N cpus]
vbus0 at mainbus0
"flashprom" at vbus0 not configured
cbus0 at vbus0
vldc0 at cbus0
vldcp0 at vldc0 chan 0x0: ivec 0x0, 0x1 channel "hvctl"
"ldom-primary" at vldc0 chan 0x1 not configured
"fmactl" at vldc0 chan 0x3 not configured
vldc1 at cbus0
"ldmfma" at vldc1 chan 0x4 not configured
vldc2 at cbus0
vldcp1 at vldc2 chan 0x14: ivec 0x28, 0x29 channel "spds"
"system-management" at vldc2 chan 0xd not configured
vcons0 at vbus0: ivec 0x111, console
vrtc0 at vbus0
"fma" at vbus0 not configured
"sunvts" at vbus0 not configured
"sunmc" at vbus0 not configured
"explorer" at vbus0 not configured
"led" at vbus0 not configured
"flashupdate" at vbus0 not configured
"ncp" at vbus0 not configured
vpci0 at mainbus0: bus 2 to 7, dvma map 8000-
pci0 at vpci0
ppb0 at pci0 dev 0 function 0 "PLX PEX 8532" rev 0xbc
pci1 at ppb0 bus 3
ppb1 at pci1 dev 1 function 0 "PLX PEX 8532" rev 0xbc
pci2 at ppb1 bus 4
em0 at pci2 dev 0 function 0 "Intel 82571EB" rev 0x06: ivec 0x795, address
00:14:4f:e6:39:9e
em1 at pci2 dev 0 function 1 "Intel 82571EB" rev 0x06: ivec 0x796, address
00:14:4f:e6:39:9f
ppb2 at pci1 dev 2 function 0 "PLX PEX 8532" rev 0xbc
pci3 at ppb2 bus 5
ppb3 at pci1 dev 8 function 0 "PLX PEX 8532" rev 0xbc: msi
pci4 at ppb3 bus 6
ppb4 at pci1 dev 9 function 0 "PLX PEX 8532" rev 0xbc
pci5 at ppb4 bus 7
mpi0 at pci5 dev 0 function 0 "Symbios Logic SAS1064E" rev 0x02: msi
mpi0: UNUSED, firmware 1.9.0.0
scsibus1 at mpi0: 63 targets
sd0 at scsibus1 targ 0 lun 0:  SCSI4
0/direct fixed naa.5000cca0222458c0
vpci1 at mainbus0: bus 2 to 9, dvma map 8000-
pci6 at vpci1
ppb5 at pci6 dev 0 function 0 "PLX PEX 8532" rev 0xbc
pci7 at ppb5 bus 3
ppb6 at pci7 dev 1 function 0 "PLX PEX 8532" rev 0xbc
pci8 at ppb6 bus 4
ppb7 at pci8 dev 0 function 0 "Intel 41210 PCIE-PCIX" rev 0x09
pci9 at ppb7 bus 5
ebus0 at pci9 dev 2 function 0 "Acer Labs M1533 ISA" rev 0x00
com0 at ebus0 addr 3f8-3ff ivec 0x2: ns16550a, 16 byte fifo
ohci0 at pci9 dev 5 function 0 "Acer Labs M5237 USB" rev 0x03: ivec 0x7c1,
version 1.0, legacy support
ohci1 at pci9 dev 6 function 0 "Acer Labs M5237 USB" rev 0x03: ivec 0x7c3,
version 1.0, legacy support
pciide0 at pci9 dev 8 function 0 "Acer Labs M5229 UDMA IDE" rev 0xc4: DMA,
channel 0 configured to native-PCI, channel 1 configured to native-PCI
pciide0: using ivec 0x7c4 for native-PCI interrupt
atapiscsi0 at pciide0 channel 0 drive 0
scsibus2 at atapiscsi0: 2 targets
cd0 at scsibus2 targ 0 lun 0:  ATAPI 5/cdrom
removable
cd0(pciide0:0:0): using PIO mode 4, Ultra-DMA mode 2
pciide0: channel 1 disabled (no drives)
usb0 at ohci0: USB revision 1.0
uhub0 at usb0 configuration 1 interface 0 "Acer Labs OHCI root hub" rev
1.00/1.00 addr 1
usb1 at ohci1: USB 

Re: Turn sowriteable(), sballoc() and sbfree() macro to inline functions

2022-11-14 Thread Vitaliy Makkoveev
On Mon, Nov 14, 2022 at 12:00:28PM +, Klemens Nanni wrote:
> On Mon, Nov 14, 2022 at 02:14:27PM +0300, Vitaliy Makkoveev wrote:
> > We have soreadable() already presented as inline function, but
> > corresponding sowriteable() is still macro. Also it's no reason to keep
> > sballoc() and sbfree() as macro.
> 
> This reads as an improvement to me.
> 
> sballoc() and sbfree()'s struct socket *so argument is entirely unused,
> can it be removed?
> 

It could, but I like to keep it for a while. It is useful for my
per buffer locking games.

> > 
> > Index: sys/sys/protosw.h
> > ===
> > RCS file: /cvs/src/sys/sys/protosw.h,v
> > retrieving revision 1.58
> > diff -u -p -r1.58 protosw.h
> > --- sys/sys/protosw.h   17 Oct 2022 14:49:02 -  1.58
> > +++ sys/sys/protosw.h   14 Nov 2022 11:07:29 -
> > @@ -53,6 +53,9 @@
> >   * described below.
> >   */
> >  
> > +#ifndef _SYS_PROTOSW_H_
> > +#define _SYS_PROTOSW_H_
> > +
> >  struct mbuf;
> >  struct sockaddr;
> >  struct socket;
> > @@ -413,3 +416,5 @@ pru_connect2(struct socket *so1, struct 
> >  }
> >  
> >  #endif
> > +
> > +#endif /* _SYS_PROTOSW_H_ */
> > Index: sys/sys/socketvar.h
> > ===
> > RCS file: /cvs/src/sys/sys/socketvar.h,v
> > retrieving revision 1.111
> > diff -u -p -r1.111 socketvar.h
> > --- sys/sys/socketvar.h 3 Oct 2022 16:43:52 -   1.111
> > +++ sys/sys/socketvar.h 14 Nov 2022 11:07:29 -
> > @@ -160,6 +160,7 @@ struct socket {
> >  
> >  #ifdef _KERNEL
> >  
> > +#include 
> >  #include 
> >  
> >  void   soassertlocked(struct socket *);
> > @@ -229,31 +230,39 @@ soreadable(struct socket *so)
> >  }
> >  
> >  /* can we write something to so? */
> > -#definesowriteable(so) \
> > -((sbspace((so), &(so)->so_snd) >= (so)->so_snd.sb_lowat && \
> > -   (((so)->so_state & SS_ISCONNECTED) || \
> > - ((so)->so_proto->pr_flags & PR_CONNREQUIRED)==0)) || \
> > -((so)->so_state & SS_CANTSENDMORE) || (so)->so_error)
> > +static inline int
> > +sowriteable(struct socket *so)
> > +{
> > +   soassertlocked(so);
> > +   return ((sbspace(so, >so_snd) >= so->so_snd.sb_lowat &&
> > +   ((so->so_state & SS_ISCONNECTED) ||
> > +   (so->so_proto->pr_flags & PR_CONNREQUIRED)==0)) ||
> > +   (so->so_state & SS_CANTSENDMORE) || so->so_error);
> > +}
> >  
> >  /* adjust counters in sb reflecting allocation of m */
> > -#definesballoc(so, sb, m) do { 
> > \
> > -   (sb)->sb_cc += (m)->m_len;  \
> > -   if ((m)->m_type != MT_CONTROL && (m)->m_type != MT_SONAME)  \
> > -   (sb)->sb_datacc += (m)->m_len;  \
> > -   (sb)->sb_mbcnt += MSIZE;\
> > -   if ((m)->m_flags & M_EXT)   \
> > -   (sb)->sb_mbcnt += (m)->m_ext.ext_size;  \
> > -} while (/* CONSTCOND */ 0)
> > +static inline void
> > +sballoc(struct socket *so, struct sockbuf *sb, struct mbuf *m)
> > +{
> > +   sb->sb_cc += m->m_len;
> > +   if (m->m_type != MT_CONTROL && m->m_type != MT_SONAME)
> > +   sb->sb_datacc += m->m_len;
> > +   sb->sb_mbcnt += MSIZE;
> > +   if (m->m_flags & M_EXT)
> > +   sb->sb_mbcnt += m->m_ext.ext_size;
> > +}
> >  
> >  /* adjust counters in sb reflecting freeing of m */
> > -#definesbfree(so, sb, m) do {  
> > \
> > -   (sb)->sb_cc -= (m)->m_len;  \
> > -   if ((m)->m_type != MT_CONTROL && (m)->m_type != MT_SONAME)  \
> > -   (sb)->sb_datacc -= (m)->m_len;  \
> > -   (sb)->sb_mbcnt -= MSIZE;\
> > -   if ((m)->m_flags & M_EXT)   \
> > -   (sb)->sb_mbcnt -= (m)->m_ext.ext_size;  \
> > -} while (/* CONSTCOND */ 0)
> > +static inline void
> > +sbfree(struct socket *so, struct sockbuf *sb, struct mbuf *m)
> > +{
> > +   sb->sb_cc -= m->m_len;
> > +   if (m->m_type != MT_CONTROL && m->m_type != MT_SONAME)
> > +   sb->sb_datacc -= m->m_len;
> > +   sb->sb_mbcnt -= MSIZE;
> > +   if (m->m_flags & M_EXT)
> > +   sb->sb_mbcnt -= m->m_ext.ext_size;
> > +}
> >  
> >  /*
> >   * Set lock on sockbuf sb; sleep if lock is already held.
> > 



Re: libX11 patch for X*IfEvent() API issues, take #2

2022-11-14 Thread Walter Alejandro Iglesias
Hi Matthiew,

On Nov 11 2022, Walter Alejandro Iglesias wrote:
> On Nov 11 2022, Matthieu Herrb wrote:
> > On Fri, Nov 11, 2022 at 03:17:21PM +0100, Walter Alejandro Iglesias wrote:
> > > On Nov 11 2022, Matthieu Herrb wrote:
> > > > Hi,
> > > > 
> > > > So the patch provided by Adam Jackson upstreams is completely buggy.
> > > > 
> > > > - the logic to setup the new locking function was busted
> > > > - I've observed  cases where even the XGetIfEvent() function gets
> > > >   re-rentered. So the flags does in fact need to be a counter.
> > > > 
> > > > New patch which works for me with fvwm2...
> > > > 
> > > > Testing is welcome...
> > > 
> > > Unfortunately I still can reproduce the bug.
> > > 
> > > This patch also makes firefox crash, (I'm not sure if it's because of
> > > firefox or cwm).  After reinstalling xenocara from the snapshots
> > > packages I could run firefox again.  
> > > 
> > > fvwm is giving a lot of problems lately, there's also this drm bug I
> > > reported: 
> > > 
> > >   https://marc.info/?l=openbsd-bugs=166332904632232=2
> > > 
> > > Now FvwmIcons is crashing on fvwm3...  I'm giving up with fvwm, I'm more
> > > comfortable with cwm.
> > 
> > Ok, I've figured out the cwm + firefox issue. There's also a
> > XInternalLock that needs to be neutered in the X*IfEvent() callbacks.
> > 
> > New diff, on which I've left my debug printfs. If you get other
> > crashes it may be interesting to look in .xsession-errors...
> 
> This patch solved the firefox cwm issue.  Firefox shows this when
> it starts: 
> 
>   ~$ firefox
>   XXX _XLockDisplay 1
>   XXX _XIfEventInternalLockDisplay 1
>   XXX _XIfEventUnlockDisplay 0
> 
> fvwm2/3 still crashing.

Today, after upgrading to the latest snapshot I can not reproduce this
fvwm bug anymore.  I ignore why.

> 
> 
> 
> > 
> > Index: Makefile.bsd-wrapper
> > ===
> > RCS file: /cvs/OpenBSD/xenocara/lib/libX11/Makefile.bsd-wrapper,v
> > retrieving revision 1.29
> > diff -u -p -u -r1.29 Makefile.bsd-wrapper
> > --- Makefile.bsd-wrapper3 Sep 2022 06:55:25 -   1.29
> > +++ Makefile.bsd-wrapper11 Nov 2022 15:13:59 -
> > @@ -14,7 +14,6 @@ SHARED_LIBS=  X11 18.0 X11_xcb 2.0
> >  
> >  CONFIGURE_ARGS= --enable-tcp-transport --enable-unix-transport 
> > --enable-ipv6 \
> > --disable-composecache \
> > -   --disable-thread-safety-constructor \
> > --without-xmlto --without-fop --without-xsltproc
> >  
> >  .include 
> > Index: include/X11/Xlibint.h
> > ===
> > RCS file: /cvs/OpenBSD/xenocara/lib/libX11/include/X11/Xlibint.h,v
> > retrieving revision 1.15
> > diff -u -p -u -r1.15 Xlibint.h
> > --- include/X11/Xlibint.h   21 Feb 2022 08:01:24 -  1.15
> > +++ include/X11/Xlibint.h   11 Nov 2022 15:14:00 -
> > @@ -207,6 +207,7 @@ struct _XDisplay
> >  
> > XIOErrorExitHandler exit_handler;
> > void *exit_handler_data;
> > +CARD32 in_ifevent;
> >  };
> >  
> >  #define XAllocIDs(dpy,ids,n) (*(dpy)->idlist_alloc)(dpy,ids,n)
> > Index: src/ChkIfEv.c
> > ===
> > RCS file: /cvs/OpenBSD/xenocara/lib/libX11/src/ChkIfEv.c,v
> > retrieving revision 1.4
> > diff -u -p -u -r1.4 ChkIfEv.c
> > --- src/ChkIfEv.c   30 May 2011 19:19:38 -  1.4
> > +++ src/ChkIfEv.c   11 Nov 2022 15:14:00 -
> > @@ -49,6 +49,7 @@ Bool XCheckIfEvent (
> > unsigned long qe_serial = 0;
> > int n;  /* time through count */
> >  
> > +dpy->in_ifevent++;
> >  LockDisplay(dpy);
> > prev = NULL;
> > for (n = 3; --n >= 0;) {
> > @@ -60,6 +61,7 @@ Bool XCheckIfEvent (
> > *event = qelt->event;
> > _XDeq(dpy, prev, qelt);
> > _XStoreEventCookie(dpy, event);
> > +dpy->in_ifevent = False;
> > UnlockDisplay(dpy);
> > return True;
> > }
> > @@ -78,6 +80,7 @@ Bool XCheckIfEvent (
> > /* another thread has snatched this event */
> > prev = NULL;
> > }
> > +dpy->in_ifevent--;
> > UnlockDisplay(dpy);
> > return False;
> >  }
> > Index: src/IfEvent.c
> > ===
> > RCS file: /cvs/OpenBSD/xenocara/lib/libX11/src/IfEvent.c,v
> > retrieving revision 1.4
> > diff -u -p -u -r1.4 IfEvent.c
> > --- src/IfEvent.c   30 May 2011 19:19:38 -  1.4
> > +++ src/IfEvent.c   11 Nov 2022 15:14:00 -
> > @@ -48,6 +48,7 @@ XIfEvent (
> > register _XQEvent *qelt, *prev;
> > unsigned long qe_serial = 0;
> >  
> > +dpy->in_ifevent++;
> >  LockDisplay(dpy);
> > prev = NULL;
> > while (1) {
> > @@ -59,6 +60,7 @@ XIfEvent (
> > *event = qelt->event;
> > _XDeq(dpy, prev, qelt);
> > _XStoreEventCookie(dpy, event);
> > +

Re: export {b,r}ootduid as sysctl, installer/sysupgrade improvements

2022-11-14 Thread Theo de Raadt
> sd1-3 are softraid chunks hosting a separate installation for testing.
> Booting into this

I think this is where you went wrong.  Expecting this to work is going
to result in 20-40 diffs bloating all the media for a configuration which
less than than 1 in a thousand people need.





Re: export {b,r}ootduid as sysctl, installer/sysupgrade improvements

2022-11-14 Thread Theo de Raadt
Klemens Nanni  wrote:

> This is because the installer always considers the first root disk it
> finds as the one to upgrade, which is certainly not what I intend or
> expect when booting/upgrading the softraid installation on sd1-3.

What does

 first root disk

Mean?

There is only one root disk.  The root disk is the one that actually contains
the / that is mounted.

It is this one:

root on sd0a (fb786f6b01042b30.a) swap on sd0b dump on sd0b

You cannot change this.  If you use the bootloader to tell the kernel do
do something else, then I argue that sysupgrade and the installer should
punish you unless you *manually tell it that every time*

The install script knows what the root filesystem is using very simple
heuristics, but by creating two new sysctl, I am afraid you will enrich
this ability to support bizzare configurations that did not work, and
I argue *should never work*.

> It is probably not that common to have multiple installations/root disks
> in one machine, but it isn't "weird" to me, either.

What?  It is not weird

I think this is unsupported bullshit.

Why do we need the install script to support this configuration you
created?  Why do we need to encourage other people to have such
configurations?  When they create such a configuration, and find the
tooling can support it, won't they go and do even stranger things, then
find the tooling doesn't support that even-stranger setup, and then
you'll come back adding support for increasingly strange setups, and
eventually we are going to end up with a large userbase *not using* a
single root filesystem?

> Overwriting the wrong system during an upgrade because the installer
> makes too big of an assumption about the first disk is weird to me.
> 
> 
> I can post console logs later showing how the installer picks the wrong
> disk, if you want.

There is only one possible root filesystem.

If you created multiple root filesystems, you have created a mess and
why is it wrong for me to argue you need to experience pain for the
decisions that led you there?

I do not think you are being honest about the reason for these extensions.



Re: mount_ntfs.8: Fix swapped -g user and -u group

2022-11-14 Thread Stuart Henderson
ha, good catch. committed, thanks.



Re: export {b,r}ootduid as sysctl, installer/sysupgrade improvements

2022-11-14 Thread Klemens Nanni
On Mon, Nov 14, 2022 at 05:45:33AM -0700, Theo de Raadt wrote:
> Mark Kettenis  wrote:
> 
> > > > The installer considers a disk a root disk if 'a' is FFS and contains
> > > > expected files.
> > > > 
> > > > Furthermore, unattended upgrades will always install to the first root
> > > > disk that is found.
> > > > 
> > > > This works fine on machines with only one root disk, but it quickly
> > > > behaves unexpectedly when having multiple disks/installations in one
> > > > machine.
> > > > 
> > > > I run such machines, esp. since fiddling with softraid and installboot.
> 
> 
> I don't understand the situation.
> 
> I suspect you are intentionally creating very weird setups, and now you
> want install script to help people create such weird setups.  Then it
> becomes an additional weird thing we must anticipate in the future.
> 
> Please show a valid reason why boot are different.  It is very hard to
> reason about this when the rest of your email shows an example where
> they are the same.
> 
> > > > The installer/sysupgrade experience can definitely be improved here, but
> > > > that takes some consideration.
> > > > 
> > > > One requirement, imho, is knowing
> > > > 1. which disk we booted from, i.e.
> > > >from which disk the kernel (/bsd.rd or /bsd.upgrade) was loaded
> > > > 2. which disk the root filesystem is on, i.e.
> > > >likely the same disk holding /home where sysupgrade put the sets
> 
> Prove it.
> 

Take a machine with multiple disks like this one:

$ sysctl -n hw.product
SolidRun CEX7 Platform
$ dmesg | grep -e ^sd -e ^root
sd0 at scsibus0 targ 1 lun 0: 
sd0: 238475MB, 512 bytes/sector, 488397168 sectors
sd1 at scsibus1 targ 0 lun 0:  
naa.5000c5002039b4e8
sd1: 152627MB, 512 bytes/sector, 312581808 sectors
sd2 at scsibus2 targ 0 lun 0:  
naa.5000c5002070e824
sd2: 152627MB, 512 bytes/sector, 312581808 sectors
sd3 at scsibus3 targ 0 lun 0:  
naa.5000c500204ada8c
sd3: 152627MB, 512 bytes/sector, 312581808 sectors
root on sd0a (8ae6b6fd29b37541.a) swap on sd0b dump on sd0b

Runnin sysupgrade(8) from sd0 upgrades sd0, as expected.

sd1-3 are softraid chunks hosting a separate installation for testing.
Booting into this

XXX

and sysupgrade'ing from there, however, upgrades sd0, not sd4.

This is because the installer always considers the first root disk it
finds as the one to upgrade, which is certainly not what I intend or
expect when booting/upgrading the softraid installation on sd1-3.

It is probably not that common to have multiple installations/root disks
in one machine, but it isn't "weird" to me, either.

Overwriting the wrong system during an upgrade because the installer
makes too big of an assumption about the first disk is weird to me.


I can post console logs later showing how the installer picks the wrong
disk, if you want.



mount_ntfs.8: Fix swapped -g user and -u group

2022-11-14 Thread Josiah Frentsos
Index: mount_ntfs.8
===
RCS file: /cvs/src/sbin/mount_ntfs/mount_ntfs.8,v
retrieving revision 1.17
diff -u -p -r1.17 mount_ntfs.8
--- mount_ntfs.820 Aug 2022 07:03:24 -  1.17
+++ mount_ntfs.814 Nov 2022 14:06:39 -
@@ -41,10 +41,10 @@
 .Sh SYNOPSIS
 .Nm mount_ntfs
 .Op Fl ai
-.Op Fl g Ar user
+.Op Fl g Ar group
 .Op Fl m Ar mask
 .Op Fl o Ar options
-.Op Fl u Ar group
+.Op Fl u Ar user
 .Ar special
 .Ar node
 .Sh DESCRIPTION



OpenBSD Errata: November 15, 2022 (pixman)

2022-11-14 Thread Alexander Bluhm
Errata patches for X11 pixman library have been released for OpenBSD
7.1 and 7.2.

Binary updates for the amd64, i386 and arm64 platform are available
via the syspatch utility.  Source code patches can be found on the
respective errata page:

  https://www.openbsd.org/errata71.html
  https://www.openbsd.org/errata72.html



Re: non regression tests for some packaging scripts

2022-11-14 Thread Jeremie Courreges-Anglas
On Mon, Nov 14 2022, Marc Espie  wrote:
> I'd like to add some non-regression tests for some scripts, most notably
> register-plist, which is... let's say not perfect at the moment.
>
> I believe
> ${PORTSDIR}/regress is probably the simplest least disturbing way to do that.

We have /usr/ports/tests.

> Since the manpages already exist under src, I could also (somehow) put them
> in /usr/src/regress.
>
> Of course, this means requiring a checkout of the ports tree to check things.
>
> What do you people think ?

I think those belong to the ports tree.

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



Re: export {b,r}ootduid as sysctl, installer/sysupgrade improvements

2022-11-14 Thread Theo de Raadt
Klemens Nanni  wrote:

> I'd like to get on with this, can also add sysctl.2 bits to document
> those before sending diffs using them.

I want you to prove the use case first.

> Mark asked whether CTLTYPE_QUAD would be more suited, but I still don't
> understand how that is supposed to work and there was no answer in this
> thread's other mail.

All the consumers of this use strings.  It is a string in the disklabel
output, a string in the disklabel command, and they are managed as
strings everywhere else.  Outside the installer, the only place it is
handled is as a partition name, which is either DUID.a or sd0a, either
on the commandline or in fstab.  Always hanled as a string, because the
shell handles numbers poorly, let alone hex numbers with extra stuff
afterwards.



Re: export {b,r}ootduid as sysctl, installer/sysupgrade improvements

2022-11-14 Thread Theo de Raadt
Mark Kettenis  wrote:

> > > The installer considers a disk a root disk if 'a' is FFS and contains
> > > expected files.
> > > 
> > > Furthermore, unattended upgrades will always install to the first root
> > > disk that is found.
> > > 
> > > This works fine on machines with only one root disk, but it quickly
> > > behaves unexpectedly when having multiple disks/installations in one
> > > machine.
> > > 
> > > I run such machines, esp. since fiddling with softraid and installboot.


I don't understand the situation.

I suspect you are intentionally creating very weird setups, and now you
want install script to help people create such weird setups.  Then it
becomes an additional weird thing we must anticipate in the future.

Please show a valid reason why boot are different.  It is very hard to
reason about this when the rest of your email shows an example where
they are the same.

> > > The installer/sysupgrade experience can definitely be improved here, but
> > > that takes some consideration.
> > > 
> > > One requirement, imho, is knowing
> > > 1. which disk we booted from, i.e.
> > >from which disk the kernel (/bsd.rd or /bsd.upgrade) was loaded
> > > 2. which disk the root filesystem is on, i.e.
> > >likely the same disk holding /home where sysupgrade put the sets

Prove it.



Re: export {b,r}ootduid as sysctl, installer/sysupgrade improvements

2022-11-14 Thread Mark Kettenis
> Date: Mon, 14 Nov 2022 09:22:13 +
> From: Klemens Nanni 
> 
> On Tue, Sep 06, 2022 at 01:16:47AM +, Klemens Nanni wrote:
> > The installer considers a disk a root disk if 'a' is FFS and contains
> > expected files.
> > 
> > Furthermore, unattended upgrades will always install to the first root
> > disk that is found.
> > 
> > This works fine on machines with only one root disk, but it quickly
> > behaves unexpectedly when having multiple disks/installations in one
> > machine.
> > 
> > I run such machines, esp. since fiddling with softraid and installboot.
> > 
> > 
> > The installer/sysupgrade experience can definitely be improved here, but
> > that takes some consideration.
> > 
> > One requirement, imho, is knowing
> > 1. which disk we booted from, i.e.
> >from which disk the kernel (/bsd.rd or /bsd.upgrade) was loaded
> > 2. which disk the root filesystem is on, i.e.
> >likely the same disk holding /home where sysupgrade put the sets
> > 
> > 
> > The boot disk could be helpful inside installer, e.g. to check if
> > /bsd.ugpraded was booted from a valid root disk -- a good indicator for
> > rebooting from the same disk the user just ran sysupgrade on.
> > 
> > The root disk is of no help inside the installer as that will always be
> > the ramdisk.  But it could be used by sysupgrade to perhaps prefill
> > /auto_upgrade.conf to decide up-front which disk to upgrade.
> > This answer to the 'Which disk is the root disk' question is currently
> > answered inside the installer during unattended upgrades... and it will
> > always be the first valid root disk, which is not always correct.
> > 
> > So to make progress, here's a diff that exports readily available
> > disklabel DUIDs:
> > 
> > # disklabel sd0 | grep duid
> > duid: 98c0c47c3ffddeb4
> > # sysctl hw | grep duid
> > hw.bootduid=98c0c47c3ffddeb4
> > hw.rootduid=98c0c47c3ffddeb4
> > 
> > Having that, working out the installer/sysupgrade bits should be easier.
> > 
> > I'm testing this on arm64 with two disks/installations.
> > 
> > Feedback? Objection? OK?
> 
> I'd like to get on with this, can also add sysctl.2 bits to document
> those before sending diffs using them.
> 
> Mark asked whether CTLTYPE_QUAD would be more suited, but I still don't
> understand how that is supposed to work and there was no answer in this
> thread's other mail.

So my thinking there was that DUIDs are 64-bit integers.  But it seems
they are always interpreted as an array of 8 bytes.  That probably
means that presenting it as a 64-bit integer (which is what
CTLTYPE_QUAD does) isn't a good idea.

> Anyone?

It is somewhat unfortunate that we are exporting binary data as a
string since it means that userland code will have to parse that
string again and turn it into an array of bytes.  We could export it
as CTLTYPE_STRUCT but then we'd need additional code to make sysctl(8)
display the data.

so ok kettenis@

> Index: sys/sys/sysctl.h
> ===
> RCS file: /cvs/src/sys/sys/sysctl.h,v
> retrieving revision 1.231
> diff -u -p -r1.231 sysctl.h
> --- sys/sys/sysctl.h  7 Nov 2022 14:25:44 -   1.231
> +++ sys/sys/sysctl.h  9 Nov 2022 12:06:31 -
> @@ -946,7 +946,9 @@ struct kinfo_file {
>  #define  HW_SMT  24  /* int: enable SMT/HT/CMT */
>  #define  HW_NCPUONLINE   25  /* int: number of cpus being 
> used */
>  #define  HW_POWER26  /* int: machine has wall-power 
> */
> -#define  HW_MAXID27  /* number of valid hw ids */
> +#define  HW_BOOTDUID 27  /* string: DUID of boot disk */
> +#define  HW_ROOTDUID 28  /* string: DUID of root disk */
> +#define  HW_MAXID29  /* number of valid hw ids */
>  
>  #define  CTL_HW_NAMES { \
>   { 0, 0 }, \
> @@ -976,6 +978,8 @@ struct kinfo_file {
>   { "smt", CTLTYPE_INT }, \
>   { "ncpuonline", CTLTYPE_INT }, \
>   { "power", CTLTYPE_INT }, \
> + { "bootduid", CTLTYPE_STRING }, \
> + { "rootduid", CTLTYPE_STRING }, \
>  }
>  
>  /*
> Index: sys/kern/kern_sysctl.c
> ===
> RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
> retrieving revision 1.408
> diff -u -p -r1.408 kern_sysctl.c
> --- sys/kern/kern_sysctl.c7 Nov 2022 14:25:44 -   1.408
> +++ sys/kern/kern_sysctl.c9 Nov 2022 12:06:30 -
> @@ -770,6 +770,12 @@ hw_sysctl(int *name, u_int namelen, void
>   case HW_SMT:
>   return (sysctl_hwsmt(oldp, oldlenp, newp, newlen));
>  #endif
> + case HW_BOOTDUID:
> + return (sysctl_rdstring(oldp, oldlenp, newp,
> + duid_format(bootduid)));
> + case HW_ROOTDUID:
> + return (sysctl_rdstring(oldp, oldlenp, newp,
> + duid_format(rootduid)));
>   default:
>   return sysctl_bounded_arr(hw_vars, 

Re: Turn sowriteable(), sballoc() and sbfree() macro to inline functions

2022-11-14 Thread Klemens Nanni
On Mon, Nov 14, 2022 at 12:00:28PM +, Klemens Nanni wrote:
> On Mon, Nov 14, 2022 at 02:14:27PM +0300, Vitaliy Makkoveev wrote:
> > We have soreadable() already presented as inline function, but
> > corresponding sowriteable() is still macro. Also it's no reason to keep
> > sballoc() and sbfree() as macro.
> 
> This reads as an improvement to me.
> 
> sballoc() and sbfree()'s struct socket *so argument is entirely unused,
> can it be removed?

This builds and runs fine on top of your diff.


diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c
index 478763592b4..e0af311ae3c 100644
--- a/sys/kern/uipc_socket.c
+++ b/sys/kern/uipc_socket.c
@@ -931,7 +931,7 @@ dontblock:
*paddr = m_copym(m, 0, m->m_len, M_NOWAIT);
m = m->m_next;
} else {
-   sbfree(so, >so_rcv, m);
+   sbfree(>so_rcv, m);
if (paddr) {
*paddr = m;
so->so_rcv.sb_mb = m->m_next;
@@ -955,7 +955,7 @@ dontblock:
*controlp = m_copym(m, 0, m->m_len, M_NOWAIT);
m = m->m_next;
} else {
-   sbfree(so, >so_rcv, m);
+   sbfree(>so_rcv, m);
so->so_rcv.sb_mb = m->m_next;
m->m_nextpkt = m->m_next = NULL;
cm = m;
@@ -1057,7 +1057,7 @@ dontblock:
orig_resid = 0;
} else {
nextrecord = m->m_nextpkt;
-   sbfree(so, >so_rcv, m);
+   sbfree(>so_rcv, m);
if (mp) {
*mp = m;
mp = >m_next;
@@ -1560,7 +1560,7 @@ somove(struct socket *so, int wait)
 * that the whole first record can be processed.
 */
m = so->so_rcv.sb_mb;
-   sbfree(so, >so_rcv, m);
+   sbfree(>so_rcv, m);
so->so_rcv.sb_mb = m_free(m);
sbsync(>so_rcv, nextrecord);
}
@@ -1570,7 +1570,7 @@ somove(struct socket *so, int wait)
 */
m = so->so_rcv.sb_mb;
while (m && m->m_type == MT_CONTROL) {
-   sbfree(so, >so_rcv, m);
+   sbfree(>so_rcv, m);
so->so_rcv.sb_mb = m_free(m);
m = so->so_rcv.sb_mb;
sbsync(>so_rcv, nextrecord);
@@ -1609,7 +1609,7 @@ somove(struct socket *so, int wait)
so->so_rcv.sb_datacc -= size;
} else {
*mp = so->so_rcv.sb_mb;
-   sbfree(so, >so_rcv, *mp);
+   sbfree(>so_rcv, *mp);
so->so_rcv.sb_mb = (*mp)->m_next;
sbsync(>so_rcv, nextrecord);
}
diff --git a/sys/kern/uipc_socket2.c b/sys/kern/uipc_socket2.c
index d8b39e44c69..478c93a3f99 100644
--- a/sys/kern/uipc_socket2.c
+++ b/sys/kern/uipc_socket2.c
@@ -845,7 +845,7 @@ sbappendrecord(struct socket *so, struct sockbuf *sb, 
struct mbuf *m0)
 * Put the first mbuf on the queue.
 * Note this permits zero length records.
 */
-   sballoc(so, sb, m0);
+   sballoc(sb, m0);
SBLASTRECORDCHK(sb, "sbappendrecord 1");
SBLINKRECORD(sb, m0);
m = m0->m_next;
@@ -900,8 +900,8 @@ sbappendaddr(struct socket *so, struct sockbuf *sb, const 
struct sockaddr *asa,
SBLASTRECORDCHK(sb, "sbappendaddr 1");
 
for (n = m; n->m_next != NULL; n = n->m_next)
-   sballoc(so, sb, n);
-   sballoc(so, sb, n);
+   sballoc(sb, n);
+   sballoc(sb, n);
nlast = n;
SBLINKRECORD(sb, m);
 
@@ -937,8 +937,8 @@ sbappendcontrol(struct socket *so, struct sockbuf *sb, 
struct mbuf *m0,
SBLASTRECORDCHK(sb, "sbappendcontrol 1");
 
for (m = control; m->m_next != NULL; m = m->m_next)
-   sballoc(so, sb, m);
-   sballoc(so, sb, m);
+   sballoc(sb, m);
+   sballoc(sb, m);
mlast = m;
SBLINKRECORD(sb, control);
 
@@ -993,7 +993,7 @@ sbcompress(struct socket *so, struct sockbuf *sb, struct 
mbuf *m,
else
sb->sb_mb = m;
sb->sb_mbtail = m;
-   sballoc(so, sb, m);
+   sballoc(sb, m);
n = m;
m->m_flags &= ~M_EOR;
m = m->m_next;
@@ -1058,12 +1058,12 @@ sbdrop(struct socket *so, struct sockbuf *sb, int len)
break;
}
len -= m->m_len;
-   sbfree(so, sb, m);
+   sbfree(sb, m);
mn = m_free(m);
m = mn;
}
while (m && m->m_len == 0) {
-   sbfree(so, sb, m);
+ 

Re: Turn sowriteable(), sballoc() and sbfree() macro to inline functions

2022-11-14 Thread Klemens Nanni
On Mon, Nov 14, 2022 at 02:14:27PM +0300, Vitaliy Makkoveev wrote:
> We have soreadable() already presented as inline function, but
> corresponding sowriteable() is still macro. Also it's no reason to keep
> sballoc() and sbfree() as macro.

This reads as an improvement to me.

sballoc() and sbfree()'s struct socket *so argument is entirely unused,
can it be removed?

> 
> Index: sys/sys/protosw.h
> ===
> RCS file: /cvs/src/sys/sys/protosw.h,v
> retrieving revision 1.58
> diff -u -p -r1.58 protosw.h
> --- sys/sys/protosw.h 17 Oct 2022 14:49:02 -  1.58
> +++ sys/sys/protosw.h 14 Nov 2022 11:07:29 -
> @@ -53,6 +53,9 @@
>   * described below.
>   */
>  
> +#ifndef _SYS_PROTOSW_H_
> +#define _SYS_PROTOSW_H_
> +
>  struct mbuf;
>  struct sockaddr;
>  struct socket;
> @@ -413,3 +416,5 @@ pru_connect2(struct socket *so1, struct 
>  }
>  
>  #endif
> +
> +#endif /* _SYS_PROTOSW_H_ */
> Index: sys/sys/socketvar.h
> ===
> RCS file: /cvs/src/sys/sys/socketvar.h,v
> retrieving revision 1.111
> diff -u -p -r1.111 socketvar.h
> --- sys/sys/socketvar.h   3 Oct 2022 16:43:52 -   1.111
> +++ sys/sys/socketvar.h   14 Nov 2022 11:07:29 -
> @@ -160,6 +160,7 @@ struct socket {
>  
>  #ifdef _KERNEL
>  
> +#include 
>  #include 
>  
>  void soassertlocked(struct socket *);
> @@ -229,31 +230,39 @@ soreadable(struct socket *so)
>  }
>  
>  /* can we write something to so? */
> -#define  sowriteable(so) \
> -((sbspace((so), &(so)->so_snd) >= (so)->so_snd.sb_lowat && \
> - (((so)->so_state & SS_ISCONNECTED) || \
> -   ((so)->so_proto->pr_flags & PR_CONNREQUIRED)==0)) || \
> -((so)->so_state & SS_CANTSENDMORE) || (so)->so_error)
> +static inline int
> +sowriteable(struct socket *so)
> +{
> + soassertlocked(so);
> + return ((sbspace(so, >so_snd) >= so->so_snd.sb_lowat &&
> + ((so->so_state & SS_ISCONNECTED) ||
> + (so->so_proto->pr_flags & PR_CONNREQUIRED)==0)) ||
> + (so->so_state & SS_CANTSENDMORE) || so->so_error);
> +}
>  
>  /* adjust counters in sb reflecting allocation of m */
> -#define  sballoc(so, sb, m) do { 
> \
> - (sb)->sb_cc += (m)->m_len;  \
> - if ((m)->m_type != MT_CONTROL && (m)->m_type != MT_SONAME)  \
> - (sb)->sb_datacc += (m)->m_len;  \
> - (sb)->sb_mbcnt += MSIZE;\
> - if ((m)->m_flags & M_EXT)   \
> - (sb)->sb_mbcnt += (m)->m_ext.ext_size;  \
> -} while (/* CONSTCOND */ 0)
> +static inline void
> +sballoc(struct socket *so, struct sockbuf *sb, struct mbuf *m)
> +{
> + sb->sb_cc += m->m_len;
> + if (m->m_type != MT_CONTROL && m->m_type != MT_SONAME)
> + sb->sb_datacc += m->m_len;
> + sb->sb_mbcnt += MSIZE;
> + if (m->m_flags & M_EXT)
> + sb->sb_mbcnt += m->m_ext.ext_size;
> +}
>  
>  /* adjust counters in sb reflecting freeing of m */
> -#define  sbfree(so, sb, m) do {  
> \
> - (sb)->sb_cc -= (m)->m_len;  \
> - if ((m)->m_type != MT_CONTROL && (m)->m_type != MT_SONAME)  \
> - (sb)->sb_datacc -= (m)->m_len;  \
> - (sb)->sb_mbcnt -= MSIZE;\
> - if ((m)->m_flags & M_EXT)   \
> - (sb)->sb_mbcnt -= (m)->m_ext.ext_size;  \
> -} while (/* CONSTCOND */ 0)
> +static inline void
> +sbfree(struct socket *so, struct sockbuf *sb, struct mbuf *m)
> +{
> + sb->sb_cc -= m->m_len;
> + if (m->m_type != MT_CONTROL && m->m_type != MT_SONAME)
> + sb->sb_datacc -= m->m_len;
> + sb->sb_mbcnt -= MSIZE;
> + if (m->m_flags & M_EXT)
> + sb->sb_mbcnt -= m->m_ext.ext_size;
> +}
>  
>  /*
>   * Set lock on sockbuf sb; sleep if lock is already held.
> 



Turn sowriteable(), sballoc() and sbfree() macro to inline functions

2022-11-14 Thread Vitaliy Makkoveev
We have soreadable() already presented as inline function, but
corresponding sowriteable() is still macro. Also it's no reason to keep
sballoc() and sbfree() as macro.

Index: sys/sys/protosw.h
===
RCS file: /cvs/src/sys/sys/protosw.h,v
retrieving revision 1.58
diff -u -p -r1.58 protosw.h
--- sys/sys/protosw.h   17 Oct 2022 14:49:02 -  1.58
+++ sys/sys/protosw.h   14 Nov 2022 11:07:29 -
@@ -53,6 +53,9 @@
  * described below.
  */
 
+#ifndef _SYS_PROTOSW_H_
+#define _SYS_PROTOSW_H_
+
 struct mbuf;
 struct sockaddr;
 struct socket;
@@ -413,3 +416,5 @@ pru_connect2(struct socket *so1, struct 
 }
 
 #endif
+
+#endif /* _SYS_PROTOSW_H_ */
Index: sys/sys/socketvar.h
===
RCS file: /cvs/src/sys/sys/socketvar.h,v
retrieving revision 1.111
diff -u -p -r1.111 socketvar.h
--- sys/sys/socketvar.h 3 Oct 2022 16:43:52 -   1.111
+++ sys/sys/socketvar.h 14 Nov 2022 11:07:29 -
@@ -160,6 +160,7 @@ struct socket {
 
 #ifdef _KERNEL
 
+#include 
 #include 
 
 void   soassertlocked(struct socket *);
@@ -229,31 +230,39 @@ soreadable(struct socket *so)
 }
 
 /* can we write something to so? */
-#definesowriteable(so) \
-((sbspace((so), &(so)->so_snd) >= (so)->so_snd.sb_lowat && \
-   (((so)->so_state & SS_ISCONNECTED) || \
- ((so)->so_proto->pr_flags & PR_CONNREQUIRED)==0)) || \
-((so)->so_state & SS_CANTSENDMORE) || (so)->so_error)
+static inline int
+sowriteable(struct socket *so)
+{
+   soassertlocked(so);
+   return ((sbspace(so, >so_snd) >= so->so_snd.sb_lowat &&
+   ((so->so_state & SS_ISCONNECTED) ||
+   (so->so_proto->pr_flags & PR_CONNREQUIRED)==0)) ||
+   (so->so_state & SS_CANTSENDMORE) || so->so_error);
+}
 
 /* adjust counters in sb reflecting allocation of m */
-#definesballoc(so, sb, m) do { 
\
-   (sb)->sb_cc += (m)->m_len;  \
-   if ((m)->m_type != MT_CONTROL && (m)->m_type != MT_SONAME)  \
-   (sb)->sb_datacc += (m)->m_len;  \
-   (sb)->sb_mbcnt += MSIZE;\
-   if ((m)->m_flags & M_EXT)   \
-   (sb)->sb_mbcnt += (m)->m_ext.ext_size;  \
-} while (/* CONSTCOND */ 0)
+static inline void
+sballoc(struct socket *so, struct sockbuf *sb, struct mbuf *m)
+{
+   sb->sb_cc += m->m_len;
+   if (m->m_type != MT_CONTROL && m->m_type != MT_SONAME)
+   sb->sb_datacc += m->m_len;
+   sb->sb_mbcnt += MSIZE;
+   if (m->m_flags & M_EXT)
+   sb->sb_mbcnt += m->m_ext.ext_size;
+}
 
 /* adjust counters in sb reflecting freeing of m */
-#definesbfree(so, sb, m) do {  
\
-   (sb)->sb_cc -= (m)->m_len;  \
-   if ((m)->m_type != MT_CONTROL && (m)->m_type != MT_SONAME)  \
-   (sb)->sb_datacc -= (m)->m_len;  \
-   (sb)->sb_mbcnt -= MSIZE;\
-   if ((m)->m_flags & M_EXT)   \
-   (sb)->sb_mbcnt -= (m)->m_ext.ext_size;  \
-} while (/* CONSTCOND */ 0)
+static inline void
+sbfree(struct socket *so, struct sockbuf *sb, struct mbuf *m)
+{
+   sb->sb_cc -= m->m_len;
+   if (m->m_type != MT_CONTROL && m->m_type != MT_SONAME)
+   sb->sb_datacc -= m->m_len;
+   sb->sb_mbcnt -= MSIZE;
+   if (m->m_flags & M_EXT)
+   sb->sb_mbcnt -= m->m_ext.ext_size;
+}
 
 /*
  * Set lock on sockbuf sb; sleep if lock is already held.



Re: special case mpe(4) in in6_ifattach()

2022-11-14 Thread Klemens Nanni
On Thu, Nov 10, 2022 at 04:13:33PM +, Klemens Nanni wrote:
> On Fri, Nov 04, 2022 at 03:40:04PM +0100, Claudio Jeker wrote:
> > So mpe(4) is a special device. It is a point-to-multipoint interface that
> > does not do multicast. So setting IFF_MULTICAST on the interface is not
> > correct but IPv6 depends on it because neighbor discovery.
> > 
> > Now there is no neighbor discovery on mpe(4) the neighbors are handled via
> > BGP. So lets adjust in6_ifattach() to succeed for mpe(4) interfaces and
> > with that BGP IPv6 L3VPN should work.
> > 
> > It may be an idea to move the IFF_MULTCAST check down into the
> > ifp->if_type switch but right now this is good enough. I wonder if we have
> > other interfaces that need similar treatment (e.g. mgre).
> 
> Good enough for now, I also have a few minues for nd6.c but prefer
> simpler changes first, then the cleanup come later.

Seeing this thread again, my OK was meant for the later
"I think this is a better diff." mail, of course, just to be clear.



Re: Merge uipc_bind() with unp_bind()

2022-11-14 Thread Vitaliy Makkoveev
On Mon, Nov 14, 2022 at 09:28:34AM +, Klemens Nanni wrote:
> On Mon, Nov 14, 2022 at 12:11:46PM +0300, Vitaliy Makkoveev wrote:
> > uipc_bind() only calls unp_bind(). Also it is the only caller of
> > unp_bind().
> 
> For *_bind() alone this looks like zapping a useless indirection, but
> the rest of uipc_*() seems to consistently use unp_*() still, so I'm not
> convinced this one-off merge is an improvement.
> 

You talk about unp_{,dis}connect(). Such unp_*() contains the code
called from multiple places, not only from corresponding uipc_*(). The
uipc_bind() and unp_bind() are the different case.

In the uipc_usrreq() times it made sense to have unp_bind() for prevent
code mess within big switch(), but since uipc_usrreq() was splitted to
multiple handlers there is no reason to keep both uipc_bind() and
unp_bind().

> > 
> > Index: sys/kern/uipc_usrreq.c
> > ===
> > RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
> > retrieving revision 1.192
> > diff -u -p -r1.192 uipc_usrreq.c
> > --- sys/kern/uipc_usrreq.c  13 Nov 2022 16:01:32 -  1.192
> > +++ sys/kern/uipc_usrreq.c  14 Nov 2022 09:07:43 -
> > @@ -322,8 +322,93 @@ int
> >  uipc_bind(struct socket *so, struct mbuf *nam, struct proc *p)
> >  {
> > struct unpcb *unp = sotounpcb(so);
> > +   struct sockaddr_un *soun;
> > +   struct mbuf *nam2;
> > +   struct vnode *vp;
> > +   struct vattr vattr;
> > +   int error;
> > +   struct nameidata nd;
> > +   size_t pathlen;
> >  
> > -   return unp_bind(unp, nam, p);
> > +   if (unp->unp_flags & (UNP_BINDING | UNP_CONNECTING))
> > +   return (EINVAL);
> > +   if (unp->unp_vnode != NULL)
> > +   return (EINVAL);
> > +   if ((error = unp_nam2sun(nam, , )))
> > +   return (error);
> > +
> > +   unp->unp_flags |= UNP_BINDING;
> > +
> > +   /*
> > +* Enforce `i_lock' -> `unplock' because fifo subsystem
> > +* requires it. The socket can't be closed concurrently
> > +* because the file descriptor reference is still held.
> > +*/
> > +
> > +   sounlock(unp->unp_socket);
> > +
> > +   nam2 = m_getclr(M_WAITOK, MT_SONAME);
> > +   nam2->m_len = sizeof(struct sockaddr_un);
> > +   memcpy(mtod(nam2, struct sockaddr_un *), soun,
> > +   offsetof(struct sockaddr_un, sun_path) + pathlen);
> > +   /* No need to NUL terminate: m_getclr() returns zero'd mbufs. */
> > +
> > +   soun = mtod(nam2, struct sockaddr_un *);
> > +
> > +   /* Fixup sun_len to keep it in sync with m_len. */
> > +   soun->sun_len = nam2->m_len;
> > +
> > +   NDINIT(, CREATE, NOFOLLOW | LOCKPARENT, UIO_SYSSPACE,
> > +   soun->sun_path, p);
> > +   nd.ni_pledge = PLEDGE_UNIX;
> > +   nd.ni_unveil = UNVEIL_CREATE;
> > +
> > +   KERNEL_LOCK();
> > +/* SHOULD BE ABLE TO ADOPT EXISTING AND wakeup() ALA FIFO's */
> > +   error = namei();
> > +   if (error != 0) {
> > +   m_freem(nam2);
> > +   solock(unp->unp_socket);
> > +   goto out;
> > +   }
> > +   vp = nd.ni_vp;
> > +   if (vp != NULL) {
> > +   VOP_ABORTOP(nd.ni_dvp, _cnd);
> > +   if (nd.ni_dvp == vp)
> > +   vrele(nd.ni_dvp);
> > +   else
> > +   vput(nd.ni_dvp);
> > +   vrele(vp);
> > +   m_freem(nam2);
> > +   error = EADDRINUSE;
> > +   solock(unp->unp_socket);
> > +   goto out;
> > +   }
> > +   VATTR_NULL();
> > +   vattr.va_type = VSOCK;
> > +   vattr.va_mode = ACCESSPERMS &~ p->p_fd->fd_cmask;
> > +   error = VOP_CREATE(nd.ni_dvp, _vp, _cnd, );
> > +   vput(nd.ni_dvp);
> > +   if (error) {
> > +   m_freem(nam2);
> > +   solock(unp->unp_socket);
> > +   goto out;
> > +   }
> > +   solock(unp->unp_socket);
> > +   unp->unp_addr = nam2;
> > +   vp = nd.ni_vp;
> > +   vp->v_socket = unp->unp_socket;
> > +   unp->unp_vnode = vp;
> > +   unp->unp_connid.uid = p->p_ucred->cr_uid;
> > +   unp->unp_connid.gid = p->p_ucred->cr_gid;
> > +   unp->unp_connid.pid = p->p_p->ps_pid;
> > +   unp->unp_flags |= UNP_FEIDSBIND;
> > +   VOP_UNLOCK(vp);
> > +out:
> > +   KERNEL_UNLOCK();
> > +   unp->unp_flags &= ~UNP_BINDING;
> > +
> > +   return (error);
> >  }
> >  
> >  int
> > @@ -724,98 +809,6 @@ unp_detach(struct unpcb *unp)
> >  }
> >  
> >  int
> > -unp_bind(struct unpcb *unp, struct mbuf *nam, struct proc *p)
> > -{
> > -   struct sockaddr_un *soun;
> > -   struct mbuf *nam2;
> > -   struct vnode *vp;
> > -   struct vattr vattr;
> > -   int error;
> > -   struct nameidata nd;
> > -   size_t pathlen;
> > -
> > -   if (unp->unp_flags & (UNP_BINDING | UNP_CONNECTING))
> > -   return (EINVAL);
> > -   if (unp->unp_vnode != NULL)
> > -   return (EINVAL);
> > -   if ((error = unp_nam2sun(nam, , )))
> > -   return (error);
> > -
> > -   unp->unp_flags |= UNP_BINDING;
> > -
> > -   /*
> > -* Enforce `i_lock' -> `unplock' because fifo subsystem
> > -* requires it. The socket can't be closed concurrently
> > -* 

Re: Merge uipc_bind() with unp_bind()

2022-11-14 Thread Klemens Nanni
On Mon, Nov 14, 2022 at 12:11:46PM +0300, Vitaliy Makkoveev wrote:
> uipc_bind() only calls unp_bind(). Also it is the only caller of
> unp_bind().

For *_bind() alone this looks like zapping a useless indirection, but
the rest of uipc_*() seems to consistently use unp_*() still, so I'm not
convinced this one-off merge is an improvement.

> 
> Index: sys/kern/uipc_usrreq.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
> retrieving revision 1.192
> diff -u -p -r1.192 uipc_usrreq.c
> --- sys/kern/uipc_usrreq.c13 Nov 2022 16:01:32 -  1.192
> +++ sys/kern/uipc_usrreq.c14 Nov 2022 09:07:43 -
> @@ -322,8 +322,93 @@ int
>  uipc_bind(struct socket *so, struct mbuf *nam, struct proc *p)
>  {
>   struct unpcb *unp = sotounpcb(so);
> + struct sockaddr_un *soun;
> + struct mbuf *nam2;
> + struct vnode *vp;
> + struct vattr vattr;
> + int error;
> + struct nameidata nd;
> + size_t pathlen;
>  
> - return unp_bind(unp, nam, p);
> + if (unp->unp_flags & (UNP_BINDING | UNP_CONNECTING))
> + return (EINVAL);
> + if (unp->unp_vnode != NULL)
> + return (EINVAL);
> + if ((error = unp_nam2sun(nam, , )))
> + return (error);
> +
> + unp->unp_flags |= UNP_BINDING;
> +
> + /*
> +  * Enforce `i_lock' -> `unplock' because fifo subsystem
> +  * requires it. The socket can't be closed concurrently
> +  * because the file descriptor reference is still held.
> +  */
> +
> + sounlock(unp->unp_socket);
> +
> + nam2 = m_getclr(M_WAITOK, MT_SONAME);
> + nam2->m_len = sizeof(struct sockaddr_un);
> + memcpy(mtod(nam2, struct sockaddr_un *), soun,
> + offsetof(struct sockaddr_un, sun_path) + pathlen);
> + /* No need to NUL terminate: m_getclr() returns zero'd mbufs. */
> +
> + soun = mtod(nam2, struct sockaddr_un *);
> +
> + /* Fixup sun_len to keep it in sync with m_len. */
> + soun->sun_len = nam2->m_len;
> +
> + NDINIT(, CREATE, NOFOLLOW | LOCKPARENT, UIO_SYSSPACE,
> + soun->sun_path, p);
> + nd.ni_pledge = PLEDGE_UNIX;
> + nd.ni_unveil = UNVEIL_CREATE;
> +
> + KERNEL_LOCK();
> +/* SHOULD BE ABLE TO ADOPT EXISTING AND wakeup() ALA FIFO's */
> + error = namei();
> + if (error != 0) {
> + m_freem(nam2);
> + solock(unp->unp_socket);
> + goto out;
> + }
> + vp = nd.ni_vp;
> + if (vp != NULL) {
> + VOP_ABORTOP(nd.ni_dvp, _cnd);
> + if (nd.ni_dvp == vp)
> + vrele(nd.ni_dvp);
> + else
> + vput(nd.ni_dvp);
> + vrele(vp);
> + m_freem(nam2);
> + error = EADDRINUSE;
> + solock(unp->unp_socket);
> + goto out;
> + }
> + VATTR_NULL();
> + vattr.va_type = VSOCK;
> + vattr.va_mode = ACCESSPERMS &~ p->p_fd->fd_cmask;
> + error = VOP_CREATE(nd.ni_dvp, _vp, _cnd, );
> + vput(nd.ni_dvp);
> + if (error) {
> + m_freem(nam2);
> + solock(unp->unp_socket);
> + goto out;
> + }
> + solock(unp->unp_socket);
> + unp->unp_addr = nam2;
> + vp = nd.ni_vp;
> + vp->v_socket = unp->unp_socket;
> + unp->unp_vnode = vp;
> + unp->unp_connid.uid = p->p_ucred->cr_uid;
> + unp->unp_connid.gid = p->p_ucred->cr_gid;
> + unp->unp_connid.pid = p->p_p->ps_pid;
> + unp->unp_flags |= UNP_FEIDSBIND;
> + VOP_UNLOCK(vp);
> +out:
> + KERNEL_UNLOCK();
> + unp->unp_flags &= ~UNP_BINDING;
> +
> + return (error);
>  }
>  
>  int
> @@ -724,98 +809,6 @@ unp_detach(struct unpcb *unp)
>  }
>  
>  int
> -unp_bind(struct unpcb *unp, struct mbuf *nam, struct proc *p)
> -{
> - struct sockaddr_un *soun;
> - struct mbuf *nam2;
> - struct vnode *vp;
> - struct vattr vattr;
> - int error;
> - struct nameidata nd;
> - size_t pathlen;
> -
> - if (unp->unp_flags & (UNP_BINDING | UNP_CONNECTING))
> - return (EINVAL);
> - if (unp->unp_vnode != NULL)
> - return (EINVAL);
> - if ((error = unp_nam2sun(nam, , )))
> - return (error);
> -
> - unp->unp_flags |= UNP_BINDING;
> -
> - /*
> -  * Enforce `i_lock' -> `unplock' because fifo subsystem
> -  * requires it. The socket can't be closed concurrently
> -  * because the file descriptor reference is still held.
> -  */
> -
> - sounlock(unp->unp_socket);
> -
> - nam2 = m_getclr(M_WAITOK, MT_SONAME);
> - nam2->m_len = sizeof(struct sockaddr_un);
> - memcpy(mtod(nam2, struct sockaddr_un *), soun,
> - offsetof(struct sockaddr_un, sun_path) + pathlen);
> - /* No need to NUL terminate: m_getclr() returns zero'd mbufs. */
> -
> - soun = mtod(nam2, struct sockaddr_un *);
> -
> - /* Fixup sun_len to keep it in sync with m_len. */
> - soun->sun_len = nam2->m_len;
> -
> - 

Re: export {b,r}ootduid as sysctl, installer/sysupgrade improvements

2022-11-14 Thread Klemens Nanni
On Tue, Sep 06, 2022 at 01:16:47AM +, Klemens Nanni wrote:
> The installer considers a disk a root disk if 'a' is FFS and contains
> expected files.
> 
> Furthermore, unattended upgrades will always install to the first root
> disk that is found.
> 
> This works fine on machines with only one root disk, but it quickly
> behaves unexpectedly when having multiple disks/installations in one
> machine.
> 
> I run such machines, esp. since fiddling with softraid and installboot.
> 
> 
> The installer/sysupgrade experience can definitely be improved here, but
> that takes some consideration.
> 
> One requirement, imho, is knowing
> 1. which disk we booted from, i.e.
>from which disk the kernel (/bsd.rd or /bsd.upgrade) was loaded
> 2. which disk the root filesystem is on, i.e.
>likely the same disk holding /home where sysupgrade put the sets
> 
> 
> The boot disk could be helpful inside installer, e.g. to check if
> /bsd.ugpraded was booted from a valid root disk -- a good indicator for
> rebooting from the same disk the user just ran sysupgrade on.
> 
> The root disk is of no help inside the installer as that will always be
> the ramdisk.  But it could be used by sysupgrade to perhaps prefill
> /auto_upgrade.conf to decide up-front which disk to upgrade.
> This answer to the 'Which disk is the root disk' question is currently
> answered inside the installer during unattended upgrades... and it will
> always be the first valid root disk, which is not always correct.
> 
> So to make progress, here's a diff that exports readily available
> disklabel DUIDs:
> 
>   # disklabel sd0 | grep duid
>   duid: 98c0c47c3ffddeb4
>   # sysctl hw | grep duid
>   hw.bootduid=98c0c47c3ffddeb4
>   hw.rootduid=98c0c47c3ffddeb4
> 
> Having that, working out the installer/sysupgrade bits should be easier.
> 
> I'm testing this on arm64 with two disks/installations.
> 
> Feedback? Objection? OK?

I'd like to get on with this, can also add sysctl.2 bits to document
those before sending diffs using them.

Mark asked whether CTLTYPE_QUAD would be more suited, but I still don't
understand how that is supposed to work and there was no answer in this
thread's other mail.

Anyone?


Index: sys/sys/sysctl.h
===
RCS file: /cvs/src/sys/sys/sysctl.h,v
retrieving revision 1.231
diff -u -p -r1.231 sysctl.h
--- sys/sys/sysctl.h7 Nov 2022 14:25:44 -   1.231
+++ sys/sys/sysctl.h9 Nov 2022 12:06:31 -
@@ -946,7 +946,9 @@ struct kinfo_file {
 #defineHW_SMT  24  /* int: enable SMT/HT/CMT */
 #defineHW_NCPUONLINE   25  /* int: number of cpus being 
used */
 #defineHW_POWER26  /* int: machine has wall-power 
*/
-#defineHW_MAXID27  /* number of valid hw ids */
+#defineHW_BOOTDUID 27  /* string: DUID of boot disk */
+#defineHW_ROOTDUID 28  /* string: DUID of root disk */
+#defineHW_MAXID29  /* number of valid hw ids */
 
 #defineCTL_HW_NAMES { \
{ 0, 0 }, \
@@ -976,6 +978,8 @@ struct kinfo_file {
{ "smt", CTLTYPE_INT }, \
{ "ncpuonline", CTLTYPE_INT }, \
{ "power", CTLTYPE_INT }, \
+   { "bootduid", CTLTYPE_STRING }, \
+   { "rootduid", CTLTYPE_STRING }, \
 }
 
 /*
Index: sys/kern/kern_sysctl.c
===
RCS file: /cvs/src/sys/kern/kern_sysctl.c,v
retrieving revision 1.408
diff -u -p -r1.408 kern_sysctl.c
--- sys/kern/kern_sysctl.c  7 Nov 2022 14:25:44 -   1.408
+++ sys/kern/kern_sysctl.c  9 Nov 2022 12:06:30 -
@@ -770,6 +770,12 @@ hw_sysctl(int *name, u_int namelen, void
case HW_SMT:
return (sysctl_hwsmt(oldp, oldlenp, newp, newlen));
 #endif
+   case HW_BOOTDUID:
+   return (sysctl_rdstring(oldp, oldlenp, newp,
+   duid_format(bootduid)));
+   case HW_ROOTDUID:
+   return (sysctl_rdstring(oldp, oldlenp, newp,
+   duid_format(rootduid)));
default:
return sysctl_bounded_arr(hw_vars, nitems(hw_vars), name,
namelen, oldp, oldlenp, newp, newlen);



Merge uipc_bind() with unp_bind()

2022-11-14 Thread Vitaliy Makkoveev
uipc_bind() only calls unp_bind(). Also it is the only caller of
unp_bind().

Index: sys/kern/uipc_usrreq.c
===
RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
retrieving revision 1.192
diff -u -p -r1.192 uipc_usrreq.c
--- sys/kern/uipc_usrreq.c  13 Nov 2022 16:01:32 -  1.192
+++ sys/kern/uipc_usrreq.c  14 Nov 2022 09:07:43 -
@@ -322,8 +322,93 @@ int
 uipc_bind(struct socket *so, struct mbuf *nam, struct proc *p)
 {
struct unpcb *unp = sotounpcb(so);
+   struct sockaddr_un *soun;
+   struct mbuf *nam2;
+   struct vnode *vp;
+   struct vattr vattr;
+   int error;
+   struct nameidata nd;
+   size_t pathlen;
 
-   return unp_bind(unp, nam, p);
+   if (unp->unp_flags & (UNP_BINDING | UNP_CONNECTING))
+   return (EINVAL);
+   if (unp->unp_vnode != NULL)
+   return (EINVAL);
+   if ((error = unp_nam2sun(nam, , )))
+   return (error);
+
+   unp->unp_flags |= UNP_BINDING;
+
+   /*
+* Enforce `i_lock' -> `unplock' because fifo subsystem
+* requires it. The socket can't be closed concurrently
+* because the file descriptor reference is still held.
+*/
+
+   sounlock(unp->unp_socket);
+
+   nam2 = m_getclr(M_WAITOK, MT_SONAME);
+   nam2->m_len = sizeof(struct sockaddr_un);
+   memcpy(mtod(nam2, struct sockaddr_un *), soun,
+   offsetof(struct sockaddr_un, sun_path) + pathlen);
+   /* No need to NUL terminate: m_getclr() returns zero'd mbufs. */
+
+   soun = mtod(nam2, struct sockaddr_un *);
+
+   /* Fixup sun_len to keep it in sync with m_len. */
+   soun->sun_len = nam2->m_len;
+
+   NDINIT(, CREATE, NOFOLLOW | LOCKPARENT, UIO_SYSSPACE,
+   soun->sun_path, p);
+   nd.ni_pledge = PLEDGE_UNIX;
+   nd.ni_unveil = UNVEIL_CREATE;
+
+   KERNEL_LOCK();
+/* SHOULD BE ABLE TO ADOPT EXISTING AND wakeup() ALA FIFO's */
+   error = namei();
+   if (error != 0) {
+   m_freem(nam2);
+   solock(unp->unp_socket);
+   goto out;
+   }
+   vp = nd.ni_vp;
+   if (vp != NULL) {
+   VOP_ABORTOP(nd.ni_dvp, _cnd);
+   if (nd.ni_dvp == vp)
+   vrele(nd.ni_dvp);
+   else
+   vput(nd.ni_dvp);
+   vrele(vp);
+   m_freem(nam2);
+   error = EADDRINUSE;
+   solock(unp->unp_socket);
+   goto out;
+   }
+   VATTR_NULL();
+   vattr.va_type = VSOCK;
+   vattr.va_mode = ACCESSPERMS &~ p->p_fd->fd_cmask;
+   error = VOP_CREATE(nd.ni_dvp, _vp, _cnd, );
+   vput(nd.ni_dvp);
+   if (error) {
+   m_freem(nam2);
+   solock(unp->unp_socket);
+   goto out;
+   }
+   solock(unp->unp_socket);
+   unp->unp_addr = nam2;
+   vp = nd.ni_vp;
+   vp->v_socket = unp->unp_socket;
+   unp->unp_vnode = vp;
+   unp->unp_connid.uid = p->p_ucred->cr_uid;
+   unp->unp_connid.gid = p->p_ucred->cr_gid;
+   unp->unp_connid.pid = p->p_p->ps_pid;
+   unp->unp_flags |= UNP_FEIDSBIND;
+   VOP_UNLOCK(vp);
+out:
+   KERNEL_UNLOCK();
+   unp->unp_flags &= ~UNP_BINDING;
+
+   return (error);
 }
 
 int
@@ -724,98 +809,6 @@ unp_detach(struct unpcb *unp)
 }
 
 int
-unp_bind(struct unpcb *unp, struct mbuf *nam, struct proc *p)
-{
-   struct sockaddr_un *soun;
-   struct mbuf *nam2;
-   struct vnode *vp;
-   struct vattr vattr;
-   int error;
-   struct nameidata nd;
-   size_t pathlen;
-
-   if (unp->unp_flags & (UNP_BINDING | UNP_CONNECTING))
-   return (EINVAL);
-   if (unp->unp_vnode != NULL)
-   return (EINVAL);
-   if ((error = unp_nam2sun(nam, , )))
-   return (error);
-
-   unp->unp_flags |= UNP_BINDING;
-
-   /*
-* Enforce `i_lock' -> `unplock' because fifo subsystem
-* requires it. The socket can't be closed concurrently
-* because the file descriptor reference is still held.
-*/
-
-   sounlock(unp->unp_socket);
-
-   nam2 = m_getclr(M_WAITOK, MT_SONAME);
-   nam2->m_len = sizeof(struct sockaddr_un);
-   memcpy(mtod(nam2, struct sockaddr_un *), soun,
-   offsetof(struct sockaddr_un, sun_path) + pathlen);
-   /* No need to NUL terminate: m_getclr() returns zero'd mbufs. */
-
-   soun = mtod(nam2, struct sockaddr_un *);
-
-   /* Fixup sun_len to keep it in sync with m_len. */
-   soun->sun_len = nam2->m_len;
-
-   NDINIT(, CREATE, NOFOLLOW | LOCKPARENT, UIO_SYSSPACE,
-   soun->sun_path, p);
-   nd.ni_pledge = PLEDGE_UNIX;
-   nd.ni_unveil = UNVEIL_CREATE;
-
-   KERNEL_LOCK();
-/* SHOULD BE ABLE TO ADOPT EXISTING AND wakeup() ALA FIFO's */
-   error = namei();
-   if (error != 0) {
-   m_freem(nam2);
-

non regression tests for some packaging scripts

2022-11-14 Thread Marc Espie
I'd like to add some non-regression tests for some scripts, most notably
register-plist, which is... let's say not perfect at the moment.

I believe
${PORTSDIR}/regress is probably the simplest least disturbing way to do that.

Since the manpages already exist under src, I could also (somehow) put them
in /usr/src/regress.

Of course, this means requiring a checkout of the ports tree to check things.

What do you people think ?