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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <CAEnQdOo5GD6TKO4rQG9q1KPbmFhoPrfMmsA1g8dySr=VsaOSJg@mail.gmail.com>
Date: Fri, 9 May 2025 14:58:27 +0800
From: henry martin <bsdhenrymartin@...il.com>
To: Thierry Reding <thierry.reding@...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()

Thierry Reding <thierry.reding@...il.com> 于2025年5月9日周五 05:54写道：
>
> 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.
>
Agreed. Since the strings are always from rodata, changing to direct
assignment makes more sense.

Regards,
Henry
> 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