linux-kernel - Re: [PATCH 3v] char: hw_random: core.c: Changed from using strncat to strlcat

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 16 Oct 2014 10:25:03 -0700
From:	Joe Perches <joe@...ches.com>
To:	Rickard Strandqvist <rickard_strandqvist@...ctrumdigital.se>
Cc:	Matt Mackall <mpm@...enic.com>,
	Herbert Xu <herbert@...dor.apana.org.au>,
	Torsten Duwe <duwe@....de>, Theodore Ts'o <tytso@....edu>,
	Jason Cooper <jason@...edaemon.net>,
	Amit Shah <amit.shah@...hat.com>,
	Stephen Boyd <sboyd@...eaurora.org>,
	Paul Gortmaker <paul.gortmaker@...driver.com>,
	Kees Cook <keescook@...omium.org>,
	Dan Carpenter <dan.carpenter@...cle.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3v] char: hw_random: core.c:  Changed from using strncat
 to strlcat

On Thu, 2014-10-16 at 19:15 +0200, Rickard Strandqvist wrote:
> The buf is used to hold the list of hwrng devices registered.
> The old code ensures we don't walk off the end of buf as we
> fill it, but it's unnecessarily complicated and thus difficult
> to maintain. Simplify it by using strlcat.
> We also ensure the string within buf is NULL terminated
> so the final strlen is ok.
[]
> diff --git a/drivers/char/hw_random/core.c b/drivers/char/hw_random/core.c
[]
> @@ -289,16 +288,13 @@ static ssize_t hwrng_attr_available_show(struct device *dev,
>  		return -ERESTARTSYS;
>  	buf[0] = '\0';
>  	list_for_each_entry(rng, &rng_list, list) {
> -		strncat(buf, rng->name, PAGE_SIZE - ret - 1);
> -		ret += strlen(rng->name);
> -		strncat(buf, " ", PAGE_SIZE - ret - 1);
> -		ret++;
> +		strlcat(buf, rng->name, PAGE_SIZE);
> +		strlcat(buf, " ", PAGE_SIZE);
>  	}
> -	strncat(buf, "\n", PAGE_SIZE - ret - 1);
> -	ret++;
> +	strlcat(buf, "\n", PAGE_SIZE);
>  	mutex_unlock(&rng_mutex);
>  
> -	return ret;
> +	return strlen(buf);
>  }
>  
>  static DEVICE_ATTR(rng_current, S_IRUGO | S_IWUSR,

Rickard, can you please use some optimizations here
(and elsewhere) so that strlcat doesn't always have
to strlen the first string and the return doesn't
have to do the strlen too?

You could use a temporary for the returned length
of the strlcat so that if it's shorter than
the buffer, the next strlcat can start at the
appropriate known position instead of having
to do the initial strlen again and again.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/