public inbox for linux-crypto@vger.kernel.org
 help / color / mirror / Atom feed
From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Saeed Mirzamohammadi <saeed.mirzamohammadi@oracle.com>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	Sasha Levin <sashal@kernel.org>,
	davem@davemloft.net, linux-crypto@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH AUTOSEL 7.0-5.10] crypto: tcrypt - clamp num_mb to avoid divide-by-zero
Date: Mon, 20 Apr 2026 09:19:18 -0400	[thread overview]
Message-ID: <20260420132314.1023554-164-sashal@kernel.org> (raw)
In-Reply-To: <20260420132314.1023554-1-sashal@kernel.org>

From: Saeed Mirzamohammadi <saeed.mirzamohammadi@oracle.com>

[ Upstream commit 32e76e3757e89f370bf2ac8dba8aeb133071834e ]

Passing num_mb=0 to the multibuffer speed tests leaves test_mb_aead_cycles()
and test_mb_acipher_cycles() dividing by (8 * num_mb). With sec=0 (the
default), the module prints "1 operation in ..." and hits a divide-by-zero
fault.

Force num_mb to at least 1 during module init and warn the caller so the
warm-up loop and the final report stay well-defined.

To reproduce:
sudo modprobe tcrypt mode=600 num_mb=0

Signed-off-by: Saeed Mirzamohammadi <saeed.mirzamohammadi@oracle.com>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Now I have all the information needed for a thorough analysis. Let me
compile my findings.

---

## PHASE 1: COMMIT MESSAGE FORENSICS

### Step 1.1: Subject Line
- **Subsystem**: `crypto: tcrypt`
- **Action verb**: "clamp" (defensive input validation)
- **Summary**: Clamps `num_mb` module parameter to minimum 1 to avoid
  divide-by-zero

### Step 1.2: Tags
- **Signed-off-by**: Saeed Mirzamohammadi (author), Herbert Xu (crypto
  subsystem maintainer)
- No Fixes: tag (expected for this review pipeline)
- No Reported-by, Tested-by, Reviewed-by, or Cc: stable tags
- Herbert Xu's SOB confirms the crypto maintainer accepted this patch

### Step 1.3: Commit Body
- **Bug**: Passing `num_mb=0` causes `test_mb_aead_cycles()` and
  `test_mb_acipher_cycles()` to divide by `(8 * num_mb)` = 0
- **Symptom**: kernel divide-by-zero fault (oops)
- **Reproduction**: `sudo modprobe tcrypt mode=600 num_mb=0`
- **Root cause**: No validation that `num_mb` (a `uint` module
  parameter) must be >= 1

### Step 1.4: Hidden Bug Fix Detection
This is an explicit bug fix. The commit message clearly describes the
divide-by-zero.

Record: Not a hidden fix; it's an explicit divide-by-zero fix.

## PHASE 2: DIFF ANALYSIS

### Step 2.1: Inventory
- **Files changed**: `crypto/tcrypt.c` (+5 lines)
- **Functions modified**: `tcrypt_mod_init()` only
- **Scope**: Single-file, single-function, surgical fix

### Step 2.2: Code Flow
- **Before**: `tcrypt_mod_init()` passed `num_mb` directly to
  `do_test()` without validation
- **After**: Checks if `num_mb == 0`, warns, and sets it to 1 before
  calling `do_test()`
- Path affected: module initialization (normal path, not error path)

### Step 2.3: Bug Mechanism
Category: **Logic/correctness fix** - missing input validation leading
to divide-by-zero.

The division expressions are at:
- Line 236: `(cycles + 4) / (8 * num_mb)` in `test_mb_aead_cycles()`
- Line 1053: `(cycles + 4) / (8 * num_mb)` in `test_mb_acipher_cycles()`

When `num_mb=0`, `8 * 0 = 0`, causing a kernel divide-by-zero
fault/oops.

### Step 2.4: Fix Quality
- **Obviously correct**: Yes - trivial clamping of an input parameter
- **Minimal**: Yes - 4 effective lines added
- **Regression risk**: Essentially zero - the only change is that
  `num_mb=0` becomes `num_mb=1` instead of crashing
- **Red flags**: None

## PHASE 3: GIT HISTORY

### Step 3.1: Blame
The division expressions were introduced by commit `4e234eed58518a`
(Kees Cook, "crypto: tcrypt - Remove VLA usage", 2018-04-26). This
commit landed in v4.18-rc1.

