From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f41.google.com (mail-wm1-f41.google.com [209.85.128.41]) (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 3290D2222C8 for ; Tue, 25 Nov 2025 13:41:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.41 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764078066; cv=none; b=AeOF83KK/HBUNJtYPZu0VvVNHomUiu9yR+vH1Z6uYgJjSPJbeut0uMKpXZyoHliog5gvKwv2QIUJA6GbIE2SotXXkhpMIo1gx8eKY/wb3HF1MEkiZmKaIVS4TWs7OuTHj9S4M10V1G9ufIB6gXXY/WcalZ9AZ6zJD8LzlPuuyFc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764078066; c=relaxed/simple; bh=aUnKbQNgYNRk5FD/94fe3iTpywXfpRwwGvDwXYNPVvE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=kkjDTChHmVGyLjURIIDlMN/hB6Az2JQfsZKOfoIYwFyDQR2DRrypWGnHgJM1oEmnOcPW/1Yp3aW2OIhEsHgDnhp+3ihIp+BfMWSeXp0F7I7nSbUwdJnPAiBnuETdrmIexzCUK4DgBUYJhv35eVxPtDOK2iUFB4ibAVmZo9HAFqA= 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=ipJIwRmi; arc=none smtp.client-ip=209.85.128.41 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="ipJIwRmi" Received: by mail-wm1-f41.google.com with SMTP id 5b1f17b1804b1-4779aa4f928so45799555e9.1 for ; Tue, 25 Nov 2025 05:41:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1764078062; x=1764682862; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=tXY0xe47aIH2gkwAiPLu9/+Y1J6dJeTatydLz8dZMD8=; b=ipJIwRmiLzeRv1bHtN78L+MhiPkccSd7iYCbuMUEoOfbjnjDVApqMhi9uqaw5DwgqC PUZJy+BC52x34/+Mk8cr7PrOmeJxLN52CEJMAVfrN0xXZMw5j1K28msJm4dKWaCWIs0r WRZrFGxN2N2YK0kdg6onXBH/+JIKCwJNjpxT80IFQToKMwraGRoXpUmo3+/pbVgkehyd xFKHZNgSS0UavHbZXMaXr6HWGPIGTk45kjTlsfPac5PvoFtGYLRfNM20qfASDng2Wdi0 xxinZOIdwuNnd8CGhB5AzEhqz/MfI22Z2xvyUe5jk+ZVfEcyg7jckz/Oseyx5q+ynyc9 QzAw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1764078062; x=1764682862; h=in-reply-to:content-transfer-encoding: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=tXY0xe47aIH2gkwAiPLu9/+Y1J6dJeTatydLz8dZMD8=; b=QwpBUTgIpZC9MH0yY8ED/cXkTDaY0SXGaFmo3yn66r3Mf4qTGx0yH3eHk8+7e7Bd9g bIvei5VsNhxfxIkbjs42SdNwE0J5M/V8CPMOi97szxdYkpQut/mYTDjEL6T/Kl9M8Ltj d7JaOSg3gyBXPpmuvwbIpY97XiItpw3fuNENLF889qoF8nk873ROoMyPwAQpMkWGpMla w3YfszxGFyYTgh7aM28aWYAGowGAAtN8vp8AR3WJou/PRX5Ci/3Zkr7V1TGa8rCON3Oc aR5fYwC6Y7vhJq0/Nc6Hg7whAia9MwgFxQ8NXHDKOehGSStpkNzOJQ3C2cMp308uthSk pTag== X-Gm-Message-State: AOJu0YwU+k/Z94pjhK7cwTWsY/j991P2xplZkmBO8Myo6VgIFsQWUCnx 3PXtU6JoAQOGLcQHlR0PFvAcWLlLjZZNDB4IFjWgYOs1jAHBSL5i7Sk9qd7Qx23EjDA= X-Gm-Gg: ASbGnctPiQnWxAMcSZUFdrQql7reSVn/deMWvZDjI3GjQoCeKEvB5CV+RUVoypkFgEE 7PNnJ/kO+xzJ7czTl/h4XmROZ5biC+r3SnSrGROhaTxrqIqZ3LMQ+A+6S4bgjFpFZjoFuImjQE+ v2Rr5T/X3vhmNyvqnius3O5WMVsSk9NpSHZrg6EhitsMdxKomdBCVJqSyEmibLtn7b6a59/SXJe 6pHTZfJiuEpOQ6RqSQKJUuhCS6DilF5Nro9T3xt65ytkVZM+J/qDAuOCFuImYqy3fZ2P/DMjiIe 0zqhi9YdsqZOQSwMNNXfGeYe9bM30kT4dIj8tFCVNQ0RyY0r372pXIa6p2cpQO6fEL2MTQHXfCk nIUKosh9eYP8hXcBwyJ52YeQRTe1AGVaqzcB1n2GAYL3TsXB0iPXsBk10e5mGOzli1EPkyEso8a wBCG0uzrCprzOQkzRN4gX0FxWtjSU= X-Google-Smtp-Source: AGHT+IGAu6+1/Q8zbkfH+KEvPagaB3p9XYnc+20ttBSWDvkBv5WLCDAbelFYeJ7DN/nugowM3DkQSQ== X-Received: by 2002:a05:600c:8b37:b0:477:9cc3:7971 with SMTP id 5b1f17b1804b1-47904b1ac02mr18980965e9.20.1764078062115; Tue, 25 Nov 2025 05:41:02 -0800 (PST) Received: from localhost ([196.207.164.177]) by smtp.gmail.com with UTF8SMTPSA id 5b1f17b1804b1-477bf1f365fsm253967315e9.8.2025.11.25.05.41.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 25 Nov 2025 05:41:01 -0800 (PST) Date: Tue, 25 Nov 2025 16:40:58 +0300 From: Dan Carpenter To: Toomas Soome Cc: smatch@vger.kernel.org Subject: Re: apparent bug about check_free_strict Message-ID: References: <719690CC-A1F0-47B7-AD43-0A1EBD632081@me.com> <13919A78-B19A-4A44-95F1-A729562C50BF@me.com> Precedence: bulk X-Mailing-List: smatch@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="R/DY2h4yeviy7e3T" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: --R/DY2h4yeviy7e3T Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit It feels like this email I'm responding to is recent because it's from Nov 18. But actually it's from Nov 18 last year... Sorry for that. I'm going to have to start again from scratch in some ways because I've forgotten everything. I've attached my reproducer script (which doesn't reproduce anything). There was a mistake in the reproducer script that you sent me because the smatch options (particularly -p=illumos_kernel) need to come before the GCC options... I guessed the dir to be in and I had to touch the usr/src/uts/i86pc/sys/priv_names.h file because I don't have a copy of that but generally the reproducer seems fine. Without the -p=illumos_kernel then "ddi_err(DER_PANIC, ..." calls were not parsed correctly so it generated some false positives but after I fixed that then there was no output. (I removed the --debug=free for now). On Mon, Nov 18, 2024 at 11:17:12PM +0200, Toomas Soome wrote: > commit 9a427ca57dc8a8b47d021f5f772ac164842bd996 (upstream/master, master) > Author: Dan Carpenter > Date: Thu Nov 14 23:12:46 2024 +0300 > > db: fix a NULL dereference in get_param() > > > > > Anyway, I've attached the diff to the latest code below with some debugging. > > Apply the diff to the lastest Smatch and apply the code-diff to the devcfg.c > > and then re-run Smatch. > > > > Now this is fun. With devcfg.c changes in place, the warnings disappeared. So I removed 'if (local_debug)’ from check_free_strict.c and, I think we have something to point to: > > /code/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i386/smatch: ../../common/os/devcfg.c:8573 e_ddi_retire_device() set_param_freed: expr='ndi_hold_devi(pdip)' param=0 key='$' > /code/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i386/smatch: ../../common/os/devcfg.c:8573 e_ddi_retire_device() set_state new [check_free_strict] 'pdip' freed > ../../common/os/devcfg.c:8573 e_ddi_retire_device() merge [check_free_strict] 'pdip' freed(L 8573) + undefined(L 8573) => merged (freed, undefined, merged) > /code/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i386/smatch: ../../common/os/devcfg.c:8573 e_ddi_retire_device() __set_sm new [check_free_strict] pdip fffff7ffe9523b90 = 'merged' [merged] (freed, undefined, merged) > /code/illumos-gate/usr/src/tools/proto/root_i386-nd/opt/onbld/bin/i386/smatch: ../../common/os/devcfg.c:8583 e_ddi_retire_device() warn: passing freed memory 'pdip' What this says is that ndi_hold_devi() is setting pdip to free. I would have thought you have a cross function database somewhere. Your dtrace output also suggests that it's coming from the DB. But you've explained later in the thread that you don't. It's possible that the ndi_hold_devi() function is being parsed inline. One way you can prevent that is by adding a bunch of semi-colons to the function since it won't inline functions with a lot of lines of code. diff --git a/usr/src/uts/common/os/devcfg.c b/usr/src/uts/common/os/devcfg.c index 787d49468e1c..f7fda31d5c23 100644 --- a/usr/src/uts/common/os/devcfg.c +++ b/usr/src/uts/common/os/devcfg.c @@ -1877,6 +1877,8 @@ ddi_remove_child(dev_info_t *dip, int dummy) void ndi_hold_devi(dev_info_t *dip) { + ;;;;;;;;;;;;; + mutex_enter(&DEVI(dip)->devi_lock); ASSERT(DEVI(dip)->devi_ref >= 0); DEVI(dip)->devi_ref++; But I don't see a kmem_free() in the ndi_hold_devi() function and I also don't see ndi_hold_devi() listed in: http://132-104-190-90.sta.estpak.ee/smatch/smatch.debug.log And in my reproducer, Smatch doesn't mark it as a free function. The fact that you're getting segfaults suggests there is memory corruption happening somewhere. That could explain it. I tried adding a valgrind before the smatch in my check script. "valgrind ~/progs/smatch/$RELEASE/smatch --full-path..." but valgrind didn't find anything. Maybe you could try again with this diff (but with the correct include path name obviously). diff --git a/usr/src/uts/common/os/devcfg.c b/usr/src/uts/common/os/devcfg.c index 787d49468e1c..a577bcbd49ee 100644 --- a/usr/src/uts/common/os/devcfg.c +++ b/usr/src/uts/common/os/devcfg.c @@ -8565,6 +8565,7 @@ e_ddi_retire_finalize(dev_info_t *dip, void *arg) * cons_array is a NULL terminated array of node paths for * which constraints have already been applied. */ +#include "/home/dcarpenter/progs/smatch/devel/check_debug.h" int e_ddi_retire_device(char *path, char **cons_array) { @@ -8596,7 +8597,9 @@ e_ddi_retire_device(char *path, char **cons_array) RIO_DEBUG((CE_NOTE, "Retire device: found dip = %p.", (void *)dip)); pdip = ddi_get_parent(dip); + __smatch_debug_db_on(); ndi_hold_devi(pdip); + __smatch_debug_db_off(); /* * Run devfs_clean() in case dip has no constraints and is I don't see any corrupted output when I run it, but maybe you will get different results. regards, dan carpenter --R/DY2h4yeviy7e3T Content-Type: application/x-sh Content-Disposition: attachment; filename="check.sh" Content-Transfer-Encoding: quoted-printable #!/bin/bash=0A=0Atouch usr/src/uts/i86pc/sys/priv_names.h=0A=0Acd usr/src/u= ts/common/os/=0A=0ARELEASE=3Drelease=0A=0A~/progs/smatch/$RELEASE/smatch --= full-path -p=3Dillumos_kernel \=0A -fident -finline -fno-inline-function= s -fno-builtin -fno-asm -fdiagnostics-show-option -nodefaultlibs -D__sun -m= 64 \=0A-mtune=3Dopteron -Ui386 -U__i386 -fno-strict-aliasing -fno-unit-at-a= -time -fno-optimize-sibling-calls -O2 -D_ASM_INLINES -ffreestanding -mno-re= d-zone -mno-mmx -mno-sse -msave-args -Wall -Wextra -g -gdwarf-2 \=0A-std=3D= gnu99 -msave-args -Werror -Wno-missing-braces -Wno-sign-compare -Wno-unknow= n-pragmas -Wno-unused-parameter -Wno-missing-field-initializers -Winline -W= no-unused -Wno-empty-body \=0A--disable=3Duninitialized,check_check_deref -= Wno-vla -Wno-one-bit-signed-bitfield -Wno-external-function-has-definition = -Wno-old-style-definition -Wno-strict-prototypes --fatal-checks --timeout= =3D0 \=0A--disable=3Dindex_overflow --disable=3Dsigned,all_func_returns -Wn= o-unused-variable -Wno-unused-value -Wno-unused-function -Wno-parentheses -= Wno-maybe-uninitialized -Wno-clobbered -Wno-empty-body \=0A-fno-inline-smal= l-functions -fno-inline-functions-called-once -fno-ipa-cp -fno-ipa-icf -fno= -clone-functions -fno-reorder-functions -fno-reorder-blocks-and-partition -= fno-aggressive-loop-optimizations \=0A--param=3Dmax-inline-insns-single=3D4= 50 -fno-shrink-wrap -mindirect-branch=3Dthunk-extern -mindirect-branch-regi= ster -fno-asynchronous-unwind-tables -fstack-protector-strong -fno-eliminat= e-unused-debug-symbols \=0A-fno-eliminate-unused-debug-types -D_KERNEL -ffr= eestanding -D_SYSCALL32 -D_SYSCALL32_IMPL -D_ELF64 -D_DDI_STRICT -Dsun -D__= sun -D__SVR4 -DOPTERON_ERRATUM_88 -DOPTERON_ERRATUM_91 -DOPTERON_ERRATUM_93= \=0A-DOPTERON_ERRATUM_95 -DOPTERON_ERRATUM_99 -DOPTERON_ERRATUM_100 -DOPTE= RON_ERRATUM_101 -DOPTERON_ERRATUM_108 -DOPTERON_ERRATUM_109 -DOPTERON_ERRAT= UM_121 -DOPTERON_ERRATUM_122 -DOPTERON_ERRATUM_123 \=0A-DOPTERON_ERRATUM_13= 1 -DOPTERON_WORKAROUND_6336786 -DOPTERON_ERRATUM_147 -DOPTERON_ERRATUM_172 = -DOPTERON_ERRATUM_298 -DOPTERON_ERRATUM_721 -I../../intel -nostdinc -I../..= /common \=0A-I/code/illumos-gate/usr/src/common -I/code/illumos-gate/usr/sr= c/uts/common/fs/zfs -I../../i86pc -c -o /tmp/cw.GCaq5P/cwICaO5P.o -mcmodel= =3Dkernel \=0Adevcfg.c=0A --R/DY2h4yeviy7e3T--