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 7600226E703 for ; Thu, 13 Nov 2025 19:49:29 +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=1763063371; cv=none; b=FCtsrwDoHCCKxIAPkq+vfm7Nnax+KdQW0yuahwx0L1h9dgZIkN6OM5J/bm8cQs22C09xXE+3G1A4dl8cNmnx+TZlfguIL41s7Yd0HE/hz9CrDtVJrnmT/UkhrjbJOP/obDT9xu0LDWsom7NNxVx9VYdjvWxBK8PpVAijm99o5hE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1763063371; c=relaxed/simple; bh=sDIqEpjMOdq33BW4w411TYnaHBx4mhjMhX8nO1aASkw=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Cj80mQ3JISPfsi64nCAcWamjPcsskZLAC246PRblz6oY1P/fePNIOUeg+QilxBLa9Wc2JMXzOgtJjJLatH6ppX9K6cT4zgE0j7yigKfOQkNnU33gD/IlyVFXp2iJD/BPOOU68yU2/SA7+YnFWQgQMyYO7bp0t+Pil9nnhSsoQNs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=f9PAnq0N; arc=none smtp.client-ip=209.85.210.169 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="f9PAnq0N" Received: by mail-pf1-f169.google.com with SMTP id d2e1a72fcca58-7ad1cd0db3bso1058572b3a.1 for ; Thu, 13 Nov 2025 11:49:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1763063369; x=1763668169; 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=sNKLoY/mkThVnS16X9wih5/iJLO2flJ/58jTcCdEcMY=; b=f9PAnq0NaW2cIi1kIAS4oOyUgCjupKrM2pNvZIR86XSd7VtWC7v6VkufZJZCY1th/2 mx5xPiA2OKO9etBHzbmbMkWoWLSjCoDpcpaxd+/YV8+aSLgPNbdZQzzUoEk6FVrv3qv7 MqkyZTZU21cOTwCAF8oaeUiQ7EYIq5t8gn0YmCHsl8f/l4rfXD0IyOntQkPU3isqfg5b 8zb/U/vUOf1D+QTKIWaRnOcSSQfli5L0ynTdJP7MXvQjK5yPpsvuJalTxe5GRtwtOoGA NEXLIbiOZQILDg7giVAwRYK9L2/HT6lcBVtX279DnBX7xfBMYKx+h3sY9l89p4bUdtus s1CA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1763063369; x=1763668169; 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=sNKLoY/mkThVnS16X9wih5/iJLO2flJ/58jTcCdEcMY=; b=lXC4NqUfITKbKdlHPRz9PP00kZzdORHstDF+E4oRw9BaRt7Y26IJue7Lx7d47+osci jeSnGTXTPZSMg0mZDwGAauJBn/Z/dkEIt75oDVNiTOQcOexcx4Z00DdP0zxg41pv/GAN cdTx+BRde2c/mnRzYROzKvxCk0HzRQydToTS8hR2N6h9GqH0Of0yEuv+lnNxp7MRKs7Z bO7jnrXZsiGmxhuUlZ9+l8HyiSrTUx9HUnB9rNddLeCUqLK5Z48azH2tEAsNPALh55rH y7OyR89dCPGOQx5Lafhxs4754trrui6FPsaaHKIoBkhkz7eOyLeU66R2ny+h5qWCqWj5 qGPw== X-Forwarded-Encrypted: i=1; AJvYcCUEW0AX6yLELu58oUF6inhWOszZav6ewr4J+GstzRozYx4EOy3SpFHilY83KZ7GGlR65RAULzmsMQ77Bb4=@vger.kernel.org X-Gm-Message-State: AOJu0YxMoxpjAcPDBoLMjJ2GrqTYh2u2twcITxPgOnIX0mtqYUzlgqNf VrTa8CUHxmmFXDehRrMBgAA/zmNQhrqJUcHtkJ07kW6cTB9pvcHBJVYI X-Gm-Gg: ASbGncuclFmyG2MlES2QSkbTGpPq1GFc49z9v/XLntvjaedVBFbhpth6Tfw+XPEuSCl QgHYnbSlDyjakkV1w9QAKP7rfbJGSwW5Kxlh+kaS6wfqCRCqs7Tz7vipK5DtBV/O2Ft3GTQkqFR wUW5OLLSkVMt2tLChagYcofCVZnz+jXh5n8PgKX/nb+dYlxZ8XEvK769ezqW3kN/gQrg5FBRO/z 1Iz8p4xw4ouLUB42yGlinXGwcnz11x8Xv6uRrMvjPvrpAo4PETq696b8CJJ9JSh1o0qxKPOG27X v1iKDKJ4U/4ber56YqHNPgE/q71bZKya0Iw4X/Lsm6k2wSuIXumE1JtMtO+Nov0djWC2MMCivnb OTubz5PkLAenCpV75Zi5PznVq3FsauKlQsoXPqt1oXzZFLOPJCfEbwBc3+bepNIWol1cAJqjcZ8 y+OZ/zqSrwVOnwrH7VecaCeQ== X-Google-Smtp-Source: AGHT+IHxCumTRfsTDJ8HCggJWAJR96dn6tK5f1f70VX3Oh/QICEx09trHr0365fnS3ElIY2ct1O8EQ== X-Received: by 2002:a05:6a00:80f:b0:7b9:9d68:4a9b with SMTP id d2e1a72fcca58-7ba3ba97bb0mr733178b3a.15.1763063367746; Thu, 13 Nov 2025 11:49:27 -0800 (PST) Received: from google.com ([118.150.148.19]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-7b927e17ed6sm3081441b3a.72.2025.11.13.11.49.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Nov 2025 11:49:27 -0800 (PST) Date: Fri, 14 Nov 2025 03:49:23 +0800 From: Kuan-Wei Chiu To: Haotian Zhang Cc: Andrew Morton , Thomas Gleixner , linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] debug: Fix a mixed use of NULL and error pointers Message-ID: References: <20251110075746.1680-1-vulab@iscas.ac.cn> <20251111021521.1906-1-vulab@iscas.ac.cn> 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: <20251111021521.1906-1-vulab@iscas.ac.cn> Hi Haotian, On Tue, Nov 11, 2025 at 10:15:21AM +0800, Haotian Zhang wrote: > The lookup_object_or_alloc() function currently returns either error > pointers (ERR_PTR(-ENOENT)) or NULL on allocation failure. Mixing error > pointers and NULL is confusing and makes the code harder to maintain. > Change lookup_object_or_alloc() to consistently return error pointers > for all error cases by returning ERR_PTR(-ENOMEM) instead of NULL when > allocation fails. > > Update all three call sites (__debug_object_init, debug_object_activate, > and debug_object_assert_init) to use IS_ERR() for error checking and > handle -ENOMEM by calling debug_objects_oom(). The code change itself LGTM. However, the v2 commit message removed the previous description about the potential for dereferencing an error pointer. This removal shifts the narrative, making it appear more like a cleanup patch than a necessary bug fix. I'll recommend you reinstate a clear explanation of why the previous behavior was buggy. Regards, Kuan-Wei > > Fixes: 63a759694eed ("debugobject: Prevent init race with static objects") > Suggested-by: Kuan-Wei Chiu > Signed-off-by: Haotian Zhang > --- > Changes in v2: > -Make error handling consistent across all call sites as suggested > by Kuan-Wei Chiu > --- > lib/debugobjects.c | 23 +++++++++++++---------- > 1 file changed, 13 insertions(+), 10 deletions(-) > > diff --git a/lib/debugobjects.c b/lib/debugobjects.c > index 7f50c4480a4e..d2d1979e2d12 100644 > --- a/lib/debugobjects.c > +++ b/lib/debugobjects.c > @@ -691,7 +691,7 @@ static struct debug_obj *lookup_object_or_alloc(void *addr, struct debug_bucket > > /* Out of memory. Do the cleanup outside of the locked region */ > debug_objects_enabled = false; > - return NULL; > + return ERR_PTR(-ENOMEM); > } > > static void debug_objects_fill_pool(void) > @@ -741,9 +741,10 @@ __debug_object_init(void *addr, const struct debug_obj_descr *descr, int onstack > raw_spin_lock_irqsave(&db->lock, flags); > > obj = lookup_object_or_alloc(addr, db, descr, onstack, false); > - if (unlikely(!obj)) { > + if (IS_ERR(obj)) { > raw_spin_unlock_irqrestore(&db->lock, flags); > - debug_objects_oom(); > + if (PTR_ERR(obj) == -ENOMEM) > + debug_objects_oom(); > return; > } > > @@ -818,11 +819,13 @@ int debug_object_activate(void *addr, const struct debug_obj_descr *descr) > raw_spin_lock_irqsave(&db->lock, flags); > > obj = lookup_object_or_alloc(addr, db, descr, false, true); > - if (unlikely(!obj)) { > - raw_spin_unlock_irqrestore(&db->lock, flags); > - debug_objects_oom(); > - return 0; > - } else if (likely(!IS_ERR(obj))) { > + if (IS_ERR(obj)) { > + if (PTR_ERR(obj) == -ENOMEM) { > + raw_spin_unlock_irqrestore(&db->lock, flags); > + debug_objects_oom(); > + return 0; > + } > + } else { > switch (obj->state) { > case ODEBUG_STATE_ACTIVE: > case ODEBUG_STATE_DESTROYED: > @@ -1007,11 +1010,11 @@ void debug_object_assert_init(void *addr, const struct debug_obj_descr *descr) > raw_spin_lock_irqsave(&db->lock, flags); > obj = lookup_object_or_alloc(addr, db, descr, false, true); > raw_spin_unlock_irqrestore(&db->lock, flags); > - if (likely(!IS_ERR_OR_NULL(obj))) > + if (!IS_ERR(obj)) > return; > > /* If NULL the allocation has hit OOM */ > - if (!obj) { > + if (PTR_ERR(obj) == -ENOMEM) { > debug_objects_oom(); > return; > } > -- > 2.50.1.windows.1 >