From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) (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 7E2AA2C21E8 for ; Sun, 16 Nov 2025 17:14:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=193.142.43.55 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763313249; cv=none; b=ZeMCevg5VP9tfVAoSZhS1peOawCjrbpWgnnkhwTEm3565BI3M3w1HikitoGEBdCW2cYIWYyyMVr71RH2nnKIqUNA30/YV2wk0ry+CAymH3t5dJFGeO6O4vBL45+UjwLlhKEs7vXNNDUkgal4bbFeH6sRS1ED8tmvI71GWkNCZuE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763313249; c=relaxed/simple; bh=o8LpJ7nKh9ewkY5q8WswKdamMbUE+aEYXAFVFsB2Ub0=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=JtELZxjUqBFtK0kau9FrAarK00mi4UbWozv2zua3Hhl5g3dYb5HA+v5/Cpo33rIdQxGCPOgxuobTERNeDWoM/WZ1vex6jVnGjep3DzC95Ev+sfB915OtzWK9lZWcpQkzs86L7ieV6pA68sV5scnSWOapu29BPOwEP0Q/98SRY74= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de; spf=pass smtp.mailfrom=linutronix.de; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=NJHdZ8FW; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=lF5SskZf; arc=none smtp.client-ip=193.142.43.55 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linutronix.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="NJHdZ8FW"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="lF5SskZf" From: Thomas Gleixner DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1763313243; 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=ZTo+TxYy2nT6b3ViY/tsN/eaKFyzFrfXfd1nyAuSiJQ=; b=NJHdZ8FWFZdRONaaNtQrh8L3R2tnbEKPi070sRkxkDO9rYQyXLfkXDENl2ikXl21uFW6Ie PNwSrgnyQKR9rlklmN11aPs0lAE9xDXgOY6T2xZ86iGhqnr0NUE1tHF+d1Riin8dkWOSe7 NhdjiPbq/S6Jl/RK/UuEKcFqsKv8xL2QqUxavkhKFSb6xlOjInQMGEHt4SyAwZpEgjm8NW mbR4x3x8fgeSlyuD7cbCIMHinLAy0UxOF2UVk6wXKo9O1vtumH4cNkMVeq7rTadC6FjAK6 slLoLeJJkCIzYUTjSp5RRh67R6XO2Oeq9mREj6jdDXkPv+/M1fTuYuzsEy0pSQ== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1763313243; 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=ZTo+TxYy2nT6b3ViY/tsN/eaKFyzFrfXfd1nyAuSiJQ=; b=lF5SskZfoiujLPEt//rrnkZ5Wj9cJyt4FAEAPeHlhmxwO4PABwhlkKyrG7Lb2y3gs8mbur zejWZzevs5g2s2CA== To: Malaya Kumar Rout Cc: linux-kernel@vger.kernel.org, lyude@redhat.com, malayarout91@gmail.com, John Stultz , Stephen Boyd Subject: Re: [PATCH] timekeeping: Fix resource leak in tk_aux_sysfs_init() error paths In-Reply-To: References: <20251110070054.9090-1-mrout@redhat.com> <87ldk8bgno.ffs@tglx> Date: Sun, 16 Nov 2025 18:14:02 +0100 Message-ID: <87y0o599yd.ffs@tglx> 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=utf-8 Content-Transfer-Encoding: quoted-printable On Sun, Nov 16 2025 at 21:19, Malaya Kumar Rout wrote: > On Sat, Nov 15, 2025 at 12:11=E2=80=AFAM Thomas Gleixner wrote: >> > auxo =3D kobject_create_and_add("aux_clocks", tko); >> > if (!auxo) { >> > - kobject_put(tko); >> > - return -ENOMEM; >> > + ret =3D -ENOMEM; >> > + goto err_put_tko; >> >> This ret variable is completely pointless as it is set to -ENOMEM in >> every error path. Just make the error path do 'return -ENOMEM;', no? >> > While it's true that most error paths in this function return -ENOMEM, the > sysfs_create_group() call can return different error codes depending on t= he > failure mode: > - -ENOMEM: Memory allocation failure > - -EEXIST: Attribute group already exists > - -EINVAL: Invalid arguments > By preserving the 'ret' variable, we ensure that the actual error code = from > sysfs_create_group() is propagated to the caller. This provides more ac= curate > error information for debugging and allows the caller to handle differe= nt > error conditions appropriately. Fair enough >> > } >> > >> > for (int i =3D 0; i < MAX_AUX_CLOCKS; i++) { >> > char id[2] =3D { [0] =3D '0' + i, }; >> > struct kobject *clk =3D kobject_create_and_add(id, auxo); >> > >> > - if (!clk) >> > - return -ENOMEM; >> > - >> > - int ret =3D sysfs_create_group(clk, &aux_clock_enable_at= tr_group); >> > + if (!clk) { >> > + ret =3D -ENOMEM; >> > + goto err_put_auxo; >> > + } >> > >> > + ret =3D sysfs_create_group(clk, &aux_clock_enable_attr_g= roup); >> > if (ret) >> > - return ret; >> > + goto err_put_auxo; >> > } >> > return 0; >> > + >> > +err_put_auxo: >> > + kobject_put(auxo); >> > +err_put_tko: >> > + kobject_put(tko); >> > + return ret; >> >> You can simplify that with _one_ error label: >> >> err: >> kobject_put(auxo); >> kobject_put(tko); >> return -ENOMEM; >> >> because kobject_put() is NULL pointer safe. > I agree with your suggestion to use a single error label since > kobject_put() is NULL-safe. > > Thank you for reviewing the patch and providing valuable feedback. > I will incorporate your suggestion for the single error label in the > next version while retaining the 'ret' variable for proper error > propagation. To avoid this ENOMEM nonsense you can split the stuff and do: static int __init tk_aux_sysfs_init(void) { struct kobject *auxo, *tko =3D kobject_create_and_add("time", kernel_kobj); int ret =3D -ENOMEM; if (!tko) return -ENOMEM; auxo =3D kobject_create_and_add("aux_clocks", tko); if (auxo) ret =3D __tk_aux_sysfs_init(auxo); if (ret) { kobject_put(auxo); kobject_put(tko); } return ret; } which spares all the extra 'ret =3D -ENOMEM;' completely. Thanks, tglx