mbox series

[GIT,PULL] firmware: arm_scmi: Fixes for v5.19

Message ID 20220614100007.1029881-1-sudeep.holla@arm.com
State New
Headers show
Series [GIT,PULL] firmware: arm_scmi: Fixes for v5.19 | expand

Pull-request

git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git tags/scmi-fixes-5.19

Message

Sudeep Holla June 14, 2022, 10 a.m. UTC
Hi ARM SoC Team,

Please pull !

Regards,
Sudeep

-->8

The following changes since commit f2906aa863381afb0015a9eb7fefad885d4e5a56:

  Linux 5.19-rc1 (2022-06-05 17:18:54 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git tags/scmi-fixes-5.19

for you to fetch changes up to 44dbdf3bb3f44bf08897ed5f22eb262edcf3d926:

  firmware: arm_scmi: Fix incorrect error propagation in scmi_voltage_descriptors_get (2022-06-12 19:59:55 +0100)

----------------------------------------------------------------
Arm SCMI firmware driver fixes for v5.19

Bunch of fixes to address:
1. Issues reported on RK3568 EVB1 and BPI-R2 pro platforms using SCMI.
   More checks were added to validate the firmware response but that
   resulted in breaking above platforms, so the checks are relaxed when
   for cases where there is no potential memory corruption issues.

2. Possible data leak by reading more than required length from the firmware.
   Recent addition of support for v3.1 extended names used larger buffers
   in the kernel and used their size to read response from the firmware even
   for cases where shorter formats are used. While that is mostly harmless
   except when firmware sends malformed non-NULL terminated buffers.

3. Possible issues sending unsupported commands to the firmware.
   SENSOR_AXIS_NAME_GET added in v3.1 needs to be used only if the firmware
   supports it. While the firmware conformant to the spec must return not
   supported error for any unsupported features, it is always safer to
   avoid issuing commands that are known to be unsupported.

4. Incorrect error propagation in scmi_voltage_descriptors_get.
   Since the return value is not reset for each iteration of the loop, the
   error value in the previous iteration will be carried for the current one.
   Fix that by not saving the return values into local variable.

5. Some warnings reported by cppcheck

----------------------------------------------------------------
Cristian Marussi (4):
      firmware: arm_scmi: Relax base protocol sanity checks on the protocol list
      firmware: arm_scmi: Remove all the unused local variables
      firmware: arm_scmi: Fix SENSOR_AXIS_NAME_GET behaviour when unsupported
      firmware: arm_scmi: Avoid using extended string-buffers sizes if not necessary

Ludvig Pärsson (1):
      firmware: arm_scmi: Fix incorrect error propagation in scmi_voltage_descriptors_get

 drivers/firmware/arm_scmi/base.c      | 24 ++++++++-----
 drivers/firmware/arm_scmi/clock.c     |  7 ++--
 drivers/firmware/arm_scmi/perf.c      |  6 ++--
 drivers/firmware/arm_scmi/power.c     |  2 +-
 drivers/firmware/arm_scmi/protocols.h |  2 --
 drivers/firmware/arm_scmi/reset.c     |  2 +-
 drivers/firmware/arm_scmi/sensors.c   | 68 ++++++++++++++++++++++++++---------
 drivers/firmware/arm_scmi/voltage.c   | 15 +++-----
 include/linux/scmi_protocol.h         |  9 ++---
 9 files changed, 85 insertions(+), 50 deletions(-)

Comments

Florian Fainelli June 15, 2022, 3:42 a.m. UTC | #1
Hello,

On 6/14/2022 12:00 PM, Sudeep Holla wrote:
> Hi ARM SoC Team,
> 
> Please pull !
> 
> Regards,
> Sudeep
> 
> -->8
> 
> The following changes since commit f2906aa863381afb0015a9eb7fefad885d4e5a56:
> 
>    Linux 5.19-rc1 (2022-06-05 17:18:54 -0700)
> 
> are available in the Git repository at:
> 
>    git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git tags/scmi-fixes-5.19
> 
> for you to fetch changes up to 44dbdf3bb3f44bf08897ed5f22eb262edcf3d926:
> 
>    firmware: arm_scmi: Fix incorrect error propagation in scmi_voltage_descriptors_get (2022-06-12 19:59:55 +0100)

Looks like "firmware: arm_scmi: Add SCMI v3.1 protocol extended names 
support" introduces the following splat which is not fixed by the 
commits in this pull request:

[    0.603737] ------------[ cut here ]------------
[    0.603752] WARNING: CPU: 1 PID: 1 at mm/page_alloc.c:5402 
__alloc_pages+0x6c/0x184
[    0.603797] Modules linked in:
[    0.603809] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 
5.19.0-rc1-g44dbdf3bb3f4 #42
[    0.603818] Hardware name: BCX972160SV (DT)
[    0.603825] pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS 
BTYPE=--)
[    0.603834] pc : __alloc_pages+0x6c/0x184
[    0.603841] lr : kmalloc_order+0x40/0x88
[    0.603851] sp : ffffffc00a40b850
[    0.603856] x29: ffffffc00a40b850 x28: 0000000000000000 x27: 
ffffffc008d60404
[    0.603867] x26: ffffff80c1e3e1a8 x25: ffffffc00877bd78 x24: 
0000000000000058
[    0.603878] x23: ffffffc0081921a8 x22: ffffffc008cb04b0 x21: 
0000000000000000
[    0.603889] x20: 000000000000000b x19: 000000000000000b x18: 
0000000000000000
[    0.603900] x17: 0000000000000001 x16: 0000000100000000 x15: 
000000000000000a
[    0.603911] x14: 0000000000000000 x13: ffffff80c1e3c20a x12: 
ffffffffffffffff
[    0.603922] x11: 0000000000000020 x10: 0000000000000880 x9 : 
ffffffc008159dac
[    0.603932] x8 : ffffff80c02708e0 x7 : 0000000000000004 x6 : 
000000000041a880
[    0.603943] x5 : 0000000000000001 x4 : ffffff8000000000 x3 : 
0000000000000000
[    0.603954] x2 : 0000000000000000 x1 : 0000000000000001 x0 : 
ffffffc00a32d3f2
[    0.603965] Call trace:
[    0.603970]  __alloc_pages+0x6c/0x184
[    0.603977]  kmalloc_order+0x40/0x88
[    0.603984]  kmalloc_order_trace+0x30/0xd0
[    0.603992]  __kmalloc_track_caller+0x64/0x19c
[    0.603999]  devm_kmalloc+0x5c/0xe0
[    0.604009]  scmi_voltage_protocol_init+0x14c/0x2f4
[    0.604020]  scmi_get_protocol_instance+0x128/0x1f4
[    0.604030]  scmi_devm_protocol_get+0x64/0xc8
[    0.604037]  scmi_regulator_probe+0x5c/0x42c
[    0.604049]  scmi_dev_probe+0x28/0x38
[    0.604056]  really_probe+0x1b8/0x380
[    0.604065]  __driver_probe_device+0x14c/0x164
[    0.604073]  driver_probe_device+0x48/0xe0
[    0.604080]  __driver_attach+0x160/0x170
[    0.604087]  bus_for_each_dev+0x78/0xb8
[    0.604095]  driver_attach+0x28/0x30
[    0.604101]  bus_add_driver+0xf4/0x208
[    0.604108]  driver_register+0xb4/0xf0
[    0.604116]  scmi_driver_register+0x5c/0xa4
[    0.604123]  scmi_drv_init+0x28/0x30
[    0.604132]  do_one_initcall+0x80/0x1a4
[    0.604141]  kernel_init_freeable+0x220/0x23c
[    0.604149]  kernel_init+0x28/0x128
[    0.604158]  ret_from_fork+0x10/0x20
[    0.604166] ---[ end trace 0000000000000000 ]---
[    0.604194] scmi-regulator: probe of scmi_dev.2 failed with error -12
[    0.604792] arm-scmi brcm_scmi@0: Failed. SCMI protocol 22 not active.
Sudeep Holla June 15, 2022, 12:29 p.m. UTC | #2
Hi Florian,

