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

listen does not allow specifying a negative backlog #2264

Closed
asomers opened this issue Dec 14, 2023 · 4 comments · Fixed by #2276
Closed

listen does not allow specifying a negative backlog #2264

asomers opened this issue Dec 14, 2023 · 4 comments · Fixed by #2276
Labels

Comments

@asomers
Copy link
Member

asomers commented Dec 14, 2023

The listen function, in C, takes a "backlog" argument as an int. It normally must be positive. However, it may be ngative:

  • On FreeBSD, since version 2.2, to use the maximum value
  • On Linux, as an undocumented feature, to use a default value, at least according to @devnexen .

But when listen was added to Nix in change f85e9bf , the backlog argument's type was specified as usize, which precludes negative values. It also will rollover at i32::MAX_VALUE with unpredictable consequences.

@asomers asomers added the A-bug label Dec 14, 2023
@asomers
Copy link
Member Author

asomers commented Dec 14, 2023

See rust-lang/rust#97963 for a discussion of the behavior on Linux.

@SteveLauC
Copy link
Member

So we know that on FreeBSD, a negative backlog or a value bigger than the max one means the default value. And from that PR, Linux will see -1 as the default value, what about other negative values, will they also be seen the default value?

Maybe we should use a wrapper type, just like the one introduced in #1876

@SteveLauC
Copy link
Member

Ok, from the Linux source code, they directly cast the backlog argument from int to unsigned int, for a somaxconn of value 4096, all the negative values will be bigger than it and then set to it:

This behavior depends on the value of somaxconn, so it is different from the FreeBSD one, we should probably not use other negative value

int __sys_listen(int fd, int backlog)
{
	struct socket *sock;
	int err, fput_needed;
	int somaxconn;

	sock = sockfd_lookup_light(fd, &err, &fput_needed);
	if (sock) {
		somaxconn = READ_ONCE(sock_net(sock->sk)->core.sysctl_somaxconn);
		if ((unsigned int)backlog > somaxconn)
			backlog = somaxconn;

		err = security_socket_listen(sock, backlog);
		if (!err)
			err = READ_ONCE(sock->ops)->listen(sock, backlog);

		fput_light(sock->file, fput_needed);
	}
	return err;
}

devnexen added a commit to devnexen/nix that referenced this issue Dec 28, 2023
changing the sys::socket::listen backlog type from `usize` to
a `i32` wrapper, offering known sane values, from -1, SOMAXCONN to
 511.

close nix-rustgh-2264
devnexen added a commit to devnexen/nix that referenced this issue Dec 28, 2023
changing the sys::socket::listen backlog type from `usize` to
a `i32` wrapper, offering known sane values, from -1, SOMAXCONN to
 511.

close nix-rustgh-2264
devnexen added a commit to devnexen/nix that referenced this issue Dec 28, 2023
changing the sys::socket::listen backlog type from `usize` to
a `i32` wrapper, offering known sane values, from -1, SOMAXCONN to
 511.

close nix-rustgh-2264
devnexen added a commit to devnexen/nix that referenced this issue Dec 28, 2023
changing the sys::socket::listen backlog type from `usize` to
a `i32` wrapper, offering known sane values, from -1, SOMAXCONN to
 511.

close nix-rustgh-2264
devnexen added a commit to devnexen/nix that referenced this issue Dec 28, 2023
changing the sys::socket::listen backlog type from `usize` to
a `i32` wrapper, offering known sane values, from -1, SOMAXCONN to
 511.

close nix-rustgh-2264
devnexen added a commit to devnexen/nix that referenced this issue Dec 28, 2023
changing the sys::socket::listen backlog type from `usize` to
a `i32` wrapper, offering known sane values, from -1, SOMAXCONN to
 511.

close nix-rustgh-2264
devnexen added a commit to devnexen/nix that referenced this issue Dec 28, 2023
changing the sys::socket::listen backlog type from `usize` to
a `i32` wrapper, offering known sane values, from -1, SOMAXCONN to
 511.

close nix-rustgh-2264
devnexen added a commit to devnexen/nix that referenced this issue Dec 29, 2023
changing the sys::socket::listen backlog type from `usize` to
a `i32` wrapper, offering known sane values, from -1, SOMAXCONN to
 511.

close nix-rustgh-2264
devnexen added a commit to devnexen/nix that referenced this issue Dec 29, 2023
changing the sys::socket::listen backlog type from `usize` to
a `i32` wrapper, offering known sane values, from -1, SOMAXCONN to
 511.

close nix-rustgh-2264
devnexen added a commit to devnexen/nix that referenced this issue Dec 29, 2023
changing the sys::socket::listen backlog type from `usize` to
a `i32` wrapper, offering known sane values, from -1, SOMAXCONN to
 511.

close nix-rustgh-2264
devnexen added a commit to devnexen/nix that referenced this issue Dec 29, 2023
changing the sys::socket::listen backlog type from `usize` to
a `i32` wrapper, offering known sane values, from -1, SOMAXCONN to
 511.

close nix-rustgh-2264
devnexen added a commit to devnexen/nix that referenced this issue Dec 30, 2023
changing the sys::socket::listen backlog type from `usize` to
a `i32` wrapper, offering known sane values, from -1, SOMAXCONN to
 511.

close nix-rustgh-2264
devnexen added a commit to devnexen/nix that referenced this issue Dec 30, 2023
changing the sys::socket::listen backlog type from `usize` to
a `i32` wrapper, offering known sane values, from -1, SOMAXCONN to
 511.

close nix-rustgh-2264
devnexen added a commit to devnexen/nix that referenced this issue Dec 30, 2023
changing the sys::socket::listen backlog type from `usize` to
a `i32` wrapper, offering known sane values, from -1/0, to SOMAXCONN.

close nix-rustgh-2264
devnexen added a commit to devnexen/nix that referenced this issue Dec 30, 2023
changing the sys::socket::listen backlog type from `usize` to
a `i32` wrapper, offering known sane values, from -1/0, to SOMAXCONN.

close nix-rustgh-2264
github-merge-queue bot pushed a commit that referenced this issue Dec 30, 2023
changing the sys::socket::listen backlog type from `usize` to
a `i32` wrapper, offering known sane values, from -1/0, to SOMAXCONN.

close gh-2264
@devnexen
Copy link
Contributor

devnexen commented Jan 5, 2024

Note: openbsd could eventually be added in some months or later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants