2020-07-25 06:05:09

by Nathan Chancellor

[permalink] [raw]
Subject: [PATCH] usb: dwc2: Fix parameter type in function pointer prototype

When booting up on a Raspberry Pi 4 with Control Flow Integrity checking
enabled, the following warning/panic happens:

[ 1.626435] CFI failure (target: dwc2_set_bcm_params+0x0/0x4):
[ 1.632408] WARNING: CPU: 0 PID: 32 at kernel/cfi.c:30 __cfi_check_fail+0x54/0x5c
[ 1.640021] Modules linked in:
[ 1.643137] CPU: 0 PID: 32 Comm: kworker/0:1 Not tainted 5.8.0-rc6-next-20200724-00051-g89ba619726de #1
[ 1.652693] Hardware name: Raspberry Pi 4 Model B Rev 1.2 (DT)
[ 1.658637] Workqueue: events deferred_probe_work_func
[ 1.663870] pstate: 60000005 (nZCv daif -PAN -UAO BTYPE=--)
[ 1.669542] pc : __cfi_check_fail+0x54/0x5c
[ 1.673798] lr : __cfi_check_fail+0x54/0x5c
[ 1.678050] sp : ffff8000102bbaa0
[ 1.681419] x29: ffff8000102bbaa0 x28: ffffab09e21c7000
[ 1.686829] x27: 0000000000000402 x26: ffff0000f6e7c228
[ 1.692238] x25: 00000000fb7cdb0d x24: 0000000000000005
[ 1.697647] x23: ffffab09e2515000 x22: ffffab09e069a000
[ 1.703055] x21: 4c550309df1cf4c1 x20: ffffab09e2433c60
[ 1.708462] x19: ffffab09e160dc50 x18: ffff0000f6e8cc78
[ 1.713870] x17: 0000000000000041 x16: ffffab09e0bce6f8
[ 1.719278] x15: ffffab09e1c819b7 x14: 0000000000000003
[ 1.724686] x13: 00000000ffffefff x12: 0000000000000000
[ 1.730094] x11: 0000000000000000 x10: 00000000ffffffff
[ 1.735501] x9 : c932f7abfc4bc600 x8 : c932f7abfc4bc600
[ 1.740910] x7 : 077207610770075f x6 : ffff0000f6c38f00
[ 1.746317] x5 : 0000000000000000 x4 : 0000000000000000
[ 1.751723] x3 : 0000000000000000 x2 : 0000000000000000
[ 1.757129] x1 : ffff8000102bb7d8 x0 : 0000000000000032
[ 1.762539] Call trace:
[ 1.765030] __cfi_check_fail+0x54/0x5c
[ 1.768938] __cfi_check+0x5fa6c/0x66afc
[ 1.772932] dwc2_init_params+0xd74/0xd78
[ 1.777012] dwc2_driver_probe+0x484/0x6ec
[ 1.781180] platform_drv_probe+0xb4/0x100
[ 1.785350] really_probe+0x228/0x63c
[ 1.789076] driver_probe_device+0x80/0xc0
[ 1.793247] __device_attach_driver+0x114/0x160
[ 1.797857] bus_for_each_drv+0xa8/0x128
[ 1.801851] __device_attach.llvm.14901095709067289134+0xc0/0x170
[ 1.808050] bus_probe_device+0x44/0x100
[ 1.812044] deferred_probe_work_func+0x78/0xb8
[ 1.816656] process_one_work+0x204/0x3c4
[ 1.820736] worker_thread+0x2f0/0x4c4
[ 1.824552] kthread+0x174/0x184
[ 1.827837] ret_from_fork+0x10/0x18

CFI validates that all indirect calls go to a function with the same
exact function pointer prototype. In this case, dwc2_set_bcm_params
is the target, which has a parameter of type 'struct dwc2_hsotg *',
but it is being implicitly cast to have a parameter of type 'void *'
because that is the set_params function pointer prototype. Make the
function pointer protoype match the definitions so that there is no
more violation.