On Wed, Jun 15, 2022 at 05:42:10AM +0200, Florian Fainelli wrote:
> Hello,
> 
> On 6/14/2022 12:00 PM, Sudeep Holla wrote:
> > Hi ARM SoC Team,
> > 
> > Please pull !
> > 
> > Regards,
> > Sudeep
> > 
> > -->8
> > 
> > The following changes since commit f2906aa863381afb0015a9eb7fefad885d4e5a56:
> > 
> >    Linux 5.19-rc1 (2022-06-05 17:18:54 -0700)
> > 
> > are available in the Git repository at:
> > 
> >    git://git.kernel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git tags/scmi-fixes-5.19
> > 
> > for you to fetch changes up to 44dbdf3bb3f44bf08897ed5f22eb262edcf3d926:
> > 
> >    firmware: arm_scmi: Fix incorrect error propagation in scmi_voltage_descriptors_get (2022-06-12 19:59:55 +0100)
> 
> Looks like "firmware: arm_scmi: Add SCMI v3.1 protocol extended names
> support" introduces the following splat which is not fixed by the commits in
> this pull request:

Thanks for the report, we weren't even aware of the issue. We can discussed
in the original patch thread as you have also reported the issue there.
Cristian is unable to reproduce it yet, it would be great if you can provide
more info to help us figure out the issue and the fix 🤞.