Re: [PATCH v3 17/70] nstree: add listns()

[Arnd Bergmann]

On Fri, Oct 24, 2025, at 12:52, Christian Brauner wrote:
> Add a new listns() system call that allows userspace to iterate through
> namespaces in the system. This provides a programmatic interface to
> discover and inspect namespaces, enhancing existing namespace apis.

I double-checked that the ABI is well-formed and works the same
way on all supported architectures, though I did not check the functional
aspects.

Acked-by: Arnd Bergmann <arnd@xxxxxxxx>

One small thing I noticed:

> +SYSCALL_DEFINE4(listns, const struct ns_id_req __user *, req,
> +		u64 __user *, ns_ids, size_t, nr_ns_ids, unsigned int, flags)
> +{
> +	struct klistns klns __free(klistns_free) = {};
> +	const size_t maxcount = 1000000;
> +	struct ns_id_req kreq;
> +	ssize_t ret;
> +
> +	if (flags)
> +		return -EINVAL;
> +
> +	if (unlikely(nr_ns_ids > maxcount))
> +		return -EOVERFLOW;
> +
> +	if (!access_ok(ns_ids, nr_ns_ids * sizeof(*ns_ids)))
> +		return -EFAULT;

I'm a bit worried about hardcoding the maxcount value here, which
seems to limit both the size of the allocation and prevent overflowing
the multiplication of the access_ok() argument, though that isn't
completely clear from the implementation.

Allowing 8MB of vmalloc space to be filled can be bad on 32-bit
systems that may only have 100MB in total. The access_ok() check
looks like it tries to provide an early-fail error return but
should not actually be needed since there is a single copy_to_user()
in the end, and that is more likely to fail for unmapped memory than
an access_ok() failure.

Would it make sense to just drop the kvmalloc() completely and
instead put_user() the output values individually? That way you
can avoid both a hardwired limit and a potential DoS from vmalloc
exhaustion.

     Arnd