From patchwork Tue Dec 24 13:37:20 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Zijun Hu X-Patchwork-Id: 13920120 Received: from pv50p00im-zteg10021401.me.com (pv50p00im-zteg10021401.me.com [17.58.6.47]) (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 CA03F1D79A6 for ; Tue, 24 Dec 2024 13:38:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=17.58.6.47 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735047491; cv=none; b=H7wWSw+J3cmQG1ifHGBgLFoFkYwWU8F49Vpg079ASstm8pzfUL5Vy+2vbD3PArFUNZooTQD4+JyQgv2Zy9YwO7fH0KeBfrMwCpXIrDr/8yhkgSmR/Ve16o4iKW6I3NcitS1R9738rzWUDLqs0VdZ+DwCKzAoNs+X/Xq6q0/i2n4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735047491; c=relaxed/simple; bh=8mn6t+QHjDaf7J6kc9SMk308hna52jlveGFlyTvTj3U=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:References: In-Reply-To:To:Cc; b=gXSU08HRC9OpKBnVp8gn9Cv4A+/nb/4LBVHV2tHA1pC2S/5EFfrKiNOVU1p3F5pOcAOYNtgIDwTmmFV3+iqqKg2nkukhc2n/VdZkd9GheNsnzLNrtaUqml8CZy8cdFU5yAO1aJtGa5LCUD5ApkRVTH7/FB/h/KsT9t5WZR8TvBY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=icloud.com; spf=pass smtp.mailfrom=icloud.com; dkim=pass (2048-bit key) header.d=icloud.com header.i=@icloud.com header.b=B/Z/8utH; arc=none smtp.client-ip=17.58.6.47 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=icloud.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=icloud.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=icloud.com header.i=@icloud.com header.b="B/Z/8utH" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=icloud.com; s=1a1hai; t=1735047489; bh=P8XwY4in8iMuG1/djQ6xOUQq3gJGLhNX0HGYx/Z+xcU=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:To: x-icloud-hme; b=B/Z/8utHrGZwwEic+ee9MMqCX12ODCZB6bTRKJ+pAKE0x2mdScRc20ZeJ/g01JZyj IT25QKU7Rdko2Eq+jPgjhThPjIoYdoZvY2Xa9/WuTSTFFx7YiyWqiwoMkU1MgFvdfp LCBl593HsLv61n7Y/5cY7O1xVG4Yj8LZsxNaEJmfr9Apkn0gwDwRP6oVHGA+GLHum9 gXYBzBd2r4MBwCS2mXLFMEKPRoPxKGPPKgA6+zaE3jhCnZSa05kTeM1onfDJzaIxao rmgqp5FR66Z4vkW0NVnUZ0JyWuLwVqfQqool4c/sY1/svURoSE8nOZvtSMqFjg2dvC pms6nuo7cltOg== Received: from [192.168.1.25] (pv50p00im-dlb-asmtp-mailmevip.me.com [17.56.9.10]) by pv50p00im-zteg10021401.me.com (Postfix) with ESMTPSA id 4D5348E041E; Tue, 24 Dec 2024 13:38:00 +0000 (UTC) From: Zijun Hu Date: Tue, 24 Dec 2024 21:37:20 +0800 Subject: [PATCH v5 1/8] driver core: class: Fix wild pointer dereferences in API class_dev_iter_next() Precedence: bulk X-Mailing-List: linux-block@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Message-Id: <20241224-class_fix-v5-1-9eaaf7abe843@quicinc.com> References: <20241224-class_fix-v5-0-9eaaf7abe843@quicinc.com> In-Reply-To: <20241224-class_fix-v5-0-9eaaf7abe843@quicinc.com> To: Greg Kroah-Hartman , "Rafael J. Wysocki" , Tejun Heo , Josef Bacik , Jens Axboe , Boris Burkov , Davidlohr Bueso , Jonathan Cameron , Dave Jiang , Alison Schofield , Vishal Verma , Ira Weiny , Dan Williams Cc: Zijun Hu , linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, linux-block@vger.kernel.org, linux-cxl@vger.kernel.org, Zijun Hu , Jonathan Cameron X-Mailer: b4 0.14.2 X-Proofpoint-GUID: IzoVIQ--U7gM-T2Mb4w_I5gy9epyeW2P X-Proofpoint-ORIG-GUID: IzoVIQ--U7gM-T2Mb4w_I5gy9epyeW2P X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.272,Aquarius:18.0.1057,Hydra:6.0.680,FMLib:17.12.68.34 definitions=2024-12-24_05,2024-12-24_01,2024-11-22_01 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 mlxlogscore=999 bulkscore=0 adultscore=0 suspectscore=0 malwarescore=0 mlxscore=0 spamscore=0 clxscore=1015 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.19.0-2308100000 definitions=main-2412240118 X-Apple-Remote-Links: v=1;h=KCk=;charset=UTF-8 From: Zijun Hu There are a potential wild pointer dereferences issue regarding APIs class_dev_iter_(init|next|exit)(), as explained by below typical usage: // All members of @iter are wild pointers. struct class_dev_iter iter; // class_dev_iter_init(@iter, @class, ...) checks parameter @class for // potential class_to_subsys() error, and it returns void type and does // not initialize its output parameter @iter, so caller can not detect // the error and continues to invoke class_dev_iter_next(@iter) even if // @iter still contains wild pointers. class_dev_iter_init(&iter, ...); // Dereference these wild pointers in @iter here once suffer the error. while (dev = class_dev_iter_next(&iter)) { ... }; // Also dereference these wild pointers here. class_dev_iter_exit(&iter); Actually, all callers of these APIs have such usage pattern in kernel tree. Fix by: - Initialize output parameter @iter by memset() in class_dev_iter_init() and give callers prompt by pr_crit() for the error. - Check if @iter is valid in class_dev_iter_next(). Fixes: 7b884b7f24b4 ("driver core: class.c: convert to only use class_to_subsys") Reviewed-by: Jonathan Cameron Signed-off-by: Zijun Hu --- Alternative fix solutions ever thought about: 1) Use BUG_ON(!sp) instead of error return in class_dev_iter_init(). 2) Change class_dev_iter_init()'s type to int, lots of jobs to do. This issue is APIs themself issues, and regardless of how various API users use them, and silent wild pointer dereferences are not what API users expect for the error absolutely. --- drivers/base/class.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/drivers/base/class.c b/drivers/base/class.c index 582b5a02a5c410113326601fe00eb6d7231f988f..d57f277978dc9033fba3484b4620bcf884a4029f 100644 --- a/drivers/base/class.c +++ b/drivers/base/class.c @@ -323,8 +323,12 @@ void class_dev_iter_init(struct class_dev_iter *iter, const struct class *class, struct subsys_private *sp = class_to_subsys(class); struct klist_node *start_knode = NULL; - if (!sp) + memset(iter, 0, sizeof(*iter)); + if (!sp) { + pr_crit("%s: class %p was not registered yet\n", + __func__, class); return; + } if (start) start_knode = &start->p->knode_class; @@ -351,6 +355,9 @@ struct device *class_dev_iter_next(struct class_dev_iter *iter) struct klist_node *knode; struct device *dev; + if (!iter->sp) + return NULL; + while (1) { knode = klist_next(&iter->ki); if (!knode)