From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 84E5738F926 for ; Mon, 27 Jul 2026 15:48:30 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.133.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785167312; cv=none; b=FHhULtwLkoyGPoheG7V0K0rkyEFU0Yn2rwYlu1H8Jfz3XIcdgFqY1D999WN9e5fKEdUUn8OUyLPXYgjvQJ+0xrjTy/Xu1xnf/0q177u0g0PnfKHD9C+0SeE21oHHSwpsZTENFizbm4e6H8MfMVapipwxv456ynsKcKA4+su1IL8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785167312; c=relaxed/simple; bh=+CJf0I8kXRaqdu5m0kltaAJrd0vSOme9F6Z6+c1yD0A=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Kqubl3gWAZFZDGjHdtTPDalcN1bQ/3NWHyeXx1LjpFj4R6L/uYqhip1Wb1B2NVqbXbFr1BpHXvd6CmfaIsutRobIU720BShd05A5MNUYfOZ5rb2JvsN+oZyWiJg4h4iqNFg9ZGVLeJNHqLXtuPMnEhONYpwkZoFETQz9igFjaQ4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=Jyy6t23v; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=Rt5Oz6VB; arc=none smtp.client-ip=170.10.133.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="Jyy6t23v"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="Rt5Oz6VB" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1785167309; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=2HmQTvTcT7QkqefKZGOjPiGMZ/ZgGZCcwnmf7IF3fIU=; b=Jyy6t23vBJKx6/Yp6w3jaNaD0KX74HR6Wec+UDKh7yCGqUDXQ9ZeJuGKl5qcLkQxxuIBBD /yO3PPRVITdRHu59vehTkPNW30p9WLJZu2udCNsRgeeBBFwrjQb23mxfZjy7GANMFnNdwF koxfUJp9oLlyunIGTwYIXNuOIF5ZGkw= Received: from mail-qt1-f197.google.com (mail-qt1-f197.google.com [209.85.160.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-108-s9P4jiyiMdO3NhyIA2ms_Q-1; Mon, 27 Jul 2026 11:48:28 -0400 X-MC-Unique: s9P4jiyiMdO3NhyIA2ms_Q-1 X-Mimecast-MFC-AGG-ID: s9P4jiyiMdO3NhyIA2ms_Q_1785167307 Received: by mail-qt1-f197.google.com with SMTP id d75a77b69052e-51c1a9764f0so37361981cf.1 for ; Mon, 27 Jul 2026 08:48:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1785167307; x=1785772107; darn=vger.kernel.org; h=user-agent:in-reply-to:content-transfer-encoding :content-disposition:content-type:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to :content-type; bh=2HmQTvTcT7QkqefKZGOjPiGMZ/ZgGZCcwnmf7IF3fIU=; b=Rt5Oz6VB9qJ9iuUBjC8XECJRLdojF+vBJykdYiT0z2twBKBt68aY9nhzIiewlRs39z YrD08Oc7estv/G1+sjcw5C2YQjJ1fwXnRCK2I/VzxvgkwjTNFFpYEAGEWm1GCZ4KAKUV 3s12i3p1OclSOwotvg/iDSilBtc1VDstg2v8+2EOpCt1TgRfva1AFQIhiMajJ4RXSo+z IkHWlqhC6Qdr+cFi/5g1W3lP7ZKL8Dg19oVOZYCv1Y/jiUmghWJxl753kjPD9OxhfVo+ fMzxKXkPPwyJvJIPXUUR4Wo/vJf+G8izky5NqyNNuRmpOSsIGuM/Vvvh7B3xZCHDej0g LWXQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1785167307; x=1785772107; h=user-agent:in-reply-to:content-transfer-encoding :content-disposition:content-type:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to:content-type; bh=2HmQTvTcT7QkqefKZGOjPiGMZ/ZgGZCcwnmf7IF3fIU=; b=f48iiG16b+H085FatW0YhLFSEDsYtmV3/4tNWpvh0qEA8SElSP8h/3ub8nYQSDZv/G /vengnLkMJIbKfB8iIY3u3WnynEpvLHOg2wHjHfmA2hP+lFnp7zGEsxBzlqLRZgOE/wW GJK6514vmNN8x85yzutC+Ua8W0vHmEW5TGZmrOv7b8wRMq7/z1ErE+uE5uMn7oQvXr7O fOaqefsoWWAOxcLtUHZmQVgNKBZOpZuV7pgeb1dNlKvgAgpV+DOrZXDfbD8jQ6j7As6Z IkCtOb0s28Af3xgAZMnJfrGmkSgnLQxCt1puFWkg2gPhN8/R+bABCpRQB6aUb1wHOxya hpOQ== X-Forwarded-Encrypted: i=1; AHgh+RoVbEs8Qo5IR5qNpL0lvLqS+lsU54MISdasw3szhtQNbveXSx84HxroMnQZgcn1UiedHxFDTGgs+UVwyQU=@vger.kernel.org X-Gm-Message-State: AOJu0YzYoICry+oBZ00v9KxEr0Ap6r6CdPtWGkQokcxWH5fzuGjwXeBe PmH7fOarJJ3+ovtn3KtkUlv2/Qu856I3pjdP/3jlUnO5k8dN+1LwuMFVcZ9htEOL2n0+Diq3pr+ A1D1JF1Xg2qKfwdQH3inVQJD8LLJ5wJoxCJjCRUHMqMEFNfhfsNdRCv4tE9S4N1Qy8w== X-Gm-Gg: AR+sD114k9O9ks4JwOtLLdyYojBNr98covax8hi2hgaZD9N8uAxi4kzL0XaKT5MeTY1 u2oPcQ/srdxY5QaG+p05dAb8UR40aduxxv+wg3i8Sotp4sucg0cYwyAbuBgSmaQy3bwChGHL5Ko hPKMpcJ0x8bRHjtNOBsRw7AjKskE21h9Lnaktr+q49747un5FIhnIuOa7Dulc/k3eJfelXvSpht 9xj9BWvb8nWVuSwQFbOV5T7HDUMrBnsQ4CHM0hi2FIa50i/IOWbkKsE0XSiyXW17ZDCtF54pA2p qmqyy1sXxAn4vTZWSiZCnkM5IGLIQOFDtB2K+hic7Rdcy7c/bMd+nI1Fw+s+vpnpR830FFyFGQl hGzV1IzAaJ1RlPzlY5NLkRmq/V6oc9VRQL5E= X-Received: by 2002:ac8:7fc2:0:b0:51e:49d7:771f with SMTP id d75a77b69052e-529a864d795mr93032471cf.57.1785167307489; Mon, 27 Jul 2026 08:48:27 -0700 (PDT) X-Received: by 2002:ac8:7fc2:0:b0:51e:49d7:771f with SMTP id d75a77b69052e-529a864d795mr93032061cf.57.1785167307010; Mon, 27 Jul 2026 08:48:27 -0700 (PDT) Received: from redhat.com (c-73-183-53-213.hsd1.pa.comcast.net. [73.183.53.213]) by smtp.gmail.com with ESMTPSA id d75a77b69052e-529a29884e7sm55881501cf.17.2026.07.27.08.48.24 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 27 Jul 2026 08:48:25 -0700 (PDT) Date: Mon, 27 Jul 2026 11:48:22 -0400 From: Brian Masney To: Sebastian Reichel Cc: Praveen Talari , bjorn.andersson@oss.qualcomm.com, Michael Turquette , Stephen Boyd , konrad.dybcio@oss.qualcomm.com, mukesh.savaliya@oss.qualcomm.com, linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org, chandana.chiluveru@oss.qualcomm.com Subject: Re: [PATCH] clk: Guard clk_round_rate() against error pointers Message-ID: References: <20260723-fix_ptr_check_on_clk-v1-1-568a7ed87746@oss.qualcomm.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/2.4.0 (2026-06-19) On Fri, Jul 24, 2026 at 11:40:19PM +0200, Sebastian Reichel wrote: > On Fri, Jul 24, 2026 at 11:44:55AM -0400, Brian Masney wrote: > > On Thu, Jul 23, 2026 at 10:10:06PM +0530, Praveen Talari wrote: > > > On 23-07-2026 19:59, Brian Masney wrote: > > > > On Thu, Jul 23, 2026 at 11:40:47AM +0530, Praveen Talari wrote: > > > > > clk_round_rate() only checks for a NULL clk pointer before > > > > > dereferencing it, but callers such as dev_pm_opp_set_rate() can pass > > > > > it an error pointer (e.g. ERR_PTR(-ENOENT) left behind by > > > > > clk_get() when a device has no Linux clock and is instead managed by > > > > > firmware via a genpd/OPP performance domain). > > > > > > > > > > Dereferencing that error pointer to read clk->exclusive_count > > > > > crashes with an unhandled kernel NULL pointer dereference, since > > > > > ERR_PTR(-ENOENT) plus the field's offset lands on a small, unmapped > > > > > address: > > > > > > > > > > Unable to handle kernel NULL pointer dereference at virtual > > > > > address 000000000000002e > > > > > ... > > > > > pc : clk_round_rate+0x3c/0x188 > > > > > ... > > > > > Call trace: > > > > > clk_round_rate+0x3c/0x188 (P) > > > > > dev_pm_opp_set_rate+0x114/0x33c > > > > > > > > > > Change the guard from "if (!clk)" to "if (IS_ERR_OR_NULL(clk))", > > > > > matching the pattern already used by other clk consumer API > > > > > functions such as clk_unprepare(), so an error pointer is rejected > > > > > the same way a NULL pointer is. > > > > > > > > > > Signed-off-by: Praveen Talari > > > > > --- > > > > > drivers/clk/clk.c | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > > > > > index 048adfa86a5d..8c1ad3d10284 100644 > > > > > --- a/drivers/clk/clk.c > > > > > +++ b/drivers/clk/clk.c > > > > > @@ -1780,7 +1780,7 @@ long clk_round_rate(struct clk *clk, unsigned long rate) > > > > > struct clk_rate_request req; > > > > > int ret; > > > > > - if (!clk) > > > > > + if (IS_ERR_OR_NULL(clk)) > > > > Can you provide more details about the clk_get() call point that starts this > > > > error? Specifically which driver this occurs in and the exact scenario that > > > > triggers this. > > > > > > On SA8255P platform there is no Linux > > > clock for the SE, and the perf domain device's OPPs are populated entirely > > > from firmware via devm_pm_opp_of_add_table() (through > > > of_genpd_add_provider_simple()/onecell()), so the perf domain's OPP table > > > has entries even though no clk_get() ever succeeds for it. > > > > > > The clk_get(-ENOENT) case comes from _update_opp_table_clk() in > > > drivers/opp/core.c: > > > > > >     opp_table->clk = clk_get(dev, NULL); > > >     ret = PTR_ERR_OR_ZERO(opp_table->clk); > > >     ... > > >     if (ret == -ENOENT) { > > >         /* ... no clk provided ... */ > > >         opp_table->clk_count = 1; > > >         return opp_table;   /* opp_table->clk left as ERR_PTR(-ENOENT) */ > > >     } > > > > > > Because the perf domain device has no "clocks" property (its OPPs are > > > supplied purely as performance states by firmware/genpd), clk_get() > > > returns -ENOENT, and opp_table->clk is left holding that error pointer > > > rather than being reset to NULL. > > > > This looks to be a reasonable change. Thanks. > > > > Reviewed-by: Brian Masney > > I suggest to instead change the OPP code, so that it calls > clk_get_optional() instead of clk_get() and thus properly > "document" that the clock is optional and use the NULL dummy > clock (it's also shorter): > > /* > * There are few platforms which don't want the OPP core to > * manage device's clock settings. In such cases neither the > * platform provides the clks explicitly to us, nor the DT > * contains a valid clk entry. The OPP nodes in DT may still > * contain "opp-hz" property though, which we need to parse and > * allow the platform to find an OPP based on freq later on. > * > * This is a simple solution to take care of such corner cases, > * i.e. make the clk_count 1, which lets us allocate space for > * frequency in opp->rates and also parse the entries in DT. > */ > opp_table->clk = clk_get_optional(dev, NULL); > > ret = PTR_ERR_OR_ZERO(opp_table->clk); > if (ret) { > dev_pm_opp_put_opp_table(opp_table); > dev_err_probe(dev, ret, "Couldn't find clock\n"); > return ERR_PTR(ret); > } > > if (opp_table->clk) > opp_table->config_clks = _opp_config_clk_single; > opp_table->clk_count = 1; > return opp_table; Yes, that makes sense. Thanks! Brian