[PATCH v2 0/3] various strscpy fixes

[PATCH v2 0/3] various strscpy fixes

[Chris Metcalf]

This patch series fixes up a couple of architecture issues where
strscpy wasn't configured correctly (missing on h8300, duplicating
local and asm-generic copies on powerpc and tile).

It also adds a use of zero_bytemask() to the final store for
strscpy to avoid writing uninitialized data to the destination.
However, to make this work we had to add support for zero_bytemask()
to the two architectures that didn't have it (alpha and tile),
because they were providing their own local copies, but didn't
provide the zero_bytemask() that was previously only required
when building with CONFIG_DCACHE_WORD_ACCESS.

The series can be pulled from:

  git://git.kernel.org/pub/scm/linux/kernel/git/cmetcalf/linux-tile.git strscpy

Chris Metcalf (3):
  word-at-a-time.h: fix some Kbuild files
  word-at-a-time.h: support zero_bytemask() on alpha and tile
  strscpy: zero any trailing garbage bytes in the destination

 arch/alpha/include/asm/word-at-a-time.h | 2 ++
 arch/h8300/include/asm/Kbuild           | 1 +
 arch/powerpc/include/asm/Kbuild         | 1 -
 arch/tile/include/asm/Kbuild            | 1 -
 arch/tile/include/asm/word-at-a-time.h  | 8 +++++++-
 lib/string.c                            | 3 ++-
 6 files changed, 12 insertions(+), 4 deletions(-)

-- 
2.1.2

[PATCH v2 1/3] word-at-a-time.h: fix some Kbuild files

[Chris Metcalf]

arch/tile added word-at-a-time.h after the patch that added generic-y
entries; the generic-y entry is now stale.

arch/h8300 is newer than the generic-y patch for word-at-a-time.h,
and needs a generic-y entry.

arch/powerpc seems to have gotten a generic-y entry by mistake in
the first patch; this change removes it.

Signed-off-by: Chris Metcalf <cmetcalf@ezchip.com>
---
 arch/h8300/include/asm/Kbuild   | 1 +
 arch/powerpc/include/asm/Kbuild | 1 -
 arch/tile/include/asm/Kbuild    | 1 -
 3 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/h8300/include/asm/Kbuild b/arch/h8300/include/asm/Kbuild
index 70e6ae1e7006..373cb23301e3 100644
--- a/arch/h8300/include/asm/Kbuild
+++ b/arch/h8300/include/asm/Kbuild
@@ -73,4 +73,5 @@ generic-y += uaccess.h
 generic-y += ucontext.h
 generic-y += unaligned.h
 generic-y += vga.h
+generic-y += word-at-a-time.h
 generic-y += xor.h
diff --git a/arch/powerpc/include/asm/Kbuild b/arch/powerpc/include/asm/Kbuild
index ac1662956e0c..ab9f4e0ed4cf 100644
--- a/arch/powerpc/include/asm/Kbuild
+++ b/arch/powerpc/include/asm/Kbuild
@@ -7,4 +7,3 @@ generic-y += mcs_spinlock.h
 generic-y += preempt.h
 generic-y += rwsem.h
 generic-y += vtime.h
-generic-y += word-at-a-time.h
diff --git a/arch/tile/include/asm/Kbuild b/arch/tile/include/asm/Kbuild
index 0b6cacaad933..ba35c41c71ff 100644
--- a/arch/tile/include/asm/Kbuild
+++ b/arch/tile/include/asm/Kbuild
@@ -40,5 +40,4 @@ generic-y += termbits.h
 generic-y += termios.h
 generic-y += trace_clock.h
 generic-y += types.h
-generic-y += word-at-a-time.h
 generic-y += xor.h
-- 
2.1.2

[PATCH v2 2/3] word-at-a-time.h: support zero_bytemask() on alpha and tile

[Chris Metcalf]

Both alpha and tile needed implementations of zero_bytemask.

The alpha version is untested.

