Re: [PATCH] params: Fix an overflow in param_attr_show

[Jean Delvare <jdelvare@suse.de>]

On Wed, 27 Sep 2017 10:10:31 +0200, Jean Delvare wrote:
> Function param_attr_show could overflow the buffer it is operating
> on. The buffer size is PAGE_SIZE, and the string returned by
> attribute->param->ops->get is generated by scnprintf(buffer,
> PAGE_SIZE, ...) so it could be PAGE_SIZE - 1 long, with the
> terminating '\0' at the very end of the buffer. Calling
> strcat(..., "\n") on this isn't safe, as the '\0' will be replaced
> by '\n' (OK) and then another '\0' will be added past the end of
> the buffer (not OK.)
> 
> Simply add the trailing '\n' when writing the attribute contents to
> the buffer originally. This is safe, and also faster.
> 
> Credits to Teradata for discovering this issue.
> 
> Signed-off-by: Jean Delvare <jdelvare@suse.de>
> ---
>  kernel/params.c |   22 +++++++++-------------
>  1 file changed, 9 insertions(+), 13 deletions(-)
> (...)

This patch turns out to be broken, so please disregard for now. I
missed several types, like bool, invbool and string (not sure why we
have types charp and string which seems to serve the same purpose...)
and most importantly arrays of parameters, which very much assume that
individual parameters are not \n-terminated. There had to be a reason
for the current code...

Sorry for the noise, I'll work some more on it and see how to address
all the remaining issues.

-- 
Jean Delvare
SUSE L3 Support