Record: Buggy code introduced in v4.18, present in ALL active stable
trees (5.4, 5.10, 5.15, 6.1, 6.6, 6.12).

### Step 3.2: Fixes Tag
No Fixes: tag present. If there were one, it would logically point to
`4e234eed58518a`.

### Step 3.3: File History
Recent tcrypt changes are mostly adding/removing test algorithms, not
related to this bug. No prerequisites identified.

### Step 3.4: Author
Saeed Mirzamohammadi has 3 commits in the tree - one HID quirk, one
fbdev divide fix, one netfilter fix. Not a regular crypto contributor,
but the patch was accepted by Herbert Xu (crypto maintainer).

### Step 3.5: Dependencies
No dependencies. The fix is self-contained and the code context
(`tcrypt_mod_init`) is stable across all kernel versions since v4.18.

## PHASE 4: MAILING LIST RESEARCH

Lore.kernel.org was unavailable due to anti-bot protection. Web searches
did not find the specific patch thread.

Record: Could not verify mailing list discussion. However, the patch was
accepted by Herbert Xu, the crypto subsystem maintainer, which is strong
evidence of review.

## PHASE 5: CODE SEMANTIC ANALYSIS

### Step 5.1: Functions Modified
Only `tcrypt_mod_init()` - the module's init function.

### Step 5.2: Callers
`tcrypt_mod_init()` is called once during `modprobe tcrypt`. It's the
module's `late_initcall` entry point.

### Step 5.3-5.4: Call Chain
The divide-by-zero path: `tcrypt_mod_init()` -> `do_test()` ->
`test_mb_aead_speed()` / `test_mb_skcipher_speed()` ->
`test_mb_aead_cycles()` / `test_mb_acipher_cycles()` -> division by `(8
* num_mb)`.

Trigger: `modprobe tcrypt mode=600 num_mb=0` (requires root).

### Step 5.5: Similar Patterns
Both `test_mb_aead_cycles()` (line 236) and `test_mb_acipher_cycles()`
(line 1053) have the identical `(8 * num_mb)` division. The fix at
module init covers both.

## PHASE 6: STABLE TREE ANALYSIS

### Step 6.1: Buggy Code in Stable
The buggy division was introduced in v4.18 (commit `4e234eed58518a`). It
exists in ALL active stable trees: 5.4.y, 5.10.y, 5.15.y, 6.1.y, 6.6.y,
6.12.y.

### Step 6.2: Backport Complications
`tcrypt_mod_init()` is straightforward and has been stable for years.
The patch should apply cleanly to all stable trees. The recent
`kzalloc_objs` refactoring (v7.0-specific) is only in the
`test_mb_*_cycles` functions, not in `tcrypt_mod_init()`.

### Step 6.3: Related Fixes
No existing fix for this specific divide-by-zero issue was found in any
stable tree.

## PHASE 7: SUBSYSTEM CONTEXT

### Step 7.1: Subsystem
- **Subsystem**: crypto (specifically the tcrypt benchmark module)
- **Criticality**: PERIPHERAL - tcrypt is a benchmarking/test module,
  not used in production crypto operations. However, it's a standard
  kernel module that can be loaded by root.

### Step 7.2: Activity
The crypto subsystem and tcrypt specifically are moderately active with
ongoing changes.

## PHASE 8: IMPACT AND RISK ASSESSMENT

### Step 8.1: Who Is Affected
Anyone who loads the tcrypt module with `num_mb=0`. This is primarily
kernel developers and system administrators running crypto benchmarks.

### Step 8.2: Trigger Conditions
- Requires root (modprobe)
- Requires deliberately passing `num_mb=0` - however, `num_mb` is a
  `uint` parameter with no documented minimum, so passing 0 is a
  "reasonable" (if mistaken) value
- Deterministic - always triggers with `num_mb=0`

### Step 8.3: Failure Mode
- **Divide-by-zero kernel fault/oops**: This is a kernel crash. On some
  configurations (panic_on_oops=1), this brings down the entire system.
- **Severity**: HIGH (kernel oops, but requires root and specific module
  parameter)

### Step 8.4: Risk-Benefit
- **Benefit**: Prevents a kernel oops in a standard kernel module. Low-
  medium benefit (affects test module users only, but the crash is
  real).
- **Risk**: VERY LOW - 4 lines of trivial input validation in module
  init. Zero regression potential.
- **Ratio**: Favorable - low-to-medium benefit with essentially zero
  risk.

