Re: [PATCH] lib/strscpy: remove word-at-a-time optimization.

From: Linus Torvalds
Date: Thu Jan 25 2018 - 12:55:07 EST


On Thu, Jan 25, 2018 at 12:32 AM, Dmitry Vyukov <dvyukov@xxxxxxxxxx> wrote:
> On Wed, Jan 24, 2018 at 6:52 PM, Linus Torvalds
> <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>>
>> So I'd *much* rather have some way to tell KASAN that word-at-a-time
>> is going on. Because that approach definitely makes a difference in
>> other places.
>
> The other option was to use READ_ONCE_NOCHECK().

How about just using the same accessor that we do for the dcache case.
That gives a reasonable example of the whole word-at-a-time model, and
should be good.

COMPLETELY UNTESTED patch attached. This needs to be verified.

It does limit the word-at-a-time code to the architectures that select
both HAVE_EFFICIENT_UNALIGNED_ACCESS and DCACHE_WORD_ACCESS, but that
seems a reasonable choice anyway.

Linus
lib/string.c | 28 +++++++---------------------
1 file changed, 7 insertions(+), 21 deletions(-)

diff --git a/lib/string.c b/lib/string.c
index 64a9e33f1daa..25a3c200307e 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -177,33 +177,18 @@ EXPORT_SYMBOL(strlcpy);
*/
ssize_t strscpy(char *dest, const char *src, size_t count)
{
- const struct word_at_a_time constants = WORD_AT_A_TIME_CONSTANTS;
- size_t max = count;
long res = 0;

if (count == 0)
return -E2BIG;

-#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
- /*
- * If src is unaligned, don't cross a page boundary,
- * since we don't know if the next page is mapped.
- */
- if ((long)src & (sizeof(long) - 1)) {
- size_t limit = PAGE_SIZE - ((long)src & (PAGE_SIZE - 1));
- if (limit < max)
- max = limit;
- }
-#else
- /* If src or dest is unaligned, don't do word-at-a-time. */
- if (((long) dest | (long) src) & (sizeof(long) - 1))
- max = 0;
-#endif
-
- while (max >= sizeof(unsigned long)) {
+#if CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS && CONFIG_DCACHE_WORD_ACCESS
+{
+ const struct word_at_a_time constants = WORD_AT_A_TIME_CONSTANTS;
+ while (count >= sizeof(unsigned long)) {
unsigned long c, data;

- c = *(unsigned long *)(src+res);
+ c = load_unaligned_zeropad(src+res);
if (has_zero(c, &data, &constants)) {
data = prep_zero_mask(c, data, &constants);
data = create_zero_mask(data);
@@ -213,8 +198,9 @@ ssize_t strscpy(char *dest, const char *src, size_t count)
*(unsigned long *)(dest+res) = c;
res += sizeof(unsigned long);
count -= sizeof(unsigned long);
- max -= sizeof(unsigned long);
}
+}
+#endif

while (count) {
char c;