Cc: [email protected]
Fixes: 7de1debcd2de ("usb: dwc2: Remove platform static params")
Link: https://github.com/ClangBuiltLinux/linux/issues/1107
Signed-off-by: Nathan Chancellor <[email protected]>
---
drivers/usb/dwc2/params.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index ce736d67c7c3..fd73ddd8eb75 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -860,7 +860,7 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg)
int dwc2_init_params(struct dwc2_hsotg *hsotg)
{
const struct of_device_id *match;
- void (*set_params)(void *data);
+ void (*set_params)(struct dwc2_hsotg *data);

dwc2_set_default_params(hsotg);
dwc2_get_device_properties(hsotg);

base-commit: 23ee3e4e5bd27bdbc0f1785eef7209ce872794c7
--
2.28.0.rc1


2020-07-25 06:22:25

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] usb: dwc2: Fix parameter type in function pointer prototype

On Fri, Jul 24, 2020 at 11:03:54PM -0700, Nathan Chancellor wrote:
> When booting up on a Raspberry Pi 4 with Control Flow Integrity checking
> enabled, the following warning/panic happens:
>
> [ 1.626435] CFI failure (target: dwc2_set_bcm_params+0x0/0x4):
> [ 1.632408] WARNING: CPU: 0 PID: 32 at kernel/cfi.c:30 __cfi_check_fail+0x54/0x5c
> [ 1.640021] Modules linked in:
> [ 1.643137] CPU: 0 PID: 32 Comm: kworker/0:1 Not tainted 5.8.0-rc6-next-20200724-00051-g89ba619726de #1
> [ 1.652693] Hardware name: Raspberry Pi 4 Model B Rev 1.2 (DT)
> [ 1.658637] Workqueue: events deferred_probe_work_func
> [ 1.663870] pstate: 60000005 (nZCv daif -PAN -UAO BTYPE=--)
> [ 1.669542] pc : __cfi_check_fail+0x54/0x5c
> [ 1.673798] lr : __cfi_check_fail+0x54/0x5c
> [ 1.678050] sp : ffff8000102bbaa0
> [ 1.681419] x29: ffff8000102bbaa0 x28: ffffab09e21c7000
> [ 1.686829] x27: 0000000000000402 x26: ffff0000f6e7c228
> [ 1.692238] x25: 00000000fb7cdb0d x24: 0000000000000005
> [ 1.697647] x23: ffffab09e2515000 x22: ffffab09e069a000
> [ 1.703055] x21: 4c550309df1cf4c1 x20: ffffab09e2433c60
> [ 1.708462] x19: ffffab09e160dc50 x18: ffff0000f6e8cc78
> [ 1.713870] x17: 0000000000000041 x16: ffffab09e0bce6f8
> [ 1.719278] x15: ffffab09e1c819b7 x14: 0000000000000003
> [ 1.724686] x13: 00000000ffffefff x12: 0000000000000000
> [ 1.730094] x11: 0000000000000000 x10: 00000000ffffffff
> [ 1.735501] x9 : c932f7abfc4bc600 x8 : c932f7abfc4bc600
> [ 1.740910] x7 : 077207610770075f x6 : ffff0000f6c38f00
> [ 1.746317] x5 : 0000000000000000 x4 : 0000000000000000
> [ 1.751723] x3 : 0000000000000000 x2 : 0000000000000000
> [ 1.757129] x1 : ffff8000102bb7d8 x0 : 0000000000000032
> [ 1.762539] Call trace:
> [ 1.765030] __cfi_check_fail+0x54/0x5c
> [ 1.768938] __cfi_check+0x5fa6c/0x66afc
> [ 1.772932] dwc2_init_params+0xd74/0xd78
> [ 1.777012] dwc2_driver_probe+0x484/0x6ec
> [ 1.781180] platform_drv_probe+0xb4/0x100
> [ 1.785350] really_probe+0x228/0x63c
> [ 1.789076] driver_probe_device+0x80/0xc0
> [ 1.793247] __device_attach_driver+0x114/0x160
> [ 1.797857] bus_for_each_drv+0xa8/0x128
> [ 1.801851] __device_attach.llvm.14901095709067289134+0xc0/0x170
> [ 1.808050] bus_probe_device+0x44/0x100
> [ 1.812044] deferred_probe_work_func+0x78/0xb8
> [ 1.816656] process_one_work+0x204/0x3c4
> [ 1.820736] worker_thread+0x2f0/0x4c4
> [ 1.824552] kthread+0x174/0x184
> [ 1.827837] ret_from_fork+0x10/0x18
>
> CFI validates that all indirect calls go to a function with the same
> exact function pointer prototype. In this case, dwc2_set_bcm_params
> is the target, which has a parameter of type 'struct dwc2_hsotg *',
> but it is being implicitly cast to have a parameter of type 'void *'
> because that is the set_params function pointer prototype. Make the
> function pointer protoype match the definitions so that there is no
> more violation.
>
> Cc: [email protected]

