[RFC PATCH linux dev-5.4 0/4] Enable kprobes on AST2500 and later

[RFC PATCH linux dev-5.4 0/4] Enable kprobes on AST2500 and later

[Andrew Jeffery]

Hello,

kprobes are pretty handy, so turn them on. I'm expecting opinions, especially
with the first and last and middle patches, so the series is RFC.

Please opine!

Andrew

Andrew Jeffery (4):
  ARM: configs: aspeed-g5: Disable netfilter
  ARM: configs: aspeed-g5: Enable modules
  ARM: configs: aspeed-g5: Enable kprobes
  ARM: configs: aspeed-g5: Set CONFIG_FORTIFY_SOURCE=n to appease
    kprobes

 arch/arm/configs/aspeed_g5_defconfig | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

-- 
2.25.1

[RFC PATCH linux dev-5.4 1/4] ARM: configs: aspeed-g5: Disable netfilter

[Andrew Jeffery]

Enabling modules with CONFIG_NETFILTER=y and CONFIG_NETFILTER_ADVANCED=n
results in a diverse set of netfilter functions being built as modules.
These functions were not previously enabled in any way because modules
were disabled, therefore we can't have been using them. Given that we
aren't using them, disable netfilter as well to prevent them from being
built.

CONFIG_NETFILTER=y was introduced in 20c90af9ea6a ("ARM: config: aspeed:
Update defconfig") with little fanfare in the commit message, which
makes me wonder whether it was enabled because we needed it or whether
its addition slipped through the cracks.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 arch/arm/configs/aspeed_g5_defconfig | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig
index cdf30fbe613e..e124b4ddd442 100644
--- a/arch/arm/configs/aspeed_g5_defconfig
+++ b/arch/arm/configs/aspeed_g5_defconfig
@@ -69,8 +69,6 @@ CONFIG_IPV6_ROUTER_PREF=y
 CONFIG_IPV6_ROUTE_INFO=y
 CONFIG_IPV6_OPTIMISTIC_DAD=y
 CONFIG_IPV6_MULTIPLE_TABLES=y
-CONFIG_NETFILTER=y
-# CONFIG_NETFILTER_ADVANCED is not set
 CONFIG_VLAN_8021Q=y
 CONFIG_NET_NCSI=y
 # CONFIG_WIRELESS is not set
-- 
2.25.1

[RFC PATCH linux dev-5.4 2/4] ARM: configs: aspeed-g5: Enable modules

[Andrew Jeffery]

This is a step on the path to enabling kprobes, which depend on
CONFIG_MODULES=y.

Setting CONFIG_MODULES=y results in the following change in kernel size:

Before:
         $ size vmlinux
            text    data     bss     dec     hex filename
         8140500 2440582  178904 10759986         a42f32 vmlinux
         $ stat --printf '%s\n' arch/arm/boot/zImage
         3445200

After:
         $ size vmlinux
            text    data     bss     dec     hex filename
         8240349 2388862  172304 10801515         a4d16b vmlinux
         $ stat --printf '%s\n' arch/arm/boot/zImage
         3458368

This gives a 41,529 byte increase to vmlinux and a 13,168 byte increase
to the size of the compressed kernel image. In the most constrained BMC
flash layout (arch/arm/boot/dts/openbmc-flash-layout.dtsi) this change
decreases the free space in the kernel partition from 1,011,248 bytes to
998,080 bytes.

CONFIG_HW_RANDOM=y seems to come along for the ride with the
`./scripts/config -e modules && make olddefconfig && make
savedefconfig`. The measurements above include it, I haven't
investigated why it gets enabled or what effect disabling it might have
on the numbers.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 arch/arm/configs/aspeed_g5_defconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig
index e124b4ddd442..e30f27a413f4 100644
--- a/arch/arm/configs/aspeed_g5_defconfig
+++ b/arch/arm/configs/aspeed_g5_defconfig
@@ -46,6 +46,7 @@ CONFIG_NEON=y
 CONFIG_KERNEL_MODE_NEON=y
 CONFIG_FIRMWARE_MEMMAP=y
 CONFIG_JUMP_LABEL=y
+CONFIG_MODULES=y
 # CONFIG_BLK_DEV_BSG is not set
 # CONFIG_BLK_DEBUG_FS is not set
 # CONFIG_MQ_IOSCHED_DEADLINE is not set
@@ -152,6 +153,7 @@ CONFIG_SERIAL_8250_DW=y
 CONFIG_SERIAL_OF_PLATFORM=y
 CONFIG_ASPEED_KCS_IPMI_BMC=y
 CONFIG_ASPEED_BT_IPMI_BMC=y
+CONFIG_HW_RANDOM=y
 CONFIG_HW_RANDOM_TIMERIOMEM=y
 # CONFIG_I2C_COMPAT is not set
 CONFIG_I2C_CHARDEV=y
-- 
2.25.1

[RFC PATCH linux dev-5.4 3/4] ARM: configs: aspeed-g5: Enable kprobes

[Andrew Jeffery]

Enable kprobes to improve the debugging experience on the AST2500 and
later.

Setting CONFIG_KPROBES=y results in the following change in kernel size:

Before:
         $ size vmlinux
            text    data     bss     dec     hex filename
         8240349 2388862  172304 10801515         a4d16b vmlinux
         $ stat --printf '%s\n' *.zImage
         3458368

After:
         $ size vmlinux
            text    data     bss     dec     hex filename
         8279214 2399530  176912 10855656         a5a4e8 vmlinux
         $ stat --printf '%s\n' *.zImage
         3471880

This gives a 54,141 byte increase to vmlinux and a 13,512 byte increase
to the size of the compressed kernel image. In the most constrained BMC
flash layout (arch/arm/boot/dts/openbmc-flash-layout.dtsi) this change
decreases the free space in the kernel partition from 998,080 bytes to
984,568 bytes.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 arch/arm/configs/aspeed_g5_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig
index e30f27a413f4..254fb7562738 100644
--- a/arch/arm/configs/aspeed_g5_defconfig
+++ b/arch/arm/configs/aspeed_g5_defconfig
@@ -45,6 +45,7 @@ CONFIG_VFP=y
 CONFIG_NEON=y
 CONFIG_KERNEL_MODE_NEON=y
 CONFIG_FIRMWARE_MEMMAP=y
+CONFIG_KPROBES=y
 CONFIG_JUMP_LABEL=y
 CONFIG_MODULES=y
 # CONFIG_BLK_DEV_BSG is not set
-- 
2.25.1

[RFC PATCH linux dev-5.4 4/4] ARM: configs: aspeed-g5: Set CONFIG_FORTIFY_SOURCE=n to appease kprobes

[Andrew Jeffery]

Setting CONFIG_FORTIFY_SOURCE=y and CONFIG_KPROBES=y on arm gives a panic when
trying to insert a new probe:

$ echo r:myretprobe do_sys_open '$retval' >> /sys/kernel/debug/tracing/kprobe_events
[   51.688589] detected buffer overflow in memcpy
[   51.689430] ------------[ cut here ]------------
[   51.689627] kernel BUG at lib/string.c:1096!
[   51.689800] Internal error: Oops - BUG: 0 [#1] SMP ARM
[   51.690107] Modules linked in:
[   51.690442] CPU: 0 PID: 107 Comm: sh Not tainted 5.4.39-00272-ga1ee7c973659 #6
[   51.690674] Hardware name: Generic DT based system
[   51.691184] PC is at fortify_panic+0x18/0x20
[   51.691371] LR is at __irq_work_queue_local+0x40/0x7c
[   51.691575] pc : [<8083eaa4>]    lr : [<8020e0ec>]    psr: 60000013
[   51.691812] sp : bd237cb0  ip : bd237bc0  fp : bd237cbc
[   51.692129] r10: 00000007  r9 : 00000000  r8 : 00000060
[   51.692395] r7 : 8011f2f0  r6 : b5092480  r5 : 7f000000  r4 : b4c53b4c
[   51.692723] r3 : 80c04c48  r2 : 00000000  r1 : bd7c5448  r0 : 00000022
[   51.693088] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[   51.693449] Control: 10c5387d  Table: b50f406a  DAC: 00000051
[   51.693769] Process sh (pid: 107, stack limit = 0x4e9b7225)
[   51.694126] Stack: (0xbd237cb0 to 0xbd238000)

YOLO it and disable fortified source.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 arch/arm/configs/aspeed_g5_defconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig
index 254fb7562738..b7f8ccc99000 100644
--- a/arch/arm/configs/aspeed_g5_defconfig
+++ b/arch/arm/configs/aspeed_g5_defconfig
@@ -269,7 +269,6 @@ CONFIG_SQUASHFS_XZ=y
 CONFIG_SQUASHFS_ZSTD=y
 # CONFIG_NETWORK_FILESYSTEMS is not set
 CONFIG_HARDENED_USERCOPY=y
-CONFIG_FORTIFY_SOURCE=y
 CONFIG_CRYPTO_HMAC=y
 CONFIG_CRYPTO_SHA256=y
 CONFIG_CRYPTO_USER_API_HASH=y
-- 
2.25.1

Re: [RFC PATCH linux dev-5.4 2/4] ARM: configs: aspeed-g5: Enable modules

[Patrick Williams]

[-- Attachment #1: Type: text/plain, Size: 2770 bytes --]

On Thu, May 14, 2020 at 10:57:01PM +0930, Andrew Jeffery wrote:
> This is a step on the path to enabling kprobes, which depend on
> CONFIG_MODULES=y.
> 
> Setting CONFIG_MODULES=y results in the following change in kernel size:
> 
> Before:
>          $ size vmlinux
>             text    data     bss     dec     hex filename
>          8140500 2440582  178904 10759986         a42f32 vmlinux
>          $ stat --printf '%s\n' arch/arm/boot/zImage
>          3445200
> 
> After:
>          $ size vmlinux
>             text    data     bss     dec     hex filename
>          8240349 2388862  172304 10801515         a4d16b vmlinux
>          $ stat --printf '%s\n' arch/arm/boot/zImage
>          3458368
> 
> This gives a 41,529 byte increase to vmlinux and a 13,168 byte increase
> to the size of the compressed kernel image. In the most constrained BMC
> flash layout (arch/arm/boot/dts/openbmc-flash-layout.dtsi) this change
> decreases the free space in the kernel partition from 1,011,248 bytes to
> 998,080 bytes.
> 
> CONFIG_HW_RANDOM=y seems to come along for the ride with the
> `./scripts/config -e modules && make olddefconfig && make
> savedefconfig`. The measurements above include it, I haven't
> investigated why it gets enabled or what effect disabling it might have
> on the numbers.
> 
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
>  arch/arm/configs/aspeed_g5_defconfig | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/arch/arm/configs/aspeed_g5_defconfig b/arch/arm/configs/aspeed_g5_defconfig
> index e124b4ddd442..e30f27a413f4 100644
> --- a/arch/arm/configs/aspeed_g5_defconfig
> +++ b/arch/arm/configs/aspeed_g5_defconfig
> @@ -46,6 +46,7 @@ CONFIG_NEON=y
>  CONFIG_KERNEL_MODE_NEON=y
>  CONFIG_FIRMWARE_MEMMAP=y
>  CONFIG_JUMP_LABEL=y
> +CONFIG_MODULES=y
>  # CONFIG_BLK_DEV_BSG is not set
>  # CONFIG_BLK_DEBUG_FS is not set
>  # CONFIG_MQ_IOSCHED_DEADLINE is not set
> @@ -152,6 +153,7 @@ CONFIG_SERIAL_8250_DW=y
>  CONFIG_SERIAL_OF_PLATFORM=y
>  CONFIG_ASPEED_KCS_IPMI_BMC=y
>  CONFIG_ASPEED_BT_IPMI_BMC=y
> +CONFIG_HW_RANDOM=y
>  CONFIG_HW_RANDOM_TIMERIOMEM=y
>  # CONFIG_I2C_COMPAT is not set
>  CONFIG_I2C_CHARDEV=y
> -- 
> 2.25.1
> 

We're already using modules on the facebook/openbmc kernels and it
hasn't seemed to cause any issues.  Is there any argument why we
shouldn't begin moving some of the drivers to be modules?  Any easy
change would be to make all the hwmon content as modules and load them
based on the device tree.

I think there is a DISTRO_FEATURE we should enable for userspace support
of modules.  Let me know if you want me to investigate that.

Reviewed-by: Patrick Williams <patrick@stwcx.xyz>

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [RFC PATCH linux dev-5.4 4/4] ARM: configs: aspeed-g5: Set CONFIG_FORTIFY_SOURCE=n to appease kprobes

[Patrick Williams]

[-- Attachment #1: Type: text/plain, Size: 1733 bytes --]

On Thu, May 14, 2020 at 10:57:03PM +0930, Andrew Jeffery wrote:
> Setting CONFIG_FORTIFY_SOURCE=y and CONFIG_KPROBES=y on arm gives a panic when
> trying to insert a new probe:
> 
> $ echo r:myretprobe do_sys_open '$retval' >> /sys/kernel/debug/tracing/kprobe_events
> [   51.688589] detected buffer overflow in memcpy
> [   51.689430] ------------[ cut here ]------------
> [   51.689627] kernel BUG at lib/string.c:1096!
> [   51.689800] Internal error: Oops - BUG: 0 [#1] SMP ARM
> [   51.690107] Modules linked in:
> [   51.690442] CPU: 0 PID: 107 Comm: sh Not tainted 5.4.39-00272-ga1ee7c973659 #6
> [   51.690674] Hardware name: Generic DT based system
> [   51.691184] PC is at fortify_panic+0x18/0x20
> [   51.691371] LR is at __irq_work_queue_local+0x40/0x7c
> [   51.691575] pc : [<8083eaa4>]    lr : [<8020e0ec>]    psr: 60000013
> [   51.691812] sp : bd237cb0  ip : bd237bc0  fp : bd237cbc
> [   51.692129] r10: 00000007  r9 : 00000000  r8 : 00000060
> [   51.692395] r7 : 8011f2f0  r6 : b5092480  r5 : 7f000000  r4 : b4c53b4c
> [   51.692723] r3 : 80c04c48  r2 : 00000000  r1 : bd7c5448  r0 : 00000022
> [   51.693088] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
> [   51.693449] Control: 10c5387d  Table: b50f406a  DAC: 00000051
> [   51.693769] Process sh (pid: 107, stack limit = 0x4e9b7225)
> [   51.694126] Stack: (0xbd237cb0 to 0xbd238000)
> 
> YOLO it and disable fortified source.
> 
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> ---
>  arch/arm/configs/aspeed_g5_defconfig | 1 -
>  1 file changed, 1 deletion(-)

Doesn't this mean we're just hiding a bug with KPROBE code and
potentially hiding more bugs with other drivers?

-- 
Patrick Williams

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [RFC PATCH linux dev-5.4 2/4] ARM: configs: aspeed-g5: Enable modules

[Milton Miller II]

On May 14, 2020 around 8:53AM in some timezone, Patrick Williams  wrote:

>On Thu, May 14, 2020 at 10:57:01PM +0930, Andrew Jeffery wrote:>> This is a step on the path to enabling kprobes, which depend on
>> CONFIG_MODULES=y.
>> 
>> Setting CONFIG_MODULES=y results in the following change in kernel
>size:
>> 
>> Before:
>>          $ size vmlinux
>>             text    data     bss     dec     hex filename
>>          8140500 2440582  178904 10759986         a42f32 vmlinux
>>          $ stat --printf '%s\n' arch/arm/boot/zImage
>>          3445200
>> 
>> After:
>>          $ size vmlinux
>>             text    data     bss     dec     hex filename
>>          8240349 2388862  172304 10801515         a4d16b vmlinux
>>          $ stat --printf '%s\n' arch/arm/boot/zImage
>>          3458368
>> 
>> This gives a 41,529 byte increase to vmlinux and a 13,168 byte
>increase
>> to the size of the compressed kernel image. In the most constrained
>BMC
>> flash layout (arch/arm/boot/dts/openbmc-flash-layout.dtsi) this
>change
>> decreases the free space in the kernel partition from 1,011,248
>bytes to
>> 998,080 bytes.
>> 
>> CONFIG_HW_RANDOM=y seems to come along for the ride with the
>> `./scripts/config -e modules && make olddefconfig && make
>> savedefconfig`. The measurements above include it, I haven't
>> investigated why it gets enabled or what effect disabling it might
>have
>> on the numbers.
>> 
>> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
>> ---
>>  arch/arm/configs/aspeed_g5_defconfig | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/arch/arm/configs/aspeed_g5_defconfig
>b/arch/arm/configs/aspeed_g5_defconfig
>> index e124b4ddd442..e30f27a413f4 100644
>> --- a/arch/arm/configs/aspeed_g5_defconfig
>> +++ b/arch/arm/configs/aspeed_g5_defconfig
>> @@ -46,6 +46,7 @@ CONFIG_NEON=y
>>  CONFIG_KERNEL_MODE_NEON=y
>>  CONFIG_FIRMWARE_MEMMAP=y
>>  CONFIG_JUMP_LABEL=y
>> +CONFIG_MODULES=y
>>  # CONFIG_BLK_DEV_BSG is not set
>>  # CONFIG_BLK_DEBUG_FS is not set
>>  # CONFIG_MQ_IOSCHED_DEADLINE is not set
>> @@ -152,6 +153,7 @@ CONFIG_SERIAL_8250_DW=y
>>  CONFIG_SERIAL_OF_PLATFORM=y
>>  CONFIG_ASPEED_KCS_IPMI_BMC=y
>>  CONFIG_ASPEED_BT_IPMI_BMC=y
>> +CONFIG_HW_RANDOM=y
>>  CONFIG_HW_RANDOM_TIMERIOMEM=y
>>  # CONFIG_I2C_COMPAT is not set
>>  CONFIG_I2C_CHARDEV=y
>> -- 
>> 2.25.1
>> 
>
>We're already using modules on the facebook/openbmc kernels and it
>hasn't seemed to cause any issues.  Is there any argument why we
>shouldn't begin moving some of the drivers to be modules?  Any easy
>change would be to make all the hwmon content as modules and load
>them
>based on the device tree.
>
>I think there is a DISTRO_FEATURE we should enable for userspace
>support
>of modules.  Let me know if you want me to investigate that.

This gerrit is still open.  It also adds content to the rootfs b
ecause it tries to pull in vmlinux into the root fs as part of the 
base kernel recipie:

https://gerrit.openbmc-project.xyz/c/openbmc/meta-phosphor/+/26092

actually this one that has failed since September:

https://gerrit.openbmc-project.xyz/c/openbmc/meta-phosphor/+/25536/2

milton

Re: [RFC PATCH linux dev-5.4 4/4] ARM: configs: aspeed-g5: Set CONFIG_FORTIFY_SOURCE=n to appease kprobes

[Andrew Jeffery]

On Thu, 14 May 2020, at 23:24, Patrick Williams wrote:
> On Thu, May 14, 2020 at 10:57:03PM +0930, Andrew Jeffery wrote:
> > Setting CONFIG_FORTIFY_SOURCE=y and CONFIG_KPROBES=y on arm gives a panic when
> > trying to insert a new probe:
> > 
> > $ echo r:myretprobe do_sys_open '$retval' >> /sys/kernel/debug/tracing/kprobe_events
> > [   51.688589] detected buffer overflow in memcpy
> > [   51.689430] ------------[ cut here ]------------
> > [   51.689627] kernel BUG at lib/string.c:1096!
> > [   51.689800] Internal error: Oops - BUG: 0 [#1] SMP ARM
> > [   51.690107] Modules linked in:
> > [   51.690442] CPU: 0 PID: 107 Comm: sh Not tainted 5.4.39-00272-ga1ee7c973659 #6
> > [   51.690674] Hardware name: Generic DT based system
> > [   51.691184] PC is at fortify_panic+0x18/0x20
> > [   51.691371] LR is at __irq_work_queue_local+0x40/0x7c
> > [   51.691575] pc : [<8083eaa4>]    lr : [<8020e0ec>]    psr: 60000013
> > [   51.691812] sp : bd237cb0  ip : bd237bc0  fp : bd237cbc
> > [   51.692129] r10: 00000007  r9 : 00000000  r8 : 00000060
> > [   51.692395] r7 : 8011f2f0  r6 : b5092480  r5 : 7f000000  r4 : b4c53b4c
> > [   51.692723] r3 : 80c04c48  r2 : 00000000  r1 : bd7c5448  r0 : 00000022
> > [   51.693088] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
> > [   51.693449] Control: 10c5387d  Table: b50f406a  DAC: 00000051
> > [   51.693769] Process sh (pid: 107, stack limit = 0x4e9b7225)
> > [   51.694126] Stack: (0xbd237cb0 to 0xbd238000)
> > 
> > YOLO it and disable fortified source.
> > 
> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > ---
> >  arch/arm/configs/aspeed_g5_defconfig | 1 -
> >  1 file changed, 1 deletion(-)
> 
> Doesn't this mean we're just hiding a bug with KPROBE code

Possibly.

> and
> potentially hiding more bugs with other drivers?

Yes.

You should view this series as "things I did to get kprobes 'working'" and not
"patches we should apply to the kernel tree". This one in particular was a
low-key troll patch to flush out any information on why
CONFIG_FORTIFY_SOURCE=y breaks kprobes in particular. I mean, it even
features "YOLO" in the commit mesage, so don't take it too seriously :)

Andrew

Re: [RFC PATCH linux dev-5.4 3/4] ARM: configs: aspeed-g5: Enable kprobes

[Andrew Jeffery]

On Thu, 14 May 2020, at 22:57, Andrew Jeffery wrote:
> Enable kprobes to improve the debugging experience on the AST2500 and
> later.
> 
> Setting CONFIG_KPROBES=y results in the following change in kernel size:
> 
> Before:
>          $ size vmlinux
>             text    data     bss     dec     hex filename
>          8240349 2388862  172304 10801515         a4d16b vmlinux
>          $ stat --printf '%s\n' *.zImage
>          3458368
> 
> After:
>          $ size vmlinux
>             text    data     bss     dec     hex filename
>          8279214 2399530  176912 10855656         a5a4e8 vmlinux
>          $ stat --printf '%s\n' *.zImage
>          3471880
> 
> This gives a 54,141 byte increase to vmlinux and a 13,512 byte increase
> to the size of the compressed kernel image. In the most constrained BMC
> flash layout (arch/arm/boot/dts/openbmc-flash-layout.dtsi) this change
> decreases the free space in the kernel partition from 998,080 bytes to
> 984,568 bytes.

Just realised I should be measuring the FIT not the zImage so ignore the
free-space numbers for the moment.

Andrew

Re: [RFC PATCH linux dev-5.4 4/4] ARM: configs: aspeed-g5: Set CONFIG_FORTIFY_SOURCE=n to appease kprobes

[Andrew Jeffery]

On Fri, 15 May 2020, at 07:40, Andrew Jeffery wrote:
> 
> 
> On Thu, 14 May 2020, at 23:24, Patrick Williams wrote:
> > On Thu, May 14, 2020 at 10:57:03PM +0930, Andrew Jeffery wrote:
> > > Setting CONFIG_FORTIFY_SOURCE=y and CONFIG_KPROBES=y on arm gives a panic when
> > > trying to insert a new probe:
> > > 
> > > $ echo r:myretprobe do_sys_open '$retval' >> /sys/kernel/debug/tracing/kprobe_events
> > > [   51.688589] detected buffer overflow in memcpy
> > > [   51.689430] ------------[ cut here ]------------
> > > [   51.689627] kernel BUG at lib/string.c:1096!
> > > [   51.689800] Internal error: Oops - BUG: 0 [#1] SMP ARM
> > > [   51.690107] Modules linked in:
> > > [   51.690442] CPU: 0 PID: 107 Comm: sh Not tainted 5.4.39-00272-ga1ee7c973659 #6
> > > [   51.690674] Hardware name: Generic DT based system
> > > [   51.691184] PC is at fortify_panic+0x18/0x20
> > > [   51.691371] LR is at __irq_work_queue_local+0x40/0x7c
> > > [   51.691575] pc : [<8083eaa4>]    lr : [<8020e0ec>]    psr: 60000013
> > > [   51.691812] sp : bd237cb0  ip : bd237bc0  fp : bd237cbc
> > > [   51.692129] r10: 00000007  r9 : 00000000  r8 : 00000060
> > > [   51.692395] r7 : 8011f2f0  r6 : b5092480  r5 : 7f000000  r4 : b4c53b4c
> > > [   51.692723] r3 : 80c04c48  r2 : 00000000  r1 : bd7c5448  r0 : 00000022
> > > [   51.693088] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
> > > [   51.693449] Control: 10c5387d  Table: b50f406a  DAC: 00000051
> > > [   51.693769] Process sh (pid: 107, stack limit = 0x4e9b7225)
> > > [   51.694126] Stack: (0xbd237cb0 to 0xbd238000)
> > > 
> > > YOLO it and disable fortified source.
> > > 
> > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > > ---
> > >  arch/arm/configs/aspeed_g5_defconfig | 1 -
> > >  1 file changed, 1 deletion(-)
> > 
> > Doesn't this mean we're just hiding a bug with KPROBE code
> 
> Possibly.
> 
> > and
> > potentially hiding more bugs with other drivers?
> 
> Yes.
> 
> You should view this series as "things I did to get kprobes 'working'" and not
> "patches we should apply to the kernel tree". This one in particular was a
> low-key troll patch to flush out any information on why
> CONFIG_FORTIFY_SOURCE=y breaks kprobes in particular.

In [1] Kees Cook suggests it's a false-positive and provides a potential fix. This
was submitted at [2]. [3] then fixes the fix. Turns out the issue needs fixing
again due to compiler advances, so I sent [4] but it looks like we might instead
resolve it by [5].

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1639397
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=e46daee53bb5
[3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0ac569bf6a798
[4] https://lore.kernel.org/linux-arm-kernel/20200517153959.293224-1-andrew@aj.id.au/
[5] https://lore.kernel.org/linux-arm-kernel/202005171447.00CFE0C@keescook/

Andrew