x/sys/unix: OpenBSD mmap syscall number (unix.SYS_MMAP) obsolete

[dusaint]

On 2021-12-23, a new syscall for mmap was introduced:

49  STD NOLOCK  { void *sys_mmap(void *addr, size_t len, int prot, \
                int flags, int fd, off_t pos); }

At the same time, the existing mmap syscall was renamed from sys_mmap to sys_pad_mmap (since it contains a now-obsolete argument for padding).

See sys/kern/syscalls.master:

https://github.com/openbsd/src/commit/1d60349d0b961891264d426ffe1c0ced24b2374c#diff-e8c6a075c6e1240e27216779540b66f3db43b14a3b3615c2ecb7a111faa54504

On 2023-02-11, the original mmap syscall was retired entirely:

https://github.com/openbsd/src/commit/8c7f5cc47d34f1bd83a08278dad544a34184dabf

With the release of OpenBSD 7.3 on 2023-04-10, any go program that uses mmap on OpenBSD will crash with SIGSYS.

For example, here's a snippet from kdump of a failing run of gotosocial:

 83839 gotosocial CALL  (via syscall) #197 (obsolete pad_mmap)()
 83839 gotosocial PSIG  SIGSYS caught handler=0x4682c0 mask=0<>
 83839 gotosocial RET   #197 (obsolete pad_mmap) -1 errno 78 Function not implemented

This is a bug both in this project and in modernc.org/libc. Here, the constant unix.SYS_MMAP needs to be changed from 197 to 49. I'll also file a bug in that project for them to switch both the number and how they're invoking syscall.

[ianlancetaylor]

CC @golang/openbsd

[landryb]

Fwiw, influxdb 1.8.10 on openbsd 7.3 had this issue (crashing with SIGSYS at startup, with a goroutine trace pointing at sys_mmap), and upgrading the x/sys module to 0.5.0 fixed it. cf https://marc.info/?l=openbsd-ports&m=168166545112152&w=2

so maybe it's already fixed 'somewhere' in the go ecosystem and projects need to update their dependencies ?

[sthen]

There are two related problems. One is that older versions of x/sys use syscall() itself, this has been replaced with calls to libc wrappers. The other is that the syscall tables are many (openbsd) releases out of date and in particular the old ones with "pad" arguments have recently been retired - https://github.com/golang/sys/commit/3086868dc2d50eaf352a77344894e7f099938c59 synced some tables but not syscalls. Fixing that will partly involve syncing the table but will also involve fixing the call parameters.

There is an additional twist in that openbsd is considering blocking syscall() completely. It has not yet been done though the linker now prints a warning when it's used. If that's done then syscall access will become impossible except through libc.

[dusaint]

FWIW, just calling unix.mmap works as expected -- the SIGSYS I saw came from another package's use of the unix.SYS_MMAP constant. I haven't tested the other changed syscalls, but wouldn't be surprised if they follow the same pattern.

[wilmhub]

As a side effect bootstrapping go by building Go 1.4 from source is no longer possible on OpenBSD 7.3. During the build of cmd/go go_bootstrap is crashing with SIGSYS, due to a mmap call.

[4a6f656c]

As a side effect bootstrapping go by building Go 1.4 from source is no longer possible on OpenBSD 7.3. During the build of cmd/go go_bootstrap is crashing with SIGSYS, due to a mmap call.

The minimum bootstrap for Go 1.20 is Go 1.17.3:

https://go.dev/doc/go1.20#bootstrap

As such, there is no effort being made to keep Go 1.4 working on OpenBSD.

[4a6f656c]

This is a bug both in this project and in modernc.org/libc. Here, the constant unix.SYS_MMAP needs to be changed from 197 to 49. I'll also file a bug in that project for them to switch both the number and how they're invoking syscall.

The short version is - I do not believe there is any way to fix this safely. It is not just the syscall number that changed, but also the arguments that are being passed to the syscall. This means that updating SYS_MMAP to the new value will break existing code on some platforms, and it would mean silently misbehaving code instead of a SIGSYS. This is why they're marked as deprecated:

https://github.com/golang/sys/blob/master/unix/zsysnum_openbsd_386.go#L9

Any code that wants to make system calls on OpenBSD should use the libc stubs directly, which the likes of unix.Mmap() does, avoiding the issue entirely. As also noted by @sthen, it is highly likely that the syscall(2) system call will be removed in future versions of OpenBSD - so even if the SYS_MMAP value was changed, it would only be kicking the can slightly further down the road.

One issue worth noting is that x/sys/unix.Mmap does not provide any way to pass in an address, which restricts some operations:

https://github.com/golang/go/issues/56123

If this is needed then calling mmap(2) via libc stubs would need to be handled outside of golang.org/x/sys/unix.

[dusaint]

This is a bug both in this project and in modernc.org/libc. Here, the constant unix.SYS_MMAP needs to be changed from 197 to 49. I'll also file a bug in that project for them to switch both the number and how they're invoking syscall.

The short version is - I do not believe there is any way to fix this safely. It is not just the syscall number that changed, but also the arguments that are being passed to the syscall. This means that updating SYS_MMAP to the new value will break existing code on some platforms, and it would mean silently misbehaving code instead of a SIGSYS. This is why they're marked as deprecated:

https://github.com/golang/sys/blob/master/unix/zsysnum_openbsd_386.go#L9

Any code that wants to make system calls on OpenBSD should use the libc stubs directly, which the likes of unix.Mmap() does, avoiding the issue entirely. As also noted by @sthen, it is highly likely that the syscall(2) system call will be removed in future versions of OpenBSD - so even if the SYS_MMAP value was changed, it would only be kicking the can slightly further down the road.

It would indeed be better for downstream packages to just call unix.MMap() or use some other libc stub. But that isn't really the concern here. That unix.SYS_MMAP is now an incorrect value is, and it's reasonable to fix that without worrying overmuch about how projects are going to use it. If source-level backward compatibility is a goal of this project, then perhaps name the updated constant something like unix.SYS_MMAP2?