linux-kernel - Re: [PATCH v1] mailbox: tegra-hsp: Fix null-ptr-deref in tegra_hsp_doorbell

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 for Android: free password hash cracker in your pocket

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

Message-ID: <s65c46x3a3jltt3nfnuop6oehsrduw6g6bdacbcugrbsy3fsdi@65xyv23uxuqd>
Date: Thu, 8 May 2025 23:54:19 +0200
From: Thierry Reding <thierry.reding@...il.com>
To: Henry Martin <bsdhenrymartin@...il.com>
Cc: jassisinghbrar@...il.com, jonathanh@...dia.com, 
	linux-kernel@...r.kernel.org, linux-tegra@...r.kernel.org
Subject: Re: [PATCH v1] mailbox: tegra-hsp: Fix null-ptr-deref in
 tegra_hsp_doorbell_create()

On Wed, Apr 02, 2025 at 10:41:15PM +0800, Henry Martin wrote:
> devm_kstrdup_const() returns NULL when memory allocation fails.
> Currently, tegra_hsp_doorbell_create() does not check for this case,
> which results in a NULL pointer dereference.
> 
> Fixes: a54d03ed01b4 ("mailbox: tegra-hsp: use devm_kstrdup_const()")
> Signed-off-by: Henry Martin <bsdhenrymartin@...il.com>
> ---
>  drivers/mailbox/tegra-hsp.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c
> index ed9a0bb2bcd8..147406149fec 100644
> --- a/drivers/mailbox/tegra-hsp.c
> +++ b/drivers/mailbox/tegra-hsp.c
> @@ -293,6 +293,8 @@ tegra_hsp_doorbell_create(struct tegra_hsp *hsp, const char *name,
>  	db->channel.hsp = hsp;
>  
>  	db->name = devm_kstrdup_const(hsp->dev, name, GFP_KERNEL);
> +	if (!db->name)
> +		return ERR_PTR(-ENOMEM);

I don't think this is needed. First and foremost, db->name ends up not
being used. It was meant to be used by debug code that never ended up
being written, so at this point it's mostly here as a way to document
what the doorbell mapping is (though even that's somewhat redundant
since we already have macros that match the strings).

Secondly, these strings always come from tegra186_hsp_db_map, which is
rodata and so the allocation path should never be taken, and hence the
allocation can never fail.

So instead of trying to fix a non-existent problem we have two other
options: one is to remove all traces of db->name (as well as the string
in the mapping table), or we turn this into an assignment since we know
that it's always rodata, so there's no need to copy it.

Alternatively we could just leave this as-is. But then it's just a
matter of time before someone else comes around to "fix" the same thing.

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)