Re: [PATCH v2] params: bound array element output to the caller's page buffer

[Petr Pavlu <petr.pavlu@suse.com>]

On 5/21/26 4:28 AM, Pengpeng Hou wrote:
> Hi Petr,
> 
> You're right, that changelog bullet was misleading.
> 
> v1 already broke out of the loop once off reached PAGE_SIZE - 1, so it
> would not enter another iteration with no remaining byte in the caller's
> page buffer.
> 
> The v2 change was narrower: after the element getter returns, it clamps
> the number of bytes to copy and only rewrites the previous '\n' separator
> when that clamped length is non-zero. That avoids turning the previous
> separator into ',' when the next element contributes no visible bytes
> after clamping, or if a getter returns 0.

The updated code in v2 looks as follows:

	for (i = off = 0; i < (arr->num ? *arr->num : arr->max); i++) {
		p.arg = arr->elem + arr->elemsize * i;
		check_kparam_locked(p.mod);
		ret = arr->ops->get(elem_buf, &p);
		if (ret < 0)
			goto out;
		ret = min(ret, (int)(PAGE_SIZE - 1 - off));
		if (!ret)
			break;
		/* Replace the previous element's trailing newline with a comma. */
		if (i)
			buffer[off - 1] = ',';
		memcpy(buffer + off, elem_buf, ret);
		off += ret;
		if (off == PAGE_SIZE - 1)
			break;
	}

The clamping is done by:

ret = min(ret, (int)(PAGE_SIZE - 1 - off));

My understanding is that the expression '(int)(PAGE_SIZE - 1 - off)'
cannot return 0 because otherwise the loop would have already broken out
in the previous iteration due to the final check
'if (off == PAGE_SIZE - 1)'.

The input ret value to the min() calculation comes from the
arr->ops->get() call. The kernel_param_ops::get() API requires the
resulting string to be terminated by '\n', so on success the call should
never return 0. Even if it does and we want to make param_array_get()
tighter, I believe it should be treated as an error rather than silently
returning success from this function.

-- 
Thanks,
Petr