Signed-off-by: Chris Metcalf <cmetcalf@ezchip.com>
---
 arch/alpha/include/asm/word-at-a-time.h | 2 ++
 arch/tile/include/asm/word-at-a-time.h  | 8 +++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/arch/alpha/include/asm/word-at-a-time.h b/arch/alpha/include/asm/word-at-a-time.h
index 6b340d0f1521..902e6ab00a06 100644
--- a/arch/alpha/include/asm/word-at-a-time.h
+++ b/arch/alpha/include/asm/word-at-a-time.h
@@ -52,4 +52,6 @@ static inline unsigned long find_zero(unsigned long bits)
 #endif
 }
 
+#define zero_bytemask(mask) ((2ul << (find_zero(mask) * 8)) - 1)
+
 #endif /* _ASM_WORD_AT_A_TIME_H */
diff --git a/arch/tile/include/asm/word-at-a-time.h b/arch/tile/include/asm/word-at-a-time.h
index 9e5ce0d7b292..b66a693c2c34 100644
--- a/arch/tile/include/asm/word-at-a-time.h
+++ b/arch/tile/include/asm/word-at-a-time.h
@@ -6,7 +6,7 @@
 struct word_at_a_time { /* unused */ };
 #define WORD_AT_A_TIME_CONSTANTS {}
 
-/* Generate 0x01 byte values for non-zero bytes using a SIMD instruction. */
+/* Generate 0x01 byte values for zero bytes using a SIMD instruction. */
 static inline unsigned long has_zero(unsigned long val, unsigned long *data,
 				     const struct word_at_a_time *c)
 {
@@ -33,4 +33,10 @@ static inline long find_zero(unsigned long mask)
 #endif
 }
 
+#ifdef __BIG_ENDIAN
+#define zero_bytemask(mask) (~1ul << (63 - __builtin_clzl(mask)))
+#else
+#define zero_bytemask(mask) ((2ul << __builtin_ctzl(mask)) - 1)
+#endif
+
 #endif /* _ASM_WORD_AT_A_TIME_H */
-- 
2.1.2

Re: [PATCH v2 2/3] word-at-a-time.h: support zero_bytemask() on alpha and tile

[Ingo Molnar]

* Chris Metcalf <cmetcalf@ezchip.com> wrote:

> Both alpha and tile needed implementations of zero_bytemask.
> 
> The alpha version is untested.
> 
> Signed-off-by: Chris Metcalf <cmetcalf@ezchip.com>
> ---
>  arch/alpha/include/asm/word-at-a-time.h | 2 ++
>  arch/tile/include/asm/word-at-a-time.h  | 8 +++++++-
>  2 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/alpha/include/asm/word-at-a-time.h b/arch/alpha/include/asm/word-at-a-time.h
> index 6b340d0f1521..902e6ab00a06 100644
> --- a/arch/alpha/include/asm/word-at-a-time.h
> +++ b/arch/alpha/include/asm/word-at-a-time.h
> @@ -52,4 +52,6 @@ static inline unsigned long find_zero(unsigned long bits)
>  #endif
>  }
>  
> +#define zero_bytemask(mask) ((2ul << (find_zero(mask) * 8)) - 1)

Small nit: please use a proper C inline function instead of CPP, as for example 
the PowerPC zero_bytemask() function is.

> -/* Generate 0x01 byte values for non-zero bytes using a SIMD instruction. */
> +/* Generate 0x01 byte values for zero bytes using a SIMD instruction. */
>  static inline unsigned long has_zero(unsigned long val, unsigned long *data,
>  				     const struct word_at_a_time *c)
>  {
> @@ -33,4 +33,10 @@ static inline long find_zero(unsigned long mask)
>  #endif
>  }
>  
> +#ifdef __BIG_ENDIAN
> +#define zero_bytemask(mask) (~1ul << (63 - __builtin_clzl(mask)))
> +#else
> +#define zero_bytemask(mask) ((2ul << __builtin_ctzl(mask)) - 1)
> +#endif

Ditto.

