From nobody Thu Nov 28 19:40:33 2024 Received: from mx0b-0031df01.pphosted.com (mx0b-0031df01.pphosted.com [205.220.180.131]) (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 109C713A3F2; Fri, 27 Sep 2024 17:42:43 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=205.220.180.131 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1727458966; cv=none; b=FQBEedGfgkRFPhNsoxQoKi2mV0krPSTkFGE3TsR9PoEWbZYKm7QIPqOvspYqP9kX2FfdmSb0SCudvLEWE54cLxWVUUEF3klfDPJtYRZlowcxx2Iw5ijKHHh9XhrCxhZYNpcgUGdjvc72HZQy/muiFs3+6w2uj43h+D3D/OK2KpE= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1727458966; c=relaxed/simple; bh=jxJATjGadpFl+gjLHfMEyMWwhsn6ZTT4U55ty2wMspA=; h=From:To:CC:Subject:Date:Message-ID:MIME-Version:Content-Type; b=MNHT0R1KWoxyEjXkUXXE9wgTQFOK1ybK1+admVeKigXKoI8cBPb9PUmyzurisvH/qOll2BqdYnpF4flfGUkn+Ukq/1hNwee0bRjNXWFEqx+jyaDC+M0Jlowks3nFcQwuIpkCpnwklmSpNYU5qhLcSouv7L9OmjeIQibaMawPBeY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=quicinc.com; spf=pass smtp.mailfrom=quicinc.com; dkim=pass (2048-bit key) header.d=quicinc.com header.i=@quicinc.com header.b=SlJQ5AuG; arc=none smtp.client-ip=205.220.180.131 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=quicinc.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=quicinc.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=quicinc.com header.i=@quicinc.com header.b="SlJQ5AuG" Received: from pps.filterd (m0279871.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.18.1.2/8.18.1.2) with ESMTP id 48RGJcqR003391; Fri, 27 Sep 2024 17:42:42 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h= cc:content-transfer-encoding:content-type:date:from:message-id :mime-version:subject:to; s=qcppdkim1; bh=O6YxYKxn18vEyjbEp2AMjt M+bqc46UqiCZ6LMbJplSc=; b=SlJQ5AuGGUF0VnKR/+m9WplivI76enCmUFjgr2 wJOftj6ZeYqL4Kg4KJ5dQFebSK+M/alx7Mf7Dd9OSd5FMPFCX4f8d27ulAGao0Zu nKU85PMjp2BecDh/GGGn+p4inZxx7NH0w+J7t5BlIc8egRxiOCo7QfrfWdUhBiRN hLXQvZh/Rc0K/gAIEfNp5dglagOARKbUnMgJ2lXaNcxw8rc6I2BBmp9F5D/N5SG4 AuExRqyzjzhfzyMxPZiKzdWQYEFMZ84cKI3ELPsNJ6mI6I7N3O+wnGGywZecuTTx raZKo1loFFn51pYcAg65V/NIpFxhcLr1NcRNyasyzIZ7Cyyg== Received: from nasanppmta02.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 41snqyusdq-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 27 Sep 2024 17:42:41 +0000 (GMT) Received: from nasanex01c.na.qualcomm.com (nasanex01c.na.qualcomm.com [10.45.79.139]) by NASANPPMTA02.qualcomm.com (8.18.1.2/8.18.1.2) with ESMTPS id 48RHge8E019621 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 27 Sep 2024 17:42:40 GMT Received: from hu-mojha-hyd.qualcomm.com (10.80.80.8) by nasanex01c.na.qualcomm.com (10.45.79.139) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.1544.9; Fri, 27 Sep 2024 10:42:38 -0700 From: Mukesh Ojha To: Linus Walleij CC: , , Mukesh Ojha Subject: [PATCH v2] pinmux: Use sequential access to access desc->pinmux data Date: Fri, 27 Sep 2024 23:12:06 +0530 Message-ID: <20240927174206.602651-1-quic_mojha@quicinc.com> X-Mailer: git-send-email 2.34.1 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable X-ClientProxiedBy: nasanex01a.na.qualcomm.com (10.52.223.231) To nasanex01c.na.qualcomm.com (10.45.79.139) X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-GUID: KPWCJgXR58yyV1u3TNmD9ksJgEAynwIf X-Proofpoint-ORIG-GUID: KPWCJgXR58yyV1u3TNmD9ksJgEAynwIf X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.680,FMLib:17.12.60.29 definitions=2024-09-06_09,2024-09-06_01,2024-09-02_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 impostorscore=0 adultscore=0 bulkscore=0 phishscore=0 mlxlogscore=999 spamscore=0 lowpriorityscore=0 clxscore=1015 mlxscore=0 malwarescore=0 suspectscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.19.0-2408220000 definitions=main-2409270128 Content-Type: text/plain; charset="utf-8" When two client of the same gpio call pinctrl_select_state() for the same functionality, we are seeing NULL pointer issue while accessing desc->mux_owner. Let's say two processes A, B executing in pin_request() for the same pin and process A updates the desc->mux_usecount but not yet updated the desc->mux_owner while process B see the desc->mux_usecount which got updated by A path and further executes strcmp and while accessing desc->mux_owner it crashes with NULL pointer. Serialize the access to mux related setting with a spin lock. cpu0 (process A) cpu1(process B) pinctrl_select_state() { pinctrl_select_state() { pin_request() { pin_request() { ... .... } else { desc->mux_usecount++; desc->mux_usecount && strcmp(desc->mux_owner, owner)) { if (desc->mux_usecount > 1) return 0; desc->mux_owner =3D owner; } } Signed-off-by: Mukesh Ojha --- Changes in v2: - Used scoped_guard and renamed lock name as per suggestion from Linus.W . drivers/pinctrl/core.c | 3 + drivers/pinctrl/core.h | 2 + drivers/pinctrl/pinmux.c | 150 +++++++++++++++++++++------------------ 3 files changed, 86 insertions(+), 69 deletions(-) diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c index 4061890a1748..b00911421cf5 100644 --- a/drivers/pinctrl/core.c +++ b/drivers/pinctrl/core.c @@ -220,6 +220,9 @@ static int pinctrl_register_one_pin(struct pinctrl_dev = *pctldev, =20 /* Set owner */ pindesc->pctldev =3D pctldev; +#ifdef CONFIG_PINMUX + spin_lock_init(&pindesc->mux_lock); +#endif =20 /* Copy basic pin info */ if (pin->name) { diff --git a/drivers/pinctrl/core.h b/drivers/pinctrl/core.h index 4e07707d2435..179e01dfacc2 100644 --- a/drivers/pinctrl/core.h +++ b/drivers/pinctrl/core.h @@ -12,6 +12,7 @@ #include #include #include +#include #include =20 #include @@ -177,6 +178,7 @@ struct pin_desc { const char *mux_owner; const struct pinctrl_setting_mux *mux_setting; const char *gpio_owner; + spinlock_t mux_lock; #endif }; =20 diff --git a/drivers/pinctrl/pinmux.c b/drivers/pinctrl/pinmux.c index 02033ea1c643..e4d535aabbb6 100644 --- a/drivers/pinctrl/pinmux.c +++ b/drivers/pinctrl/pinmux.c @@ -14,6 +14,7 @@ =20 #include #include +#include #include #include #include @@ -127,29 +128,31 @@ static int pin_request(struct pinctrl_dev *pctldev, dev_dbg(pctldev->dev, "request pin %d (%s) for %s\n", pin, desc->name, owner); =20 - if ((!gpio_range || ops->strict) && - desc->mux_usecount && strcmp(desc->mux_owner, owner)) { - dev_err(pctldev->dev, - "pin %s already requested by %s; cannot claim for %s\n", - desc->name, desc->mux_owner, owner); - goto out; - } + scoped_guard(spinlock_irqsave, &desc->mux_lock) { + if ((!gpio_range || ops->strict) && + desc->mux_usecount && strcmp(desc->mux_owner, owner)) { + dev_err(pctldev->dev, + "pin %s already requested by %s; cannot claim for %s\n", + desc->name, desc->mux_owner, owner); + goto out; + } =20 - if ((gpio_range || ops->strict) && desc->gpio_owner) { - dev_err(pctldev->dev, - "pin %s already requested by %s; cannot claim for %s\n", - desc->name, desc->gpio_owner, owner); - goto out; - } + if ((gpio_range || ops->strict) && desc->gpio_owner) { + dev_err(pctldev->dev, + "pin %s already requested by %s; cannot claim for %s\n", + desc->name, desc->gpio_owner, owner); + goto out; + } =20 - if (gpio_range) { - desc->gpio_owner =3D owner; - } else { - desc->mux_usecount++; - if (desc->mux_usecount > 1) - return 0; + if (gpio_range) { + desc->gpio_owner =3D owner; + } else { + desc->mux_usecount++; + if (desc->mux_usecount > 1) + return 0; =20 - desc->mux_owner =3D owner; + desc->mux_owner =3D owner; + } } =20 /* Let each pin increase references to this module */ @@ -178,12 +181,14 @@ static int pin_request(struct pinctrl_dev *pctldev, =20 out_free_pin: if (status) { - if (gpio_range) { - desc->gpio_owner =3D NULL; - } else { - desc->mux_usecount--; - if (!desc->mux_usecount) - desc->mux_owner =3D NULL; + scoped_guard(spinlock_irqsave, &desc->mux_lock) { + if (gpio_range) { + desc->gpio_owner =3D NULL; + } else { + desc->mux_usecount--; + if (!desc->mux_usecount) + desc->mux_owner =3D NULL; + } } } out: @@ -223,11 +228,13 @@ static const char *pin_free(struct pinctrl_dev *pctld= ev, int pin, /* * A pin should not be freed more times than allocated. */ - if (WARN_ON(!desc->mux_usecount)) - return NULL; - desc->mux_usecount--; - if (desc->mux_usecount) - return NULL; + scoped_guard(spinlock_irqsave, &desc->mux_lock) { + if (WARN_ON(!desc->mux_usecount)) + return NULL; + desc->mux_usecount--; + if (desc->mux_usecount) + return NULL; + } } =20 /* @@ -239,13 +246,15 @@ static const char *pin_free(struct pinctrl_dev *pctld= ev, int pin, else if (ops->free) ops->free(pctldev, pin); =20 - if (gpio_range) { - owner =3D desc->gpio_owner; - desc->gpio_owner =3D NULL; - } else { - owner =3D desc->mux_owner; - desc->mux_owner =3D NULL; - desc->mux_setting =3D NULL; + scoped_guard(spinlock_irqsave, &desc->mux_lock) { + if (gpio_range) { + owner =3D desc->gpio_owner; + desc->gpio_owner =3D NULL; + } else { + owner =3D desc->mux_owner; + desc->mux_owner =3D NULL; + desc->mux_setting =3D NULL; + } } =20 module_put(pctldev->owner); @@ -608,40 +617,43 @@ static int pinmux_pins_show(struct seq_file *s, void = *what) if (desc =3D=3D NULL) continue; =20 - if (desc->mux_owner && - !strcmp(desc->mux_owner, pinctrl_dev_get_name(pctldev))) - is_hog =3D true; - - if (pmxops->strict) { - if (desc->mux_owner) - seq_printf(s, "pin %d (%s): device %s%s", - pin, desc->name, desc->mux_owner, + scoped_guard(spinlock_irqsave, &desc->mux_lock) { + if (desc->mux_owner && + !strcmp(desc->mux_owner, pinctrl_dev_get_name(pctldev))) + is_hog =3D true; + + if (pmxops->strict) { + if (desc->mux_owner) + seq_printf(s, "pin %d (%s): device %s%s", + pin, desc->name, desc->mux_owner, + is_hog ? " (HOG)" : ""); + else if (desc->gpio_owner) + seq_printf(s, "pin %d (%s): GPIO %s", + pin, desc->name, desc->gpio_owner); + else + seq_printf(s, "pin %d (%s): UNCLAIMED", + pin, desc->name); + } else { + /* For non-strict controllers */ + seq_printf(s, "pin %d (%s): %s %s%s", pin, desc->name, + desc->mux_owner ? desc->mux_owner + : "(MUX UNCLAIMED)", + desc->gpio_owner ? desc->gpio_owner + : "(GPIO UNCLAIMED)", is_hog ? " (HOG)" : ""); - else if (desc->gpio_owner) - seq_printf(s, "pin %d (%s): GPIO %s", - pin, desc->name, desc->gpio_owner); + } + + /* If mux: print function+group claiming the pin */ + if (desc->mux_setting) + seq_printf(s, " function %s group %s\n", + pmxops->get_function_name(pctldev, + desc->mux_setting->func), + pctlops->get_group_name(pctldev, + desc->mux_setting->group)); else - seq_printf(s, "pin %d (%s): UNCLAIMED", - pin, desc->name); - } else { - /* For non-strict controllers */ - seq_printf(s, "pin %d (%s): %s %s%s", pin, desc->name, - desc->mux_owner ? desc->mux_owner - : "(MUX UNCLAIMED)", - desc->gpio_owner ? desc->gpio_owner - : "(GPIO UNCLAIMED)", - is_hog ? " (HOG)" : ""); - } + seq_putc(s, '\n'); =20 - /* If mux: print function+group claiming the pin */ - if (desc->mux_setting) - seq_printf(s, " function %s group %s\n", - pmxops->get_function_name(pctldev, - desc->mux_setting->func), - pctlops->get_group_name(pctldev, - desc->mux_setting->group)); - else - seq_putc(s, '\n'); + } } =20 mutex_unlock(&pctldev->mutex); --=20 2.34.1