Re: [PATCH v2] arch: s390: appldata: using strncpy() and strnlen() instead of sprintf()

[walter harms <wharms@bfs.de>]

Am 27.05.2013 11:55, schrieb Chen Gang:
> 
> 'buf[2]' is 2 bytes length, and sprintf() will append '\0' at the end
> of string "?\n", so original implementation is memory overflow.
> 
> Need use strncpy() and strnlen() instead of sprintf().
> 
> 
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
>  arch/s390/appldata/appldata_base.c |    7 +++++--
>  1 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/s390/appldata/appldata_base.c b/arch/s390/appldata/appldata_base.c
> index bae0f40..87a2209 100644
> --- a/arch/s390/appldata/appldata_base.c
> +++ b/arch/s390/appldata/appldata_base.c
> @@ -212,7 +212,9 @@ appldata_timer_handler(ctl_table *ctl, int write,
>  		return 0;
>  	}
>  	if (!write) {
> -		len = sprintf(buf, appldata_timer_active ? "1\n" : "0\n");
> +		strncpy(buf, appldata_timer_active ? "1\n" : "0\n",
> +			ARRAY_SIZE(buf));

an other way would be

buf[]="0\n";
if (appldata_timer_active) buf[0]='1';


> +		len = strnlen(buf, ARRAY_SIZE(buf));

can len ever change ?


re,
 wh



>  		if (len > *lenp)
>  			len = *lenp;
>  		if (copy_to_user(buffer, buf, len))
> @@ -317,7 +319,8 @@ appldata_generic_handler(ctl_table *ctl, int write,
>  		return 0;
>  	}
>  	if (!write) {
> -		len = sprintf(buf, ops->active ? "1\n" : "0\n");
> +		strncpy(buf, ops->active ? "1\n" : "0\n", ARRAY_SIZE(buf));
> +		len = strnlen(buf, ARRAY_SIZE(buf));
>  		if (len > *lenp)
>  			len = *lenp;
>  		if (copy_to_user(buffer, buf, len)) {