Why does this matter for stable kernels, given that CFI is not in any
kernel tree yet?

thanks,

greg k-h

2020-07-25 06:25:16

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH] usb: dwc2: Fix parameter type in function pointer prototype

On Sat, Jul 25, 2020 at 08:19:47AM +0200, Greg Kroah-Hartman wrote:
> On Fri, Jul 24, 2020 at 11:03:54PM -0700, Nathan Chancellor wrote:
> > When booting up on a Raspberry Pi 4 with Control Flow Integrity checking
> > enabled, the following warning/panic happens:
> >
> > [ 1.626435] CFI failure (target: dwc2_set_bcm_params+0x0/0x4):
> > [ 1.632408] WARNING: CPU: 0 PID: 32 at kernel/cfi.c:30 __cfi_check_fail+0x54/0x5c
> > [ 1.640021] Modules linked in:
> > [ 1.643137] CPU: 0 PID: 32 Comm: kworker/0:1 Not tainted 5.8.0-rc6-next-20200724-00051-g89ba619726de #1
> > [ 1.652693] Hardware name: Raspberry Pi 4 Model B Rev 1.2 (DT)
> > [ 1.658637] Workqueue: events deferred_probe_work_func
> > [ 1.663870] pstate: 60000005 (nZCv daif -PAN -UAO BTYPE=--)
> > [ 1.669542] pc : __cfi_check_fail+0x54/0x5c
> > [ 1.673798] lr : __cfi_check_fail+0x54/0x5c
> > [ 1.678050] sp : ffff8000102bbaa0
> > [ 1.681419] x29: ffff8000102bbaa0 x28: ffffab09e21c7000
> > [ 1.686829] x27: 0000000000000402 x26: ffff0000f6e7c228
> > [ 1.692238] x25: 00000000fb7cdb0d x24: 0000000000000005
> > [ 1.697647] x23: ffffab09e2515000 x22: ffffab09e069a000
> > [ 1.703055] x21: 4c550309df1cf4c1 x20: ffffab09e2433c60
> > [ 1.708462] x19: ffffab09e160dc50 x18: ffff0000f6e8cc78
> > [ 1.713870] x17: 0000000000000041 x16: ffffab09e0bce6f8
> > [ 1.719278] x15: ffffab09e1c819b7 x14: 0000000000000003
> > [ 1.724686] x13: 00000000ffffefff x12: 0000000000000000
> > [ 1.730094] x11: 0000000000000000 x10: 00000000ffffffff
> > [ 1.735501] x9 : c932f7abfc4bc600 x8 : c932f7abfc4bc600
> > [ 1.740910] x7 : 077207610770075f x6 : ffff0000f6c38f00
> > [ 1.746317] x5 : 0000000000000000 x4 : 0000000000000000
> > [ 1.751723] x3 : 0000000000000000 x2 : 0000000000000000
> > [ 1.757129] x1 : ffff8000102bb7d8 x0 : 0000000000000032
> > [ 1.762539] Call trace:
> > [ 1.765030] __cfi_check_fail+0x54/0x5c
> > [ 1.768938] __cfi_check+0x5fa6c/0x66afc
> > [ 1.772932] dwc2_init_params+0xd74/0xd78
> > [ 1.777012] dwc2_driver_probe+0x484/0x6ec
> > [ 1.781180] platform_drv_probe+0xb4/0x100
> > [ 1.785350] really_probe+0x228/0x63c
> > [ 1.789076] driver_probe_device+0x80/0xc0
> > [ 1.793247] __device_attach_driver+0x114/0x160
> > [ 1.797857] bus_for_each_drv+0xa8/0x128
> > [ 1.801851] __device_attach.llvm.14901095709067289134+0xc0/0x170
> > [ 1.808050] bus_probe_device+0x44/0x100
> > [ 1.812044] deferred_probe_work_func+0x78/0xb8
> > [ 1.816656] process_one_work+0x204/0x3c4
> > [ 1.820736] worker_thread+0x2f0/0x4c4
> > [ 1.824552] kthread+0x174/0x184
> > [ 1.827837] ret_from_fork+0x10/0x18
> >
> > CFI validates that all indirect calls go to a function with the same
> > exact function pointer prototype. In this case, dwc2_set_bcm_params
> > is the target, which has a parameter of type 'struct dwc2_hsotg *',
> > but it is being implicitly cast to have a parameter of type 'void *'
> > because that is the set_params function pointer prototype. Make the
> > function pointer protoype match the definitions so that there is no
> > more violation.
> >
> > Cc: [email protected]
>
> Why does this matter for stable kernels, given that CFI is not in any
> kernel tree yet?
>
> thanks,
>
> greg k-h

