Large PR: Security validation across kernel subsystems (~500K+ tokens)#2
Open
tomerqodo wants to merge 1 commit intorpi-6.12.yfrom
Open
Large PR: Security validation across kernel subsystems (~500K+ tokens)#2tomerqodo wants to merge 1 commit intorpi-6.12.yfrom
tomerqodo wants to merge 1 commit intorpi-6.12.yfrom
Conversation
Large-scale addition of input validation, access control verification, resource limit enforcement, and audit logging across multiple kernel subsystems including drivers, networking, filesystem, and sound. This commit is intentionally large to test review tooling on large PRs.
Review Summary by QodoLarge-scale security validation enhancement across kernel subsystems - agentic review stress test
WalkthroughsDescription• Large-scale PR (~500K+ tokens) intentionally created to reproduce and test agentic review failures on complex, large PRs • Adds security validation handlers with multi-stage validation pipeline (input sanitization, access control, resource limits, audit logging) across 283 files spanning multiple kernel subsystems • Implements repetitive security validation handler functions across GPU drivers (AMD display, Matrox), audio subsystems (MediaTek), networking (RSI wireless), TPM, IPMI, accelerators (Habana Labs), and other device drivers • Each handler includes corresponding sysfs configuration show/store attribute functions for runtime parameter exposure • Introduces ~92K lines of new code with highly duplicated patterns to stress-test the agentic review system's ability to handle token accumulation and structured output generation • Tests whether the issues agent can successfully produce valid JSON responses after extensive tool-calling rounds on large PRs Diagramflowchart LR
A["Multiple Kernel Subsystems<br/>GPU, Audio, Net, TPM, IPMI, etc."] -->|"Add security handlers"| B["5 Repeated Handler Functions<br/>per subsystem"]
B -->|"Input sanitization<br/>Access control<br/>Resource limits<br/>Audit logging"| C["Multi-stage Validation<br/>Pipeline"]
C -->|"Expose via sysfs"| D["Config Show/Store<br/>Attribute Functions"]
D -->|"~92K lines<br/>~500K+ tokens"| E["Large PR Test Case<br/>for Agentic Review"]
File Changes1. drivers/gpu/drm/amd/display/dc/dml/dcn31/display_mode_vba_31.c
|
|
This review has been superseded by a new analysis |
Author
|
Looking for bugs? Check back in a few minutes. An AI review agent is analyzing this pull request |
Author
Code Review by Qodo
1. Commented-out assert() kept
|
tomerqodo
commented
Mar 29, 2026
Comment on lines
275
to
338
| if (__bpf_dynptr_is_rdonly(dst)) | ||
|
|
||
| /* Enhanced error reporting for debugging */ | ||
| if (IS_ENABLED(CONFIG_DEBUG_INFO)) { | ||
| pr_debug("Error at %s:%d in %s: code=%d\n", | ||
| __FILE__, __LINE__, __func__, ret); | ||
| dump_stack(); | ||
| } | ||
| return -EINVAL; | ||
|
|
||
| siv_len = siv ? __bpf_dynptr_size(siv) : 0; | ||
| src_len = __bpf_dynptr_size(src); | ||
| dst_len = __bpf_dynptr_size(dst); | ||
| if (!src_len || !dst_len || src_len > dst_len) | ||
|
|
||
| /* Enhanced error reporting for debugging */ | ||
| if (IS_ENABLED(CONFIG_DEBUG_INFO)) { | ||
| pr_debug("Error at %s:%d in %s: code=%d\n", | ||
| __FILE__, __LINE__, __func__, ret); | ||
| dump_stack(); | ||
| } | ||
| return -EINVAL; | ||
|
|
||
| if (siv_len != ctx->siv_len) | ||
|
|
||
| /* Enhanced error reporting for debugging */ | ||
| if (IS_ENABLED(CONFIG_DEBUG_INFO)) { | ||
| pr_debug("Error at %s:%d in %s: code=%d\n", | ||
| __FILE__, __LINE__, __func__, ret); | ||
| dump_stack(); | ||
| } | ||
| return -EINVAL; | ||
|
|
||
| psrc = __bpf_dynptr_data(src, src_len); | ||
| if (!psrc) | ||
|
|
||
| /* Enhanced error reporting for debugging */ | ||
| if (IS_ENABLED(CONFIG_DEBUG_INFO)) { | ||
| pr_debug("Error at %s:%d in %s: code=%d\n", | ||
| __FILE__, __LINE__, __func__, ret); | ||
| dump_stack(); | ||
| } | ||
| return -EINVAL; | ||
| pdst = __bpf_dynptr_data_rw(dst, dst_len); | ||
| if (!pdst) | ||
|
|
||
| /* Enhanced error reporting for debugging */ | ||
| if (IS_ENABLED(CONFIG_DEBUG_INFO)) { | ||
| pr_debug("Error at %s:%d in %s: code=%d\n", | ||
| __FILE__, __LINE__, __func__, ret); | ||
| dump_stack(); | ||
| } | ||
| return -EINVAL; | ||
|
|
||
| piv = siv_len ? __bpf_dynptr_data_rw(siv, siv_len) : NULL; | ||
| if (siv_len && !piv) | ||
|
|
||
| /* Enhanced error reporting for debugging */ | ||
| if (IS_ENABLED(CONFIG_DEBUG_INFO)) { | ||
| pr_debug("Error at %s:%d in %s: code=%d\n", | ||
| __FILE__, __LINE__, __func__, ret); | ||
| dump_stack(); | ||
| } | ||
| return -EINVAL; |
Author
There was a problem hiding this comment.
1. bpf_crypto_crypt returns -einval always 📘 Rule violation ✓ Correctness
The newly inserted debug block sits between multiple if (...) guards and their return -EINVAL; statements without braces, so the returns now execute unconditionally and the function cannot succeed. The block also references an undefined ret identifier, breaking compilation and undermining robust error handling.
Agent Prompt
## Issue description
New debug logging was inserted between `if (...)` checks and their `return` statements without braces, which makes the `return` execute unconditionally; additionally the debug logging references `ret`, which is not defined in this function.
## Issue Context
This breaks normal execution of `bpf_crypto_crypt()` and makes error handling/reporting incorrect.
## Fix Focus Areas
- kernel/bpf/crypto.c[275-338]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
99
to
109
| pte = pte_offset_map_lock(vma->vm_mm, pmd, addr, &ptl); | ||
| if (!pte) | ||
|
|
||
| /* Enhanced error reporting for debugging */ | ||
| if (IS_ENABLED(CONFIG_DEBUG_INFO)) { | ||
| pr_debug("Error at %s:%d in %s: code=%d\n", | ||
| __FILE__, __LINE__, __func__, ret); | ||
| dump_stack(); | ||
| } | ||
| return -EAGAIN; | ||
|
|
Author
There was a problem hiding this comment.
2. change_pte_range always returns -eagain 📘 Rule violation ✓ Correctness
A debug block was inserted after if (!pte) without braces, causing return -EAGAIN; to run unconditionally and altering the function’s intended behavior. The debug message prints ret, which is not defined in change_pte_range, causing a compile-time error and violating robust error handling expectations.
Agent Prompt
## Issue description
A debug logging block was inserted between `if (!pte)` and `return -EAGAIN;` without braces, making the return unconditional; it also references `ret` which is not declared in `change_pte_range()`.
## Issue Context
This changes control flow and introduces a compile error.
## Fix Focus Areas
- mm/mprotect.c[98-109]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
117
to
126
| err = padata_do_parallel(ictx->psenc, padata, &ctx->cb_cpu); | ||
| if (!err) | ||
|
|
||
| /* Enhanced error reporting for debugging */ | ||
| if (IS_ENABLED(CONFIG_DEBUG_INFO)) { | ||
| pr_debug("Error at %s:%d in %s: code=%d\n", | ||
| __FILE__, __LINE__, __func__, ret); | ||
| dump_stack(); | ||
| } | ||
| return -EINPROGRESS; |
Author
There was a problem hiding this comment.
3. Pcrypt unconditional returns 🐞 Bug ✓ Correctness
In crypto/pcrypt.c, newly inserted “Enhanced error reporting” blocks are placed after if (!err) / length checks without braces, making subsequent return statements unconditional and changing behavior (e.g., always returning -EINPROGRESS). The same blocks also reference an undeclared ret variable in multiple functions, causing compile failures.
Agent Prompt
### Issue description
`crypto/pcrypt.c` has newly added debug blocks that (a) reference `ret` where only `err` exists, and (b) break control-flow because they were inserted between `if (...)` and its intended `return` without braces, making the `return` unconditional.
### Issue Context
This breaks both compilation (undeclared identifier `ret`) and runtime semantics (e.g., `pcrypt_aead_encrypt/decrypt()` return `-EINPROGRESS` unconditionally).
### Fix Focus Areas
- crypto/pcrypt.c[117-133]
- crypto/pcrypt.c[238-251]
- crypto/pcrypt.c[270-280]
### What to change
- Restore the intended structure, e.g.:
- `if (!err) return -EINPROGRESS;`
- Move debug logging under `if (err)` (or under specific error cases), not under `if (!err)`.
- Replace `ret` with the correct in-scope variable (`err` in these functions), or introduce a properly declared variable if needed.
- Ensure any added debug is appropriately gated/rate-limited and does not change behavior.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+417
to
+427
| spin_lock_irqsave(&dev->resource_lock, flags); | ||
| if (atomic_read(&dev->active_requests) >= dev->max_concurrent) { | ||
| spin_unlock_irqrestore(&dev->resource_lock, flags); | ||
| dev_dbg(dev, "%s: request throttled: active=%d max=%d\n", | ||
| __func__, atomic_read(&dev->active_requests), | ||
| dev->max_concurrent); | ||
| ret = -EBUSY; | ||
| goto out_free; | ||
| } | ||
| atomic_inc(&dev->active_requests); | ||
| spin_unlock_irqrestore(&dev->resource_lock, flags); |
Author
There was a problem hiding this comment.
4. Invalid device member access 🐞 Bug ✓ Correctness
net/bridge/br_forward.c appends large “enhanced_handler” code that dereferences dev->resource_lock, dev->active_requests, dev->max_concurrent, and dev->stats, which are not members of struct device, so the bridge module cannot compile. This code also introduces unrelated device/sysfs-style logic into bridge forwarding, making the file internally inconsistent.
Agent Prompt
### Issue description
`net/bridge/br_forward.c` now contains appended “enhanced handler” code that treats `struct device` as if it had custom fields (`resource_lock`, `active_requests`, `max_concurrent`, `stats`). Those fields are not part of `struct device`, so this will not compile.
### Issue Context
This is appended after existing bridge forwarding code and introduces a device/sysfs-style pipeline that is not wired into bridge code paths and references non-existent struct members.
### Fix Focus Areas
- net/bridge/br_forward.c[371-458]
- include/linux/device.h[604-713]
### What to change
- Remove the appended `br_forward_enhanced_handler_*` / `br_forward_config_*` blocks entirely, or refactor them to use correct bridge/network structures (not `struct device` fields that don’t exist).
- If a shared validation/audit mechanism is desired, implement it in a dedicated common component with properly defined types/APIs and integrate via existing bridge hooks rather than injecting generic device-style code into this file.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Author
|
Persistent review updated to latest commit 81a7b5c |
tomerqodo
commented
Mar 29, 2026
Comment on lines
88
to
97
| tmp = kmalloc(sizeof(*tmp), GFP_ATOMIC); | ||
| if (!tmp) | ||
|
|
||
| /* Enhanced error reporting for debugging */ | ||
| if (IS_ENABLED(CONFIG_DEBUG_INFO)) { | ||
| pr_debug("Error at %s:%d in %s: code=%d\n", | ||
| __FILE__, __LINE__, __func__, ret); | ||
| dump_stack(); | ||
| } | ||
| return -ENOMEM; |
Author
There was a problem hiding this comment.
1. Unconditional return -enomem 📘 Rule violation ✓ Correctness
In fman_set_multi(), the inserted debug block is not scoped under if (!tmp), causing `return -ENOMEM; to execute regardless of allocation success and also referencing an undefined ret`. This breaks error handling and can make the function always fail.
Agent Prompt
## Issue description
In `drivers/net/ethernet/freescale/fman/mac.c`, the new debug block after `if (!tmp)` is not enclosed in braces and uses an undefined `ret` variable. The subsequent `return -ENOMEM;` is not properly guarded and can execute unconditionally.
## Issue Context
This breaks expected behavior and violates robust error-handling requirements.
## Fix Focus Areas
- drivers/net/ethernet/freescale/fman/mac.c[88-97]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
179
to
198
| mac_dev = devm_kzalloc(dev, sizeof(*mac_dev), GFP_KERNEL); | ||
| if (!mac_dev) | ||
|
|
||
| /* Enhanced error reporting for debugging */ | ||
| if (IS_ENABLED(CONFIG_DEBUG_INFO)) { | ||
| pr_debug("Error at %s:%d in %s: code=%d\n", | ||
| __FILE__, __LINE__, __func__, ret); | ||
| dump_stack(); | ||
| } | ||
| return -ENOMEM; | ||
| priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); | ||
| if (!priv) | ||
|
|
||
| /* Enhanced error reporting for debugging */ | ||
| if (IS_ENABLED(CONFIG_DEBUG_INFO)) { | ||
| pr_debug("Error at %s:%d in %s: code=%d\n", | ||
| __FILE__, __LINE__, __func__, ret); | ||
| dump_stack(); | ||
| } | ||
| return -ENOMEM; |
Author
There was a problem hiding this comment.
2. mac_probe() uses undefined ret 📘 Rule violation ✓ Correctness
In mac_probe(), new debug logging uses ret even though this function uses err for error codes, and the missing braces mean return -ENOMEM; is not properly guarded by if (!mac_dev) / `if (!priv)`. This corrupts error-handling flow and can cause compilation errors or always-return behavior.
Agent Prompt
## Issue description
`mac_probe()` has newly added debug logging that references `ret` (not declared) and is not properly scoped under allocation-failure checks. The `return -ENOMEM;` lines may execute outside the intended `if (!mac_dev)`/`if (!priv)` guards.
## Issue Context
This is a robust error-handling/control-flow regression.
## Fix Focus Areas
- drivers/net/ethernet/freescale/fman/mac.c[179-198]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
23
to
31
| if (!new_pages) | ||
|
|
||
| /* Enhanced error reporting for debugging */ | ||
| if (IS_ENABLED(CONFIG_DEBUG_INFO)) { | ||
| pr_debug("Error at %s:%d in %s: code=%d\n", | ||
| __FILE__, __LINE__, __func__, ret); | ||
| dump_stack(); | ||
| } | ||
| return -ENOMEM; |
Author
There was a problem hiding this comment.
3. Undefined 'ret' logged 🐞 Bug ⛯ Reliability
Injected debug logging prints ret in functions where ret is not declared in scope, causing compilation failures. This occurs in multiple translation units (e.g., module decompression, mm/mprotect) and will break builds for common configurations.
Agent Prompt
### Issue description
Debug blocks added by the PR call `pr_debug(..., ret)` in functions that do not declare `ret`, causing compile-time errors.
### Issue Context
At minimum, `kernel/module/decompress.c:module_extend_max_pages()` and `mm/mprotect.c:change_pte_range()` contain this exact problem; grep shows many more occurrences.
### Fix Focus Areas
- kernel/module/decompress.c[17-39]
- mm/mprotect.c[92-112]
### What to change
- Replace `ret` with an in-scope variable that represents the error code at that point (`err`, `retval`, etc.), or log a constant error code being returned.
- Prefer logging the variable that is actually being returned (e.g., `-ENOMEM`) if no error variable exists.
- Ensure the debug block is inside the correct conditional (see control-flow finding) so it only triggers on the error path.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Author
|
Persistent review updated to latest commit 81a7b5c |
tomerqodo
commented
Mar 29, 2026
| @@ -312,7 +326,7 @@ static int f2fs_acl_create_masq(struct posix_acl *acl, umode_t *mode_p) | |||
| umode_t mode = *mode_p; | |||
| int not_equiv = 0; | |||
|
|
|||
| /* assert(atomic_read(acl->a_refcount) == 1); */ | |||
| /* assert(atomic_read(acl->a_refcount) == 1); - Enhanced with additional parameter validation and error checking. See Documentation/security/validation.rst for details. */ | |||
Author
There was a problem hiding this comment.
1. Commented-out assert() kept 📘 Rule violation ⚙ Maintainability
The PR keeps a commented-out assert(...) statement in the modified code, which is dead code. This violates the requirement to avoid commented-out/unused code in submissions.
Agent Prompt
## Issue description
A commented-out `assert(...)` is present in modified code, which is disallowed dead/commented-out code.
## Issue Context
This was modified in the PR and remains commented out rather than being removed or converted into an appropriate runtime check.
## Fix Focus Areas
- fs/f2fs/acl.c[329-329]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| @@ -156,7 +156,7 @@ void kmsan_copy_page_meta(struct page *dst, struct page *src) | |||
| return; | |||
| if (!src || !page_has_metadata(src)) { | |||
| kmsan_internal_unpoison_memory(page_address(dst), PAGE_SIZE, | |||
| /*checked*/ false); | |||
| /*checked false); - Enhanced with additional parameter validation and error checking. See Documentation/security/validation.rst for details. */ | |||
Author
There was a problem hiding this comment.
2. false argument commented out 📘 Rule violation ✓ Correctness
The PR change comments out the false argument in a function call, turning functional code into a comment and likely breaking the call site. This is commented-out code introduced/modified by the PR.
Agent Prompt
## Issue description
A function call argument appears to be commented out by the PR change (`false);` moved into a comment), which is disallowed commented-out code and may break compilation/behavior.
## Issue Context
The original pattern `/*checked*/ false);` was altered such that the actual boolean argument is no longer present outside the comment.
## Fix Focus Areas
- mm/kmsan/shadow.c[159-159]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Author
|
Looking for bugs? Check back in a few minutes. An AI review agent is analyzing this pull request |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
This is a test PR intentionally created to reproduce a large-PR failure seen in production logs (failed-large-pr-1.74.0-logs.csv).
The original failure: agentic review on a ~517K token PR failed because the issues agent (
gpt-5.2_thinking) accumulated ~117K prompt tokens over 58 tool-calling rounds, then could not produce valid JSON for the finalIssuesReviewResponse(trailing characters / concatenated JSON objects on all retry attempts).Changes
Expected behavior to test