linux-kernel - Re: [PATCH 2/2] firmware: qcom: qcom

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0c91b95f-1945-4c9d-a119-fac5dc4f5661@quicinc.com>
Date: Tue, 8 Oct 2024 00:53:33 +0530
From: Kuldeep Singh <quic_kuldsing@...cinc.com>
To: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>,
        Bjorn Andersson
	<andersson@...nel.org>
CC: Konrad Dybcio <konradybcio@...nel.org>, <linux-arm-msm@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/2] firmware: qcom: qcom_tzmem: Implement sanity checks


On 10/7/2024 7:53 PM, Bartosz Golaszewski wrote:
> On Mon, 7 Oct 2024 at 03:18, Bjorn Andersson <andersson@...nel.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