2017-08-31 08:21:07

by Stephen Rothwell

[permalink] [raw]
Subject: linux-next: build failure after merge of the akpm-current tree

Hi Andrew,

After merging the akpm-current tree, today's linux-next build (arm
multi_v7_defconfig) failed like this:

In file included from /home/sfr/next/next/include/uapi/linux/uuid.h:21:0,
from /home/sfr/next/next/include/linux/uuid.h:19,
from /home/sfr/next/next/include/linux/mod_devicetable.h:12,
from /home/sfr/next/next/scripts/mod/devicetable-offsets.c:2:
/home/sfr/next/next/include/linux/string.h: In function 'memcpy_and_pad':
/home/sfr/next/next/include/linux/string.h:450:3: error: implicit declaration of function 'fortify_panic' [-Werror=implicit-function-declaration]
fortify_panic(__func__);
^

Caused by commit

9b04e51112ba ("fortify: use WARN instead of BUG for now")

interacting with commit

01f33c336e2d ("string.h: add memcpy_and_pad()")

from the block tree.

I have applied the following merge fix patch:

From: Stephen Rothwell <[email protected]>
Date: Thu, 31 Aug 2017 18:13:43 +1000
Subject: [PATCH] fortify: use WARN instead of BUG for now fix

Signed-off-by: Stephen Rothwell <[email protected]>
---
include/linux/string.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/string.h b/include/linux/string.h
index edd2b6154b80..e3b713114732 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -447,7 +447,7 @@ __FORTIFY_INLINE void memcpy_and_pad(void *dest, size_t dest_len,
__read_overflow3();
}
if (dest_size < dest_len)
- fortify_panic(__func__);
+ fortify_overflow(__func__);
if (dest_len > count) {
memcpy(dest, src, count);
memset(dest + count, pad, dest_len - count);
--
2.13.2

--
Cheers,
Stephen Rothwell


2017-09-06 12:33:05

by Martin Wilck

[permalink] [raw]
Subject: Re: linux-next: build failure after merge of the akpm-current tree

Hello Stephen,

On Thu, 2017-08-31 at 18:21 +1000, Stephen Rothwell wrote:
> Hi Andrew,
>
> After merging the akpm-current tree, today's linux-next build (arm
> multi_v7_defconfig) failed like this:
>
> In file included from
> /home/sfr/next/next/include/uapi/linux/uuid.h:21:0,
> from /home/sfr/next/next/include/linux/uuid.h:19,
> from
> /home/sfr/next/next/include/linux/mod_devicetable.h:12,
> from /home/sfr/next/next/scripts/mod/devicetable-
> offsets.c:2:
> /home/sfr/next/next/include/linux/string.h: In function
> 'memcpy_and_pad':
> /home/sfr/next/next/include/linux/string.h:450:3: error: implicit
> declaration of function 'fortify_panic' [-Werror=implicit-function-
> declaration]
> fortify_panic(__func__);
> ^
>
> Caused by commit
>
> 9b04e51112ba ("fortify: use WARN instead of BUG for now")
>
> interacting with commit
>
> 01f33c336e2d ("string.h: add memcpy_and_pad()")
>
> from the block tree.
>
> I have applied the following merge fix patch:
>
> From: Stephen Rothwell <[email protected]>
> Date: Thu, 31 Aug 2017 18:13:43 +1000
> Subject: [PATCH] fortify: use WARN instead of BUG for now fix
>
> Signed-off-by: Stephen Rothwell <[email protected]>
> ---
> include/linux/string.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/string.h b/include/linux/string.h
> index edd2b6154b80..e3b713114732 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -447,7 +447,7 @@ __FORTIFY_INLINE void memcpy_and_pad(void *dest,
> size_t dest_len,
> __read_overflow3();
> }
> if (dest_size < dest_len)
> - fortify_panic(__func__);
> + fortify_overflow(__func__);
> if (dest_len > count) {
> memcpy(dest, src, count);
> memset(dest + count, pad, dest_len - count);
> --
> 2.13.2

Arnd Bergmann spotted another problem with that patch. I decided to rip
out the "fortify" related code. I'll send a patch in a follow-up email.

Regards,
Martin

--
Dr. Martin Wilck <[email protected]>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

2017-09-06 12:37:55

by Martin Wilck

[permalink] [raw]
Subject: [PATCH] string.h: un-fortify memcpy_and_pad

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 <[email protected]>
Acked-by: Arnd Bergmann <[email protected]>

---
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 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);
--
2.14.0

2017-09-11 09:44:43

by Martin Wilck

[permalink] [raw]
Subject: Re: [PATCH] string.h: un-fortify memcpy_and_pad

On Wed, 2017-09-06 at 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 <[email protected]>
> Acked-by: Arnd Bergmann <[email protected]>
>
> ---
> 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);

2017-09-11 11:57:28

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH] string.h: un-fortify memcpy_and_pad

> 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.