[PATCH] string.h: un-fortify memcpy_and_pad

[PATCH] string.h: un-fortify memcpy_and_pad

[Martin Wilck]

The way I'd implemented the new helper memcpy_and_pad  with
__FORTIFY_INLINE caused compiler warnings for certain kernel
configurations.

This helper is only used in a single place at this time, and thus
doesn't benefit much from fortification. So simplify the code
by dropping fortification support for now.

Fixes: 3c5fa8cd18f8 "string.h: add memcpy_and_pad()"
Signed-off-by: Martin Wilck <mwilck at suse.com>
---
 include/linux/string.h | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/include/linux/string.h b/include/linux/string.h
index 0bec4151b0eb9..0495cd3c81689 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -404,20 +404,9 @@ __FORTIFY_INLINE char *strcpy(char *p, const char *q)
  * @count: The number of bytes to copy
  * @pad: Character to use for padding if space is left in destination.
  */
-__FORTIFY_INLINE void memcpy_and_pad(void *dest, size_t dest_len,
-				     const void *src, size_t count, int pad)
+static inline void memcpy_and_pad(void *dest, size_t dest_len,
+				  const void *src, size_t count, int pad)
 {
-	size_t dest_size = __builtin_object_size(dest, 0);
-	size_t src_size = __builtin_object_size(src, 0);
-
-	if (__builtin_constant_p(dest_len) && __builtin_constant_p(count)) {
-		if (dest_size < dest_len && dest_size < count)
-			__write_overflow();
-		else if (src_size < dest_len && src_size < count)
-			__read_overflow3();
-	}
-	if (dest_size < dest_len)
-		fortify_panic(__func__);
 	if (dest_len > count) {
 		memcpy(dest, src, count);
 		memset(dest + count, pad,  dest_len - count);
-- 
2.14.0

[PATCH] string.h: un-fortify memcpy_and_pad

[Arnd Bergmann]

On Tue, Sep 5, 2017@8:23 PM, Martin Wilck <mwilck@suse.com> wrote:
> The way I'd implemented the new helper memcpy_and_pad  with
> __FORTIFY_INLINE caused compiler warnings for certain kernel
> configurations.
>
> This helper is only used in a single place at this time, and thus
> doesn't benefit much from fortification. So simplify the code
> by dropping fortification support for now.
>
> Fixes: 3c5fa8cd18f8 "string.h: add memcpy_and_pad()"
> Signed-off-by: Martin Wilck <mwilck at suse.com>

Looks good to me,

Acked-by: Arnd Bergmann <arnd at arndb.de>

I've added this to my randconfig testing tree, if you don't hear anything
from me by tomorrow, you can assume that it caused no other failures.

      Arnd

[PATCH] string.h: un-fortify memcpy_and_pad

[Arnd Bergmann]

On Tue, Sep 5, 2017@9:26 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tue, Sep 5, 2017@8:23 PM, Martin Wilck <mwilck@suse.com> wrote:
>> The way I'd implemented the new helper memcpy_and_pad  with
>> __FORTIFY_INLINE caused compiler warnings for certain kernel
>> configurations.
>>
>> This helper is only used in a single place at this time, and thus
>> doesn't benefit much from fortification. So simplify the code
>> by dropping fortification support for now.
>>
>> Fixes: 3c5fa8cd18f8 "string.h: add memcpy_and_pad()"
>> Signed-off-by: Martin Wilck <mwilck at suse.com>
>
> Looks good to me,
>
> Acked-by: Arnd Bergmann <arnd at arndb.de>
>
> I've added this to my randconfig testing tree, if you don't hear anything
> from me by tomorrow, you can assume that it caused no other failures.

build-tested successfully.

Tested-by: Arnd Bergmann <arnd at arndb.de>

[PATCH] string.h: un-fortify memcpy_and_pad

[Martin Wilck]

On Wed, 2017-09-06@14:36 +0200, Martin Wilck wrote:
> The way I'd implemented the new helper memcpy_and_pad  with
> __FORTIFY_INLINE caused compiler warnings for certain kernel
> configurations.
> 
> This helper is only used in a single place at this time, and thus
> doesn't benefit much from fortification. So simplify the code
> by dropping fortification support for now.
> 
> Fixes: 01f33c336e2d "string.h: add memcpy_and_pad()"
> Signed-off-by: Martin Wilck <mwilck at suse.com>
> Acked-by: Arnd Bergmann <arnd at arndb.de>
> 
> ---
>  include/linux/string.h | 15 ++-------------
>  1 file changed, 2 insertions(+), 13 deletions(-)

Hello Stephen and Christoph,

my broken patch 01f33c336e2d is in Linus' tree and causing compiler
warnings there. Could you please take care that this fix is pulled in
on top of it? Or should I take another action myself?

Thanks,
Martin


> 
> diff --git a/include/linux/string.h b/include/linux/string.h
> index e1eeb0a8a9693..54d21783e18dd 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -434,20 +434,9 @@ __FORTIFY_INLINE char *strcpy(char *p, const
> char *q)
>   * @count: The number of bytes to copy
>   * @pad: Character to use for padding if space is left in
> destination.
>   */
> -__FORTIFY_INLINE void memcpy_and_pad(void *dest, size_t dest_len,
> -				     const void *src, size_t count,
> int pad)
> +static inline void memcpy_and_pad(void *dest, size_t dest_len,
> +				  const void *src, size_t count, int
> pad)
>  {
> -	size_t dest_size = __builtin_object_size(dest, 0);
> -	size_t src_size = __builtin_object_size(src, 0);
> -
> -	if (__builtin_constant_p(dest_len) &&
> __builtin_constant_p(count)) {
> -		if (dest_size < dest_len && dest_size < count)
> -			__write_overflow();
> -		else if (src_size < dest_len && src_size < count)
> -			__read_overflow3();
> -	}
> -	if (dest_size < dest_len)
> -		fortify_panic(__func__);
>  	if (dest_len > count) {
>  		memcpy(dest, src, count);
>  		memset(dest + count, pad,  dest_len - count);

[PATCH] string.h: un-fortify memcpy_and_pad

[Christoph Hellwig]

> Hello Stephen and Christoph,
> 
> my broken patch 01f33c336e2d is in Linus' tree and causing compiler
> warnings there. Could you please take care that this fix is pulled in
> on top of it? Or should I take another action myself?

string.h is a bit of a maintainers no mans land.  Given that the
problematic commit came in through the nvme tree I've applied your
patch and will sned it to Jens with the next batch of nvme updates
tonight.