Re: [PATCH 2/2] firmware: qcom: qcom_tzmem: Implement sanity checks

[Kuldeep Singh <quic_kuldsing@quicinc.com>]

On 10/7/2024 7:53 PM, Bartosz Golaszewski wrote:
> On Mon, 7 Oct 2024 at 03:18, Bjorn Andersson <andersson@kernel.org> wrote:
>>
>> On Sat, Oct 05, 2024 at 07:31:50PM GMT, Kuldeep Singh wrote:
>>> The qcom_tzmem driver currently has multiple exposed APIs that lack
>>> validations on input parameters. This oversight can lead to unexpected
>>> crashes due to null pointer dereference when incorrect inputs are
>>> provided.
>>>
>>> To address this issue, add required sanity for all input parameters in
>>> the exposed APIs.
>>>
>>
>> Unless there's good reason for the opposite, I rather see that we define
>> the API to only accept valid pointers. Then if a client passes a NULL we
>> get a oops with a nice callstack, which is easy to debug>>
>> The alternative is that we return -EINVAL, which not unlikely is
>> propagated to some application which may or may not result in a bug
>> report from a user - without any tangible information about where things
>> went wrong.

Discussing with Dmitry as well on other thread over same point.
Not all checks are needed but I believe some sanity is still needed to avoid crashes.

> 
> Agreed, I don't think this is a good pattern in a kernel API (as
> opposed to user-space interfaces where we validate everything). We
> expect a certain level of sanity from in-kernel users.
> 
> Bart

-- 
Regards
Kuldeep