Other than that, for the whole series:

  Reviewed-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo

Re: [PATCH v2 2/3] word-at-a-time.h: support zero_bytemask() on alpha and tile

[Chris Metcalf]

On 10/07/2015 01:53 AM, Ingo Molnar wrote:
> * Chris Metcalf <cmetcalf@ezchip.com> wrote:
>
>> Both alpha and tile needed implementations of zero_bytemask.
>>
>> The alpha version is untested.
>>
>> Signed-off-by: Chris Metcalf <cmetcalf@ezchip.com>
>> ---
>>   arch/alpha/include/asm/word-at-a-time.h | 2 ++
>>   arch/tile/include/asm/word-at-a-time.h  | 8 +++++++-
>>   2 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/alpha/include/asm/word-at-a-time.h b/arch/alpha/include/asm/word-at-a-time.h
>> index 6b340d0f1521..902e6ab00a06 100644
>> --- a/arch/alpha/include/asm/word-at-a-time.h
>> +++ b/arch/alpha/include/asm/word-at-a-time.h
>> @@ -52,4 +52,6 @@ static inline unsigned long find_zero(unsigned long bits)
>>   #endif
>>   }
>>   
>> +#define zero_bytemask(mask) ((2ul << (find_zero(mask) * 8)) - 1)
> Small nit: please use a proper C inline function instead of CPP, as for example
> the PowerPC zero_bytemask() function is.

Indeed, I normally would do that.  But, I was trying to match the
existing style; there are nine definitions that are specified as macros
(including the one in asm-generic), and only one (powerpc 64-bit LE)
that was specified as an inline.

I did put together a v3 of the patch series that included all your
excellent suggestions, including the Reported-by, Tested-by, and
Reviewed-by tags, but then found out v2 was already pulled into
Linus's tree, so I guess that ship has sailed; oh well.

My inclination would be not to churn the tree with a patch to
convert those new zero_bytemask() macros to inlines, but I'm happy
to do so if you think it would be better.

-- 
Chris Metcalf, EZChip Semiconductor
http://www.ezchip.com

[PATCH v2 3/3] strscpy: zero any trailing garbage bytes in the destination

[Chris Metcalf]

It's possible that the destination can be shadowed in userspace
(as, for example, the perf buffers are now).  So we should take
care not to leak data that could be inspected by userspace.

Signed-off-by: Chris Metcalf <cmetcalf@ezchip.com>
---
 lib/string.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/string.c b/lib/string.c
index 8dbb7b1eab50..84775ba873b9 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -203,12 +203,13 @@ ssize_t strscpy(char *dest, const char *src, size_t count)
 		unsigned long c, data;
 
 		c = *(unsigned long *)(src+res);
-		*(unsigned long *)(dest+res) = c;
 		if (has_zero(c, &data, &constants)) {
 			data = prep_zero_mask(c, data, &constants);
 			data = create_zero_mask(data);
+			*(unsigned long *)(dest+res) = c & zero_bytemask(data);
 			return res + find_zero(data);
 		}
+		*(unsigned long *)(dest+res) = c;
 		res += sizeof(unsigned long);
 		count -= sizeof(unsigned long);
 		max -= sizeof(unsigned long);
-- 
2.1.2

Re: [PATCH v2 3/3] strscpy: zero any trailing garbage bytes in the destination

[Ingo Molnar]

* Chris Metcalf <cmetcalf@ezchip.com> wrote:

> It's possible that the destination can be shadowed in userspace
> (as, for example, the perf buffers are now).  So we should take
> care not to leak data that could be inspected by userspace.
> 
> Signed-off-by: Chris Metcalf <cmetcalf@ezchip.com>

Please always preserve credits for who found the bug:

  Reported-by: Alexey Dobriyan <adobriyan@gmail.com>

Also, you should credit the kbuild-bot for the cross-arch build breakages that 
your other patches address:

  Reported-by: kbuild test robot <fengguang.wu@intel.com>

Thanks,

	Ingo