## PHASE 9: FINAL SYNTHESIS

### Step 9.1: Evidence

**FOR backporting:**
- Fixes a real divide-by-zero that causes a kernel oops
- Reproducible with a simple command (`modprobe tcrypt mode=600
  num_mb=0`)
- Fix is 4 lines, obviously correct, and zero regression risk
- Bug has existed since v4.18, present in all active stable trees
- Accepted by the crypto subsystem maintainer (Herbert Xu)
- No dependencies - standalone fix

**AGAINST backporting:**
- tcrypt is a benchmark module, not production code
- Requires root to trigger
- Requires a non-default parameter value (default is 8)
- Low real-world impact

### Step 9.2: Stable Rules Checklist
1. **Obviously correct and tested?** YES - trivial clamping, author
   provided reproduction steps
2. **Fixes a real bug?** YES - divide-by-zero causing kernel oops
3. **Important issue?** MODERATE - kernel oops, but in a test module
   with root-only trigger
4. **Small and contained?** YES - 4 lines in one function
5. **No new features?** YES - input validation only
6. **Can apply to stable?** YES - clean apply expected

### Step 9.3: Exception Categories
Not applicable (not a device ID, quirk, DT, or build fix).

### Step 9.4: Decision

This is a small, obviously correct fix for a real divide-by-zero that
causes a kernel oops. While the impact is limited to users of the tcrypt
benchmark module who pass `num_mb=0`, the fix is trivial, risk-free, and
the bug exists in all active stable trees since v4.18. The kernel should
not oops on any valid `uint` module parameter value, and the fix follows
the principle of defensive input validation.

## Verification

- [Phase 1] Parsed tags: Signed-off-by from author and Herbert Xu
  (crypto maintainer)
- [Phase 2] Diff analysis: 4 lines added in `tcrypt_mod_init()`,
  clamping `num_mb` to min 1
- [Phase 2] Confirmed divide-by-zero at lines 236 and 1053: `(cycles +
  4) / (8 * num_mb)`
- [Phase 3] git blame: Division expression introduced in commit
  `4e234eed58518a` (Kees Cook, v4.18-rc1)
- [Phase 3] git describe --contains: Bug present since v4.18, in all
  stable trees
- [Phase 3] git log: No related fixes or prerequisites found
- [Phase 3] Author has 3 commits in tree; patch accepted by crypto
  maintainer Herbert Xu
- [Phase 4] UNVERIFIED: Lore discussion not accessible (anti-bot
  protection blocked access)
- [Phase 5] Traced call chain: `tcrypt_mod_init` -> `do_test` ->
  `test_mb_*_speed` -> `test_mb_*_cycles` -> division by `(8 * num_mb)`
- [Phase 6] Code exists in all active stable trees (v5.4+) - verified
  buggy commit in v4.18
- [Phase 6] Backport expected to apply cleanly - `tcrypt_mod_init()` is
  stable
- [Phase 8] Failure mode: kernel divide-by-zero oops, severity HIGH but
  limited user base

**YES**

 crypto/tcrypt.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index aded375461374..61c8cf55c4f1e 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -2808,6 +2808,11 @@ static int __init tcrypt_mod_init(void)
 			goto err_free_tv;
 	}
 
+	if (!num_mb) {
+		pr_warn("num_mb must be at least 1; forcing to 1\n");
+		num_mb = 1;
+	}
+
 	err = do_test(alg, type, mask, mode, num_mb);
 
 	if (err) {
-- 
2.53.0


  parent reply	other threads:[~2026-04-20 13:27 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260420132314.1023554-1-sashal@kernel.org>
2026-04-20 13:17 ` [PATCH AUTOSEL 6.18] crypto: algif_aead - Fix minimum RX size check for decryption Sasha Levin
2026-04-20 13:19 ` [PATCH AUTOSEL 6.18] crypto: af_alg - Fix page reassignment overflow in af_alg_pull_tsgl Sasha Levin
2026-04-20 13:19 ` Sasha Levin [this message]
2026-04-20 13:21 ` [PATCH AUTOSEL 6.18] crypto: af_alg - limit RX SG extraction by receive buffer budget Sasha Levin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260420132314.1023554-164-sashal@kernel.org \
    --to=sashal@kernel.org \
    --cc=davem@davemloft.net \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=patches@lists.linux.dev \
    --cc=saeed.mirzamohammadi@oracle.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox