From nobody Tue Jun 16 12:41:37 2026 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 1E7DE402BA3; Tue, 28 Apr 2026 10:42:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777372952; cv=none; b=p9+a4sDbXso/8j4lmj/pQ0Gn+92hfRwKhEm/0uejO7AxisGBEVBl3r2DXTUbVv/JwhbzN0jhVOLd66Oo2amg3zDc9jYsusReZfkZI3j7tBy2axHwbqKXrbQ5axcJieaXrk8adRZ0LEzEDPI2MzxY1CtqxBSTAzi/k2NuNfqCJyI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1777372952; c=relaxed/simple; bh=VbCd9suTkAq0QTyiaF+WctXyADNMKX2UwVQqlIBS9ok=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=UkguVUU9gCMB56vaGxwhCEnaG7mtt47GA1OkIfgvEcKHBBlL4SiMGnV640d56+AGt6XjcHFbnj55dIBQY3c/erkB67kT2+hpXZC5MQYQwJsPNLKpZiHUDTa9cFEsJLlzLT9RCSDnAb1rDEy3LhwT46x0mOYVZJPWrWO0F0WT0lg= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=QUqY/J5U; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="QUqY/J5U" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 9E4DCC2BCAF; Tue, 28 Apr 2026 10:42:30 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1777372951; bh=VbCd9suTkAq0QTyiaF+WctXyADNMKX2UwVQqlIBS9ok=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=QUqY/J5UE/I0cXMxImdEMMq81tVUpP8i6xo1j08H/KYvAkD1b+1gjRykoWssb72Gn ut8M/8hsyC7OAgQr/gO40z6fn9R+Fy9byWQtJPgM1jVzg0TgcD/j1JtLjmGejamcB/ 70bmzgwARXme9Moc/Sg3/6HMm4DfVXGqXBnMPA9eVtdtwRv/2c9bYKVnAt9NmKfJDN x5kD9U7cRd2PVCnSF4sCpG2m+A/KWc9sF8LRDtMlpb1WEgghoKeCc768XW/8LJbCpK CfXsUR70VpHVR4s9JOALCYO4zKjwjbXJNdUEy7/+WigVPfulNfB/v4nrVK1qbTxcP1 RZPiYNdhTYY6Q== From: Sasha Levin To: patches@lists.linux.dev, stable@vger.kernel.org Cc: Yuho Choi , Myeonghun Pak , Ijae Kim , Taegyu Kim , Helge Deller , Sasha Levin , adaplas@gmail.com, linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: [PATCH AUTOSEL 7.0-5.10] fbdev: savage: fix probe-path EDID cleanup leaks Date: Tue, 28 Apr 2026 06:40:52 -0400 Message-ID: <20260428104133.2858589-41-sashal@kernel.org> X-Mailer: git-send-email 2.53.0 In-Reply-To: <20260428104133.2858589-1-sashal@kernel.org> References: <20260428104133.2858589-1-sashal@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-stable: review X-Patchwork-Hint: Ignore X-stable-base: Linux 7.0.2 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable From: Yuho Choi [ Upstream commit 9b8a9a3a6f57edd02b7c8db14a316e6fab7fa772 ] When CONFIG_FB_SAVAGE_I2C is enabled, savagefb_probe() can build both an EDID-derived monspecs.modedb and a modelist from it before later failing. The normal success path frees monspecs.modedb after the initial mode select= ion, but the probe error path only deletes the I2C busses and misses the EDID-derived allocations. Free both the modelist and monspecs.modedb on the failed: unwind path. Co-developed-by: Myeonghun Pak Signed-off-by: Myeonghun Pak Co-developed-by: Ijae Kim Signed-off-by: Ijae Kim Co-developed-by: Taegyu Kim Signed-off-by: Taegyu Kim Signed-off-by: Yuho Choi Signed-off-by: Helge Deller Signed-off-by: Sasha Levin --- LLM Generated explanations, may be completely bogus: ## Phase 1: Commit Message Forensics Step 1.1 Record: subsystem `fbdev: savage`; action verb `fix`; claimed intent is to free EDID-derived probe-path allocations on `savagefb_probe()` failure. Step 1.2 Record: tags present: `Co-developed-by` Myeonghun Pak, Ijae Kim, Taegyu Kim; `Signed-off-by` Myeonghun Pak, Ijae Kim, Taegyu Kim, Yuho Choi, Helge Deller. No `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-by:`, `Link:`, or `Cc: stable@vger.kernel.org` tag in the supplied message. Step 1.3 Record: the body describes a real resource leak when `CONFIG_FB_SAVAGE_I2C=3Dy`, EDID parsing creates `info->monspecs.modedb` and modelist entries, and later probe failure reaches `failed:` without freeing those allocations. Symptom is leaked kernel memory on failed probe. No explicit affected kernel versions or user report are provided. Step 1.4 Record: not hidden; this is explicitly a probe error-path cleanup leak fix. ## Phase 2: Diff Analysis Step 2.1 Record: one file changed, `drivers/video/fbdev/savage/savagefb_driver.c`; 2 lines added, 0 removed; function modified: `savagefb_probe()`; scope is a single-file surgical error-path fix. Step 2.2 Record: before, `failed:` under `CONFIG_FB_SAVAGE_I2C` only deleted I2C busses. After, it also calls `fb_destroy_modelist(&info->modelist)` and `fb_destroy_modedb(info->monspecs.modedb)`. This affects probe unwind paths after EDID/modelist setup. Step 2.3 Record: bug category is resource leak. Verified allocation sources: `fb_edid_to_monspecs()` stores `specs->modedb =3D fb_create_modedb(...)`; `fb_create_modedb()` allocates with `kzalloc_objs()`/`kmalloc_objs()`; `fb_videomode_to_modelist()` calls `fb_add_videomode()`, which allocates `struct fb_modelist`. Verified cleanup helpers free those objects. Step 2.4 Record: fix quality is good: minimal, uses existing fbdev cleanup APIs, no new feature/API. Regression risk is very low. `fb_destroy_modedb(NULL)` is just `kfree(NULL)`, and `fb_destroy_modelist()` safely iterates an initialized empty list. ## Phase 3: Git History Investigation Step 3.1 Record: `git blame` shows the EDID/modelist setup and missing `failed:` cleanup originate from very old code, much of it from the initial imported history; the local EDID pointer handling was adjusted by `0f8a1cae923670` in v5.18-rc1, but the leak pattern existed before that with `par->edid`. Step 3.2 Record: no `Fixes:` tag is present, so no target commit to follow. Step 3.3 Record: recent file history includes related probe fixes: `e8d35898a78e3` fixed a savage probe leak in 2020, `04e5eac8f3ab` handled zero pixclock, and `6ad959b6703e` fixed error handling for `savagefb_check_var()`. No prerequisite was found for this cleanup, because the failed label and cleanup helpers exist independently. Step 3.4 Record: local history has no commits by Yuho Choi under `drivers/video/fbdev`; Helge Deller signed off the supplied commit and is verified in `MAINTAINERS` as framebuffer layer maintainer. The S3 Savage driver entry lists Antonino Daplas as maintainer. Step 3.5 Record: dependency risk is low. The patch only uses `fb_destroy_modelist()` and `fb_destroy_modedb()`, both verified present in v5.15, v6.1, and v6.6 tags. ## Phase 4: Mailing List And External Research Step 4.1 Record: no local commit hash was found with `git log --grep`, so `b4 dig -c ` could not be performed on a real commit object. Attempts to use `b4 dig` with the subject failed: =E2=80=9CCannot find a co= mmit matching ...=E2=80=9D. Lore `WebFetch` searches were blocked by Anubis; web search found no exact subject match. Step 4.2 Record: `b4 dig -w` could not identify recipients for the same reason: no commit object found. Step 4.3 Record: no `Link:` or `Reported-by:` tags were supplied; no external bug report was verified. Step 4.4 Record: no patch series context was verified. Local git history suggests this is standalone. Step 4.5 Record: stable-specific lore search could not be verified because lore fetch was blocked; web search found no exact stable discussion. ## Phase 5: Code Semantic Analysis Step 5.1 Record: modified function: `savagefb_probe()`. Step 5.2 Record: `savagefb_probe()` is assigned as `.probe` in `savagefb_driver`; `savagefb_init()` calls `pci_register_driver(&savagefb_driver)`; `pci_register_driver` maps to `__pci_register_driver()`, which registers the driver with the PCI core. Impact is limited to S3 Savage PCI/AGP devices. Step 5.3 Record: relevant callees are `savagefb_create_i2c_busses()`, `savagefb_probe_i2c_connector()`, `fb_edid_to_monspecs()`, `fb_videomode_to_modelist()`, `register_framebuffer()`, and the cleanup helpers. Verified `savagefb_probe_i2c_connector()` can obtain EDID via DDC or firmware copy. Step 5.4 Record: reachable during PCI device probe at boot, module load, hotplug, or driver bind. I did not verify an unprivileged direct trigger; this appears hardware/config/probe-path reachable, not syscall- hot-path reachable. Step 5.5 Record: similar cleanup patterns exist in other fbdev drivers: `udlfb`, `smscufx`, and `uvesafb` free both `monspecs.modedb` and `modelist` on teardown/error paths. ## Phase 6: Stable Tree Analysis Step 6.1 Record: buggy pattern verified in v4.14, v4.19, v5.10, v5.15, v6.1, v6.6, v6.10, and v6.12 tags: EDID/modelist are created, normal path destroys `monspecs.modedb`, but `failed:` only deletes I2C busses. Step 6.2 Record: expected backport difficulty is clean or minor line- offset adjustment. For older trees, EDID is stored as `par->edid`, but the new cleanup lines only reference `info`, so no semantic dependency on the v5.18 local-variable cleanup. Step 6.3 Record: no related fix for this exact EDID/modelist failed-path leak found in local history. ## Phase 7: Subsystem Context Step 7.1 Record: subsystem is fbdev driver code, specifically S3 Savage framebuffer. Criticality is peripheral/driver-specific, but kernel memory leaks in probe error paths are still real bugs for affected hardware/configurations. Step 7.2 Record: `drivers/video/fbdev` is active, but `drivers/video/fbdev/savage` has low churn. Recent changes are mostly small fixes and cleanups. ## Phase 8: Impact And Risk Step 8.1 Record: affected users are systems with `CONFIG_FB_SAVAGE` and `CONFIG_FB_SAVAGE_I2C` using S3 Savage hardware or matching devices. Step 8.2 Record: trigger requires successful EDID-derived allocation followed by later `savagefb_probe()` failure, such as mode validation/geometry failure or framebuffer registration failure. Repeated privileged probe attempts could accumulate leaks; no unprivileged trigger was verified. Step 8.3 Record: failure mode is kernel memory/resource leak, severity medium: not a crash or corruption, but real unreclaimed kernel memory on failed probe. Step 8.4 Record: benefit is moderate for affected systems because it fixes a verified leak across many stable versions. Risk is very low: two cleanup calls in an existing unwind path, using established helpers. ## Phase 9: Final Synthesis Step 9.1 Record: evidence for backporting: verified real leak, tiny scoped patch, existing helper APIs, same buggy pattern present in many stable tags, no feature/API change, low regression risk. Evidence against: driver-specific old hardware, only a probe failure path, no verified user report/test/lore review. Unresolved: exact upstream commit hash and mailing-list discussion could not be verified. Step 9.2 Record: stable rules checklist: obviously correct by code inspection: yes; fixes a real bug: yes, leaked `modedb` and modelist allocations; important issue: medium, resource leak rather than crash; small and contained: yes, 2 lines in one function; no new features/APIs: yes; can apply to stable: likely yes, helpers and buggy code verified in stable tags. Step 9.3 Record: not a device-ID, quirk, DT, build, or documentation exception. It is a standard resource-leak fix. Step 9.4 Decision: backport. The issue is not high severity, but the fix is minimal, correct, uses existing cleanup APIs, and addresses a verified kernel memory leak present across stable trees. Verification: - [Phase 1] Parsed supplied subject/body/tags; confirmed no Fixes/Reported/Tested/Reviewed/Acked/Link/Cc stable tags in the supplied message. - [Phase 2] Read `savagefb_probe()` and helper implementations; confirmed missing failed-path cleanup and verified allocation/free behavior. - [Phase 3] Ran `git blame`, `git log`, `git show`, and ancestry checks without `--all`; found related savage probe/error-path history and no local candidate commit. - [Phase 4] Ran `b4 dig` attempts, web search, and lore fetch attempts; no exact commit/thread verified, lore fetch blocked by Anubis. - [Phase 5] Traced `savagefb_probe()` registration through the PCI driver structure and `pci_register_driver()`. - [Phase 6] Used tag-scoped `git grep` on v4.14, v4.19, v5.10, v5.15, v6.1, v6.6, v6.10, and v6.12; confirmed the buggy pattern exists. - [Phase 7] Checked `MAINTAINERS`; verified fbdev and S3 Savage maintainer entries. - [Phase 8] Verified trigger and severity from code paths; unprivileged trigger remains unverified and did not drive the decision. **YES** drivers/video/fbdev/savage/savagefb_driver.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/video/fbdev/savage/savagefb_driver.c b/drivers/video/f= bdev/savage/savagefb_driver.c index ac41f8f37589f..c2f79357c8da0 100644 --- a/drivers/video/fbdev/savage/savagefb_driver.c +++ b/drivers/video/fbdev/savage/savagefb_driver.c @@ -2322,6 +2322,8 @@ static int savagefb_probe(struct pci_dev *dev, const = struct pci_device_id *id) failed: #ifdef CONFIG_FB_SAVAGE_I2C savagefb_delete_i2c_busses(info); + fb_destroy_modelist(&info->modelist); + fb_destroy_modedb(info->monspecs.modedb); #endif fb_alloc_cmap(&info->cmap, 0, 0); savage_unmap_video(info); --=20 2.53.0