Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

syscall: regression in Select() nil timeout parameter #24189

Closed
zhsj opened this issue Mar 1, 2018 · 4 comments
Closed

syscall: regression in Select() nil timeout parameter #24189

zhsj opened this issue Mar 1, 2018 · 4 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@zhsj
Copy link
Contributor

zhsj commented Mar 1, 2018

What version of Go are you using (go version)?

go1.10

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

linux, arm64 and mips64le

What did you do?

package main

import "syscall"

func main() {
        syscall.Select(0, nil, nil, nil, nil)
}

What did you expect to see?

No panic

What did you see instead?

panic: runtime error: invalid memory address or nil pointer dereference

However it's fine in 1.9 and on other architectures.

The releated commit is bf237f5

@bradfitz
Copy link
Contributor

bradfitz commented Mar 1, 2018

Indeed:

func Select(nfd int, r *FdSet, w *FdSet, e *FdSet, timeout *Timeval) (n int, err error) {
        ts := Timespec{Sec: timeout.Sec, Nsec: timeout.Usec * 1000}
        return pselect(nfd, r, w, e, &ts, nil)
}

Per https://golang.org/doc/go1compat#operating_systems , the syscall package is not under the Go 1 compatibility rules, so technically this isn't a regression or worthy of a Go 1.10.1 point release cherry pick.

I suppose we should fix it, though.

In the meantime, you probably want to migrate away from the syscall package and use golang.org/x/sys/unix instead.

/cc @tklauser @ianlancetaylor

@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label Mar 1, 2018
@bradfitz bradfitz added this to the Go1.11 milestone Mar 1, 2018
@tklauser
Copy link
Member

tklauser commented Mar 1, 2018

Oops, sorry about that. CL is on the way.

And FWIW, unix.Select from golang.org/x/sys/unix also suffers from the same problem on linux/{mips64x,arm64}, though you can use unix.Pselect there directly. I'll send a CL there as well.

@gopherbot
Copy link

Change https://golang.org/cl/97819 mentions this issue: syscall: fix nil pointer dereference in Select on linux/{arm64,mips64x}

@gopherbot
Copy link

Change https://golang.org/cl/97820 mentions this issue: unix: fix nil pointer dereference in Select on linux/{arm64,mips64x}

gopherbot pushed a commit to golang/sys that referenced this issue Mar 2, 2018
The timeout parameter might be nil, don't dereference it
unconditionally.

CL 97819 did the same for the syscall package.

Updates golang/go#24189

Change-Id: I95a93468c7d8431abf2e9a3a9b8d0fbd1a223e0d
Reviewed-on: https://go-review.googlesource.com/97820
Run-TryBot: Tobias Klauser <tobias.klauser@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@golang golang locked and limited conversation to collaborators Mar 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants