Re: CVS: cvs.openbsd.org: src
On Wed, Apr 17, 2024 at 08:55:38AM +0200, Anton Lindqvist wrote: > On Tue, Apr 16, 2024 at 08:37:49AM -0600, Florian Obser wrote: > > CVSROOT:/cvs > > Module name:src > > Changes by: flor...@cvs.openbsd.org 2024/04/16 08:37:49 > > > > Modified files: > > sys/netinet6 : in6.c > > > > Log message: > > Destination addresses make no sense on loopback interfaces. > > > > While here use (variable & FLAG) or !(variable & FLAG) consistently in > > in6_update_ifa(). > > > > Discussed with claudio > > OK denis > > This change introduced 15 regress failures, all related to inet6. > > https://regress.basename.se/ > > Excerpt from sys/net/mpath showing the symptom: > > doas -n /sbin/ifconfig lo19 inet6 fc00::19 alias > ifconfig: lo19: SIOCIFAFATTACH: Invalid argument > ifconfig: SIOCAIFADDR: Invalid argument Localhost ::1 and all inet6 adresses are missing at loopback. ==> /etc/hostname.lo0 <== inet alias 10.188.0.74 255.255.255.255 inet6 fdd7:e83e:66bc:0::74 128 before # ifconfig lo0 lo0: flags=2008049 mtu 32768 index 5 priority 0 llprio 3 groups: lo inet6 ::1 prefixlen 128 inet6 fe80::1%lo0 prefixlen 64 scopeid 0x5 inet 127.0.0.1 netmask 0xff00 inet 10.188.0.74 netmask 0x inet6 fdd7:e83e:66bc::74 prefixlen 128 after # ifconfig lo0 lo0: flags=2008049 mtu 32768 index 5 priority 0 llprio 3 groups: lo inet 127.0.0.1 netmask 0xff00 inet 10.188.0.74 netmask 0x
Re: CVS: cvs.openbsd.org: src
reverted, sorry for the breakage, I did my tests on an old kernel :( On 2024-04-17 02:07 -06, Theo de Raadt wrote: > There's a long history of using loX interfaces for such things. > > What might make sense is to restrict it on lo0 ? > it's not about configuring additional IP addresses on loopback interfaces but on how they are configured. What ifconfig does internally is set a *destination* of ::1, i.e. as if called like this: /sbin/ifconfig lo19 inet6 fc00::19/128 ::1 I suspect that something somewhere in our stack depends on ::1 being there to deliver packets correctly. I'll leave this alone and punt it to future IPv6 hackers. I think we probably should enforce in the kernel that ifconfig sends us ::1 as the destination address and nothing else. >>On Tue, Apr 16, 2024 at 08:37:49AM -0600, Florian Obser wrote: >>> CVSROOT:/cvs >>> Module name:src >>> Changes by: flor...@cvs.openbsd.org 2024/04/16 08:37:49 >>> >>> Modified files: >>> sys/netinet6 : in6.c >>> >>> Log message: >>> Destination addresses make no sense on loopback interfaces. >>> >>> While here use (variable & FLAG) or !(variable & FLAG) consistently in >>> in6_update_ifa(). >>> >>> Discussed with claudio >>> OK denis >> >>This change introduced 15 regress failures, all related to inet6. >> >> https://regress.basename.se/ >> >>Excerpt from sys/net/mpath showing the symptom: >> >> doas -n /sbin/ifconfig lo19 inet6 fc00::19 alias >> ifconfig: lo19: SIOCIFAFATTACH: Invalid argument >> ifconfig: SIOCAIFADDR: Invalid argument >> >> > -- In my defence, I have been left unsupervised.
Re: CVS: cvs.openbsd.org: src
There's a long history of using loX interfaces for such things. What might make sense is to restrict it on lo0 ? >On Tue, Apr 16, 2024 at 08:37:49AM -0600, Florian Obser wrote: >> CVSROOT: /cvs >> Module name: src >> Changes by: flor...@cvs.openbsd.org 2024/04/16 08:37:49 >> >> Modified files: >> sys/netinet6 : in6.c >> >> Log message: >> Destination addresses make no sense on loopback interfaces. >> >> While here use (variable & FLAG) or !(variable & FLAG) consistently in >> in6_update_ifa(). >> >> Discussed with claudio >> OK denis > >This change introduced 15 regress failures, all related to inet6. > > https://regress.basename.se/ > >Excerpt from sys/net/mpath showing the symptom: > > doas -n /sbin/ifconfig lo19 inet6 fc00::19 alias > ifconfig: lo19: SIOCIFAFATTACH: Invalid argument > ifconfig: SIOCAIFADDR: Invalid argument > >
Re: CVS: cvs.openbsd.org: src
On Tue, Apr 16, 2024 at 08:37:49AM -0600, Florian Obser wrote: > CVSROOT: /cvs > Module name: src > Changes by: flor...@cvs.openbsd.org 2024/04/16 08:37:49 > > Modified files: > sys/netinet6 : in6.c > > Log message: > Destination addresses make no sense on loopback interfaces. > > While here use (variable & FLAG) or !(variable & FLAG) consistently in > in6_update_ifa(). > > Discussed with claudio > OK denis This change introduced 15 regress failures, all related to inet6. https://regress.basename.se/ Excerpt from sys/net/mpath showing the symptom: doas -n /sbin/ifconfig lo19 inet6 fc00::19 alias ifconfig: lo19: SIOCIFAFATTACH: Invalid argument ifconfig: SIOCAIFADDR: Invalid argument
Re: CVS: cvs.openbsd.org: src
Sebastien Marie writes: > CVSROOT: /cvs > Module name: src > Changes by: sema...@cvs.openbsd.org 2024/04/04 10:00:51 > > Modified files: > usr.bin/ssh/ssh-agent: Makefile > > Log message: > set right mode on ssh-agent at boot-time > > which sthen@ s/which/with/ > ok deraadt@ -- Sebastien Marie
Re: CVS: cvs.openbsd.org: src
On Mon, Apr 01, 2024 at 09:50:17AM -0600, Theo de Raadt wrote: > CVSROOT: /cvs > Module name: src > Changes by: dera...@cvs.openbsd.org 2024/04/01 09:50:17 > > Modified files: > usr.bin/ssh/ssh-agent: Makefile > > Log message: > also create a relink kit for ssh-agent, since it is a long-running setgid > program carrying keys with some (not very powerful) communication channels. > solution for testing the binary from dtucker. agreement from djm. > Will add it into /etc/rc in a few days. The BINMODE change, removing read permissions from group and others, caused ssh regress to fail. Flooded with the following message: cat: /usr/bin/ssh-agent: Permission denied
Re: CVS: cvs.openbsd.org: src
Scott Soule Cheloha wrote in <3d329ff2b7df9...@cvs.openbsd.org>: ... |Log message: |gnu/usr.bin: import xz-utils 5.6.1 Dschihad(-75) in Ramadan! That is very unreligious. --steffen | |Der Kragenbaer,The moon bear, |der holt sich munter he cheerfully and one by one |einen nach dem anderen runter wa.ks himself off |(By Robert Gernhardt)
Re: CVS: cvs.openbsd.org: src
On Thu, Mar 28, 2024 at 05:16:32AM -0600, Joel Sing wrote: > CVSROOT: /cvs > Module name: src > Changes by: js...@cvs.openbsd.org 2024/03/28 05:16:32 > > Modified files: > lib/libcrypto : x86_64cpuid.pl > > Log message: > Remove .init section that calls OPENSSL_cpuid_setup on amd64. > > OPENSSL_cpuid_setup() is already called from OPENSSL_crypto_init(), hence > we do not need or want automatic initialisation on library load. > > ok tb@ > This was discovered after investigating a diff by Viktor Szakats who tracked down strange crashes on MinGW when experimenting with CET on this platform to be related to an endbr64 in the .init section.
Re: CVS: cvs.openbsd.org: src
On Wed, Mar 27, 2024 at 11:35:01PM +0100, Alexander Bluhm wrote: > On Thu, Mar 28, 2024 at 01:21:17AM +0300, Vitaliy Makkoveev wrote: > > On Wed, Mar 27, 2024 at 10:51:09PM +0100, Alexander Bluhm wrote: > > > On Wed, Mar 27, 2024 at 10:26:27PM +0300, Vitaliy Makkoveev wrote: > > > > On Wed, Mar 27, 2024 at 08:17:33AM +0100, Anton Lindqvist wrote: > > > > > > > > > > Observing two regress hangs in the kernel on netio. Both seems make > > > > > use > > > > > of unix sockets. Could this be the culprit? > > > > > > > > > > regress/lib/libc/fread > > > > > regress/usr.bin/ssh (scp.sh) > > > > > > > > Sorry for delay. > > > > > > > > It was exposed that `sb_mtx' should not be released between `so_rcv' > > > > usage check and corresponding sbwait() sleep. Otherwise wakeup() could > > > > be lost sometimes. > > > > > > > > This diff fixed regress tests. It introduces sbunlock_locked() and > > > > sbwait_locked() to perform with `sb_mtx' held. > > > > > > Do we really need a restart_unlocked label? > > > Instead of goto restart_unlocked you could call solock_shared(so) > > > and fall through goto restart; > > > > > > > This was the first version, but later I decided 'restart_unlocked' is > > better. No problems to return it back. > > > > > I don't like pr_protocol == AF_UNIX check in generic soreceive() > > > function. Could we check SB_MTXLOCK instead? > > > > > > > Me too. I used "pr_protocol == AF_UNIX" check because sosend() does the > > same. The existing SB_MTXLOCK used by some inet sockets and I strictly > > want to move them separately of this fix. > > > > I propose to introduce SB_OWNLOCK flag and check it. I wanted to use > > this flag for socket with standalone sblock() ability to modify > > sblock()/sbwait() behaviour. I need this flag to convert all > > SB_MTXLOCK'ed sockets separately. After conversion, both SB_MTXLOCK and > > SB_OWNLOCK will be removed. > > > > I propose to introduce SB_OWNLOCK right now and use it instead of > > "pr_protocol == AF_UNIX" check to keep inet sockets as is. I have no > > objections to convert them too, but separately. > > sb_flags is short, can you use 4 digits 0x for the flags? > And please use a space after #define, then the diff is less ugly. > Done. Thanks.
Re: CVS: cvs.openbsd.org: src
On Thu, Mar 28, 2024 at 01:21:17AM +0300, Vitaliy Makkoveev wrote:
> On Wed, Mar 27, 2024 at 10:51:09PM +0100, Alexander Bluhm wrote:
> > On Wed, Mar 27, 2024 at 10:26:27PM +0300, Vitaliy Makkoveev wrote:
> > > On Wed, Mar 27, 2024 at 08:17:33AM +0100, Anton Lindqvist wrote:
> > > >
> > > > Observing two regress hangs in the kernel on netio. Both seems make use
> > > > of unix sockets. Could this be the culprit?
> > > >
> > > > regress/lib/libc/fread
> > > > regress/usr.bin/ssh (scp.sh)
> > >
> > > Sorry for delay.
> > >
> > > It was exposed that `sb_mtx' should not be released between `so_rcv'
> > > usage check and corresponding sbwait() sleep. Otherwise wakeup() could
> > > be lost sometimes.
> > >
> > > This diff fixed regress tests. It introduces sbunlock_locked() and
> > > sbwait_locked() to perform with `sb_mtx' held.
> >
> > Do we really need a restart_unlocked label?
> > Instead of goto restart_unlocked you could call solock_shared(so)
> > and fall through goto restart;
> >
>
> This was the first version, but later I decided 'restart_unlocked' is
> better. No problems to return it back.
>
> > I don't like pr_protocol == AF_UNIX check in generic soreceive()
> > function. Could we check SB_MTXLOCK instead?
> >
>
> Me too. I used "pr_protocol == AF_UNIX" check because sosend() does the
> same. The existing SB_MTXLOCK used by some inet sockets and I strictly
> want to move them separately of this fix.
>
> I propose to introduce SB_OWNLOCK flag and check it. I wanted to use
> this flag for socket with standalone sblock() ability to modify
> sblock()/sbwait() behaviour. I need this flag to convert all
> SB_MTXLOCK'ed sockets separately. After conversion, both SB_MTXLOCK and
> SB_OWNLOCK will be removed.
>
> I propose to introduce SB_OWNLOCK right now and use it instead of
> "pr_protocol == AF_UNIX" check to keep inet sockets as is. I have no
> objections to convert them too, but separately.
sb_flags is short, can you use 4 digits 0x for the flags?
And please use a space after #define, then the diff is less ugly.
OK bluhm@
> Index: sys/kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.322
> diff -u -p -r1.322 uipc_socket.c
> --- sys/kern/uipc_socket.c26 Mar 2024 09:46:47 - 1.322
> +++ sys/kern/uipc_socket.c27 Mar 2024 22:12:55 -
> @@ -161,7 +161,7 @@ soalloc(const struct protosw *prp, int w
> }
> break;
> case AF_UNIX:
> - so->so_rcv.sb_flags |= SB_MTXLOCK;
> + so->so_rcv.sb_flags |= SB_MTXLOCK | SB_OWNLOCK;
> break;
> }
>
> @@ -903,12 +903,23 @@ restart:
> }
> SBLASTRECORDCHK(>so_rcv, "soreceive sbwait 1");
> SBLASTMBUFCHK(>so_rcv, "soreceive sbwait 1");
> - sb_mtx_unlock(>so_rcv);
> - sbunlock(so, >so_rcv);
> - error = sbwait(so, >so_rcv);
> - if (error) {
> +
> + if (so->so_rcv.sb_flags & SB_OWNLOCK) {
> + sbunlock_locked(so, >so_rcv);
> sounlock_shared(so);
> - return (error);
> + error = sbwait_locked(so, >so_rcv);
> + sb_mtx_unlock(>so_rcv);
> + if (error)
> + return (error);
> + solock_shared(so);
> + } else {
> + sb_mtx_unlock(>so_rcv);
> + sbunlock(so, >so_rcv);
> + error = sbwait(so, >so_rcv);
> + if (error) {
> + sounlock_shared(so);
> + return (error);
> + }
> }
> goto restart;
> }
> Index: sys/kern/uipc_socket2.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
> retrieving revision 1.145
> diff -u -p -r1.145 uipc_socket2.c
> --- sys/kern/uipc_socket2.c 26 Mar 2024 09:46:47 - 1.145
> +++ sys/kern/uipc_socket2.c 27 Mar 2024 22:12:55 -
> @@ -523,6 +523,18 @@ sbmtxassertlocked(struct socket *so, str
> * Wait for data to arrive at/drain from a socket buffer.
> */
> int
> +sbwait_locked(struct socket *so, struct sockbuf *sb)
> +{
> + int prio = (sb->sb_flags & SB_NOINTR) ? PSOCK : PSOCK | PCATCH;
> +
> + MUTEX_ASSERT_LOCKED(>sb_mtx);
> +
> + sb->sb_flags |= SB_WAIT;
> + return msleep_nsec(>sb_cc, >sb_mtx, prio, "sbwait",
> + sb->sb_timeo_nsecs);
> +}
> +
> +int
> sbwait(struct socket *so, struct sockbuf *sb)
> {
> uint64_t timeo_nsecs;
> @@ -573,20 +585,23 @@ out:
> }
>
> void
> -sbunlock(struct socket *so, struct sockbuf *sb)
> +sbunlock_locked(struct socket *so, struct sockbuf *sb)
> {
> - int dowakeup = 0;
> + MUTEX_ASSERT_LOCKED(>sb_mtx);
>
> - mtx_enter(>sb_mtx);
>
Re: CVS: cvs.openbsd.org: src
On Wed, Mar 27, 2024 at 10:51:09PM +0100, Alexander Bluhm wrote:
> On Wed, Mar 27, 2024 at 10:26:27PM +0300, Vitaliy Makkoveev wrote:
> > On Wed, Mar 27, 2024 at 08:17:33AM +0100, Anton Lindqvist wrote:
> > >
> > > Observing two regress hangs in the kernel on netio. Both seems make use
> > > of unix sockets. Could this be the culprit?
> > >
> > > regress/lib/libc/fread
> > > regress/usr.bin/ssh (scp.sh)
> >
> > Sorry for delay.
> >
> > It was exposed that `sb_mtx' should not be released between `so_rcv'
> > usage check and corresponding sbwait() sleep. Otherwise wakeup() could
> > be lost sometimes.
> >
> > This diff fixed regress tests. It introduces sbunlock_locked() and
> > sbwait_locked() to perform with `sb_mtx' held.
>
> Do we really need a restart_unlocked label?
> Instead of goto restart_unlocked you could call solock_shared(so)
> and fall through goto restart;
>
This was the first version, but later I decided 'restart_unlocked' is
better. No problems to return it back.
> I don't like pr_protocol == AF_UNIX check in generic soreceive()
> function. Could we check SB_MTXLOCK instead?
>
Me too. I used "pr_protocol == AF_UNIX" check because sosend() does the
same. The existing SB_MTXLOCK used by some inet sockets and I strictly
want to move them separately of this fix.
I propose to introduce SB_OWNLOCK flag and check it. I wanted to use
this flag for socket with standalone sblock() ability to modify
sblock()/sbwait() behaviour. I need this flag to convert all
SB_MTXLOCK'ed sockets separately. After conversion, both SB_MTXLOCK and
SB_OWNLOCK will be removed.
I propose to introduce SB_OWNLOCK right now and use it instead of
"pr_protocol == AF_UNIX" check to keep inet sockets as is. I have no
objections to convert them too, but separately.
Index: sys/kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.322
diff -u -p -r1.322 uipc_socket.c
--- sys/kern/uipc_socket.c 26 Mar 2024 09:46:47 - 1.322
+++ sys/kern/uipc_socket.c 27 Mar 2024 22:12:55 -
@@ -161,7 +161,7 @@ soalloc(const struct protosw *prp, int w
}
break;
case AF_UNIX:
- so->so_rcv.sb_flags |= SB_MTXLOCK;
+ so->so_rcv.sb_flags |= SB_MTXLOCK | SB_OWNLOCK;
break;
}
@@ -903,12 +903,23 @@ restart:
}
SBLASTRECORDCHK(>so_rcv, "soreceive sbwait 1");
SBLASTMBUFCHK(>so_rcv, "soreceive sbwait 1");
- sb_mtx_unlock(>so_rcv);
- sbunlock(so, >so_rcv);
- error = sbwait(so, >so_rcv);
- if (error) {
+
+ if (so->so_rcv.sb_flags & SB_OWNLOCK) {
+ sbunlock_locked(so, >so_rcv);
sounlock_shared(so);
- return (error);
+ error = sbwait_locked(so, >so_rcv);
+ sb_mtx_unlock(>so_rcv);
+ if (error)
+ return (error);
+ solock_shared(so);
+ } else {
+ sb_mtx_unlock(>so_rcv);
+ sbunlock(so, >so_rcv);
+ error = sbwait(so, >so_rcv);
+ if (error) {
+ sounlock_shared(so);
+ return (error);
+ }
}
goto restart;
}
Index: sys/kern/uipc_socket2.c
===
RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
retrieving revision 1.145
diff -u -p -r1.145 uipc_socket2.c
--- sys/kern/uipc_socket2.c 26 Mar 2024 09:46:47 - 1.145
+++ sys/kern/uipc_socket2.c 27 Mar 2024 22:12:55 -
@@ -523,6 +523,18 @@ sbmtxassertlocked(struct socket *so, str
* Wait for data to arrive at/drain from a socket buffer.
*/
int
+sbwait_locked(struct socket *so, struct sockbuf *sb)
+{
+ int prio = (sb->sb_flags & SB_NOINTR) ? PSOCK : PSOCK | PCATCH;
+
+ MUTEX_ASSERT_LOCKED(>sb_mtx);
+
+ sb->sb_flags |= SB_WAIT;
+ return msleep_nsec(>sb_cc, >sb_mtx, prio, "sbwait",
+ sb->sb_timeo_nsecs);
+}
+
+int
sbwait(struct socket *so, struct sockbuf *sb)
{
uint64_t timeo_nsecs;
@@ -573,20 +585,23 @@ out:
}
void
-sbunlock(struct socket *so, struct sockbuf *sb)
+sbunlock_locked(struct socket *so, struct sockbuf *sb)
{
- int dowakeup = 0;
+ MUTEX_ASSERT_LOCKED(>sb_mtx);
- mtx_enter(>sb_mtx);
sb->sb_flags &= ~SB_LOCK;
if (sb->sb_flags & SB_WANT) {
sb->sb_flags &= ~SB_WANT;
- dowakeup = 1;
+ wakeup(>sb_flags);
}
- mtx_leave(>sb_mtx);
+}
- if (dowakeup)
- wakeup(>sb_flags);
+void
+sbunlock(struct socket *so, struct sockbuf *sb)
+{
+
Re: CVS: cvs.openbsd.org: src
On Wed, Mar 27, 2024 at 10:26:27PM +0300, Vitaliy Makkoveev wrote:
> On Wed, Mar 27, 2024 at 08:17:33AM +0100, Anton Lindqvist wrote:
> >
> > Observing two regress hangs in the kernel on netio. Both seems make use
> > of unix sockets. Could this be the culprit?
> >
> > regress/lib/libc/fread
> > regress/usr.bin/ssh (scp.sh)
>
> Sorry for delay.
>
> It was exposed that `sb_mtx' should not be released between `so_rcv'
> usage check and corresponding sbwait() sleep. Otherwise wakeup() could
> be lost sometimes.
>
> This diff fixed regress tests. It introduces sbunlock_locked() and
> sbwait_locked() to perform with `sb_mtx' held.
Do we really need a restart_unlocked label?
Instead of goto restart_unlocked you could call solock_shared(so)
and fall through goto restart;
I don't like pr_protocol == AF_UNIX check in generic soreceive()
function. Could we check SB_MTXLOCK instead?
bluhm
> Index: sys/kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.322
> diff -u -p -r1.322 uipc_socket.c
> --- sys/kern/uipc_socket.c26 Mar 2024 09:46:47 - 1.322
> +++ sys/kern/uipc_socket.c27 Mar 2024 19:17:52 -
> @@ -834,6 +834,7 @@ bad:
> if (mp)
> *mp = NULL;
>
> +restart_unlocked:
> solock_shared(so);
> restart:
> if ((error = sblock(so, >so_rcv, SBLOCKWAIT(flags))) != 0) {
> @@ -903,12 +904,23 @@ restart:
> }
> SBLASTRECORDCHK(>so_rcv, "soreceive sbwait 1");
> SBLASTMBUFCHK(>so_rcv, "soreceive sbwait 1");
> - sb_mtx_unlock(>so_rcv);
> - sbunlock(so, >so_rcv);
> - error = sbwait(so, >so_rcv);
> - if (error) {
> +
> + if (so->so_proto->pr_protocol == AF_UNIX) {
> + sbunlock_locked(so, >so_rcv);
> sounlock_shared(so);
> - return (error);
> + error = sbwait_locked(so, >so_rcv);
> + sb_mtx_unlock(>so_rcv);
> + if (error)
> + return (error);
> + goto restart_unlocked;
> + } else {
> + sb_mtx_unlock(>so_rcv);
> + sbunlock(so, >so_rcv);
> + error = sbwait(so, >so_rcv);
> + if (error) {
> + sounlock_shared(so);
> + return (error);
> + }
> }
> goto restart;
> }
> Index: sys/kern/uipc_socket2.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
> retrieving revision 1.145
> diff -u -p -r1.145 uipc_socket2.c
> --- sys/kern/uipc_socket2.c 26 Mar 2024 09:46:47 - 1.145
> +++ sys/kern/uipc_socket2.c 27 Mar 2024 19:17:52 -
> @@ -523,6 +523,18 @@ sbmtxassertlocked(struct socket *so, str
> * Wait for data to arrive at/drain from a socket buffer.
> */
> int
> +sbwait_locked(struct socket *so, struct sockbuf *sb)
> +{
> + int prio = (sb->sb_flags & SB_NOINTR) ? PSOCK : PSOCK | PCATCH;
> +
> + MUTEX_ASSERT_LOCKED(>sb_mtx);
> +
> + sb->sb_flags |= SB_WAIT;
> + return msleep_nsec(>sb_cc, >sb_mtx, prio, "sbwait",
> + sb->sb_timeo_nsecs);
> +}
> +
> +int
> sbwait(struct socket *so, struct sockbuf *sb)
> {
> uint64_t timeo_nsecs;
> @@ -573,20 +585,23 @@ out:
> }
>
> void
> -sbunlock(struct socket *so, struct sockbuf *sb)
> +sbunlock_locked(struct socket *so, struct sockbuf *sb)
> {
> - int dowakeup = 0;
> + MUTEX_ASSERT_LOCKED(>sb_mtx);
>
> - mtx_enter(>sb_mtx);
> sb->sb_flags &= ~SB_LOCK;
> if (sb->sb_flags & SB_WANT) {
> sb->sb_flags &= ~SB_WANT;
> - dowakeup = 1;
> + wakeup(>sb_flags);
> }
> - mtx_leave(>sb_mtx);
> +}
>
> - if (dowakeup)
> - wakeup(>sb_flags);
> +void
> +sbunlock(struct socket *so, struct sockbuf *sb)
> +{
> + mtx_enter(>sb_mtx);
> + sbunlock_locked(so, sb);
> + mtx_leave(>sb_mtx);
> }
>
> /*
> Index: sys/sys/socketvar.h
> ===
> RCS file: /cvs/src/sys/sys/socketvar.h,v
> retrieving revision 1.126
> diff -u -p -r1.126 socketvar.h
> --- sys/sys/socketvar.h 26 Mar 2024 09:46:47 - 1.126
> +++ sys/sys/socketvar.h 27 Mar 2024 19:17:53 -
> @@ -320,6 +320,7 @@ int sblock(struct socket *, struct sockb
>
> /* release lock on sockbuf sb */
> void sbunlock(struct socket *, struct sockbuf *);
> +void sbunlock_locked(struct socket *, struct sockbuf *);
>
> #define SB_EMPTY_FIXUP(sb) do {
> \
> if ((sb)->sb_mb == NULL) { \
> @@ -367,6 +368,7 @@ int sbcheckreserve(u_long, u_long);
> int sbchecklowmem(void);
> int
Re: CVS: cvs.openbsd.org: src
On Wed, Mar 27, 2024 at 08:17:33AM +0100, Anton Lindqvist wrote:
>
> Observing two regress hangs in the kernel on netio. Both seems make use
> of unix sockets. Could this be the culprit?
>
> regress/lib/libc/fread
> regress/usr.bin/ssh (scp.sh)
Sorry for delay.
It was exposed that `sb_mtx' should not be released between `so_rcv'
usage check and corresponding sbwait() sleep. Otherwise wakeup() could
be lost sometimes.
This diff fixed regress tests. It introduces sbunlock_locked() and
sbwait_locked() to perform with `sb_mtx' held.
Index: sys/kern/uipc_socket.c
===
RCS file: /cvs/src/sys/kern/uipc_socket.c,v
retrieving revision 1.322
diff -u -p -r1.322 uipc_socket.c
--- sys/kern/uipc_socket.c 26 Mar 2024 09:46:47 - 1.322
+++ sys/kern/uipc_socket.c 27 Mar 2024 19:17:52 -
@@ -834,6 +834,7 @@ bad:
if (mp)
*mp = NULL;
+restart_unlocked:
solock_shared(so);
restart:
if ((error = sblock(so, >so_rcv, SBLOCKWAIT(flags))) != 0) {
@@ -903,12 +904,23 @@ restart:
}
SBLASTRECORDCHK(>so_rcv, "soreceive sbwait 1");
SBLASTMBUFCHK(>so_rcv, "soreceive sbwait 1");
- sb_mtx_unlock(>so_rcv);
- sbunlock(so, >so_rcv);
- error = sbwait(so, >so_rcv);
- if (error) {
+
+ if (so->so_proto->pr_protocol == AF_UNIX) {
+ sbunlock_locked(so, >so_rcv);
sounlock_shared(so);
- return (error);
+ error = sbwait_locked(so, >so_rcv);
+ sb_mtx_unlock(>so_rcv);
+ if (error)
+ return (error);
+ goto restart_unlocked;
+ } else {
+ sb_mtx_unlock(>so_rcv);
+ sbunlock(so, >so_rcv);
+ error = sbwait(so, >so_rcv);
+ if (error) {
+ sounlock_shared(so);
+ return (error);
+ }
}
goto restart;
}
Index: sys/kern/uipc_socket2.c
===
RCS file: /cvs/src/sys/kern/uipc_socket2.c,v
retrieving revision 1.145
diff -u -p -r1.145 uipc_socket2.c
--- sys/kern/uipc_socket2.c 26 Mar 2024 09:46:47 - 1.145
+++ sys/kern/uipc_socket2.c 27 Mar 2024 19:17:52 -
@@ -523,6 +523,18 @@ sbmtxassertlocked(struct socket *so, str
* Wait for data to arrive at/drain from a socket buffer.
*/
int
+sbwait_locked(struct socket *so, struct sockbuf *sb)
+{
+ int prio = (sb->sb_flags & SB_NOINTR) ? PSOCK : PSOCK | PCATCH;
+
+ MUTEX_ASSERT_LOCKED(>sb_mtx);
+
+ sb->sb_flags |= SB_WAIT;
+ return msleep_nsec(>sb_cc, >sb_mtx, prio, "sbwait",
+ sb->sb_timeo_nsecs);
+}
+
+int
sbwait(struct socket *so, struct sockbuf *sb)
{
uint64_t timeo_nsecs;
@@ -573,20 +585,23 @@ out:
}
void
-sbunlock(struct socket *so, struct sockbuf *sb)
+sbunlock_locked(struct socket *so, struct sockbuf *sb)
{
- int dowakeup = 0;
+ MUTEX_ASSERT_LOCKED(>sb_mtx);
- mtx_enter(>sb_mtx);
sb->sb_flags &= ~SB_LOCK;
if (sb->sb_flags & SB_WANT) {
sb->sb_flags &= ~SB_WANT;
- dowakeup = 1;
+ wakeup(>sb_flags);
}
- mtx_leave(>sb_mtx);
+}
- if (dowakeup)
- wakeup(>sb_flags);
+void
+sbunlock(struct socket *so, struct sockbuf *sb)
+{
+ mtx_enter(>sb_mtx);
+ sbunlock_locked(so, sb);
+ mtx_leave(>sb_mtx);
}
/*
Index: sys/sys/socketvar.h
===
RCS file: /cvs/src/sys/sys/socketvar.h,v
retrieving revision 1.126
diff -u -p -r1.126 socketvar.h
--- sys/sys/socketvar.h 26 Mar 2024 09:46:47 - 1.126
+++ sys/sys/socketvar.h 27 Mar 2024 19:17:53 -
@@ -320,6 +320,7 @@ int sblock(struct socket *, struct sockb
/* release lock on sockbuf sb */
void sbunlock(struct socket *, struct sockbuf *);
+void sbunlock_locked(struct socket *, struct sockbuf *);
#defineSB_EMPTY_FIXUP(sb) do {
\
if ((sb)->sb_mb == NULL) { \
@@ -367,6 +368,7 @@ int sbcheckreserve(u_long, u_long);
intsbchecklowmem(void);
intsbreserve(struct socket *, struct sockbuf *, u_long);
intsbwait(struct socket *, struct sockbuf *);
+intsbwait_locked(struct socket *, struct sockbuf *);
void soinit(void);
void soabort(struct socket *);
intsoaccept(struct socket *, struct mbuf *);
Re: CVS: cvs.openbsd.org: src
On Tue, Mar 26, 2024 at 03:46:47AM -0600, Vitaliy Makkoveev wrote: > CVSROOT: /cvs > Module name: src > Changes by: m...@cvs.openbsd.org2024/03/26 03:46:47 > > Modified files: > sys/kern : sys_socket.c uipc_socket.c uipc_socket2.c >uipc_usrreq.c > sys/miscfs/fifofs: fifo_vnops.c > sys/sys: socketvar.h > > Log message: > Use `sb_mtx' to protect `so_rcv' receive buffer of unix(4) sockets. > > This makes re-locking unnecessary in the uipc_*send() paths, because > it's enough to lock one socket to prevent peer from concurrent > disconnection. As the little bonus, one unix(4) socket can perform > simultaneous transmission and reception with one exception for > uipc_rcvd(), which still requires the re-lock for connection oriented > sockets. > > The socket lock is not held while filt_soread() and filt_soexcept() > called from uipc_*send() through sorwakeup(). However, the unlocked > access to the `so_options', `so_state' and `so_error' is fine. > > The receiving socket can't be or became listening socket. It also can't > be disconnected concurrently. This makes immutable SO_ACCEPTCONN, > SS_ISDISCONNECTED and SS_ISCONNECTED bits which are clean and set > respectively. > > `so_error' is set on the peer sockets only by unp_detach(), which also > can't be called concurrently on sending socket. > > This is also true for filt_fiforead() and filt_fifoexcept(). For other > callers like kevent(2) or doaccept() the socket lock is still held. > > ok bluhm Observing two regress hangs in the kernel on netio. Both seems make use of unix sockets. Could this be the culprit? regress/lib/libc/fread regress/usr.bin/ssh (scp.sh)
Re: CVS: cvs.openbsd.org: src
Should it be removed from changelist too? On 2024/03/15 23:18, Job Snijders wrote: > CVSROOT: /cvs > Module name: src > Changes by: j...@cvs.openbsd.org2024/03/15 23:18:01 > > Modified files: > distrib/sets/lists/base: mi > distrib/sets/lists/etc: mi > > Log message: > Move RPKI Trust Anchor constraints from etc set to base > > The cadence of updates being applied to the RPKI Trust Anchor constraints > seems sufficiently low, while the required understanding of context to make > educated decisions quite high, so centralized coordination of updates through > t...@openbsd.org is more appropriate. > > requested by & OK deraadt@, OK tb@ >
Re: CVS: cvs.openbsd.org: src
On Fri, Mar 01, 2024 at 01:10:09AM -0700, Theo Buehler wrote: > CVSROOT: /cvs > Module name: src > Changes by: t...@cvs.openbsd.org2024/03/01 01:10:09 > > Modified files: > usr.sbin/rpki-client: main.c output-json.c parser.c > rpki-client.8 > > Log message: > Add -x to opt into experimental file formats > > Instead of burning one letter for each new file format (sidrops is known > to crank out new things faster than a normal person can read), use -x to > opt into parsing and processing file formats that aren't yet considered > stable. This is currently only the Signed Prefix List. While a repetition > of the ASPA debacle, this code hasn't yet seen enough stress testing to be > enabled by default. This should have said: "while a repetition of the ASPA debacle is unlikely". > > ok claudio job >
Re: CVS: cvs.openbsd.org: src
Alexander Bluhm: > CVSROOT: /cvs > Module name: src > Changes by: bl...@cvs.openbsd.org 2024/02/27 05:37:49 > > Modified files: > sys/net: route.c route.h > sys/netinet: in_pcb.c > sys/netinet6 : in6_pcb.c in6_src.c > > Log message: > Combine route_cache() and rtalloc_mpath() in new route_mpath(). > > Fill and check the cache and call rtalloc_mpath() together. Then > the caller of route_mpath() does not have to care about the uint32_t > *src pointer and just pass struct in_addr. All the conversions are > done inside the functions. ro->ro_rt is either valid or NULL. Note > that some places have a stricter rtisvalid() now compared to the > previous NULL check. > > OK claudio@ This change breaks NFS in some way: starting RPC daemons: portmap mountd nfsd. NFS Portmap: RPC: Port mapper failure - RPC: Unable to send NFS Portmap: RPC: Port mapper failure - RPC: Unable to send NFS Portmap: RPC: Port mapper failure - RPC: Unable to send NFS Portmap: RPC: Port mapper failure - RPC: Unable to send NFS Portmap: RPC: Port mapper failure - RPC: Unable to send -- Christian "naddy" Weisgerber na...@mips.inka.de
Re: CVS: cvs.openbsd.org: src
David Gwynne wrote: > CVSROOT: /cvs > Module name: src > Changes by: d...@cvs.openbsd.org2024/02/20 18:45:14 > > Modified files: > sys/arch/riscv64/riscv64: machdep.c > sys/arch/arm64/arm64: machdep.c > > Log message: > handle /reserved-memory nodes from device trees on arm64. > > u-boot is supposed to take these entries and put them in the efi > memory map, but i keep hitting machines where an otherwise functional > u-boot does not do this, resulting in weird errors. > > i have an espressobin with a vendor u-boot that has a reserved-memory > region for psci. without this diff the machine faults when the > kernel tries to reboot using a psci handler. > > a macchiatobin with an otherwise working u-boot throws SErrors or > panics on weird memory corruption problems without this. i thought > it was bad RAM, but the problems persisted with completely different > ram, and very underclocked and well cooled ram. > > riscv64 already has code to handle reserved-memory regions. the > riscv64 change is to add handling for the "no-map" property. This locks up the kernel on riscv64 Unmatched: After Copyright, but before printing OpenBSD 7.5...
Re: CVS: cvs.openbsd.org: src
February 19, 2024 10:00 PM, "Gilles Chehade" wrote: > CVSROOT: /cvs > Module name: src > Changes by: gil...@cvs.openbsd.org 2024/02/19 14:00:19 > > Modified files: > usr.sbin/smtpd : parse.y > > Log message: > no need to be as strict with table formats on various match constraints, > this prevents the reuse of T_HASH tables in T_LIST contexts when the key > column actually makes sense by itself. > > diff from Philipp (philipp+openbsd [at] bureaucracy [dot] de) sorry, this was ok op@
Re: CVS: cvs.openbsd.org: src
On Thu, Feb 15, 2024 at 03:56:53AM -0700, Marcus Glocker wrote: > CVSROOT: /cvs > Module name: src > Changes by: mgloc...@cvs.openbsd.org2024/02/15 03:56:53 > > Modified files: > sys/dev/pci: if_ix.c > > Log message: > Fix bogus packet length calculation in the RX/LRO path, which can lead to > TSO tagging forwarded packets which shouldn't. This will also fix the em(4) > watchdog timeouts seen after the em(4) TSO commit. > > Thanks to Hrvoje Popovski for providing the infrastructure the reproduce > the issue, and test the fix. > > Discussed with bluhm@. OK claudio@ And OK bluhm@
Re: CVS: cvs.openbsd.org: src
On Fri, Feb 02, 2024 at 08:39:23AM -0700, Alexander Bluhm wrote: > CVSROOT: /cvs > Module name: src > Changes by: bl...@cvs.openbsd.org 2024/02/02 08:39:23 > > Modified files: > sys/netinet: in_pcb.c > > Log message: > In in_pcbrtentry() add missing return of in6_pcbrtentry() value. > > Reported-by: syzbot+5f2286d5962355f0c...@syzkaller.appspotmail.com > OK kn@ and OK claudio@
Re: CVS: cvs.openbsd.org: src
On Tue, Jan 16, 2024 at 06:33:13AM -0700, Claudio Jeker wrote: > CVSROOT: /cvs > Module name: src > Changes by: clau...@cvs.openbsd.org 2024/01/16 06:33:13 > > Modified files: > usr.sbin/snmpd : proc.c snmpd.h snmpe.c > > Log message: > Convert to imsg_get_fd() and remove unused proc_forward_imsg(). > OK martijn@ > Also OK florian@ -- :wq Claudio
Re: CVS: cvs.openbsd.org: src
>However, to ease this transition, change ld.so to only load the first libc >version that we encounter (in a breadth first sense) and substitute that >libc version for all further loads of libc, even if different versions are >requested. This is done silently since I can't come up with a good warning >message. In practice this means the libc version requested by the >executable itself will be loaded. This means that shared libraries may >fail to load if they use a symbol that has been removed. But given the >constraints, this is the best that we can do. Even when we bump the >libc major, the set of changes is typically small and most binaries and >shared libraries will continue to run and allow the user to run pkg_add -u >without any fallout. When there are API additions and deletions, we tend to be pretty careful. Meaning, additions tend to happen well in advance of deletions. I've done a vague retrospective inspection of this, and it looks like we would not have been negatively affected in the last 4 years, during a 6-month window. Maybe we can be more careful. Generally, anyone doing a development that will remove an API is going to hit the issue first by themselves. And I think it will be more a more obvious failure for that developer now, than it was in the past. So this behaviour is probably an improvement.
Re: CVS: cvs.openbsd.org: src
On Sat, Jan 06, 2024 at 04:29:00AM -0700, Marc Espie wrote: > CVSROOT: /cvs > Module name: src > Changes by: es...@cvs.openbsd.org 2024/01/06 04:29:00 > > Modified files: > usr.sbin/httpd : js.h.in server_file.c > > Log message: > put the real sizes into the "title" attribute so that hovering shows the > exact value. > > adjust/refactor javascript sorter accordingly > > Forgot to say this was okay tb@
Re: CVS: cvs.openbsd.org: src
On Wed, Jan 03, 2024 at 02:19:22AM -0700, Kevin Lo wrote: > > CVSROOT: /cvs > Module name: src > Changes by: ke...@cvs.openbsd.org 2024/01/03 02:19:22 > > Modified files: > share/man/man4 : uoaklux.4 uoakrh.4 uoakv.4 > > Log message: > Update website URL. > > ok miod@ Also ok sthen@
Re: CVS: cvs.openbsd.org: src
On Sat, 23 Dec 2023 09:05:54 +0100, Anton Lindqvist wrote: > On Fri, Dec 22, 2023 at 10:12:13AM -0700, Todd C. Miller wrote: > > CVSROOT:/cvs > > Module name:src > > Changes by: mill...@cvs.openbsd.org 2023/12/22 10:12:13 > > > > Modified files: > > regress/usr.bin/xargs: xargs-L.sh > > usr.bin/xargs : xargs.c > > > > Log message: > > xargs: fix parsing of empty fields when "xargs -0" is used. > > Previously, these fields would be skipped. From Hiltjo Posthuma. > > This broke the common `find -print0 | xargs -0` idiom: > > $ mkdir -p /tmp/a > $ find /tmp/a -print0 | xargs -0r rm -r > rm: : No such file or directory Thanks, this should be fixed now. - todd
Re: CVS: cvs.openbsd.org: src
> On 23 Dec 2023, at 13:15, Alexander Bluhm wrote: > > On Fri, Dec 22, 2023 at 04:01:50PM -0700, Vitaliy Makkoveev wrote: >> CVSROOT: /cvs >> Module name: src >> Changes by: m...@cvs.openbsd.org2023/12/22 16:01:50 >> >> Modified files: >> sys/net: if.c if_aggr.c if_bpe.c if_etherip.c if_gif.c >> if_gre.c if_mpe.c if_mpip.c if_mpw.c if_pflow.c >> if_pfsync.c if_pppx.c if_sec.c if_tpmr.c >> if_trunk.c if_tun.c if_var.h if_veb.c if_vlan.c >> if_vxlan.c if_wg.c >> sys/netinet: ip_carp.c >> >> Log message: >> Always allocate per-CPU statistics counters for network interface >> descriptor. > > It looks like this breaks interface attach. > It should be backed out. > Thanks for backout and sorry for delay.
Re: CVS: cvs.openbsd.org: src
On Fri, Dec 22, 2023 at 04:01:50PM -0700, Vitaliy Makkoveev wrote: > CVSROOT: /cvs > Module name: src > Changes by: m...@cvs.openbsd.org2023/12/22 16:01:50 > > Modified files: > sys/net: if.c if_aggr.c if_bpe.c if_etherip.c if_gif.c >if_gre.c if_mpe.c if_mpip.c if_mpw.c if_pflow.c >if_pfsync.c if_pppx.c if_sec.c if_tpmr.c >if_trunk.c if_tun.c if_var.h if_veb.c if_vlan.c >if_vxlan.c if_wg.c > sys/netinet: ip_carp.c > > Log message: > Always allocate per-CPU statistics counters for network interface > descriptor. It looks like this breaks interface attach. It should be backed out. bluhm pci4 at ppb3 bus 4 em0 at pci4 dev 0 function 0 "Intel 82571EB" rev 0x06: apic 8 int 17uvm_fault(0 x824bf360, 0x8, 0, 1) -> e kernel: page fault trap, code=0 Stopped at pool_get+0x6e: movq0x8(%rax),%r11 TIDPIDUID PRFLAGS PFLAGS CPU COMMAND * 0 0 0 0x1 0x2000K swapper pool_get(825aac78,1,a619daefe381a0c,8017a048,d,8017a7e8 ) at pool_get+0x6e counters_alloc(c,c,a1f03e534e639ae1,8017a048,8017a048,8 017a7e8) at counters_alloc+0x3f if_attach_common(8017a048,8017a048,ce773033c5bc7a4c,801 7a048,8017a048,e) at if_attach_common+0xa7 if_attach(8017a048,8017a048,8017a000,e,81618525 ,8295be70) at if_attach+0x15 em_setup_interface(8017a000,8017a000,8017a578,8 017a000,8017a548,0) at em_setup_interface+0x1c8 em_attach(80177900,8017a000,8295bfa8,80177900,b d8850f5395a4cef,80177900) at em_attach+0x410 config_attach(80177900,8245e5a8,8295bfa8,81b39c 50,e47e39a69ab854bf,8004) at config_attach+0x1e2 pci_probe_device(80177900,8004,0,0,e374438be72c4e6d,0) at pci_probe _device+0x4c4 pci_enumerate_bus(80177900,0,0,80177900,f5afadb07e1609f4,80 179000) at pci_enumerate_bus+0x183 config_attach(80179000,8245d9d8,8295c1d0,82037f 00,e47e39a699089b75,8295c348) at config_attach+0x1e2 ppbattach(80177700,80179000,8295c348,80177700,f 7ff70c80a4ae9eb,80177700) at ppbattach+0x74e config_attach(80177700,8245e2d0,8295c348,81b39c 50,e47e39a69ab854bf,80020800) at config_attach+0x1e2 pci_probe_device(80177700,80020800,0,0,e374438be72c4e6d,0) at pci_probe _device+0x4c4 pci_enumerate_bus(80177700,0,0,80177700,f5afadb07e1609f4,80 178800) at pci_enumerate_bus+0x183 end trace frame: 0x8295c550, count: 0 > We have the mess in network interface statistics. Only pseudo drivers > do per-CPU counters allocation, all other network devices use the old > `if_data'. The network stack partially uses per-CPU counters and > partially use `if_data', but the protection is inconsistent: some times > counters accessed with exclusive netlock, some times with shared > netlock, some times with kernel lock, but without netlock, some times > with another locks. > > To make network interfaces statistics more consistent, always allocate > per-CPU counters at interface attachment time and use it instead of > `if_data'. At this step only move counters allocation to the if_attach() > internals. The `if_data' removal will be performed with the following > diffs to make review and tests easier. > > ok bluhm
Re: CVS: cvs.openbsd.org: src
On Fri, Dec 22, 2023 at 10:12:13AM -0700, Todd C. Miller wrote: > CVSROOT: /cvs > Module name: src > Changes by: mill...@cvs.openbsd.org 2023/12/22 10:12:13 > > Modified files: > regress/usr.bin/xargs: xargs-L.sh > usr.bin/xargs : xargs.c > > Log message: > xargs: fix parsing of empty fields when "xargs -0" is used. > Previously, these fields would be skipped. From Hiltjo Posthuma. This broke the common `find -print0 | xargs -0` idiom: $ mkdir -p /tmp/a $ find /tmp/a -print0 | xargs -0r rm -r rm: : No such file or directory
Re: CVS: cvs.openbsd.org: src
On 2023/12/04 07:24, Robert Nagy wrote: > CVSROOT: /cvs > Module name: src > Changes by: rob...@cvs.openbsd.org 2023/12/04 07:24:29 > > Modified files: > gnu/llvm/lld/ELF: InputFiles.cpp Relocations.cpp > > Log message: > protect access to the gnu warning map with a mutex to avoid random crashes > > discussed with, tested by and ok tb@ > (seen fairly frequently with -static)
Re: CVS: cvs.openbsd.org: src
And ok miod@
> ROOT: /cvs
> Module name: src
> Changes by: mgloc...@cvs.openbsd.org2023/12/03 22:28:25
>
> Modified files:
> sys/dev/usb: uthum.c
>
> Log message:
> Make the TEMPer{1,2} devices display minus degC. From the NetBSD driver.
>
> ok deraadt@
Re: CVS: cvs.openbsd.org: src
On Tue, Nov 21, 2023 at 09:21:03AM +0100, Florian Obser wrote:
> On 2023-11-21 08:04 +01, Anton Lindqvist wrote:
> > On Mon, Nov 20, 2023 at 05:15:16AM -0700, Florian Obser wrote:
> >> CVSROOT: /cvs
> >> Module name: src
> >> Changes by:flor...@cvs.openbsd.org 2023/11/20 05:15:16
> >>
> >> Modified files:
> >>lib/libc/asr : asr_private.h asr_utils.c getaddrinfo_async.c
> >> gethostnamadr_async.c
> >>
> >> Log message:
> >> localhost is either 127.0.0.1 or ::1, nothing else.
> >>
> >> RFC 6761, 6.3 Domain Name Reservation Considerations for "localhost.":
> >> 3. Name resolution APIs and libraries SHOULD recognize localhost
> >> names as special and SHOULD always return the IP loopback address
> >> for address queries and negative responses for all other query
> >> types. Name resolution APIs SHOULD NOT send queries for
> >> localhost names to their configured caching DNS server(s).
> >>
> >> This makes sure that the getaddrinfo(3) and gethostbyname(3) family of
> >> functions always return the loopback address and do not send queries
> >> to name servers. This includes "localhost", "localhost." and
> >> everything under ".localhost" and ".localhost.".
> >>
> >> For example, a host underneath the .com.ar zone will per default have
> >> a search list of "com.ar.". resolv.conf(5) has a default of "lookup
> >> bind file". Both combined will result in lookups for "localhost" to
> >> not return 127.0.0.1 because localhost.com.ar is registered in DNS.
> >>
> >> It has been known for decades that this is a problem, especially for
> >> localhost.
> >>
> >> Problem recently spotted by gonzalo@ and debugged by sthen@
> >>
> >> Testing sthen, gonzalo
> >> Input & OK phessler, eric, millert
> >> OK sthen, kn, deraadt
> >
> > Seeing two new failures. The offending pf rule:
> >
> > pass in inet6 proto tcp to port 25 divert-to localhost port 8025
> >
> > pfctl complains about:
> >
> > stdin:10: divert-to address family mismatch
> >
> > Changing localhost to ::1 makes it pass again. Is this expected or a
> > regression? The ipsecctl failure looks like a symptom of the same
> > problem.
>
> We were a bit too enthusiastic short-circuiting "localhost". It has to
> fail if AI_NUMERICHOST is set, because "localhost" is not an IP address.
>
> This makes the pfctl and ipsecctl regress tests pass again. (At least
> with a non-weird /etc/resolv.conf).
>
> OK?
Fixes the tests here as well; ok anton@ FWIW
> diff --git getaddrinfo_async.c getaddrinfo_async.c
> index f85d541398a..2e5898b6411 100644
> --- getaddrinfo_async.c
> +++ getaddrinfo_async.c
> @@ -115,7 +115,7 @@ getaddrinfo_async_run(struct asr_query *as, struct
> asr_result *ar)
> char fqdn[MAXDNAME];
> const char *str;
> struct addrinfo *ai;
> - int i, family, r, is_localhost;
> + int i, family, r, is_localhost = 0;
> FILE*f;
> union {
> struct sockaddr sa;
> @@ -228,7 +228,8 @@ getaddrinfo_async_run(struct asr_query *as, struct
> asr_result *ar)
>
> ar->ar_gai_errno = 0;
>
> - is_localhost = _asr_is_localhost(as->as.ai.hostname);
> + if (!(ai->ai_flags & AI_NUMERICHOST))
> + is_localhost = _asr_is_localhost(as->as.ai.hostname);
> /*
>* If hostname is NULL, "localhost" or falls within the
>* ".localhost." domain, use local address.
>
>
> >
> >> sbin/ipsecctl
> > Exit: 1
> > Duration: 00:00:06
> > Log: 181-sbin-ipsecctl.log
> >
> > ike
> > cat /home/src/regress/sbin/ipsecctl/ike56.in | sed -e
> > 's,DIR,/home/src/regress/sbin/ipsecctl,g' | /sbin/ipsecctl -nv -f - |
> > diff -u /home/src/regress/sbin/ipsecctl/ike56.ok /dev/stdin
> > stdin: 1: source/destination address families do not match
> > ipsecctl: Syntax error in config file: ipsec rules not loaded
> > --- /home/src/regress/sbin/ipsecctl/ike56.okFri Sep 2 12:58:24 2016
> > +++ /dev/stdin Tue Nov 21 02:26:19 2023
> > @@ -1,33 +0,0 @@
> > -C set [Phase 1]:127.0.0.1=peer-127.0.0.1 force
> > -C set [peer-127.0.0.1]:Phase=1 force
> > -C set [peer-127.0.0.1]:Address=127.0.0.1 force
> > -C set [peer-127.0.0.1]:Configuration=phase1-peer-127.0.0.1 force
> > -C set [phase1-peer-127.0.0.1]:EXCHANGE_TYPE=ID_PROT force
> > -C add
> > [phase1-peer-127.0.0.1]:Transforms=phase1-transform-peer-127.0.0.1-RSA_SIG-SHA-AES128-MODP_3072
> > force
> > -C set
> > [phase1-transform-peer-127.0.0.1-RSA_SIG-SHA-AES128-MODP_3072]:AUTHENTICATION_METHOD=RSA_SIG
> > force
> > -C set
> > [phase1-transform-peer-127.0.0.1-RSA_SIG-SHA-AES128-MODP_3072]:HASH_ALGORITHM=SHA
> > force
> > -C set
> > [phase1-transform-peer-127.0.0.1-RSA_SIG-SHA-AES128-MODP_3072]:ENCRYPTION_ALGORITHM=AES_CBC
> > force
> > -C set
> > [phase1-transform-peer-127.0.0.1-RSA_SIG-SHA-AES128-MODP_3072]:KEY_LENGTH=128,128:256
> > force
> > -C set
> >
Re: CVS: cvs.openbsd.org: src
On 2023-11-20 23:33 +01, Alexander Bluhm wrote:
> On Tue, Nov 14, 2023 at 01:27:33AM -0700, Florian Obser wrote:
>> CVSROOT: /cvs
>> Module name: src
>> Changes by: flor...@cvs.openbsd.org 2023/11/14 01:27:33
>>
>> Modified files:
>> lib/libc/asr : gethostnamadr_async.c
>>
>> Log message:
>> Add break to error out on weird characters in gethostbyname().
>>
>> Missed in previous some time ago; while here remove confusing else.
>>
>> Input & OK eric
>
> This breaks DNS error handling in ifconfig.
>
> # ifconfig foo0 foo:bar
> Segmentation fault (core dumped)
>
> #0 0x05eb2d9dc132 in in_getaddr (s=0x70483ea0f184 "foo:bar", which=1)
> at /usr/src/sbin/ifconfig/ifconfig.c:6413
> 6413memcpy(>sin_addr, hp->h_addr,
> hp->h_length);
>
> bluhm
We need to set the correct ar_ errno, otherwise we return stack garbage.
OK?
diff --git gethostnamadr_async.c gethostnamadr_async.c
index d211679400b..5df767c 100644
--- gethostnamadr_async.c
+++ gethostnamadr_async.c
@@ -205,7 +205,8 @@ gethostnamadr_async_run(struct asr_query *as, struct
asr_result *ar)
}
if (!hnok_lenient(as->as.hostnamadr.name)) {
- ar->ar_gai_errno = EAI_FAIL;
+ ar->ar_h_errno = NETDB_INTERNAL;
+ ar->ar_errno = EINVAL;
async_set_state(as, ASR_STATE_HALT);
break;
}
--
In my defence, I have been left unsupervised.
Re: CVS: cvs.openbsd.org: src
On 2023-11-21 08:04 +01, Anton Lindqvist wrote:
> On Mon, Nov 20, 2023 at 05:15:16AM -0700, Florian Obser wrote:
>> CVSROOT: /cvs
>> Module name: src
>> Changes by: flor...@cvs.openbsd.org 2023/11/20 05:15:16
>>
>> Modified files:
>> lib/libc/asr : asr_private.h asr_utils.c getaddrinfo_async.c
>> gethostnamadr_async.c
>>
>> Log message:
>> localhost is either 127.0.0.1 or ::1, nothing else.
>>
>> RFC 6761, 6.3 Domain Name Reservation Considerations for "localhost.":
>> 3. Name resolution APIs and libraries SHOULD recognize localhost
>> names as special and SHOULD always return the IP loopback address
>> for address queries and negative responses for all other query
>> types. Name resolution APIs SHOULD NOT send queries for
>> localhost names to their configured caching DNS server(s).
>>
>> This makes sure that the getaddrinfo(3) and gethostbyname(3) family of
>> functions always return the loopback address and do not send queries
>> to name servers. This includes "localhost", "localhost." and
>> everything under ".localhost" and ".localhost.".
>>
>> For example, a host underneath the .com.ar zone will per default have
>> a search list of "com.ar.". resolv.conf(5) has a default of "lookup
>> bind file". Both combined will result in lookups for "localhost" to
>> not return 127.0.0.1 because localhost.com.ar is registered in DNS.
>>
>> It has been known for decades that this is a problem, especially for
>> localhost.
>>
>> Problem recently spotted by gonzalo@ and debugged by sthen@
>>
>> Testing sthen, gonzalo
>> Input & OK phessler, eric, millert
>> OK sthen, kn, deraadt
>
> Seeing two new failures. The offending pf rule:
>
> pass in inet6 proto tcp to port 25 divert-to localhost port 8025
>
> pfctl complains about:
>
> stdin:10: divert-to address family mismatch
>
> Changing localhost to ::1 makes it pass again. Is this expected or a
> regression? The ipsecctl failure looks like a symptom of the same
> problem.
We were a bit too enthusiastic short-circuiting "localhost". It has to
fail if AI_NUMERICHOST is set, because "localhost" is not an IP address.
This makes the pfctl and ipsecctl regress tests pass again. (At least
with a non-weird /etc/resolv.conf).
OK?
diff --git getaddrinfo_async.c getaddrinfo_async.c
index f85d541398a..2e5898b6411 100644
--- getaddrinfo_async.c
+++ getaddrinfo_async.c
@@ -115,7 +115,7 @@ getaddrinfo_async_run(struct asr_query *as, struct
asr_result *ar)
char fqdn[MAXDNAME];
const char *str;
struct addrinfo *ai;
- int i, family, r, is_localhost;
+ int i, family, r, is_localhost = 0;
FILE*f;
union {
struct sockaddr sa;
@@ -228,7 +228,8 @@ getaddrinfo_async_run(struct asr_query *as, struct
asr_result *ar)
ar->ar_gai_errno = 0;
- is_localhost = _asr_is_localhost(as->as.ai.hostname);
+ if (!(ai->ai_flags & AI_NUMERICHOST))
+ is_localhost = _asr_is_localhost(as->as.ai.hostname);
/*
* If hostname is NULL, "localhost" or falls within the
* ".localhost." domain, use local address.
>
>> sbin/ipsecctl
> Exit: 1
> Duration: 00:00:06
> Log: 181-sbin-ipsecctl.log
>
> ike
> cat /home/src/regress/sbin/ipsecctl/ike56.in | sed -e
> 's,DIR,/home/src/regress/sbin/ipsecctl,g' | /sbin/ipsecctl -nv -f - |
> diff -u /home/src/regress/sbin/ipsecctl/ike56.ok /dev/stdin
> stdin: 1: source/destination address families do not match
> ipsecctl: Syntax error in config file: ipsec rules not loaded
> --- /home/src/regress/sbin/ipsecctl/ike56.ok Fri Sep 2 12:58:24 2016
> +++ /dev/stdinTue Nov 21 02:26:19 2023
> @@ -1,33 +0,0 @@
> -C set [Phase 1]:127.0.0.1=peer-127.0.0.1 force
> -C set [peer-127.0.0.1]:Phase=1 force
> -C set [peer-127.0.0.1]:Address=127.0.0.1 force
> -C set [peer-127.0.0.1]:Configuration=phase1-peer-127.0.0.1 force
> -C set [phase1-peer-127.0.0.1]:EXCHANGE_TYPE=ID_PROT force
> -C add
> [phase1-peer-127.0.0.1]:Transforms=phase1-transform-peer-127.0.0.1-RSA_SIG-SHA-AES128-MODP_3072
> force
> -C set
> [phase1-transform-peer-127.0.0.1-RSA_SIG-SHA-AES128-MODP_3072]:AUTHENTICATION_METHOD=RSA_SIG
> force
> -C set
> [phase1-transform-peer-127.0.0.1-RSA_SIG-SHA-AES128-MODP_3072]:HASH_ALGORITHM=SHA
> force
> -C set
> [phase1-transform-peer-127.0.0.1-RSA_SIG-SHA-AES128-MODP_3072]:ENCRYPTION_ALGORITHM=AES_CBC
> force
> -C set
> [phase1-transform-peer-127.0.0.1-RSA_SIG-SHA-AES128-MODP_3072]:KEY_LENGTH=128,128:256
> force
> -C set
> [phase1-transform-peer-127.0.0.1-RSA_SIG-SHA-AES128-MODP_3072]:GROUP_DESCRIPTION=MODP_3072
> force
> -C set
> [phase1-transform-peer-127.0.0.1-RSA_SIG-SHA-AES128-MODP_3072]:Life=LIFE_MAIN_MODE
> force
> -C set [from-127.0.0.1-to-127.0.0.1]:Phase=2 force
> -C set [from-127.0.0.1-to-127.0.0.1]:ISAKMP-peer=peer-127.0.0.1 force
> -C set
>
Re: CVS: cvs.openbsd.org: src
On Mon, Nov 20, 2023 at 05:15:16AM -0700, Florian Obser wrote: > CVSROOT: /cvs > Module name: src > Changes by: flor...@cvs.openbsd.org 2023/11/20 05:15:16 > > Modified files: > lib/libc/asr : asr_private.h asr_utils.c getaddrinfo_async.c >gethostnamadr_async.c > > Log message: > localhost is either 127.0.0.1 or ::1, nothing else. > > RFC 6761, 6.3 Domain Name Reservation Considerations for "localhost.": > 3. Name resolution APIs and libraries SHOULD recognize localhost > names as special and SHOULD always return the IP loopback address > for address queries and negative responses for all other query > types. Name resolution APIs SHOULD NOT send queries for > localhost names to their configured caching DNS server(s). > > This makes sure that the getaddrinfo(3) and gethostbyname(3) family of > functions always return the loopback address and do not send queries > to name servers. This includes "localhost", "localhost." and > everything under ".localhost" and ".localhost.". > > For example, a host underneath the .com.ar zone will per default have > a search list of "com.ar.". resolv.conf(5) has a default of "lookup > bind file". Both combined will result in lookups for "localhost" to > not return 127.0.0.1 because localhost.com.ar is registered in DNS. > > It has been known for decades that this is a problem, especially for > localhost. > > Problem recently spotted by gonzalo@ and debugged by sthen@ > > Testing sthen, gonzalo > Input & OK phessler, eric, millert > OK sthen, kn, deraadt Seeing two new failures. The offending pf rule: pass in inet6 proto tcp to port 25 divert-to localhost port 8025 pfctl complains about: stdin:10: divert-to address family mismatch Changing localhost to ::1 makes it pass again. Is this expected or a regression? The ipsecctl failure looks like a symptom of the same problem. > sbin/ipsecctl Exit: 1 Duration: 00:00:06 Log: 181-sbin-ipsecctl.log ike cat /home/src/regress/sbin/ipsecctl/ike56.in | sed -e 's,DIR,/home/src/regress/sbin/ipsecctl,g' | /sbin/ipsecctl -nv -f - | diff -u /home/src/regress/sbin/ipsecctl/ike56.ok /dev/stdin stdin: 1: source/destination address families do not match ipsecctl: Syntax error in config file: ipsec rules not loaded --- /home/src/regress/sbin/ipsecctl/ike56.okFri Sep 2 12:58:24 2016 +++ /dev/stdin Tue Nov 21 02:26:19 2023 @@ -1,33 +0,0 @@ -C set [Phase 1]:127.0.0.1=peer-127.0.0.1 force -C set [peer-127.0.0.1]:Phase=1 force -C set [peer-127.0.0.1]:Address=127.0.0.1 force -C set [peer-127.0.0.1]:Configuration=phase1-peer-127.0.0.1 force -C set [phase1-peer-127.0.0.1]:EXCHANGE_TYPE=ID_PROT force -C add [phase1-peer-127.0.0.1]:Transforms=phase1-transform-peer-127.0.0.1-RSA_SIG-SHA-AES128-MODP_3072 force -C set [phase1-transform-peer-127.0.0.1-RSA_SIG-SHA-AES128-MODP_3072]:AUTHENTICATION_METHOD=RSA_SIG force -C set [phase1-transform-peer-127.0.0.1-RSA_SIG-SHA-AES128-MODP_3072]:HASH_ALGORITHM=SHA force -C set [phase1-transform-peer-127.0.0.1-RSA_SIG-SHA-AES128-MODP_3072]:ENCRYPTION_ALGORITHM=AES_CBC force -C set [phase1-transform-peer-127.0.0.1-RSA_SIG-SHA-AES128-MODP_3072]:KEY_LENGTH=128,128:256 force -C set [phase1-transform-peer-127.0.0.1-RSA_SIG-SHA-AES128-MODP_3072]:GROUP_DESCRIPTION=MODP_3072 force -C set [phase1-transform-peer-127.0.0.1-RSA_SIG-SHA-AES128-MODP_3072]:Life=LIFE_MAIN_MODE force -C set [from-127.0.0.1-to-127.0.0.1]:Phase=2 force -C set [from-127.0.0.1-to-127.0.0.1]:ISAKMP-peer=peer-127.0.0.1 force -C set [from-127.0.0.1-to-127.0.0.1]:Configuration=phase2-from-127.0.0.1-to-127.0.0.1 force -C set [from-127.0.0.1-to-127.0.0.1]:Local-ID=from-127.0.0.1 force -C set [from-127.0.0.1-to-127.0.0.1]:Remote-ID=to-127.0.0.1 force -C set [phase2-from-127.0.0.1-to-127.0.0.1]:EXCHANGE_TYPE=QUICK_MODE force -C set [phase2-from-127.0.0.1-to-127.0.0.1]:Suites=phase2-suite-from-127.0.0.1-to-127.0.0.1 force -C set [phase2-suite-from-127.0.0.1-to-127.0.0.1]:Protocols=phase2-protocol-from-127.0.0.1-to-127.0.0.1 force -C set [phase2-protocol-from-127.0.0.1-to-127.0.0.1]:PROTOCOL_ID=IPSEC_ESP force -C set [phase2-protocol-from-127.0.0.1-to-127.0.0.1]:Transforms=phase2-transform-from-127.0.0.1-to-127.0.0.1-AES128-SHA2_256-MODP_3072-TUNNEL force -C set [phase2-transform-from-127.0.0.1-to-127.0.0.1-AES128-SHA2_256-MODP_3072-TUNNEL]:TRANSFORM_ID=AES force -C set [phase2-transform-from-127.0.0.1-to-127.0.0.1-AES128-SHA2_256-MODP_3072-TUNNEL]:KEY_LENGTH=128,128:256 force -C set [phase2-transform-from-127.0.0.1-to-127.0.0.1-AES128-SHA2_256-MODP_3072-TUNNEL]:ENCAPSULATION_MODE=TUNNEL force -C set [phase2-transform-from-127.0.0.1-to-127.0.0.1-AES128-SHA2_256-MODP_3072-TUNNEL]:AUTHENTICATION_ALGORITHM=HMAC_SHA2_256 force -C set [phase2-transform-from-127.0.0.1-to-127.0.0.1-AES128-SHA2_256-MODP_3072-TUNNEL]:GROUP_DESCRIPTION=MODP_3072 force -C set
Re: CVS: cvs.openbsd.org: src
On Tue, Nov 14, 2023 at 01:27:33AM -0700, Florian Obser wrote: > CVSROOT: /cvs > Module name: src > Changes by: flor...@cvs.openbsd.org 2023/11/14 01:27:33 > > Modified files: > lib/libc/asr : gethostnamadr_async.c > > Log message: > Add break to error out on weird characters in gethostbyname(). > > Missed in previous some time ago; while here remove confusing else. > > Input & OK eric This breaks DNS error handling in ifconfig. # ifconfig foo0 foo:bar Segmentation fault (core dumped) #0 0x05eb2d9dc132 in in_getaddr (s=0x70483ea0f184 "foo:bar", which=1) at /usr/src/sbin/ifconfig/ifconfig.c:6413 6413memcpy(>sin_addr, hp->h_addr, hp->h_length); bluhm
Re: CVS: cvs.openbsd.org: src
On Mon, Oct 23, 2023 at 01:12:09PM -0400, Daniel Jakots wrote: > On Mon, 23 Oct 2023 02:38:14 -0600 (MDT), Marc Espie > wrote: > > > CVSROOT:/cvs > > Module name:src > > Changes by: es...@cvs.openbsd.org 2023/10/23 02:38:14 > > > > Modified files: > > usr.sbin/pkg_add/OpenBSD: AddCreateDelete.pm > > > > Log message: > > split on spaces as well, to take advantage of += > > > > I just updated a machine and now pkg_add fails: > > # pkg_add -u > syntax error at /usr/libdata/perl5/OpenBSD/Subst.pm line 54, near "" $v")" > Compilation failed in require at /usr/libdata/perl5/OpenBSD/State.pm line 73. > BEGIN failed--compilation aborted at /usr/libdata/perl5/OpenBSD/State.pm line > 73. Yeah, not sure how I managed to fuck that one. Fixed for real
Re: CVS: cvs.openbsd.org: src
On 23/10/2023 19:12, Daniel Jakots wrote: On Mon, 23 Oct 2023 02:38:14 -0600 (MDT), Marc Espie wrote: CVSROOT:/cvs Module name:src Changes by: es...@cvs.openbsd.org 2023/10/23 02:38:14 Modified files: usr.sbin/pkg_add/OpenBSD: AddCreateDelete.pm Log message: split on spaces as well, to take advantage of += I just updated a machine and now pkg_add fails: # pkg_add -u syntax error at /usr/libdata/perl5/OpenBSD/Subst.pm line 54, near "" $v")" Compilation failed in require at /usr/libdata/perl5/OpenBSD/State.pm line 73. BEGIN failed--compilation aborted at /usr/libdata/perl5/OpenBSD/State.pm line 73. Compilation failed in require at /usr/libdata/perl5/OpenBSD/AddCreateDelete.pm line 26. BEGIN failed--compilation aborted at /usr/libdata/perl5/OpenBSD/AddCreateDelete.pm line 26. Compilation failed in require at /usr/libdata/perl5/OpenBSD/AddDelete.pm line 60. BEGIN failed--compilation aborted at /usr/libdata/perl5/OpenBSD/AddDelete.pm line 60. Compilation failed in require at /usr/libdata/perl5/OpenBSD/PkgAdd.pm line 22. BEGIN failed--compilation aborted at /usr/libdata/perl5/OpenBSD/PkgAdd.pm line 22. Compilation failed in require at (eval 1) line 1. could it be related? Cheers, Daniel I don't think so, if I revert the following commit pkg_add works again on my machine CVSROOT:/cvs Module name:src Changes by: es...@cvs.openbsd.org 2023/10/23 02:37:45 Modified files: usr.sbin/pkg_add/OpenBSD: Subst.pm Log message: add some limited capability to -DVAR+=value to be used with REGRESSION_TESTING, where it's somewhat inconvenient to know the full list in advance.
Re: CVS: cvs.openbsd.org: src
On Mon, 23 Oct 2023 02:38:14 -0600 (MDT), Marc Espie wrote: > CVSROOT: /cvs > Module name: src > Changes by: es...@cvs.openbsd.org 2023/10/23 02:38:14 > > Modified files: > usr.sbin/pkg_add/OpenBSD: AddCreateDelete.pm > > Log message: > split on spaces as well, to take advantage of += > I just updated a machine and now pkg_add fails: # pkg_add -u syntax error at /usr/libdata/perl5/OpenBSD/Subst.pm line 54, near "" $v")" Compilation failed in require at /usr/libdata/perl5/OpenBSD/State.pm line 73. BEGIN failed--compilation aborted at /usr/libdata/perl5/OpenBSD/State.pm line 73. Compilation failed in require at /usr/libdata/perl5/OpenBSD/AddCreateDelete.pm line 26. BEGIN failed--compilation aborted at /usr/libdata/perl5/OpenBSD/AddCreateDelete.pm line 26. Compilation failed in require at /usr/libdata/perl5/OpenBSD/AddDelete.pm line 60. BEGIN failed--compilation aborted at /usr/libdata/perl5/OpenBSD/AddDelete.pm line 60. Compilation failed in require at /usr/libdata/perl5/OpenBSD/PkgAdd.pm line 22. BEGIN failed--compilation aborted at /usr/libdata/perl5/OpenBSD/PkgAdd.pm line 22. Compilation failed in require at (eval 1) line 1. could it be related? Cheers, Daniel
Re: CVS: cvs.openbsd.org: src
On 2023/10/03 04:22, Stuart Henderson wrote: > CVSROOT: /cvs > Module name: src > Changes by: st...@cvs.openbsd.org 2023/10/03 04:22:10 > > Modified files: > sys/net: if_pfsync.c > > Log message: > Reinstate setting rtableid based on rdomain for pfsync, > lost during the rewrite, reported by Mark Patruck. > > ok phessler claudio sashan deraadt > and dlg
Re: CVS: cvs.openbsd.org: src
On Thu, 21 Sep 2023 11:19:06 -0600, Todd C. Miller wrote: > CVSROOT: /cvs > Module name: src > Changes by: mill...@cvs.openbsd.org 2023/09/21 11:19:06 > > Modified files: > usr.bin/awk: b.c > > Log message: > Fix a potential out-of-bounds read caused by the big-endian fix. > > We must store a UTF-32 empty string, not UTF-8 empty string, for > an empty CCL. Found running the awk test suite with address sanitizer. Actually, this bug is also present in upstream awk, without the big-endian fix. - todd
Re: CVS: cvs.openbsd.org: src
On 2023/09/16 01:59:44 -0600, Omar Polo wrote: > CVSROOT: /cvs > Module name: src > Changes by: o...@cvs.openbsd.org2023/09/16 01:59:44 > > Modified files: > share/man/man5 : bsd.port.mk.5 > > Log message: > document CHECK_LIB_DEPENDS_ARGS sigh, and this improved and okay'd espie@ too. sorry...
Re: CVS: cvs.openbsd.org: src
Hello, I was about to start looking into it. however there was another mail in my inbox where I learned naddy@ has backed out the commit. bluhm@ and I agree with him. the change attempts to fix awkward corner case doing more harm than good. thanks and regards sashan pá 8. 9. 2023 v 12:59 odesÃlatel Anton Lindqvist napsal: > > On Thu, Sep 07, 2023 at 03:59:43AM -0600, Alexandr Nedvedicky wrote: > > CVSROOT: /cvs > > Module name: src > > Changes by: sas...@cvs.openbsd.org 2023/09/07 03:59:43 > > > > Modified files: > > sys/net: pf.c > > > > Log message: > > pf(4) ignores 'keep state' and 'nat-to' actions for unsolicited > > icmp error responses. Fix tightens rule matching logic so icmp > > error responses no longer match 'keep state' rule. In typical > > scenarios icmp errors (if solicited) should match existing state. > > The change is going to bite firewalls which deal with asymmetric > > routes. In those cases the 'keep state' action should be relaxed > > to sloppy or new 'no state' rule to explicitly match icmp > > errors should be added. > > > > The issue has been reported by Peter J. Philip (pjp _at_ delphinusdns.org). > > > > Discussed with bluhm@ and florian@ > > > > OK bluhm@ > > Couple of new failures. Reverting this commit makes the problem go away. > > https://regress.basename.se/amd64/2023-09-08.1/296-sys-net-pair.log > https://regress.basename.se/amd64/2023-09-08.1/310-sys-net-vxlan.log > https://regress.basename.se/amd64/2023-09-08.1/397-usr.sbin-bgpd.log > https://regress.basename.se/amd64/2023-09-08.1/405-usr.sbin-ospf6d.log
Re: CVS: cvs.openbsd.org: src
On Thu, Sep 07, 2023 at 03:59:43AM -0600, Alexandr Nedvedicky wrote: > CVSROOT: /cvs > Module name: src > Changes by: sas...@cvs.openbsd.org 2023/09/07 03:59:43 > > Modified files: > sys/net: pf.c > > Log message: > pf(4) ignores 'keep state' and 'nat-to' actions for unsolicited > icmp error responses. Fix tightens rule matching logic so icmp > error responses no longer match 'keep state' rule. In typical > scenarios icmp errors (if solicited) should match existing state. > The change is going to bite firewalls which deal with asymmetric > routes. In those cases the 'keep state' action should be relaxed > to sloppy or new 'no state' rule to explicitly match icmp > errors should be added. > > The issue has been reported by Peter J. Philip (pjp _at_ delphinusdns.org). > > Discussed with bluhm@ and florian@ > > OK bluhm@ Couple of new failures. Reverting this commit makes the problem go away. https://regress.basename.se/amd64/2023-09-08.1/296-sys-net-pair.log https://regress.basename.se/amd64/2023-09-08.1/310-sys-net-vxlan.log https://regress.basename.se/amd64/2023-09-08.1/397-usr.sbin-bgpd.log https://regress.basename.se/amd64/2023-09-08.1/405-usr.sbin-ospf6d.log
Re: CVS: cvs.openbsd.org: src
On Mon, Sep 04, 2023 at 11:42:55PM -0400, Thomas Frohwein wrote: > On Mon, Sep 04, 2023 at 04:36:55PM -0600, Thomas Frohwein wrote: > > CVSROOT:/cvs > > Module name:src > > Changes by: t...@cvs.openbsd.org2023/09/04 16:36:55 > > > > Modified files: > > share/man/man5 : bsd.port.mk.5 > > > > Log message: > > document DIST_TUPLE and related variables: EXTRACT_SUFX., > > TEMPLATE_DISTFILES., and TEMPLATE_HOMEPAGE.. > > ok espie@ > > > > This wasn't only ok espie@, but a large part of this documentation was > written and/or modified by espie@. Credit where credit is due. We've been doing so much back on this, it's not even clear who wrote this. Stuart gave a lot of useful input as well. It's really the kind of work that goes so much faster during hackathons ! Lots of fun.
Re: CVS: cvs.openbsd.org: src
On Mon, Sep 04, 2023 at 04:36:55PM -0600, Thomas Frohwein wrote: > CVSROOT: /cvs > Module name: src > Changes by: t...@cvs.openbsd.org2023/09/04 16:36:55 > > Modified files: > share/man/man5 : bsd.port.mk.5 > > Log message: > document DIST_TUPLE and related variables: EXTRACT_SUFX., > TEMPLATE_DISTFILES., and TEMPLATE_HOMEPAGE.. > ok espie@ > This wasn't only ok espie@, but a large part of this documentation was written and/or modified by espie@. Credit where credit is due.
Re: CVS: cvs.openbsd.org: src
> Maybe ci is now "unused" on non-MULTIPROCESSOR kernels and this angers > the compiler? Yes. /usr/src/sys/kern/kern_clock.c:114:19: error: unused variable 'ci' [-Werror,-Wunused-variable] struct cpu_info *ci = curcpu(); ^ 1 error generated.
Re: CVS: cvs.openbsd.org: src
On Sat, Aug 12, 2023 at 07:19:28AM -0600, Miod Vallat wrote: > CVSROOT: /cvs > Module name: src > Changes by: m...@cvs.openbsd.org2023/08/12 07:19:28 > > Modified files: > sys/kern : kern_clock.c > > Log message: > Repair compilability for non-MULTIPROCESSOR kernels. I'm confused, what did I do? Maybe ci is now "unused" on non-MULTIPROCESSOR kernels and this angers the compiler?
Re: CVS: cvs.openbsd.org: src
On Thu, Jul 27, 2023 at 02:21:25PM -0600, Jan Klemkow wrote: > CVSROOT: /cvs > Module name: src > Changes by: j...@cvs.openbsd.org2023/07/27 14:21:25 > > Modified files: > sys/dev/pci: if_ix.c > sys/net: if_ethersubr.c > sys/netinet: if_ether.h > > Log message: > Fix inline vlan-tag handling of forwarded LRO packets from ix(4) > > Implement vlan-tag parsing ether_extract_header() to use this information > to adjust the MSS calculation of LRO packets. > > pointed out by mbuhl and bluhm and also found by sthen the hard way > with tweaks from bluhm > > ok bluhm@
Re: CVS: cvs.openbsd.org: src
On Mon, Jul 24, 2023 at 01:38:48PM -0600, Alexander Bluhm wrote: > CVSROOT: /cvs > Module name: src > Changes by: bl...@cvs.openbsd.org 2023/07/24 13:38:48 > > Modified files: > sys/dev/wscons : Tag: OPENBSD_7_3 wsemul_sun.c wsemul_vt100.c > > Log message: > Make sure we do not increase the escape sequence argument count beyond usable > bounds, in case escape sequences end up with too many semicolons. > Without this, the kernel could be made to access random memory after receiving > some specially crafted DCS or CSI terminal escape sequences. > > Reported by David Leadbeater (dgl, dgl dot cx) > from miod@ this is errata/7.3/014_wscons.patch.sig
Re: CVS: cvs.openbsd.org: src
On Wed, Jul 12, 2023 at 06:37:28AM -0600, Theo Buehler wrote: > CVSROOT: /cvs > Module name: src > Changes by: t...@cvs.openbsd.org2023/07/12 06:37:28 > > Modified files: > usr.sbin/httpd : httpd.h server.c server_fcgi.c > > Log message: > Work around use after free in httpd(8) > > A malformed HTTP request can cause httpd in fastcgi mode to crash due to a > use-after-free. This is an awful hack, but it's good enough until someone > figures out the correct way of dealing with server_close() here. > > "this will do the trick for now" claudio > ok beck deraadt > This was reported a while ago by Jesper Wallin
Re: CVS: cvs.openbsd.org: src
Yes, of course it did.
filedev/acpi/acpi_x86.c acpi & suspend & !small_kernel
The other ways of doing this will require a few #ifdef
>On Wed, Jul 05, 2023 at 12:51:56PM -0600, Tobias Heider wrote:
>> CVSROOT: /cvs
>> Module name: src
>> Changes by: to...@cvs.openbsd.org 2023/07/05 12:51:56
>>
>> Modified files:
>> sys/dev/acpi : acpi.c acpi_x86.c
>>
>> Log message:
>> Move acpiioctl to acpi_x86.c, it is only used up on i386 and amd64.
>>
>> ok kettenis@ deraadt@
>
>This broke the amd64 ramdisk.
>
>ld -T ld.script -X --warn-common -nopie -o bsd ${SYSTEM_HEAD} vers.o ${OBJS}
>ld: error: undefined symbol: acpiioctl
referenced by conf.c
conf.o:(cdevsw)
>*** Error 1 in /home/src2/sys/arch/amd64/compile/RAMDISK_CD (Makefile:610
>'bsd': @echo ld -T ld.script -X --warn-common -nopie -o bsd '${SYS...)
>*** Error 2 in /home/src2/distrib/amd64/ramdisk_cd (Makefile:71 'bsd')
>*** Error 2 in /home/src2/distrib/amd64 (:48 'all')
>*** Error 2 in /home/src2/distrib (:48 'all')
>*** Error 2 in . (Makefile:301 'distrib')
>*** Error 2 in . (Makefile:277 'do-release')
>*** Error 2 in /home/src2/etc (Makefile:260 'release')
>
>
Re: CVS: cvs.openbsd.org: src
On Wed, Jul 05, 2023 at 12:51:56PM -0600, Tobias Heider wrote:
> CVSROOT: /cvs
> Module name: src
> Changes by: to...@cvs.openbsd.org 2023/07/05 12:51:56
>
> Modified files:
> sys/dev/acpi : acpi.c acpi_x86.c
>
> Log message:
> Move acpiioctl to acpi_x86.c, it is only used up on i386 and amd64.
>
> ok kettenis@ deraadt@
This broke the amd64 ramdisk.
ld -T ld.script -X --warn-common -nopie -o bsd ${SYSTEM_HEAD} vers.o ${OBJS}
ld: error: undefined symbol: acpiioctl
>>> referenced by conf.c
>>> conf.o:(cdevsw)
*** Error 1 in /home/src2/sys/arch/amd64/compile/RAMDISK_CD (Makefile:610
'bsd': @echo ld -T ld.script -X --warn-common -nopie -o bsd '${SYS...)
*** Error 2 in /home/src2/distrib/amd64/ramdisk_cd (Makefile:71 'bsd')
*** Error 2 in /home/src2/distrib/amd64 (:48 'all')
*** Error 2 in /home/src2/distrib (:48 'all')
*** Error 2 in . (Makefile:301 'distrib')
*** Error 2 in . (Makefile:277 'do-release')
*** Error 2 in /home/src2/etc (Makefile:260 'release')
Re: CVS: cvs.openbsd.org: src
Diff below adds some debug output to narrow issue down if indeed it was
the editor_allocspace() that causes the problem.
Kenneth Westerback writes:
> Anton Lindqvist writes:
>
>> On Wed, Jun 28, 2023 at 06:12:48AM -0600, Kenneth R Westerback wrote:
>>> CVSROOT:/cvs
>>> Module name:src
>>> Changes by: k...@cvs.openbsd.org2023/06/28 06:12:48
>>>
>>> Modified files:
>>> sbin/disklabel : editor.c
>>>
>>> Log message:
>>> Refactor editor_allocspace() into easier to follow pieces.
>>>
>>> editor_allocspace() interates over alloc_tables calling
>>> allocate_space(). allocate_space() iterates over
>>> space_allocations calling allocate_partition().
>>> allocate_partition() calls allocate_diskchunk() which finds disk
>>> space for the partition.
>>>
>>> No intentional functional change.
>>>
>>> ok otto@
>>>
>> My regress machines failed today during autoinstall with the following
>> error:
>>
>> disklabel: autoalloc failed
>> Autopartitioning failed
>>
>> These machines are using disk templates. I haven't been able to test a
>> revert yet but this looks like the prime suspect according to the cvs
>> delta.
>
> Some more data would be helpful. Like, what are the 'template' files you
> are using? What is in the autoinstall script? What architecture is this?
>
> The regress tests in the tree pass with no problems noted.
--
Ken
diff --git a/sbin/disklabel/editor.c b/sbin/disklabel/editor.c
index a84900331dc..8193cbdd315 100644
--- a/sbin/disklabel/editor.c
+++ b/sbin/disklabel/editor.c
@@ -578,8 +578,11 @@ allocate_diskchunk(const struct disklabel *lp,
#endif
if (maxstop < largest.stop)
largest.stop = maxstop;
- if (CHUNKSZ() < DL_BLKTOSEC(lp, sa->minsz))
+ if (CHUNKSZ() < DL_BLKTOSEC(lp, sa->minsz)) {
+ fprintf(stderr, "CHUNKSZ() (%llu) < '%s' sa->minsz
(%llu)\n",
+ CHUNKSZ(), sa->mp, sa->minsz);
return NULL;
+ }
return
}
@@ -598,8 +601,10 @@ allocate_partition(struct disklabel *lp, struct
space_allocation *sa)
if (DL_GETPSIZE(pp) == 0 || pp->p_fstype == FS_UNUSED)
break;
}
- if (partno >= nitems(lp->d_partitions))
+ if (partno >= nitems(lp->d_partitions)) {
+ fprintf(stderr, "No free partition available\n");
return 1; /* No free partition. */
+ }
/* Find appropriate chunk of free space. */
chunk = allocate_diskchunk(lp, sa);
@@ -612,8 +617,10 @@ allocate_partition(struct disklabel *lp, struct
space_allocation *sa)
pp->p_fstype = FS_SWAP;
else if (sa->mp[0] == '/')
pp->p_fstype = FS_BSDFFS;
- else
+ else {
+ fprintf(stderr, "Invalid mount point '%s'\n", sa->mp);
return 1;
+ }
DL_SETPSIZE(pp, chunk->stop - chunk->start);
DL_SETPOFFSET(pp, chunk->start);
@@ -622,8 +629,10 @@ allocate_partition(struct disklabel *lp, struct
space_allocation *sa)
mountpoints[partno] = strdup(sa->mp);
if (mountpoints[partno] == NULL)
err(1, NULL);
- if (set_fragblock(lp, partno))
+ if (set_fragblock(lp, partno)) {
+ fprintf(stderr, "set_fragblock() failed\n");
return 1;
+ }
}
return 0;
@@ -662,8 +671,11 @@ allocate_space(struct disklabel *lp, const struct
alloc_table *alloc_table)
sa[i] = alloc_table->table[i];
if (alloc_table->table == alloc_big)
allocate_physmemincr([i]);
- if (xtrablks < sa[i].minsz)
+ if (xtrablks < sa[i].minsz) {
+ fprintf(stderr, "xtrablks (%llu) < sa[%d].minsz
(%llu)\n",
+ xtrablks, i, sa[i].minsz);
return 1; /* Too few free blocks. */
+ }
xtrablks -= sa[i].minsz;
}
sa[alloc_table->sz - 1].rate = 100; /* Last allocation is greedy. */
Re: CVS: cvs.openbsd.org: src
Anton Lindqvist writes: > On Wed, Jun 28, 2023 at 06:12:48AM -0600, Kenneth R Westerback wrote: >> CVSROOT: /cvs >> Module name: src >> Changes by: k...@cvs.openbsd.org2023/06/28 06:12:48 >> >> Modified files: >> sbin/disklabel : editor.c >> >> Log message: >> Refactor editor_allocspace() into easier to follow pieces. >> >> editor_allocspace() interates over alloc_tables calling >> allocate_space(). allocate_space() iterates over >> space_allocations calling allocate_partition(). >> allocate_partition() calls allocate_diskchunk() which finds disk >> space for the partition. >> >> No intentional functional change. >> >> ok otto@ >> > My regress machines failed today during autoinstall with the following > error: > > disklabel: autoalloc failed > Autopartitioning failed > > These machines are using disk templates. I haven't been able to test a > revert yet but this looks like the prime suspect according to the cvs > delta. Some more data would be helpful. Like, what are the 'template' files you are using? What is in the autoinstall script? What architecture is this? The regress tests in the tree pass with no problems noted. -- Ken
Re: CVS: cvs.openbsd.org: src
On Wed, Jun 28, 2023 at 06:12:48AM -0600, Kenneth R Westerback wrote: > CVSROOT: /cvs > Module name: src > Changes by: k...@cvs.openbsd.org2023/06/28 06:12:48 > > Modified files: > sbin/disklabel : editor.c > > Log message: > Refactor editor_allocspace() into easier to follow pieces. > > editor_allocspace() interates over alloc_tables calling > allocate_space(). allocate_space() iterates over > space_allocations calling allocate_partition(). > allocate_partition() calls allocate_diskchunk() which finds disk > space for the partition. > > No intentional functional change. > > ok otto@ > My regress machines failed today during autoinstall with the following error: disklabel: autoalloc failed Autopartitioning failed These machines are using disk templates. I haven't been able to test a revert yet but this looks like the prime suspect according to the cvs delta.
Re: CVS: cvs.openbsd.org: src
On 1.6.2023. 20:57, Klemens Nanni wrote: > CVSROOT: /cvs > Module name: src > Changes by: k...@cvs.openbsd.org2023/06/01 12:57:54 > > Modified files: > sys/net: if_wg.c if_wg.h > sbin/ifconfig : ifconfig.8 ifconfig.c > > Log message: > Add support for wireguard peer descriptions > > "wgdescr[iption] foo" to label one peer (amongst many) on a wg(4) interface, > "-wgdescr[iption]" or "wgdescr ''" to remove the label, completely analogous > to existing interface discriptions. > > Idea/initial diff from Mikolaj Kucharski (OK sthen) > Tests/prodded by Hrvoje Popovski > Tweaks/manual bits from me > Feedback deraadt sthen mvs claudio > OK claudio > > Thank you
Re: CVS: cvs.openbsd.org: src
On Thu, Jun 01, 2023 at 12:57:54PM -0600, Klemens Nanni wrote: > CVSROOT: /cvs > Module name: src > Changes by: k...@cvs.openbsd.org2023/06/01 12:57:54 > > Modified files: > sys/net: if_wg.c if_wg.h > sbin/ifconfig : ifconfig.8 ifconfig.c > > Log message: > Add support for wireguard peer descriptions > > "wgdescr[iption] foo" to label one peer (amongst many) on a wg(4) interface, > "-wgdescr[iption]" or "wgdescr ''" to remove the label, completely analogous > to existing interface discriptions. > > Idea/initial diff from Mikolaj Kucharski (OK sthen) > Tests/prodded by Hrvoje Popovski > Tweaks/manual bits from me > Feedback deraadt sthen mvs claudio > OK claudio > I would like to point out, that initial version was written by Noah Meier https://marc.info/?l=openbsd-tech=163478285129091=2 -- Regards, Mikolaj
Re: CVS: cvs.openbsd.org: src
On 2023/06/01 03:47, Claudio Jeker wrote: > CVSROOT: /cvs > Module name: src > Changes by: clau...@cvs.openbsd.org 2023/06/01 03:47:35 > > Modified files: > usr.sbin/bgpd : kroute.c > > Log message: > Check the F_NEXTHOP flag on the right kroute6 object. > > On multipath routes the check ended up checking the wrong route for the > nexthop update. This resulted in a use-after-free in kroute_detach_nexthop(). > This only affects IPv6 in the IPv4 code path the right object was already > used. > > Thanks to sthen@ for providing the debug information to track this down. > OK sthen@ tb@ > I think this is one where we can definitely say "found the hard way", thank you Claudio for staring at the code..
Re: CVS: cvs.openbsd.org: src
On 28.4.2023. 17:50, Alexandr Nedvedicky wrote: > CVSROOT: /cvs > Module name: src > Changes by: sas...@cvs.openbsd.org 2023/04/28 09:50:05 > > Modified files: > sys/net: if_pfsync.c > > Log message: > remove superfluous/invalid KASSERT() in pfsync_q_del(). > > pointed and OK bluhm@ > > I thought that this diff would repair this panic r620-2# uvm_fault(0x825c8000, 0x17, 0, 2) -> e kernel: page fault trap, code=2 Stopped at pfsync_q_del+0x8d: movq%rdx,0x8(%rax) TIDPIDUID PRFLAGS PFLAGS CPU COMMAND *461341 27834 0 0x14000 0x2005K softnet pfsync_q_del(fd832ba5d320) at pfsync_q_del+0x8d pf_remove_state(fd832ba5d320) at pf_remove_state+0x156 pfsync_in_del_c(fd8002ce6738,c,79,0) at pfsync_in_del_c+0x6f pfsync_input(800022d60a98,800022d60aa4,f0,2) at pfsync_input+0x34c ip_deliver(800022d60a98,800022d60aa4,f0,2) at ip_deliver+0x113 ipintr() at ipintr+0x69 if_netisr(0) at if_netisr+0xe0 taskq_thread(8003) at taskq_thread+0x100 end trace frame: 0x0, count: 7 https://www.openbsd.org/ddb.html describes the minimum info required in bug reports. Insufficient info makes it difficult to find and fix bugs. but it didn't :)
Re: CVS: cvs.openbsd.org: src
Or as it should have said: "Make 'bsd' before 'mr.fs' so that 'mr.fs' can use the actual space available in the kernel image rather than relying on third parties like disktab to be correct. No intentional functional change." Sigh. Kenneth R Westerback writes: > CVSROOT: /cvs > Module name: src > Changes by: k...@cvs.openbsd.org2023/04/28 02:45:26 > > Modified files: > distrib/alpha/miniroot: Makefile > distrib/amd64/ramdiskA: Makefile > distrib/amd64/ramdisk_cd: Makefile > distrib/arm64/ramdisk: Makefile > distrib/armv7/ramdisk: Makefile > distrib/hppa/ramdisk: Makefile > distrib/i386/ramdisk: Makefile > distrib/i386/ramdisk_cd: Makefile > distrib/landisk/ramdisk: Makefile > distrib/loongson/ramdisk: Makefile > distrib/luna88k/ramdisk: Makefile > distrib/macppc/ramdisk: Makefile > distrib/octeon/ramdisk: Makefile > distrib/powerpc64/ramdisk: Makefile > distrib/riscv64/ramdisk: Makefile > distrib/sparc64/miniroot: Makefile > distrib/sparc64/ramdisk: Makefile > distrib/sparc64/ramdiskB: Makefile > sys/arch/octeon/stand/boot: Makefile > sys/arch/powerpc64/stand/boot: Makefile > > Log message: > Retire -E's "expert" mode. Introduced 23 years ago to avoid > confusing users with FFS attributes that only experts should > fiddle with. Actual use has withered away with functionality > rendered moot or moved elsewhere. > > '-e' remains for the truly obscure corner cases. > > Simply excise the code for now to see if hidden users/uses are > exposed. Further simplifications are possible if no such > users/uses surface. > > ok with sthen@ millert@ kn@ otto@ -- Ken
Re: CVS: cvs.openbsd.org: src
Sigh. Greg Steuck wrote: > "Theo de Raadt" writes: > > > Can you dig deeper into why? > > The easiest reproducer is to compile a file with a single > #include > > > Should syslog.h be pulling in sys/_types.h instead of machine/_types.h? > > Neither of them seems to have the magic define, unlike the 13 > unobviously chosen places in /usr/include which have: > > #ifndef _SIZE_T_DEFINED_ > #define _SIZE_T_DEFINED_ > typedef __size_t size_t; > #endif > > Should I replicate this block into sys/syslog.h? That fixes the problem > but clearly one can never tell what's gonna break with system include > files. > > Thanks > Greg
Re: CVS: cvs.openbsd.org: src
"Todd C. Miller" writes: >> Should I replicate this block into sys/syslog.h? That fixes the >> problem >> but clearly one can never tell what's gonna break with system >> include >> files. > > That should be OK but you could also just use __size_t in the > function prototype. We use __va_list in syslog.h instead of va_list > for a similar reason. I'm running this through make build just in case, but it seems to be fixing the original problem. OK? diff --git a/sys/sys/syslog.h b/sys/sys/syslog.h index 063105952b1..c5f24339ecc 100644 --- a/sys/sys/syslog.h +++ b/sys/sys/syslog.h @@ -204,7 +204,7 @@ int setlogmask_r(int, struct syslog_data *); void syslog_r(int, struct syslog_data *, const char *, ...) __attribute__((__format__(__syslog__,3,4))); void vsyslog_r(int, struct syslog_data *, const char *, __va_list); -intsendsyslog(const char *, size_t, int); +intsendsyslog(const char *, __size_t, int); __END_DECLS #else /* !_KERNEL */
Re: CVS: cvs.openbsd.org: src
On Thu, 27 Apr 2023 15:40:10 +0100, Greg Steuck wrote: > Neither of them seems to have the magic define, unlike the 13 > unobviously chosen places in /usr/include which have: > > #ifndef _SIZE_T_DEFINED_ > #define _SIZE_T_DEFINED_ > typedef __size_t size_t; > #endif > > Should I replicate this block into sys/syslog.h? That fixes the problem > but clearly one can never tell what's gonna break with system include > files. That should be OK but you could also just use __size_t in the function prototype. We use __va_list in syslog.h instead of va_list for a similar reason. - todd
Re: CVS: cvs.openbsd.org: src
"Theo de Raadt" writes: > Can you dig deeper into why? The easiest reproducer is to compile a file with a single #include > Should syslog.h be pulling in sys/_types.h instead of machine/_types.h? Neither of them seems to have the magic define, unlike the 13 unobviously chosen places in /usr/include which have: #ifndef _SIZE_T_DEFINED_ #define _SIZE_T_DEFINED_ typedef __size_tsize_t; #endif Should I replicate this block into sys/syslog.h? That fixes the problem but clearly one can never tell what's gonna break with system include files. Thanks Greg
Re: CVS: cvs.openbsd.org: src
Can you dig deeper into why? Should syslog.h be pulling in sys/_types.h instead of machine/_types.h? Antoine Jacoutot wrote: > On Wed, Apr 26, 2023 at 12:52:45AM -0600, Greg Steuck wrote: > > CVSROOT:/cvs > > Module name:src > > Changes by: gne...@cvs.openbsd.org 2023/04/26 00:52:45 > > > > Modified files: > > lib/libc/hidden: syslog.h > > sys/sys: syslog.h > > > > Log message: > > Declare sendsyslog to match its man page > > > > This simplifies syzkaller revival after the removal of __syscall. > > > > OK bluhm, millert, deraadt > > This broke (at least): > > www/e2guardian > www/dansguardian > > /usr/include/syslog.h:207:30: error: unknown type name 'size_t'; did you mean > '__size_t'? > > > -- > Antoine >
Re: CVS: cvs.openbsd.org: src
On Wed, Apr 26, 2023 at 12:52:45AM -0600, Greg Steuck wrote: > CVSROOT: /cvs > Module name: src > Changes by: gne...@cvs.openbsd.org 2023/04/26 00:52:45 > > Modified files: > lib/libc/hidden: syslog.h > sys/sys: syslog.h > > Log message: > Declare sendsyslog to match its man page > > This simplifies syzkaller revival after the removal of __syscall. > > OK bluhm, millert, deraadt This broke (at least): www/e2guardian www/dansguardian /usr/include/syslog.h:207:30: error: unknown type name 'size_t'; did you mean '__size_t'? -- Antoine
Re: CVS: cvs.openbsd.org: src
On Wed, Apr 26, 2023 at 11:39:20AM -0600, Theo Buehler wrote: > CVSROOT: /cvs > Module name: src > Changes by: t...@cvs.openbsd.org2023/04/26 11:39:20 > > Modified files: > usr.sbin/bgpctl: json.c > > Log message: > Whitespace > > "please fix" json > json == claudio
Re: CVS: cvs.openbsd.org: src
Jonathan Gray writes: > On Wed, Apr 26, 2023 at 07:39:38AM +0200, Anton Lindqvist wrote: >> On Mon, Apr 24, 2023 at 10:53:57AM -0600, Dave Voutila wrote: >> > CVSROOT: /cvs >> > Module name: src >> > Changes by:d...@cvs.openbsd.org2023/04/24 10:53:57 >> > >> > Modified files: >> >sys/arch/amd64/amd64: vmm.c >> > >> > Log message: >> > vmm(4): allow guests to enable and use supervisor IBT. >> > >> > Why should hosts have all the fun? Conditionally unmask the cpuid >> > bits for IBT and allow r/w access to the supervisor CET msr. >> > >> > Will need revisiting when we introduce usage of userland CET msr. >> > >> > ok marlkin@ >> >> Running on older hardware where CET is not enumerated causes guests to >> panic in cpu_fix_msrs() during RDMSR 0x6a2. > > vcpu_reset_regs_vmx() also has: > > if (rcr4() | CR4_CET) > vmx_setmsrbrw(vcpu, MSR_S_CET); > Thanks!
Re: CVS: cvs.openbsd.org: src
On Wed, Apr 26, 2023 at 07:39:38AM +0200, Anton Lindqvist wrote: > On Mon, Apr 24, 2023 at 10:53:57AM -0600, Dave Voutila wrote: > > CVSROOT:/cvs > > Module name:src > > Changes by: d...@cvs.openbsd.org2023/04/24 10:53:57 > > > > Modified files: > > sys/arch/amd64/amd64: vmm.c > > > > Log message: > > vmm(4): allow guests to enable and use supervisor IBT. > > > > Why should hosts have all the fun? Conditionally unmask the cpuid > > bits for IBT and allow r/w access to the supervisor CET msr. > > > > Will need revisiting when we introduce usage of userland CET msr. > > > > ok marlkin@ > > Running on older hardware where CET is not enumerated causes guests to > panic in cpu_fix_msrs() during RDMSR 0x6a2. vcpu_reset_regs_vmx() also has: if (rcr4() | CR4_CET) vmx_setmsrbrw(vcpu, MSR_S_CET); > > diff --git sys/arch/amd64/amd64/vmm.c sys/arch/amd64/amd64/vmm.c > index 42ac8007029..35b05033cdc 100644 > --- sys/arch/amd64/amd64/vmm.c > +++ sys/arch/amd64/amd64/vmm.c > @@ -7059,7 +7059,7 @@ vmm_handle_cpuid(struct vcpu *vcpu) > *rcx &= ~SEFF0ECX_PKU; > > /* Expose IBT bit if we've enabled CET on the host. */ > - if (rcr4() | CR4_CET) > + if (rcr4() & CR4_CET) > *rdx |= SEFF0EDX_IBT; > else > *rdx &= ~SEFF0EDX_IBT; > >
Re: CVS: cvs.openbsd.org: src
On Mon, Apr 24, 2023 at 10:53:57AM -0600, Dave Voutila wrote: > CVSROOT: /cvs > Module name: src > Changes by: d...@cvs.openbsd.org2023/04/24 10:53:57 > > Modified files: > sys/arch/amd64/amd64: vmm.c > > Log message: > vmm(4): allow guests to enable and use supervisor IBT. > > Why should hosts have all the fun? Conditionally unmask the cpuid > bits for IBT and allow r/w access to the supervisor CET msr. > > Will need revisiting when we introduce usage of userland CET msr. > > ok marlkin@ Running on older hardware where CET is not enumerated causes guests to panic in cpu_fix_msrs() during RDMSR 0x6a2. diff --git sys/arch/amd64/amd64/vmm.c sys/arch/amd64/amd64/vmm.c index 42ac8007029..35b05033cdc 100644 --- sys/arch/amd64/amd64/vmm.c +++ sys/arch/amd64/amd64/vmm.c @@ -7059,7 +7059,7 @@ vmm_handle_cpuid(struct vcpu *vcpu) *rcx &= ~SEFF0ECX_PKU; /* Expose IBT bit if we've enabled CET on the host. */ - if (rcr4() | CR4_CET) + if (rcr4() & CR4_CET) *rdx |= SEFF0EDX_IBT; else *rdx &= ~SEFF0EDX_IBT;
Re: CVS: cvs.openbsd.org: src
On Mon, Apr 24, 2023 at 10:55:06AM -0600, Theo Buehler wrote:
> CVSROOT: /cvs
> Module name: src
> Changes by: t...@cvs.openbsd.org2023/04/24 10:55:06
>
> Modified files:
> lib/libssl : ssl_tlsext.c
>
> Log message:
> Free and calloc() the tlsext_build_order and remember its length
>
> Aligns tlsext_randomize_build_order() with tlsext_linearize_build_order()
> and will help regression testing.
>
> ok jsing
Does not compile without the following:
Index: ssl_local.h
===
RCS file: /cvs/src/lib/libssl/ssl_local.h,v
retrieving revision 1.4
diff -u -p -r1.4 ssl_local.h
--- ssl_local.h 23 Apr 2023 18:51:53 - 1.4
+++ ssl_local.h 25 Apr 2023 05:13:01 -
@@ -975,6 +975,7 @@ struct ssl_st {
unsigned int max_send_fragment;
const struct tls_extension **tlsext_build_order;
+ size_t tlsext_build_order_len;
char *tlsext_hostname;
/* certificate status request info */
Re: CVS: cvs.openbsd.org: src
On Mon, Mar 27, 2023 at 08:02:20AM +0200, Anton Lindqvist wrote: > On Sat, Mar 25, 2023 at 12:27:28PM -0600, Klemens Nanni wrote: > > CVSROOT:/cvs > > Module name:src > > Changes by: k...@cvs.openbsd.org2023/03/25 12:27:28 > > > > Modified files: > > distrib/miniroot: install.sub > > > > Log message: > > simplify final MAKEDEV call > > > > No need to loop here, the script takes multiple args. > > > > OK tb afresh1 > > Seen in install log on my regress machines during autoinstall. Is this > commit the culprit? Yes. MAKEDEV doesn't like repeated args and makes noise if it sees them. If you try to create an already existing device with a single argument it is silent. Odd. I reverted the commit.
Re: CVS: cvs.openbsd.org: src
It looks like the diff wasn't tested. Back it out. Anton Lindqvist wrote: > On Sat, Mar 25, 2023 at 12:27:28PM -0600, Klemens Nanni wrote: > > CVSROOT:/cvs > > Module name:src > > Changes by: k...@cvs.openbsd.org2023/03/25 12:27:28 > > > > Modified files: > > distrib/miniroot: install.sub > > > > Log message: > > simplify final MAKEDEV call > > > > No need to loop here, the script takes multiple args. > > > > OK tb afresh1 > > Seen in install log on my regress machines during autoinstall. Is this > commit the culprit? > > Making all device nodes...mknod: sd0a: File exists > mknod: rsd0a: File exists > mknod: sd0b: File exists > mknod: rsd0b: File exists > mknod: sd0c: File exists > mknod: rsd0c: File exists > mknod: sd0d: File exists > mknod: rsd0d: File exists > mknod: sd0e: File exists > mknod: rsd0e: File exists > mknod: sd0f: File exists > mknod: rsd0f: File exists > mknod: sd0g: File exists > mknod: rsd0g: File exists > mknod: sd0h: File exists > mknod: rsd0h: File exists > mknod: sd0i: File exists > mknod: rsd0i: File exists > mknod: sd0j: File exists > mknod: rsd0j: File exists > mknod: sd0k: File exists > mknod: rsd0k: File exists > mknod: sd0l: File exists > mknod: rsd0l: File exists > mknod: sd0m: File exists > mknod: rsd0m: File exists > mknod: sd0n: File exists > mknod: rsd0n: File exists > mknod: sd0o: File exists > mknod: rsd0o: File exists > mknod: sd0p: File exists > mknod: rsd0p: File exists > mknod: cd0a: File exists > mknod: rcd0a: File exists > mknod: cd0c: File exists > mknod: rcd0c: File exists > done. >
Re: CVS: cvs.openbsd.org: src
On Sat, Mar 25, 2023 at 12:27:28PM -0600, Klemens Nanni wrote: > CVSROOT: /cvs > Module name: src > Changes by: k...@cvs.openbsd.org2023/03/25 12:27:28 > > Modified files: > distrib/miniroot: install.sub > > Log message: > simplify final MAKEDEV call > > No need to loop here, the script takes multiple args. > > OK tb afresh1 Seen in install log on my regress machines during autoinstall. Is this commit the culprit? Making all device nodes...mknod: sd0a: File exists mknod: rsd0a: File exists mknod: sd0b: File exists mknod: rsd0b: File exists mknod: sd0c: File exists mknod: rsd0c: File exists mknod: sd0d: File exists mknod: rsd0d: File exists mknod: sd0e: File exists mknod: rsd0e: File exists mknod: sd0f: File exists mknod: rsd0f: File exists mknod: sd0g: File exists mknod: rsd0g: File exists mknod: sd0h: File exists mknod: rsd0h: File exists mknod: sd0i: File exists mknod: rsd0i: File exists mknod: sd0j: File exists mknod: rsd0j: File exists mknod: sd0k: File exists mknod: rsd0k: File exists mknod: sd0l: File exists mknod: rsd0l: File exists mknod: sd0m: File exists mknod: rsd0m: File exists mknod: sd0n: File exists mknod: rsd0n: File exists mknod: sd0o: File exists mknod: rsd0o: File exists mknod: sd0p: File exists mknod: rsd0p: File exists mknod: cd0a: File exists mknod: rcd0a: File exists mknod: cd0c: File exists mknod: rcd0c: File exists done.
Re: CVS: cvs.openbsd.org: src
copied out.
Re: CVS: cvs.openbsd.org: src
On 2/21/2023 4:38 PM, Jonathan Gray wrote: > On Tue, Feb 21, 2023 at 06:42:59AM -0700, Brian Callahan wrote: >> CVSROOT: /cvs >> Module name: src >> Changes by: bcal...@cvs.openbsd.org 2023/02/21 06:42:59 >> >> Modified files: >> sys/dev/pci: azalia.c >> >> Log message: >> Attach Apollo Lake HD Audio device, enabling audio on machines with it. >> ok jsg@ phessler@ > > It already attached on other Apollo Lake machines, matching with > the HD Audio pci subclass. Your machine (and likely others) has a > different subclass. > Thanks for the clarification. I've never encountered another Apollo Lake machine. ~Brian
Re: CVS: cvs.openbsd.org: src
On Tue, Feb 21, 2023 at 06:42:59AM -0700, Brian Callahan wrote: > CVSROOT: /cvs > Module name: src > Changes by: bcal...@cvs.openbsd.org 2023/02/21 06:42:59 > > Modified files: > sys/dev/pci: azalia.c > > Log message: > Attach Apollo Lake HD Audio device, enabling audio on machines with it. > ok jsg@ phessler@ It already attached on other Apollo Lake machines, matching with the HD Audio pci subclass. Your machine (and likely others) has a different subclass.
Re: CVS: cvs.openbsd.org: src
Oops. The commit message was supposed to be hppa does not do pinstall() correctly because of that weird callgate stuff. Not sure how to fix it, so disabling this on hppa for now. Theo de Raadt wrote: > CVSROOT: /cvs > Module name: src > Changes by: dera...@cvs.openbsd.org 2023/02/21 09:21:40 > > Modified files: > lib/libc/dlfcn : init.c > > Log message: > dlfcn/init.c >
Re: CVS: cvs.openbsd.org: src
On 2023/02/15 20:10, Theo de Raadt wrote: > transistor shortage, in these trying times > > Jonathan Gray wrote: > > > CVSROOT:/cvs > > Module name:src > > Changes by: j...@cvs.openbsd.org2023/02/15 20:09:33 > > > > Modified files: > > usr.sbin/fw_update: patterns.c > > > > Log message: > > remove the '(R)' from the intel cpu match pattern > > > > Intel(R) does not appear in > > cpu0: Intel Atom(R) x6425RE Processor @ 1.90GHz, 1895.90 MHz, 06-96-01 > > reported by patrick@ ok deraadt@ > > > Perhaps they need a new cpuid function that returns UTF-8 (the model name they use in some other places has a unicode registered trademark symbol in place of the now-missing '(R)').
Re: CVS: cvs.openbsd.org: src
transistor shortage, in these trying times Jonathan Gray wrote: > CVSROOT: /cvs > Module name: src > Changes by: j...@cvs.openbsd.org2023/02/15 20:09:33 > > Modified files: > usr.sbin/fw_update: patterns.c > > Log message: > remove the '(R)' from the intel cpu match pattern > > Intel(R) does not appear in > cpu0: Intel Atom(R) x6425RE Processor @ 1.90GHz, 1895.90 MHz, 06-96-01 > reported by patrick@ ok deraadt@ >
Re: CVS: cvs.openbsd.org: src
On Wed, 25 Jan 2023 12:06:50 -0700, Todd C. Miller wrote: > CVSROOT: /cvs > Module name: src > Changes by: mill...@cvs.openbsd.org 2023/01/25 12:06:50 > > Modified files: > usr.bin/pkg-config/OpenBSD: PkgConfig.pm > > Log message: > Fix CVE-2023-24056, unbounded variable expansion in pkg-config. > We now die with an error when trying to expand a variable that is > already longer than 64K. This was never a buffer overflow in our > pkg-config, but rather an unbounded memory allocation that would > eventually run up against resource limits. OK sthen@ jasper@ To avoid confusion on the matter, the CVE listed is for the C version of pkg-config, not our Perl version. This is not a security issue on OpenBSD because: 1) there is no buffer overflow in our perl version 2) only root can install .pc files anyway However, it still makes sense to limit the amount of variable expansion to avoid using excessive memory. The 64K limit was chosen to be compatible with the C version. - todd
Re: CVS: cvs.openbsd.org: src
On Tue, Dec 27 2022, Jeremie Courreges-Anglas wrote:
> CVSROOT: /cvs
> Module name: src
> Changes by: j...@cvs.openbsd.org2022/12/27 00:34:05
>
> Modified files:
> sys/lib/libsa : netif.c
> sys/arch/amd64/stand/libsa: pxe.c
> sys/arch/i386/stand/libsa: pxe.c
>
> Log message:
> Ansify pxe_netif_close() and {,pxe}socktodesc()
>
> To appease the clang 15 warning -Wdeprecated-non-prototype (turned on
> by -Wall). ok millert@
Actually with clang 15 -Wdeprecated-non-prototype is on by default,
with or without -Wall.
--
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF DDCC 0DFA 74AE 1524 E7EE
Re: CVS: cvs.openbsd.org: src
Hi Theo, Theo de Raadt wrote on Thu, Dec 22, 2022 at 09:42:36AM -0700: > Hmm. Are you sure about this? Or do you have it backwards? If you harbour doubts, feel free to submit a new file /usr/src/lib/libcrypto/man/BIO_s_log.3 for review, but maybe wait three months and ten days before doing so. > Should we be re-writing libc/stdio as a layerr on top of BIO? Spectacularly good idea, but please make sure jsing@ gets professional psychological support when he is then forced to resolve the resulting circular dependencies in libcrypto/bio/bss_file.c / BIO_s_file(3). Then again, i suspect adding BIO_accept_ex(3) and friends (as documented in OpenSSL 1.1.1 doc/man3/BIO_connect.pod) to LibreSSL and then replacing the Berkeley socket(2) layer with that is an even more pressing task. Yours, Ingo >> CVSROOT: /cvs >> Module name: src >> Changes by: schwa...@cvs.openbsd.org2022/12/22 09:38:45 >> >> Modified files: >> lib/libcrypto/man: BIO_s_null.3 >> >> Log message: >> Mark BIO_s_log(3) as intentionally undocumented. >> >> Ben Laurie invented the system logging BIO in 1999 and yet, >> nothing whatsoever uses it according to codesearch.debian.net. >> Besides, it is poorly designed and a crypto library is absolutely >> not the place for putting a clumsy system logging facility. >> Not everything needs to be a BIO!
Re: CVS: cvs.openbsd.org: src
Hmm. Are you sure about this? Or do you have it backwards? Should we be re-writing libc/stdio as a layerr on top of BIO? Ingo Schwarze wrote: > CVSROOT: /cvs > Module name: src > Changes by: schwa...@cvs.openbsd.org2022/12/22 09:38:45 > > Modified files: > lib/libcrypto/man: BIO_s_null.3 > > Log message: > Mark BIO_s_log(3) as intentionally undocumented. > > Ben Laurie invented the system logging BIO in 1999 and yet, > nothing whatsoever uses it according to codesearch.debian.net. > Besides, it is poorly designed and a crypto library is absolutely > not the place for putting a clumsy system logging facility. > Not everything needs to be a BIO! >
Re: CVS: cvs.openbsd.org: src
On Mon, Dec 19, 2022 at 2:55 PM Philip Guenther wrote: > CVSROOT:/cvs > Module name:src > Changes by: guent...@cvs.openbsd.org2022/12/19 15:55:12 > > Modified files: > usr.bin/kdump : kdump.c kdump_subr.h mksubr > > Log message: > Improve reporting of waitid(2)'s idtype/id and options arguments > Add mimmutable(2) to report like munmap(2) > ...and this is ok deraadt@ and millert@ (not my day for typing commit messages)
Re: CVS: cvs.openbsd.org: src
On Mon, Dec 19, 2022 at 2:45 PM Philip Guenther wrote: > CVSROOT:/cvs > Module name:src > Changes by: guent...@cvs.openbsd.org2022/12/19 15:44:54 > > Modified files: > regress/lib/libc/sys: t_wait_noproc.c > > Log message: > WTRAPPED is now supported by waitid(2) > Don't test waitid(WUNTRACED) as that's not portable and only 'works' due > to an implementation decision > This was ok kettenis@
Re: CVS: cvs.openbsd.org: src
Nicholas Marriott wrote (2022-12-19 08:30 CET): > Try this again now please. Works as expected again. Thanks! - Stefan > On Sat, 17 Dec 2022 at 11:59, Stefan Hagen wrote: > > > Nicholas Marriott wrote (2022-12-16 09:13 CET): > > > CVSROOT: /cvs > > > Module name: src > > > Changes by: n...@cvs.openbsd.org2022/12/16 01:13:40 > > > > > > Modified files: > > > usr.bin/tmux : arguments.c cmd-find-window.c cmd-send-keys.c > > >tmux.1 tmux.h > > > > > > Log message: > > > Add send-keys -K to handle keys directly as if typed (so look up in key > > > table). GitHub issue 3361. > > > > This broke sending keys to a detached session: > > > > Before: > > ./obj/tmux new-session -s mysession -d > > ./obj/tmux send-keys -t mysession:0 '# Hello World!' Enter > > > > After: > > ./obj/tmux new-session -s mysession -d > > ./obj/tmux send-keys -t mysession:0 '# Hello World!' Enter > > no current client > > > > - Stefan > >
Re: CVS: cvs.openbsd.org: src
Try this again now please. On Sat, 17 Dec 2022 at 11:59, Stefan Hagen wrote: > Nicholas Marriott wrote (2022-12-16 09:13 CET): > > CVSROOT: /cvs > > Module name: src > > Changes by: n...@cvs.openbsd.org2022/12/16 01:13:40 > > > > Modified files: > > usr.bin/tmux : arguments.c cmd-find-window.c cmd-send-keys.c > >tmux.1 tmux.h > > > > Log message: > > Add send-keys -K to handle keys directly as if typed (so look up in key > > table). GitHub issue 3361. > > This broke sending keys to a detached session: > > Before: > ./obj/tmux new-session -s mysession -d > ./obj/tmux send-keys -t mysession:0 '# Hello World!' Enter > > After: > ./obj/tmux new-session -s mysession -d > ./obj/tmux send-keys -t mysession:0 '# Hello World!' Enter > no current client > > - Stefan >
Re: CVS: cvs.openbsd.org: src
Nicholas Marriott wrote (2022-12-16 09:13 CET): > CVSROOT: /cvs > Module name: src > Changes by: n...@cvs.openbsd.org2022/12/16 01:13:40 > > Modified files: > usr.bin/tmux : arguments.c cmd-find-window.c cmd-send-keys.c >tmux.1 tmux.h > > Log message: > Add send-keys -K to handle keys directly as if typed (so look up in key > table). GitHub issue 3361. This broke sending keys to a detached session: Before: ./obj/tmux new-session -s mysession -d ./obj/tmux send-keys -t mysession:0 '# Hello World!' Enter After: ./obj/tmux new-session -s mysession -d ./obj/tmux send-keys -t mysession:0 '# Hello World!' Enter no current client - Stefan
Re: CVS: cvs.openbsd.org: src
No to this proposal.
The tree will remain broken until the change is backed out or fixed.
Anton Lindqvist wrote:
> On Sun, Dec 11, 2022 at 02:19:08PM -0700, Vitaliy Makkoveev wrote:
> > CVSROOT:/cvs
> > Module name:src
> > Changes by: m...@cvs.openbsd.org2022/12/11 14:19:08
> >
> > Modified files:
> > sys/kern : sys_socket.c uipc_socket.c uipc_socket2.c
> > uipc_usrreq.c
> > sys/miscfs/fifofs: fifo_vnops.c
> > sys/netinet: tcp_usrreq.c
> > sys/sys: socketvar.h
> >
> > Log message:
> > This time, socket's buffer lock requires solock() to be held. As a part of
> > socket buffers standalone locking work, move socket state bits which
> > represent its buffers state to per buffer state. Introduce `sb_state' and
> > turn SS_CANTSENDMORE to SBS_CANTSENDMORE. This bit will be processed on
> > `so_snd' buffer only.
> >
> > Move SS_CANTRCVMORE and SS_RCVATMARK bits with separate diff to make
> > review easier and exclude possible so_rcv/so_snd mistypes.
> >
> > Also, don't adjust the remaining SS_* bits right now.
> >
> > ok millert@
>
> Tree is broken due to the SS_CANTSENDMORE removal.
>
> Index: fstat.c
> ===
> RCS file: /cvs/src/usr.bin/fstat/fstat.c,v
> retrieving revision 1.103
> diff -u -p -r1.103 fstat.c
> --- fstat.c 20 Jun 2022 01:39:44 - 1.103
> +++ fstat.c 12 Dec 2022 06:30:21 -
> @@ -807,8 +807,6 @@ socktrans(struct kinfo_file *kf)
> if (!(kf->so_state & SS_CANTRCVMORE))
> *cp++ = '<';
> *cp++ = '-';
> - if (!(kf->so_state & SS_CANTSENDMORE))
> - *cp++ = '>';
> *cp = '\0';
> printf(" %s ", shoconn);
> hide((void *)(uintptr_t)kf->unp_conn);
>
Re: CVS: cvs.openbsd.org: src
On Sun, Dec 11, 2022 at 02:19:08PM -0700, Vitaliy Makkoveev wrote:
> CVSROOT: /cvs
> Module name: src
> Changes by: m...@cvs.openbsd.org2022/12/11 14:19:08
>
> Modified files:
> sys/kern : sys_socket.c uipc_socket.c uipc_socket2.c
>uipc_usrreq.c
> sys/miscfs/fifofs: fifo_vnops.c
> sys/netinet: tcp_usrreq.c
> sys/sys: socketvar.h
>
> Log message:
> This time, socket's buffer lock requires solock() to be held. As a part of
> socket buffers standalone locking work, move socket state bits which
> represent its buffers state to per buffer state. Introduce `sb_state' and
> turn SS_CANTSENDMORE to SBS_CANTSENDMORE. This bit will be processed on
> `so_snd' buffer only.
>
> Move SS_CANTRCVMORE and SS_RCVATMARK bits with separate diff to make
> review easier and exclude possible so_rcv/so_snd mistypes.
>
> Also, don't adjust the remaining SS_* bits right now.
>
> ok millert@
Tree is broken due to the SS_CANTSENDMORE removal.
Index: fstat.c
===
RCS file: /cvs/src/usr.bin/fstat/fstat.c,v
retrieving revision 1.103
diff -u -p -r1.103 fstat.c
--- fstat.c 20 Jun 2022 01:39:44 - 1.103
+++ fstat.c 12 Dec 2022 06:30:21 -
@@ -807,8 +807,6 @@ socktrans(struct kinfo_file *kf)
if (!(kf->so_state & SS_CANTRCVMORE))
*cp++ = '<';
*cp++ = '-';
- if (!(kf->so_state & SS_CANTSENDMORE))
- *cp++ = '>';
*cp = '\0';
printf(" %s ", shoconn);
hide((void *)(uintptr_t)kf->unp_conn);
Re: CVS: cvs.openbsd.org: src
On Fri, Dec 09, 2022 at 10:32:53AM -0700, Claudio Jeker wrote: > CVSROOT: /cvs > Module name: src > Changes by: clau...@cvs.openbsd.org 2022/12/09 10:32:53 > > Modified files: > sys/netinet6 : icmp6.c nd6.c nd6.h nd6_nbr.c nd6_rtr.c > > Log message: > Switch nd_opts from a union to just a struct. > The ND6 option handling in the kernel got a lot simpler since only > the tgt and src lladdr option are inspected by the kernel. The magic > of assigning options via one side of the union and accessing them > via the other is total overkill and actually quite error prone. > OK florian@ Also OK kn@ who actually pointed me at this madness by sending me some other nd6 cleanup diff. -- :wq Claudio
Re: CVS: cvs.openbsd.org: src
On 16 November 2022 19:30:12 CET, Florian Obser wrote: >CVSROOT: /cvs >Module name: src >Changes by:flor...@cvs.openbsd.org 2022/11/16 11:30:12 > >Modified files: > lib/libc/net : res_comp.c > >Log message: >tolower(3) guarantees to return its argument unchanged if it's not >uppercase. >While here use the correct idiom of casting to unsigned char. > >OK millert, farewell to ultrix deraadt > and ok guenther@ -- Sent from a mobile device. Please excuse poor formatting.
Re: CVS: cvs.openbsd.org: src
On Wed, Nov 09, 2022 at 07:06:43AM +0100, Stefan Sperling wrote:
> On Wed, Nov 09, 2022 at 06:58:32AM +0100, Anton Lindqvist wrote:
> > GENERIC is broken:
> >
> > vmm.c:900:3: error: implicit declaration of function 'x86_send_ipi' is
> > invalid in C99 [-Werror,-Wimplicit-function-declaration]
>
> ok stsp@
I have committed this to unbreak the tree.
Seems like everyone else is absent right now.
> > Index: vmm.c
> > ===
> > RCS file: /cvs/src/sys/arch/amd64/amd64/vmm.c,v
> > retrieving revision 1.329
> > diff -u -p -r1.329 vmm.c
> > --- vmm.c 8 Nov 2022 19:38:34 - 1.329
> > +++ vmm.c 9 Nov 2022 05:57:26 -
> > @@ -877,7 +877,9 @@ vm_intr_pending(struct vm_intr_params *v
> > {
> > struct vm *vm;
> > struct vcpu *vcpu;
> > +#ifdef MULTIPROCESSOR
> > struct cpu_info *ci;
> > +#endif
> > int error, ret = 0;
> >
> > /* Find the desired VM */
> > @@ -894,10 +896,12 @@ vm_intr_pending(struct vm_intr_params *v
> > goto out;
> > }
> >
> > +#ifdef MULTIPROCESSOR
> > vcpu->vc_intr = vip->vip_intr;
> > ci = READ_ONCE(vcpu->vc_curcpu);
> > if (ci != NULL)
> > x86_send_ipi(ci, X86_IPI_NOP);
> > +#endif
> >
> > refcnt_rele_wake(>vc_refcnt);
> > out:
> >
> >
>
>
Re: CVS: cvs.openbsd.org: src
On Wed, Nov 09, 2022 at 06:58:32AM +0100, Anton Lindqvist wrote:
> On Tue, Nov 08, 2022 at 12:18:47PM -0700, David Gwynne wrote:
> > CVSROOT:/cvs
> > Module name:src
> > Changes by: d...@cvs.openbsd.org2022/11/08 12:18:47
> >
> > Modified files:
> > sys/arch/amd64/include: vmmvar.h
> > sys/arch/amd64/amd64: vmm.c
> >
> > Log message:
> > further speed up delivery of interrupts to a running vcpu.
> >
> > this records which physical cpu a vcpu is running on. this is used
> > by the code that marks a vcpu as having a pending interrupt to check
> > if the vcpu is currently running. if it thinks the vcpu is running,
> > it sends a nop IPI to the physical cpu it is running on to trigger
> > a vmexit, which in turn runs interrupt handling in the guest.
> >
> > ok mlarkin@
>
> GENERIC is broken:
>
> vmm.c:900:3: error: implicit declaration of function 'x86_send_ipi' is
> invalid in C99 [-Werror,-Wimplicit-function-declaration]
ok stsp@
> Index: vmm.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/vmm.c,v
> retrieving revision 1.329
> diff -u -p -r1.329 vmm.c
> --- vmm.c 8 Nov 2022 19:38:34 - 1.329
> +++ vmm.c 9 Nov 2022 05:57:26 -
> @@ -877,7 +877,9 @@ vm_intr_pending(struct vm_intr_params *v
> {
> struct vm *vm;
> struct vcpu *vcpu;
> +#ifdef MULTIPROCESSOR
> struct cpu_info *ci;
> +#endif
> int error, ret = 0;
>
> /* Find the desired VM */
> @@ -894,10 +896,12 @@ vm_intr_pending(struct vm_intr_params *v
> goto out;
> }
>
> +#ifdef MULTIPROCESSOR
> vcpu->vc_intr = vip->vip_intr;
> ci = READ_ONCE(vcpu->vc_curcpu);
> if (ci != NULL)
> x86_send_ipi(ci, X86_IPI_NOP);
> +#endif
>
> refcnt_rele_wake(>vc_refcnt);
> out:
>
>
Re: CVS: cvs.openbsd.org: src
On Tue, Nov 08, 2022 at 12:18:47PM -0700, David Gwynne wrote:
> CVSROOT: /cvs
> Module name: src
> Changes by: d...@cvs.openbsd.org2022/11/08 12:18:47
>
> Modified files:
> sys/arch/amd64/include: vmmvar.h
> sys/arch/amd64/amd64: vmm.c
>
> Log message:
> further speed up delivery of interrupts to a running vcpu.
>
> this records which physical cpu a vcpu is running on. this is used
> by the code that marks a vcpu as having a pending interrupt to check
> if the vcpu is currently running. if it thinks the vcpu is running,
> it sends a nop IPI to the physical cpu it is running on to trigger
> a vmexit, which in turn runs interrupt handling in the guest.
>
> ok mlarkin@
GENERIC is broken:
vmm.c:900:3: error: implicit declaration of function 'x86_send_ipi' is
invalid in C99 [-Werror,-Wimplicit-function-declaration]
Index: vmm.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/vmm.c,v
retrieving revision 1.329
diff -u -p -r1.329 vmm.c
--- vmm.c 8 Nov 2022 19:38:34 - 1.329
+++ vmm.c 9 Nov 2022 05:57:26 -
@@ -877,7 +877,9 @@ vm_intr_pending(struct vm_intr_params *v
{
struct vm *vm;
struct vcpu *vcpu;
+#ifdef MULTIPROCESSOR
struct cpu_info *ci;
+#endif
int error, ret = 0;
/* Find the desired VM */
@@ -894,10 +896,12 @@ vm_intr_pending(struct vm_intr_params *v
goto out;
}
+#ifdef MULTIPROCESSOR
vcpu->vc_intr = vip->vip_intr;
ci = READ_ONCE(vcpu->vc_curcpu);
if (ci != NULL)
x86_send_ipi(ci, X86_IPI_NOP);
+#endif
refcnt_rele_wake(>vc_refcnt);
out:
Re: CVS: cvs.openbsd.org: src
Oops, unintented commit of the new ld.script They will not be used until Makefile.inc references them, and are still undergoing test. Theo de Raadt wrote: > CVSROOT: /cvs > Module name: src > Changes by: dera...@cvs.openbsd.org 2022/11/08 06:47:22 > > Modified files: > libexec/ld.so : loader.c > Added files: > libexec/ld.so/alpha: ld.script > libexec/ld.so/hppa: ld.script > libexec/ld.so/i386: ld.script > > Log message: > In the new scheme, the main executable object needs to be marked > nodelete, so that _dl_relro() will immutable it's relro. >
Re: CVS: cvs.openbsd.org: src
On Thu, Nov 03, 2022 at 11:41:22AM -0600, Patrick Wildt wrote: > CVSROOT: /cvs > Module name: src > Changes by: patr...@cvs.openbsd.org 2022/11/03 11:41:22 > > Modified files: > sys/arch/arm64/conf: RAMDISK > > Log message: > Enable smbios0 on arm64 RAMDISK so the code mitigating crashes on the x13s > gets correct info from hw.version. > That commit was: ok kettenis@ phessler@
Re: CVS: cvs.openbsd.org: src
On Tue, Nov 01, 2022 at 02:26:20PM -0600, Theo Buehler wrote: > CVSROOT: /cvs > Module name: src > Changes by: t...@cvs.openbsd.org2022/11/01 14:26:20 > > Modified files: > gnu/lib/libreadline: rlstdc.h > > Log message: > Do not neuter __attribute__ with __STRICT_ANSI__ > > This broke readline support in newer Pythons and generally seems a > bad idea. Upstream have removed this conditional in 5.0. > > ok millert > Also ok daniel jca