It might not be available upstream but it is in all downstream Android
kernels. Furthermore, all of the previous CFI fixes I have done have
inevitably ended up in stable trees through AUTOSEL, I figured I would
save Sasha the hassle this time around. It does not personally matter to
me though, I am fine with stripping the tag since I do all of my
personal testing with mainline/next so if this is needed in stable
later due to an OEM or someone else tripping over it, it can just be
added then.

Let me know if you want me to resend it without that tag.

Cheers,
Nathan

2020-07-25 06:27:31

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH] usb: dwc2: Fix parameter type in function pointer prototype

Greg Kroah-Hartman <[email protected]> writes:

> On Fri, Jul 24, 2020 at 11:03:54PM -0700, Nathan Chancellor wrote:
>> When booting up on a Raspberry Pi 4 with Control Flow Integrity checking
>> enabled, the following warning/panic happens:
>>
>> [ 1.626435] CFI failure (target: dwc2_set_bcm_params+0x0/0x4):
>> [ 1.632408] WARNING: CPU: 0 PID: 32 at kernel/cfi.c:30 __cfi_check_fail+0x54/0x5c
>> [ 1.640021] Modules linked in:
>> [ 1.643137] CPU: 0 PID: 32 Comm: kworker/0:1 Not tainted 5.8.0-rc6-next-20200724-00051-g89ba619726de #1
>> [ 1.652693] Hardware name: Raspberry Pi 4 Model B Rev 1.2 (DT)
>> [ 1.658637] Workqueue: events deferred_probe_work_func
>> [ 1.663870] pstate: 60000005 (nZCv daif -PAN -UAO BTYPE=--)
>> [ 1.669542] pc : __cfi_check_fail+0x54/0x5c
>> [ 1.673798] lr : __cfi_check_fail+0x54/0x5c
>> [ 1.678050] sp : ffff8000102bbaa0
>> [ 1.681419] x29: ffff8000102bbaa0 x28: ffffab09e21c7000
>> [ 1.686829] x27: 0000000000000402 x26: ffff0000f6e7c228
>> [ 1.692238] x25: 00000000fb7cdb0d x24: 0000000000000005
>> [ 1.697647] x23: ffffab09e2515000 x22: ffffab09e069a000
>> [ 1.703055] x21: 4c550309df1cf4c1 x20: ffffab09e2433c60
>> [ 1.708462] x19: ffffab09e160dc50 x18: ffff0000f6e8cc78
>> [ 1.713870] x17: 0000000000000041 x16: ffffab09e0bce6f8
>> [ 1.719278] x15: ffffab09e1c819b7 x14: 0000000000000003
>> [ 1.724686] x13: 00000000ffffefff x12: 0000000000000000
>> [ 1.730094] x11: 0000000000000000 x10: 00000000ffffffff
>> [ 1.735501] x9 : c932f7abfc4bc600 x8 : c932f7abfc4bc600
>> [ 1.740910] x7 : 077207610770075f x6 : ffff0000f6c38f00
>> [ 1.746317] x5 : 0000000000000000 x4 : 0000000000000000
>> [ 1.751723] x3 : 0000000000000000 x2 : 0000000000000000
>> [ 1.757129] x1 : ffff8000102bb7d8 x0 : 0000000000000032
>> [ 1.762539] Call trace:
>> [ 1.765030] __cfi_check_fail+0x54/0x5c
>> [ 1.768938] __cfi_check+0x5fa6c/0x66afc
>> [ 1.772932] dwc2_init_params+0xd74/0xd78
>> [ 1.777012] dwc2_driver_probe+0x484/0x6ec
>> [ 1.781180] platform_drv_probe+0xb4/0x100
>> [ 1.785350] really_probe+0x228/0x63c
>> [ 1.789076] driver_probe_device+0x80/0xc0
>> [ 1.793247] __device_attach_driver+0x114/0x160
>> [ 1.797857] bus_for_each_drv+0xa8/0x128
>> [ 1.801851] __device_attach.llvm.14901095709067289134+0xc0/0x170
>> [ 1.808050] bus_probe_device+0x44/0x100
>> [ 1.812044] deferred_probe_work_func+0x78/0xb8
>> [ 1.816656] process_one_work+0x204/0x3c4
>> [ 1.820736] worker_thread+0x2f0/0x4c4
>> [ 1.824552] kthread+0x174/0x184
>> [ 1.827837] ret_from_fork+0x10/0x18
>>
>> CFI validates that all indirect calls go to a function with the same
>> exact function pointer prototype. In this case, dwc2_set_bcm_params
>> is the target, which has a parameter of type 'struct dwc2_hsotg *',
>> but it is being implicitly cast to have a parameter of type 'void *'
>> because that is the set_params function pointer prototype. Make the
>> function pointer protoype match the definitions so that there is no
>> more violation.
>>
>> Cc: [email protected]
>
> Why does this matter for stable kernels, given that CFI is not in any
> kernel tree yet?

