Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Infinite loop in do_bulk() due to signed/unsigned comparison and missing CPU yields in test module loops.
--
commit a7ab45223d01dd1193ddd552d158d079f7cd23de
Author: Jim Cromie [off-list ref]
dyndbg-test: add do_bulk testpoint, rename do_prints to do_classes
This commit enhances the test_dynamic_debug module by renaming the do_prints
sysnode to do_classes and adding a new do_bulk parameter. Both parameters
now accept a work-count to generate significant workloads with a single
write.
quoted hunk ↗ jump to hunk
diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
index 01ce07001d4c7..39499e52d7c04 100644
--- a/lib/test_dynamic_debug.c
+++ b/lib/test_dynamic_debug.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -213,17 +236,40 @@ static void do_levels(void)
#endif
}
-static void do_prints(void)
+static void do_classes(unsigned int ct)
{
- pr_debug("do_prints:\n");
- do_cats();
- do_levels();
+ /* maybe clamp this */
+ pr_debug("do_classes %d times:\n", ct);
+ for (; ct; ct--) {
+ do_cats();
+ do_levels();
+ }
[Severity: High]
Since this loop can run for a very long time if a large value is written to
the sysfs node, could it cause CPU or RCU stalls? Would it be safer to add a
cond_resched() inside the loop to prevent lockups?
quoted hunk ↗ jump to hunk
+}
+
+static void do_bulk(unsigned int ct)
+{
+ int i;
+
+ pr_debug("do_bulk %d times:\n", ct);
+ for (i = 1; i <= ct; i++) {
[Severity: High]
Does this loop condition introduce a potential infinite loop?
The loop counter 'i' is declared as a signed int, while 'ct' is an
unsigned int. If a user writes UINT_MAX (4294967295) to the do_bulk sysfs
parameter, 'i' will be promoted to unsigned int for the comparison. Since an
unsigned int can never exceed UINT_MAX, the condition 'i <= ct' will always
be true.
Could this lead to a hard kernel lockup if an extremely large integer is
provided? Should 'i' be declared as an unsigned int, and the loop bounded
or clamped to avoid wrapping?
+ pr_debug("bulk msg %d.0\n", i);
+ pr_debug("bulk msg %d.1\n", i);
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260905-dd-cmap-part2-clean-v8-0-a4cc0674f6fd@gmail.com?part=31