From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pf1-f169.google.com (mail-pf1-f169.google.com [209.85.210.169]) (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 26FFA2E5B26 for ; Fri, 14 Nov 2025 19:04:57 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.169 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763147100; cv=none; b=WjXSfxk8APcCSK2fL84UZSi1McsISdP3S8jb0YcNudq6brT8a6xq9+WSQwaw2YrJ/A/ClsmTy9+FO0fIiiSk3hfcHFX84Tl2TRA/SYH5tgIHnCw+erwha0UcgTO8C9C32ae8ZkS88Zv6D0ryoKdF0azwC2TvWtgvPETo3RWQ/vg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763147100; c=relaxed/simple; bh=M3e3utOPbpx8hPf1y1icwX9zY21LD2B25JJVojjQ19k=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=TuRHdtCjuxloFgGBTGjLmlO6Rnux1ywEzNrgSaDUq0u3hbVsq6LK1+Znrq3JUeTDErkaHORqWrUiCSvh3zaAT1FxxY4rsRHzquxUMWxwUtDyYaEvudeFpQNlLUr1e4Z5+igKMk0hUE7j4oduWbSnprSNyeVEJMzSj3UC+tvErEU= 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=CHRyhLzB; arc=none smtp.client-ip=209.85.210.169 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="CHRyhLzB" Received: by mail-pf1-f169.google.com with SMTP id d2e1a72fcca58-7aab061e7cbso2563340b3a.1 for ; Fri, 14 Nov 2025 11:04:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1763147097; x=1763751897; 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=BetbtNepftNPwtT7TMrGjbCKY0xag6LCqeb+veSVYD0=; b=CHRyhLzBpcTTyoBZzgyqbX50ZKOmASHquIdG24UyKCOaivBx6SmkQR69CLFVJtRT+q gFOUyXc5N6PL9qRwaekZbwbOeujbjPPjhRol8yHBnVYUBsms8ltKXcLz4OxrvmtW1RHU S1wittSCZnn9Id8sfw/XGGYocUu2r4+sIHuNrgETYXVC4SK9DBHKLtu1D+gOrpnlVKxK deoOSuhpzjMvQvaOGaI+g4+ChNvHypNTte+LjyfZL8kcVtKVlpsI1dwfsiyoGqdVWA86 wvDGgb6blBSUOkZ+X+aCglOsJrNKEll+B7/HWwSgdfj0R+ZXzE1CqSGVAPyLjSmsVy4G yFog== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1763147097; x=1763751897; 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=BetbtNepftNPwtT7TMrGjbCKY0xag6LCqeb+veSVYD0=; b=kzxv4PQ6uVcNbcP/q7d78fBZVQceQKX2amgADHNItZbyWBaCHFTvgFa3cBN8EILzKX VuiUELbUmEDPLUlYz3tozIzoYQ6Abp25VjogZZOpl5HJLk/TLMctF6m4TLwVB1eFTjen kGgJMB0YhvFb2znKRxuzd7UAY0zoAdr98oXjI0Cwtvy/DcwNugF5ByiVJ4q6BnTZ16nR H/tPsUZP09iwQUVhDhDRX8TvingxfZf/TlQCpxsSxIbursalNBtmx/0gQ6gBs5wL29LT FnuM8L/B+VIdxQLNwworRAQCezsqPuAoMBHU9vQOtA663/U/0IbSq1hwG5ftn8y+Aht8 mzow== X-Forwarded-Encrypted: i=1; AJvYcCU7SinmlNZiYWWWDBLScNeFOCuM0Po82xWWGCo+3GMBg/LTT2XeSOB/LUQHu91IchhKJFx0YOvt61vtTZc=@vger.kernel.org X-Gm-Message-State: AOJu0Yzs3tvhJfokGJtR0ECqmanOUW0+KIRv3LkW3H7UrM9jOjh7TAyx zIQjg7Bp90VITgKmymTHRDCeyxQU8nC2dmxpOLl0zfwNEcbSNERt3S0Yqf7x8DZixHz2nvajy/5 YCrn2EfQ= X-Gm-Gg: ASbGncu5IlFLI6T5/MfQb2k0aVS6yt5WclO/qoAKz8Ph3B/rgZec2QI4hMkGDwMZb4B x3XmFbyq9rLU0epEllVWOplKCmoXh1ufvzCR4NWDMNQ5oelKvu9cOXBMj/KHcWQ39jbgzYOCxDP ItDVUOxyIcMGY1+wfqSVyTF+wzCgqq+XQDR34pZfpOjysH7q961m9X8Y1i5JoTbNwCkml2dpfBi +0MKB1ldiFdB8aH0GfVhPWTzwLiPJ4A675mZkhytHJj3j83IKhkiTG2umQgxXcI/lM/FfT/3Jon osoR7PheyP6qm/G0gMu97wa9WqruegzTL51OnGfq/+byHeWw1S2PC9q5SH/IK51MnfLGiHTDAiF XDYu/loMrzd4qBexR9ZcGUuCBsPBmdLG4cGFeN8t54JRGGhuzz9ushHBWbUdgcotwuKv02zr2T0 SnLvY= X-Google-Smtp-Source: AGHT+IHYwFf7+NaXyMe8AHBJnYbpi5NdrBt0F5VZtx+xzw5AtAbIt4qRmikBMKsFagWSIpq2Heha3g== X-Received: by 2002:a05:6a00:986:b0:7b7:ac37:9235 with SMTP id d2e1a72fcca58-7ba3be8dae4mr5002740b3a.15.1763147097445; Fri, 14 Nov 2025 11:04:57 -0800 (PST) Received: from p14s ([2604:3d09:148c:c800:734d:a808:2eb1:a1ea]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-7b924aee8afsm6010118b3a.8.2025.11.14.11.04.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 14 Nov 2025 11:04:56 -0800 (PST) Date: Fri, 14 Nov 2025 12:04:54 -0700 From: Mathieu Poirier To: Dawei Li Cc: andersson@kernel.org, linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org, set_pte_at@outlook.com, Dan Carpenter Subject: Re: [PATCH v3 3/3] rpmsg: char: Rework exception handling of rpmsg_eptdev_add() Message-ID: References: <20251113153909.3789-1-dawei.li@linux.dev> <20251113153909.3789-4-dawei.li@linux.dev> 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: <20251113153909.3789-4-dawei.li@linux.dev> On Thu, Nov 13, 2025 at 11:39:09PM +0800, Dawei Li wrote: > Rework error handling of rpmsg_eptdev_add() and its callers, following > rule of "release resource where it's allocated". > > Fixes: 2410558f5f11 ("rpmsg: char: Implement eptdev based on anonymous inode") > Reported-by: Dan Carpenter > Closes: https://lore.kernel.org/all/aPi6gPZE2_ztOjIW@stanley.mountain/ > > Signed-off-by: Dawei Li > --- > drivers/rpmsg/rpmsg_char.c | 60 +++++++++++++++++++++----------------- > 1 file changed, 33 insertions(+), 27 deletions(-) > > diff --git a/drivers/rpmsg/rpmsg_char.c b/drivers/rpmsg/rpmsg_char.c > index 0919ad0a19df..92c176e9b0e4 100644 > --- a/drivers/rpmsg/rpmsg_char.c > +++ b/drivers/rpmsg/rpmsg_char.c > @@ -460,44 +460,34 @@ static int rpmsg_eptdev_add(struct rpmsg_eptdev *eptdev, > > eptdev->chinfo = chinfo; > > - if (cdev) { > - ret = ida_alloc_max(&rpmsg_minor_ida, RPMSG_DEV_MAX - 1, GFP_KERNEL); > - if (ret < 0) > - goto free_eptdev; > - > - dev->devt = MKDEV(MAJOR(rpmsg_major), ret); > - } > - > /* Anonymous inode device still need device name for dev_err() and friends */ > ret = ida_alloc(&rpmsg_ept_ida, GFP_KERNEL); > if (ret < 0) > - goto free_minor_ida; > + return ret; > dev->id = ret; > dev_set_name(dev, "rpmsg%d", ret); > > - ret = 0; > - > if (cdev) { > + ret = ida_alloc_max(&rpmsg_minor_ida, RPMSG_DEV_MAX - 1, GFP_KERNEL); > + if (ret < 0) { > + ida_free(&rpmsg_ept_ida, dev->id); > + return ret; > + } > + > + dev->devt = MKDEV(MAJOR(rpmsg_major), ret); > + > ret = cdev_device_add(&eptdev->cdev, &eptdev->dev); > - if (ret) > - goto free_ept_ida; > + if (ret) { > + ida_free(&rpmsg_ept_ida, dev->id); > + ida_free(&rpmsg_minor_ida, MINOR(dev->devt)); > + return ret; > + } > } > > /* We can now rely on the release function for cleanup */ > dev->release = rpmsg_eptdev_release_device; > > - return ret; > - > -free_ept_ida: > - ida_free(&rpmsg_ept_ida, dev->id); > -free_minor_ida: > - if (cdev) > - ida_free(&rpmsg_minor_ida, MINOR(dev->devt)); > -free_eptdev: > - dev_err(&eptdev->dev, "failed to add %s\n", eptdev->chinfo.name); > - kfree(eptdev); > - > - return ret; > + return 0; > } > > static int rpmsg_chrdev_eptdev_add(struct rpmsg_eptdev *eptdev, struct rpmsg_channel_info chinfo) > @@ -509,12 +499,17 @@ int rpmsg_chrdev_eptdev_create(struct rpmsg_device *rpdev, struct device *parent > struct rpmsg_channel_info chinfo) > { > struct rpmsg_eptdev *eptdev; > + int ret; > > eptdev = rpmsg_chrdev_eptdev_alloc(rpdev, parent); > if (IS_ERR(eptdev)) > return PTR_ERR(eptdev); > > - return rpmsg_chrdev_eptdev_add(eptdev, chinfo); > + ret = rpmsg_chrdev_eptdev_add(eptdev, chinfo); > + if (ret) > + kfree(eptdev); Previous patch. > + > + return ret; > } > EXPORT_SYMBOL(rpmsg_chrdev_eptdev_create); > > @@ -545,6 +540,12 @@ int rpmsg_anonymous_eptdev_create(struct rpmsg_device *rpdev, struct device *par > > ret = rpmsg_eptdev_add(eptdev, chinfo, false); > if (ret) { > + dev_err(&eptdev->dev, "failed to add %s\n", eptdev->chinfo.name); > + /* > + * Avoid put_device() or WARN() will be triggered due to absence of > + * device::release(), refer to device_release(). > + */ > + kfree(eptdev); Previous patch. > return ret; > } > > @@ -572,6 +573,7 @@ static int rpmsg_chrdev_probe(struct rpmsg_device *rpdev) > struct rpmsg_channel_info chinfo; > struct rpmsg_eptdev *eptdev; > struct device *dev = &rpdev->dev; > + int ret; > > memcpy(chinfo.name, rpdev->id.name, RPMSG_NAME_SIZE); > chinfo.src = rpdev->src; > @@ -590,7 +592,11 @@ static int rpmsg_chrdev_probe(struct rpmsg_device *rpdev) > */ > eptdev->default_ept->priv = eptdev; > > - return rpmsg_chrdev_eptdev_add(eptdev, chinfo); > + ret = rpmsg_chrdev_eptdev_add(eptdev, chinfo); > + if (ret) > + kfree(eptdev); Previous patch. > + > + return ret; > } > > static void rpmsg_chrdev_remove(struct rpmsg_device *rpdev) > -- > 2.25.1 >