remove stable while applying to testing/next.

--
balbi


Attachments:
signature.asc (847.00 B)

2020-07-25 06:28:16

by Felipe Balbi

[permalink] [raw]
Subject: Re: [PATCH] usb: dwc2: Fix parameter type in function pointer prototype


Hi,

Nathan Chancellor <[email protected]> writes:

> On Sat, Jul 25, 2020 at 08:19:47AM +0200, Greg Kroah-Hartman wrote:
>> On Fri, Jul 24, 2020 at 11:03:54PM -0700, Nathan Chancellor wrote:
>> > When booting up on a Raspberry Pi 4 with Control Flow Integrity checking
>> > enabled, the following warning/panic happens:
>> >
>> > [ 1.626435] CFI failure (target: dwc2_set_bcm_params+0x0/0x4):
>> > [ 1.632408] WARNING: CPU: 0 PID: 32 at kernel/cfi.c:30 __cfi_check_fail+0x54/0x5c
>> > [ 1.640021] Modules linked in:
>> > [ 1.643137] CPU: 0 PID: 32 Comm: kworker/0:1 Not tainted 5.8.0-rc6-next-20200724-00051-g89ba619726de #1
>> > [ 1.652693] Hardware name: Raspberry Pi 4 Model B Rev 1.2 (DT)
>> > [ 1.658637] Workqueue: events deferred_probe_work_func
>> > [ 1.663870] pstate: 60000005 (nZCv daif -PAN -UAO BTYPE=--)
>> > [ 1.669542] pc : __cfi_check_fail+0x54/0x5c
>> > [ 1.673798] lr : __cfi_check_fail+0x54/0x5c
>> > [ 1.678050] sp : ffff8000102bbaa0
>> > [ 1.681419] x29: ffff8000102bbaa0 x28: ffffab09e21c7000
>> > [ 1.686829] x27: 0000000000000402 x26: ffff0000f6e7c228
>> > [ 1.692238] x25: 00000000fb7cdb0d x24: 0000000000000005
>> > [ 1.697647] x23: ffffab09e2515000 x22: ffffab09e069a000
>> > [ 1.703055] x21: 4c550309df1cf4c1 x20: ffffab09e2433c60
>> > [ 1.708462] x19: ffffab09e160dc50 x18: ffff0000f6e8cc78
>> > [ 1.713870] x17: 0000000000000041 x16: ffffab09e0bce6f8
>> > [ 1.719278] x15: ffffab09e1c819b7 x14: 0000000000000003
>> > [ 1.724686] x13: 00000000ffffefff x12: 0000000000000000
>> > [ 1.730094] x11: 0000000000000000 x10: 00000000ffffffff
>> > [ 1.735501] x9 : c932f7abfc4bc600 x8 : c932f7abfc4bc600
>> > [ 1.740910] x7 : 077207610770075f x6 : ffff0000f6c38f00
>> > [ 1.746317] x5 : 0000000000000000 x4 : 0000000000000000
>> > [ 1.751723] x3 : 0000000000000000 x2 : 0000000000000000
>> > [ 1.757129] x1 : ffff8000102bb7d8 x0 : 0000000000000032
>> > [ 1.762539] Call trace:
>> > [ 1.765030] __cfi_check_fail+0x54/0x5c
>> > [ 1.768938] __cfi_check+0x5fa6c/0x66afc
>> > [ 1.772932] dwc2_init_params+0xd74/0xd78
>> > [ 1.777012] dwc2_driver_probe+0x484/0x6ec
>> > [ 1.781180] platform_drv_probe+0xb4/0x100
>> > [ 1.785350] really_probe+0x228/0x63c
>> > [ 1.789076] driver_probe_device+0x80/0xc0
>> > [ 1.793247] __device_attach_driver+0x114/0x160
>> > [ 1.797857] bus_for_each_drv+0xa8/0x128
>> > [ 1.801851] __device_attach.llvm.14901095709067289134+0xc0/0x170
>> > [ 1.808050] bus_probe_device+0x44/0x100
>> > [ 1.812044] deferred_probe_work_func+0x78/0xb8
>> > [ 1.816656] process_one_work+0x204/0x3c4
>> > [ 1.820736] worker_thread+0x2f0/0x4c4
>> > [ 1.824552] kthread+0x174/0x184
>> > [ 1.827837] ret_from_fork+0x10/0x18
>> >
>> > CFI validates that all indirect calls go to a function with the same
>> > exact function pointer prototype. In this case, dwc2_set_bcm_params
>> > is the target, which has a parameter of type 'struct dwc2_hsotg *',
>> > but it is being implicitly cast to have a parameter of type 'void *'
>> > because that is the set_params function pointer prototype. Make the
>> > function pointer protoype match the definitions so that there is no
>> > more violation.
>> >
>> > Cc: [email protected]
>>
>> Why does this matter for stable kernels, given that CFI is not in any
>> kernel tree yet?
>>
>> thanks,
>>
>> greg k-h
>
> It might not be available upstream but it is in all downstream Android
> kernels. Furthermore, all of the previous CFI fixes I have done have

If we were to accept patches in stable because some downstream kernel
needs it even though the feature isn't in upstream, Greg would have a
hard time sorting through all the patches :-)

I think this falls into the category of "downstream folks can manually
pick this into their tree".

> inevitably ended up in stable trees through AUTOSEL, I figured I would
> save Sasha the hassle this time around. It does not personally matter to
> me though, I am fine with stripping the tag since I do all of my
> personal testing with mainline/next so if this is needed in stable
> later due to an OEM or someone else tripping over it, it can just be
> added then.

Makes sense to me, thanks :-)

> Let me know if you want me to resend it without that tag.

Just applied to my testing/next without the stable tag.

--
balbi


Attachments:
signature.asc (847.00 B)