All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Boyd <sboyd@kernel.org>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	linux-clk@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, mturquette@baylibre.com,
	Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Subject: Re: [PATCH RFC v1] clk: Fix potential NULL dereference in clk_fetch_parent_index()
Date: Thu, 15 Aug 2019 16:29:50 -0700	[thread overview]
Message-ID: <20190815232951.AA402206C2@mail.kernel.org> (raw)
In-Reply-To: <20190815223155.21384-1-martin.blumenstingl@googlemail.com>

Quoting Martin Blumenstingl (2019-08-15 15:31:55)
> Don't compare the parent clock name with a NULL name in the
> clk_parent_map. This prevents a kernel crash when passing NULL
> core->parents[i].name to strcmp().
> 
> An example which triggered this is a mux clock with four parents when
> each of them is referenced in the clock driver using
> clk_parent_data.fw_name and then calling clk_set_parent(clk, 3rd_parent)
> on this mux.
> In this case the first parent is also the HW default so
> core->parents[i].hw is populated when the clock is registered. Calling
> clk_set_parent(clk, 3rd_parent) will then go through all parents and
> skip the first parent because it's hw pointer doesn't match. For the
> second parent no hw pointer is cached yet and clk_core_get(core, 1)
> returns a non-matching pointer (which is correct because we are comparing
> the second with the third parent). Comparing the result of
> clk_core_get(core, 2) with the requested parent gives a match. However
> we don't reach this point because right after the clk_core_get(core, 1)
> mismatch the old code tried to !strcmp(parent->name, NULL) (where the
> second argument is actually core->parents[i].name, but that was never
> populated by the clock driver).
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
> I have seen the original crash when I was testing an MMC driver which
> is not upstream yet on v5.3-rc4. I'm not sure whether this fix is
> "correct" (it fixes the crash for me) or where to point the Fixes tag
> to, it may be one of:
> - fc0c209c147f ("clk: Allow parents to be specified without string names")
> - 1a079560b145 ("clk: Cache core in clk_fetch_parent_index() without names")
> 
> This is meant to be applied on top of v5.3-rc4.
> 

Ah ok. I thought that strcmp() would ignore NULL arguments, but
apparently not. I can apply this to clk-fixes.


  reply	other threads:[~2019-08-15 23:29 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-15 22:31 [PATCH RFC v1] clk: Fix potential NULL dereference in clk_fetch_parent_index() Martin Blumenstingl
2019-08-15 23:29 ` Stephen Boyd [this message]
2019-08-16  6:48   ` Martin Blumenstingl
2019-08-16 17:31     ` Stephen Boyd
2019-08-19  8:43       ` Neil Armstrong
2019-08-19  8:43         ` Neil Armstrong
2019-08-16 17:30 ` Stephen Boyd

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190815232951.AA402206C2@mail.kernel.org \
    --to=sboyd@kernel.org \
    --cc=linux-clk@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.blumenstingl@googlemail.com \
    --cc=mturquette@baylibre.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.