[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3144d7cd-0b17-6236-5575-cfbb1a71d1ab@gmail.com>
Date: Wed, 8 Mar 2023 16:09:49 +0100
From: Maximilian Luz <luzmaximilian@...il.com>
To: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
Bjorn Andersson <andersson@...nel.org>
Cc: Andy Gross <agross@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
Ard Biesheuvel <ardb@...nel.org>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Johan Hovold <johan@...nel.org>,
Sudeep Holla <sudeep.holla@....com>,
Ilias Apalodimas <ilias.apalodimas@...aro.org>,
Sumit Garg <sumit.garg@...aro.org>,
Steev Klimaszewski <steev@...i.org>,
linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/4] firmware: qcom_scm: Export SCM call functions
On 3/8/23 15:20, Srinivas Kandagatla wrote:
>
>
> On 08/03/2023 13:48, Maximilian Luz wrote:
>> On 3/8/23 13:53, Srinivas Kandagatla wrote:
>>>
>>>
>>> On 07/03/2023 15:23, Dmitry Baryshkov wrote:
>>>>
>>>>> Make qcom_scm_call, qcom_scm_call_atomic and associated types accessible
>>>>> to other modules.
>>>>
>>>> Generally all the qcom_scm calls are a part of qcom_scm.c. I think it is better to make qseecom_scm_call a part qcom_scm.c (as we were previously doing) rather than exporting the core function.
>>>>
>>>
>>> Other big issue I see in exporting qcom_scm_call() is that there is danger of misuse of this api as this could lead to a path where new apis and its payloads can come directly from userspace via a rogue/hacking modules. This will bypass scm layer completely within kernel.
>>
>> I'm not sure I follow your argument here. If you have the possibility to
>> load your own kernel modules, can you not always bypass the kernel and
>> just directly invoke the respective SCM calls manually? So this is
>> superficial security at best.
> qcom_scm_call() will expose a much bigger window where the user can add new SCM APIs but with the current model of exporting symbols at SCM API level will narrow that down to that API.
>
>>
>> I guess keeping it in qcom_scm could make it easier to spot new
>> in-kernel users of that function and with that better prevent potential
>> misuse in the kernel itself. But then again I'd hope that our review
>> system is good enough to catch such issues regardless and thoroughly
>> question calls to that function (especially ones involving user-space
>> APIs).
>
> One problem I can immediately see here is the facility that will be exploited and promote more development outside upstream.
>
> ex: vendor modules with GKI compliance.
Fair point.
I still believe that squashing everything into a single driver is not
particularly great code/architecture style, but then again I'm not a fan of
vendors not integrating their stuff upstream either.
I guess we should be able to find something that has sufficiently low
complexity so that it can be implemented in qcom_scm while also preventing
external use like that.
Regards,
Max
>
> --srini
>>
>> Regards,
>> Max
>>
>>>
>>> --srini
>>>
>>>> If you wish to limit the kernel bloat, you can split the qcom_scm into per-driver backend and add Kconfig symbols to limit the impact. However I think that these functions are pretty small to justify the effort.
>>>>
>>>
>>>
>>>>>
>>>>> Signed-off-by: Maximilian Luz <luzmaximilian@...il.com>
Powered by blists - more mailing lists