Message-ID: <afee5468-6c73-d088-e3ab-e9314492e49b@linaro.org>
Date:   Wed, 8 Mar 2023 14:20:32 +0000
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH v3 1/4] firmware: qcom_scm: Export SCM call functions
Content-Language: en-US
To:     Maximilian Luz <luzmaximilian@gmail.com>,
        Dmitry Baryshkov <dmitry.baryshkov@linaro.org>,
        Bjorn Andersson <andersson@kernel.org>
Cc:     Andy Gross <agross@kernel.org>,
        Konrad Dybcio <konrad.dybcio@linaro.org>,
        Ard Biesheuvel <ardb@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Johan Hovold <johan@kernel.org>,
        Sudeep Holla <sudeep.holla@arm.com>,
        Ilias Apalodimas <ilias.apalodimas@linaro.org>,
        Sumit Garg <sumit.garg@linaro.org>,
        Steev Klimaszewski <steev@kali.org>,
        linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20230305022119.1331495-1-luzmaximilian@gmail.com>
 <20230305022119.1331495-2-luzmaximilian@gmail.com>
 <a2c97f09-3360-b2b1-184a-8e3b869a70ef@linaro.org>
 <615d907e-fd7c-f235-405b-d112f1373280@linaro.org>
 <81468734-d25e-ddda-7bd1-1498ca6fa6ae@gmail.com>
From:   Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
In-Reply-To: <81468734-d25e-ddda-7bd1-1498ca6fa6ae@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk


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.


--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@gmail.com>