From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f51.google.com (mail-pj1-f51.google.com [209.85.216.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 006291E5734 for ; Mon, 15 Dec 2025 01:39:24 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.51 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1765762766; cv=none; b=hBNxC8J7yXWzvWSk4Wl1sMb7PdVooPvGTUZ3a1lJJODbXVtlnpANkCPsYCdka7uWjjvVQW6vN/MUzU0LdSgt9RyWRPaZPe55Yznit88dtQ/5tVDk04G75n0+Bc9B0jgzW196Rgn/wZWdqti6TmHcVDoeBF4uQJpY05mpys5DQ/o= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1765762766; c=relaxed/simple; bh=k0AIvnDWVMG7Y4wKZy8PhAxFrYCzZJ0PZRlPyQMp2d8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=i3p/z5U3DiDw3qsnMXRyIyWmGj2YAIkwaS9bld5ryFiLzLnsKW8J3q3jRIiLqzwJ0zxutsWtIfR9RBOuCwTm/iglNHpXwGQB3o5t3FX0n7LvNQOmpvB2s2SibzMS7pizMXDXEfhIKQNFHdrsBuRIB0D+pfu0wpaEQx5q6OUqELk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=kqGvJDZT; arc=none smtp.client-ip=209.85.216.51 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="kqGvJDZT" Received: by mail-pj1-f51.google.com with SMTP id 98e67ed59e1d1-34c902f6845so509748a91.2 for ; Sun, 14 Dec 2025 17:39:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1765762764; x=1766367564; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=ANBknulCzJ+NpT/RV9+tvFzXk2SLbrdyzL4bSFdcmvU=; b=kqGvJDZTxNqlZhVm3XoMzYF0aT4mbjfjNYu1/ZMWAg6La+VpbvyGcSU7ZENHnsJ2BR hPo1n4cUNXJIFKiowsXxwogTiSt/UUhwN+7Yi6yxRw6HX5kh/8j/mNZd8No250JJ7/QR FBDl1eZgG6FZnG+E3bZ8kVgESKnQT9EtwzeCp3S6GojrFszRx3wewHCgUKL1ZkuftbXl p7zO4cXGqcEaTKiPBUJcDoa53byMafJ5MsGn5qmFxUILfIfWCkbtG1S6xlhR4xu8w3rX 3sTHuMfqcMxJ4pYAURdzvScqAvEbpjr8rlpehFxtIpl1z2SXYDo1ons25kcasdY9/ide LzEA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1765762764; x=1766367564; h=in-reply-to:content-disposition: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; bh=ANBknulCzJ+NpT/RV9+tvFzXk2SLbrdyzL4bSFdcmvU=; b=Vt0zKI9N80yC3+C7sVDPZa2CjqB2JRhMRCeQnOjE7elwzkEKXRTTn/1fftW7x67xtA /dGkkBVH+G2L2VVQhEirT9hrPb/3Jku6S3OGTR8HhpSoW/qcVfVGnFtrSowV9ze5NnuN ojFtS1cl/TAUOpI28FlwUi3Ru2H3gm5HKCljwsC39mbuB7Vewymv2TjEye5WDoWjPydG bmYPvatOv33crPF2HGrRXSep2Jtb1yQcxEg5Uqb4ZuH4X8g9UwZTlsF0ej0UoInm18BG 5jvGXQauog7Eg433odmEKdePzLnIUmWhfq+OsTYWOkhCEDOtlginqekIorVYiDm9azwx AovQ== X-Forwarded-Encrypted: i=1; AJvYcCUMbU5rSjIXgyUPknL2DVaD8uAGOXn2+AAnPESwuMycx+4BFNv5z/B6rGvzi+Nm7OAvhlgzn7+/Z1u38YI=@vger.kernel.org X-Gm-Message-State: AOJu0YwpENK11tDcix9XDU5UdxHMI4AB7uRJ8aGgdsj0CES1gPSF0Hx6 zRFVuLmhJt3XEdgln8Wch5jh8jFOx2o9/fniAUcqMr+datAd3nK2pLL/0V3zZurt9dkuSTAlD/9 0lyRqMHimug== X-Gm-Gg: AY/fxX7nInKoaGO4Dj8GcqeRXAt4Ad2Y2C0wTHAo65q1bKrX9HxSYNRjblK5Dq/qM/4 Z8KYoHw6Yg0kdGbg/iVgS/Q5t4Q9y0qOf9PLW/7ECkhLu6HSmADTbqhnlyPyogeD/oQ3YUPB+jC h2g6rxTvmyjoweGIO1RrMnaOJLwNVmkw0yPIVGZegAbvqKGFI3h93hJgBcgThjW6vgI2HxM5DMY RXObXI89EmiQA7n1ZNQRrVyr2V/6T9Kfwfg201XYqTOxDIUK4cO+lEFJGp2gAxNfKpWMtwyaGd7 PH8JqlV9EzcpRI6EY5OcPPQaY0h+pPZreTgTe/1+y3i5VbobjRAF7eiAIQH/nEcCIKGa5XI1+xT 76BLwh9dd7tJ8P07KHE/pL5wjo6h01n9ztU0k46S3sbkQ1Ze3xys67TpH4gAP5ZJObbikPXsXnI QkOhYGNZ/KGfRorxInkRyJV9t12tE057nCQVYvfJiqvQ== X-Google-Smtp-Source: AGHT+IEEH/MPRy5s7U+kyHScGR0GHLIOXl1SxGpgweU/Mtzit1FYvEDkVU28mycvGRDH2rPMUZtm9A== X-Received: by 2002:a17:90b:35d2:b0:34a:48ff:694 with SMTP id 98e67ed59e1d1-34abd7a4d37mr6767705a91.31.1765762764021; Sun, 14 Dec 2025 17:39:24 -0800 (PST) Received: from p14s (p7838222-ipoefx.ipoe.ocn.ne.jp. [123.225.39.221]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-34abe13f7e6sm3093132a91.0.2025.12.14.17.39.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 14 Dec 2025 17:39:23 -0800 (PST) Date: Sun, 14 Dec 2025 18:39:19 -0700 From: Mathieu Poirier To: Gui-Dong Han Cc: andersson@kernel.org, linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org Subject: Re: [PATCH] rpmsg: core: fix race in driver_override_show() and use core helper Message-ID: References: <20251202174948.12693-1-hanguidong02@gmail.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=us-ascii Content-Disposition: inline In-Reply-To: <20251202174948.12693-1-hanguidong02@gmail.com> On Wed, Dec 03, 2025 at 01:49:48AM +0800, Gui-Dong Han wrote: > The driver_override_show function reads the driver_override string > without holding the device_lock. However, the store function modifies > and frees the string while holding the device_lock. This creates a race > condition where the string can be freed by the store function while > being read by the show function, leading to a use-after-free. > > To fix this, replace the rpmsg_string_attr macro with explicit show and > store functions. The new driver_override_store uses the standard > driver_set_override helper. Since the introduction of > driver_set_override, the comments in include/linux/rpmsg.h have stated > that this helper must be used to set or clear driver_override, but the > implementation was not updated until now. > > Because driver_set_override modifies and frees the string while holding > the device_lock, the new driver_override_show now correctly holds the > device_lock during the read operation to prevent the race. > > Additionally, since rpmsg_string_attr has only ever been used for > driver_override, removing the macro simplifies the code. > > Fixes: 39e47767ec9b ("rpmsg: Add driver_override device attribute for rpmsg_device") > Cc: stable@vger.kernel.org > Signed-off-by: Gui-Dong Han Applied. Thanks, Mathieu > --- > I verified this with a stress test that continuously writes/reads the > attribute. It triggered KASAN and leaked bytes like a0 f4 81 9f a3 ff ff > (likely kernel pointers). Since driver_override is world-readable (0644), > this allows unprivileged users to leak kernel pointers and bypass KASLR. > Similar races were fixed in other buses (e.g., commits 9561475db680 and > 91d44c1afc61). Currently, 9 of 11 buses handle this correctly; this patch > fixes one of the remaining two. > --- > drivers/rpmsg/rpmsg_core.c | 66 ++++++++++++++++---------------------- > 1 file changed, 27 insertions(+), 39 deletions(-) > > diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c > index 5d661681a9b6..96964745065b 100644 > --- a/drivers/rpmsg/rpmsg_core.c > +++ b/drivers/rpmsg/rpmsg_core.c > @@ -352,50 +352,38 @@ field##_show(struct device *dev, \ > } \ > static DEVICE_ATTR_RO(field); > > -#define rpmsg_string_attr(field, member) \ > -static ssize_t \ > -field##_store(struct device *dev, struct device_attribute *attr, \ > - const char *buf, size_t sz) \ > -{ \ > - struct rpmsg_device *rpdev = to_rpmsg_device(dev); \ > - const char *old; \ > - char *new; \ > - \ > - new = kstrndup(buf, sz, GFP_KERNEL); \ > - if (!new) \ > - return -ENOMEM; \ > - new[strcspn(new, "\n")] = '\0'; \ > - \ > - device_lock(dev); \ > - old = rpdev->member; \ > - if (strlen(new)) { \ > - rpdev->member = new; \ > - } else { \ > - kfree(new); \ > - rpdev->member = NULL; \ > - } \ > - device_unlock(dev); \ > - \ > - kfree(old); \ > - \ > - return sz; \ > -} \ > -static ssize_t \ > -field##_show(struct device *dev, \ > - struct device_attribute *attr, char *buf) \ > -{ \ > - struct rpmsg_device *rpdev = to_rpmsg_device(dev); \ > - \ > - return sprintf(buf, "%s\n", rpdev->member); \ > -} \ > -static DEVICE_ATTR_RW(field) > - > /* for more info, see Documentation/ABI/testing/sysfs-bus-rpmsg */ > rpmsg_show_attr(name, id.name, "%s\n"); > rpmsg_show_attr(src, src, "0x%x\n"); > rpmsg_show_attr(dst, dst, "0x%x\n"); > rpmsg_show_attr(announce, announce ? "true" : "false", "%s\n"); > -rpmsg_string_attr(driver_override, driver_override); > + > +static ssize_t driver_override_store(struct device *dev, > + struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + struct rpmsg_device *rpdev = to_rpmsg_device(dev); > + int ret; > + > + ret = driver_set_override(dev, &rpdev->driver_override, buf, count); > + if (ret) > + return ret; > + > + return count; > +} > + > +static ssize_t driver_override_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + struct rpmsg_device *rpdev = to_rpmsg_device(dev); > + ssize_t len; > + > + device_lock(dev); > + len = sysfs_emit(buf, "%s\n", rpdev->driver_override); > + device_unlock(dev); > + return len; > +} > +static DEVICE_ATTR_RW(driver_override); > > static ssize_t modalias_show(struct device *dev, > struct device_attribute *attr, char *buf) > -- > 2.43.0 >