| Message ID | 20200725060354.177009-1-natechancellor@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 362b9398c962c9ec563653444e15ef9032ef3a90 |
| Headers | show |
| Series | usb: dwc2: Fix parameter type in function pointer prototype | expand |
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: stable@vger.kernel.org Why does this matter for stable kernels, given that CFI is not in any kernel tree yet? thanks, greg k-h
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: stable@vger.kernel.org > > 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
Greg Kroah-Hartman <gregkh@linuxfoundation.org> 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: stable@vger.kernel.org > > Why does this matter for stable kernels, given that CFI is not in any > kernel tree yet? remove stable while applying to testing/next.
Hi, Nathan Chancellor <natechancellor@gmail.com> 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: stable@vger.kernel.org >> >> 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.
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);
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: stable@vger.kernel.org Fixes: 7de1debcd2de ("usb: dwc2: Remove platform static params") Link: https://github.com/ClangBuiltLinux/linux/issues/1107 Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> --- drivers/usb/dwc2/params.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) base-commit: 23ee3e4e5bd27bdbc0f1785eef7209ce872794c7