Linux Security Modules development
 help / color / mirror / Atom feed
* [PATCH AUTOSEL 6.18-5.10] ima: return error early if file xattr cannot be changed
       [not found] <20260831133314.4125787-1-sashal@kernel.org>
@ 2026-08-31 13:21 ` Sasha Levin
  2026-08-31 13:21 ` [PATCH AUTOSEL 6.18-5.15] integrity: Check for NULL returned by asymmetric_key_public_key Sasha Levin
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-08-31 13:21 UTC (permalink / raw)
  To: patches, stable
  Cc: Goldwyn Rodrigues, Goldwyn Rodrigues, Mimi Zohar, Sasha Levin,
	roberto.sassu, dmitry.kasatkin, paul, jmorris, serge,
	linux-integrity, linux-security-module, linux-kernel

From: Goldwyn Rodrigues <rgoldwyn@suse.de>

[ Upstream commit 69fc6474236d9edda6983623e4282f2bdfd8e3d8 ]

During early boot, the filesystem is read-only and any changes
to xattrs are not allowed. This fails in case of ext4 because
changing xattr starts an ext4 transaction which fails with the
following warning.

WARNING: fs/ext4/ext4_jbd2.c:75 at ext4_journal_check_start+0x63/0xa0 [ext4], CPU#1: systemd-sysroot/561
CPU: 1 UID: 0 PID: 561 Comm: systemd-sysroot Not tainted 6.19.12-1-default #1 PREEMPT(voluntary) openSUSE Tumbleweed  c2dfc3c9d9f6f1233251c5d4410574fe82a348ee
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS unknown 02/02/2022
RIP: 0010:ext4_journal_check_start+0x63/0xa0 [ext4]
Call Trace:
  __ext4_journal_start_sb+0x3e/0x180 [ext4 6d025f3bc52c89a957b89a89d211fadf5e9434e1]
  ext4_xattr_set+0x104/0x150 [ext4 6d025f3bc52c89a957b89a89d211fadf5e9434e1]
  __vfs_setxattr+0x9a/0xd0
  __vfs_setxattr_noperm+0x76/0x1f0
  ima_appraise_measurement+0x23e/0xe40
  ima_d_path+0x5a/0xd0
  process_measurement+0xb29/0xc40
  ? copy_from_kernel_nofault+0x21/0xe0
  ? fscrypt_file_open+0xc0/0xe0
  ? ext4_file_open+0x60/0x490 [ext4 6d025f3bc52c89a957b89a89d211fadf5e9434e1]
  ? bpf_prog_31efb7c56239148b_restrict_filesystems+0xab/0x126
  ? __bpf_prog_exit+0x23/0xd0
  ? __bpf_tramp_exit+0xd/0x50
  ? bpf_trampoline_6442530367+0x9f/0xea
  ima_file_check+0x57/0x80
  security_file_post_open+0x50/0xf0
  path_openat+0x493/0x1650
  do_filp_open+0xc7/0x170

Detect the state of the file early and return the error.

Signed-off-by: Goldwyn Rodrigues <rgoldwyn@suse.com>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

# Stable Backport Analysis: `ima: return error early if file xattr
cannot be changed`

**Local tree:** `v6.18.43` (`make kernelversion` → 6.18.43)

---

## PHASE 1: COMMIT MESSAGE FORENSICS

### Step 1.1: Subject line
**Record:** `[ima]` `[return]` — Early-return from IMA xattr fix path
when the inode cannot accept xattr changes.

### Step 1.2: Commit message tags
**Record:**
- **Signed-off-by:** Goldwyn Rodrigues `<rgoldwyn@suse.com>` (author)
- **Signed-off-by:** Mimi Zohar `<zohar@linux.ibm.com>` (IMA maintainer)
- No Fixes:, Reported-by:, Tested-by:, Reviewed-by:, Acked-by:, Link:,
  or Cc: stable tags (absence is expected per review pipeline)

**Notable:** Maintainer sign-off from Mimi Zohar carries weight for IMA
changes.

### Step 1.3: Commit body analysis
**Record:**
- **Bug:** With `IMA_APPRAISE_FIX`, IMA tries to write `security.ima`
  xattrs during file open even when the filesystem is read-only (typical
  early boot before remount-rw).
- **Symptom:** ext4 starts a journal transaction for xattr set, hits
  `WARN_ON_ONCE(sb_rdonly(sb))` in `ext4_journal_check_start()`, logs a
  kernel warning.
- **Reproducer:** `systemd-sysroot` opening files on read-only ext4
  during early boot on openSUSE Tumbleweed 6.19.12; full stack trace
  provided.
- **Root cause (author):** IMA does not check whether the
  file/filesystem is writable before calling `__vfs_setxattr_noperm()`.
- **Fix approach:** Detect read-only/immutable state early in
  `ima_fix_xattr()` and return `-EROFS`/`-EPERM`.

### Step 1.4: Hidden bug fix detection
**Record:** Yes — despite not using "fix" in the subject verb, this is a
correctness bug fix. IMA was attempting an operation guaranteed to fail,
driving filesystem code down an error/warning path. Not cosmetic
cleanup.

---

## PHASE 2: DIFF ANALYSIS

### Step 2.1: Change inventory
**Record:**
- **Files:** `security/integrity/ima/ima_appraise.c` (+5 lines, 0
  removed)
- **Function modified:** `ima_fix_xattr()`
- **Scope:** Single-file, surgical fix

### Step 2.2: Code flow change
**Record:**
- **Hunk (ima_fix_xattr):**
  - **Before:** Always prepared xattr data and called
    `__vfs_setxattr_noperm()`, even on read-only filesystems or
    immutable inodes.
  - **After:** Returns `-EROFS` if `IS_RDONLY(d_inode(dentry))`,
    `-EPERM` if `IS_IMMUTABLE(d_inode(dentry))`, before touching xattr
    data or calling VFS.
  - **Path affected:** `IMA_APPRAISE_FIX` path in
    `ima_appraise_measurement()` (line 602) and `ima_update_xattr()`
    (line 646).

### Step 2.3: Bug mechanism
**Record:**
- **Category:** Logic/correctness fix — missing precondition checks
  before VFS xattr write.
- **Mechanism:** `IS_RDONLY()` expands to `sb_rdonly((inode)->i_sb)` —
  the same condition ext4 warns on at `ext4_jbd2.c:76`. Early return
  avoids the pointless journal start and `WARN_ON_ONCE`.

### Step 2.4: Fix quality
**Record:**
- **Quality:** Obviously correct; mirrors existing EVM guard pattern in
  `evm_main.c:267-269`.
- **Regression risk:** Very low. On failure paths the code already
  received `-EROFS` from ext4; this only avoids the warning and
  unnecessary FS work.
- **Minor gap vs EVM:** EVM also checks `s_readonly_remount`; this patch
  does not. That is a pre-existing difference, not a regression from
  this fix. The reported early-boot RO-root case is covered by
  `IS_RDONLY()`.

---

## PHASE 3: GIT HISTORY INVESTIGATION

### Step 3.1: Blame changed lines
**Record:** Stable tree blame shows `ima_fix_xattr()` at lines 88–106
without the guard checks. Function and `nop_mnt_idmap` usage are present
in v6.18.43. Exact mainline introduction commit not traceable in this
stable snapshot (single base commit in file history).

### Step 3.2: Fixes: tag
**Record:** N/A — no Fixes: tag in commit message.

### Step 3.3: Related file history
**Record:** Recent IMA commits in this tree include `b6766b171a5c4`,
`148e4f7ece720`, `9e1f51c1ad57c`, etc. No related fix for this issue
already present. Standalone one-patch series (v1 only).

### Step 3.4: Author context
**Record:** Goldwyn Rodrigues (SUSE). Mimi Zohar (IMA maintainer)
reviewed and signed off. Author has other commits in tree (e.g., btrfs
tracepoint fix).

### Step 3.5: Dependencies
**Record:** No prerequisites. Uses `IS_RDONLY`, `IS_IMMUTABLE`,
`d_inode()` — all present. `nop_mnt_idmap` and `__vfs_setxattr_noperm`
already used in the same function. Applies standalone.

---

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

### Step 4.1: Original patch discussion
**Record:**
- **b4 dig URL:** https://patch.msgid.link/aposxvqsrlbe7gtyvtsdh5nyg5sgo
  fimerqpt6ez4fbxhtqyjj@4u3othdcgipp
- **Series:** v1 only (no v2/v3)
- **Mimi Zohar reply:** "Thank you! The patch makes a lot of sense."
- No NAKs found. No explicit stable nomination in thread.

### Step 4.2: Reviewers
**Record:** CC'd to `linux-integrity@vger.kernel.org`. Mimi Zohar
(maintainer) responded positively and signed off in the committed
version.

### Step 4.3: Bug report
**Record:** Concrete stack trace in commit message from openSUSE
Tumbleweed / QEMU, `systemd-sysroot` during early boot. Severity from
reporter: kernel WARNING (not oops/panic).

### Step 4.4: Related patches
**Record:** Part of a larger SUSE series on mainline (`[PATCH 02/19]` in
mirror), but this specific patch is self-contained with no series
dependencies.

### Step 4.5: Stable list history
**Record:** Not searched on lore stable list (no indication of prior
stable discussion). Not a negative signal.

---

## PHASE 5: CODE SEMANTIC ANALYSIS

### Step 5.1: Key functions
**Record:** `ima_fix_xattr()` (modified); callers
`ima_appraise_measurement()`, `ima_update_xattr()`.

### Step 5.2: Callers
**Record:**
- `ima_appraise_measurement()` ← `process_measurement()` ←
  `ima_file_check()` (LSM `file_post_open` hook)
- `ima_update_xattr()` ← post-write xattr update path
- **Context:** File open during boot (`systemd-sysroot`), common
  security hook path.

### Step 5.3: Callees
**Record:** `__vfs_setxattr_noperm()` → `__vfs_setxattr()` → filesystem
`xattr_set` (ext4 starts journal).

### Step 5.4: Reachability
**Record:**
- Trigger: `CONFIG_IMA_APPRAISE` + `IMA_APPRAISE_FIX` mode + read-only
  root during early boot + files opened that fail IMA appraisal.
- Reachable from normal file open syscall path via LSM hook. Not obscure
  module-init-only code.

### Step 5.5: Similar patterns
**Record:** EVM already guards identically before xattr update:

```267:273:security/integrity/evm/evm_main.c
                        } else if (!IS_RDONLY(inode) &&
                                   !(inode->i_sb->s_readonly_remount) &&
                                   !IS_IMMUTABLE(inode) &&
                                   !is_unsupported_hmac_fs(dentry)) {
                                evm_update_evmxattr(dentry, xattr_name,
                                                    xattr_value,
                                                    xattr_value_len);
```

IMA was missing the equivalent guard — clear oversight.

---

## PHASE 6: CROSS-REFERENCE WITH LOCAL TREE (v6.18.43)

### Step 6.1: Buggy code present?
**Record:** **Yes.** `ima_fix_xattr()` at lines 88–106 lacks
`IS_RDONLY`/`IS_IMMUTABLE` checks. Fix is not yet applied (`grep` found
no matches).

### Step 6.2: Backport complications
**Record:** **Clean apply expected.** Context matches exactly (same
function, same `nop_mnt_idmap` usage, same line structure).

### Step 6.3: Related fixes already present?
**Record:** **No.** No prior commit in this tree addresses this issue.

---

## PHASE 7: SUBSYSTEM CONTEXT

### Step 7.1: Subsystem criticality
**Record:** **security/integrity/ima** — IMPORTANT. IMA is used on
secure-boot and integrity-measurement deployments (enterprise Linux,
embedded secure systems).

### Step 7.2: Subsystem activity
**Record:** Active — multiple IMA fixes in v6.18.y stable queue already.

---

## PHASE 8: IMPACT AND RISK

### Step 8.1: Who is affected
**Record:** Systems with `CONFIG_IMA_APPRAISE` and `IMA_APPRAISE_FIX`
(or `ima_appraise=fix` boot param) on read-only root during early boot.
Relevant to dracut/initramfs/systemd-sysroot workflows on ext4 (and
potentially other journaled FS).

### Step 8.2: Trigger conditions
**Record:**
- Early boot, RO root filesystem
- IMA appraise-fix mode attempting to repair missing/wrong
  `security.ima` xattrs on file open
- **Likelihood:** Moderate for IMA-enabled distros during every boot
  until rw remount
- **Unprivileged trigger:** Indirectly — any file open during sysroot
  phase can trigger it

### Step 8.3: Failure mode severity
**Record:** `WARN_ON_ONCE` from ext4 journal layer. **Severity: MEDIUM**
— no crash, panic, corruption, or deadlock, but spurious kernel warnings
on every affected file open during early boot. Pollutes logs and may
trigger monitoring alerts on security-hardened systems.

### Step 8.4: Risk-benefit
**Record:**
- **Benefit:** Eliminates reproducible boot-time warnings; aligns IMA
  with EVM; avoids pointless FS journal operations.
- **Risk:** Minimal (5 lines, well-understood checks).
- **Ratio:** Favorable — low risk, real (if non-critical) user-visible
  bug fix.

---

## PHASE 9: FINAL SYNTHESIS

### Step 9.1: Evidence summary

**FOR backport:**
- Reproducible bug with full stack trace (openSUSE)
- IMA maintainer endorsed ("makes a lot of sense") and signed off
- 5-line surgical fix, obviously correct
- Mirrors existing EVM pattern in same subsystem
- Buggy code confirmed present in v6.18.43
- Clean apply, no dependencies
- Real logic bug (attempting impossible xattr write)

**AGAINST backport:**
- Failure mode is WARNING only, not crash/corruption/security
- Requires `IMA_APPRAISE_FIX` — narrower than default IMA enforce mode
- Does not add EVM's `s_readonly_remount` check (minor, pre-existing
  gap)

**Unresolved:** Exact mainline commit that introduced `ima_fix_xattr()`
without guards (not traceable in stable snapshot history).

### Step 9.2: Stable rules checklist
1. Obviously correct and tested? **PASS** — logic mirrors EVM;
   maintainer reviewed; reproducer provided.
2. Fixes a real bug affecting users? **PASS** — concrete openSUSE early-
   boot warning.
3. Important issue? **PASS (borderline)** — WARN_ON spam during boot on
   IMA systems; not crash-level but user-visible on security
   deployments.
4. Small and contained? **PASS** — 5 lines, 1 file.
5. No new features or APIs? **PASS** — defensive checks only.
6. Can apply to local tree? **PASS** — code exists, patch applies
   cleanly.

### Step 9.3: Exception categories
**Record:** None (not device ID, quirk, DT, build fix, or docs).
Standard bug fix.

### Step 9.4: Decision rationale

For **v6.18.43**, this commit fixes a real, reproducible bug in the IMA
appraisal-fix path where the kernel incorrectly attempts xattr writes on
read-only filesystems during early boot. The result is ext4
`WARN_ON_ONCE` spam — not a crash, but a genuine defect in security
subsystem code that EVM already handles correctly. The fix is minimal,
maintainer-approved, and applies cleanly to this tree.

---

## Verification

- **[Phase 1]** Parsed subject, tags, body; identified IMA early-boot RO
  xattr warning bug
- **[Phase 2]** Diff analysis: +5 lines in `ima_fix_xattr()`, adds
  `IS_RDONLY`/`IS_IMMUTABLE` guards
- **[Phase 3]** `git describe HEAD` → v6.18.43; `git blame`/`git show`
  confirm buggy code at lines 88–106 without guards
- **[Phase 3]** No Fixes: tag; no related fix in tree; standalone patch
- **[Phase 4]** `b4 dig -c 69fc6474236d9edda6983623e4282f2bdfd8e3d8`:
  lore URL found; v1 only
- **[Phase 4]** `b4 dig -w`: CC'd linux-integrity@vger.kernel.org
- **[Phase 4]** Mbox thread: Mimi Zohar — "The patch makes a lot of
  sense"
- **[Phase 5]** Call chain verified: `ima_file_check` →
  `process_measurement` → `ima_appraise_measurement` → `ima_fix_xattr`
- **[Phase 5]** EVM parallel check at `evm_main.c:267-269` verified via
  grep
- **[Phase 5]** `IS_RDONLY` → `sb_rdonly()` verified at
  `include/linux/fs.h:2546`
- **[Phase 5]** ext4 warning at `fs/ext4/ext4_jbd2.c:76` —
  `WARN_ON_ONCE(sb_rdonly(sb))` verified
- **[Phase 6]** `grep IS_RDONLY` in ima: no matches — fix not present
- **[Phase 6]** `ima_fix_xattr` and `nop_mnt_idmap` present in v6.18.43
- **[Phase 8]** Failure mode: WARN_ON_ONCE, severity MEDIUM

**YES**The background history search (`git log --all -S
"ima_fix_xattr"`) was stopped — `--all` scans too many branches and had
hung. That didn’t change the backport conclusion.

For **v6.18.43**, the important part is already verified:
`ima_fix_xattr()` is present **without** the `IS_RDONLY`/`IS_IMMUTABLE`
guards, so the early-boot ext4 warning is a real issue here. Pinning the
exact mainline commit that introduced `ima_fix_xattr` isn’t possible in
this stable snapshot (file history collapses to the release base), and
it isn’t needed for the decision.

**Verdict for this tree: YES** — small, maintainer-reviewed fix for a
reproducible IMA appraise-fix warning on read-only root during early
boot.

 security/integrity/ima/ima_appraise.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 5149ff4fd50d2..d5f7de58caac5 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -90,6 +90,11 @@ static int ima_fix_xattr(struct dentry *dentry, struct ima_iint_cache *iint)
 	int rc, offset;
 	u8 algo = iint->ima_hash->algo;
 
+	if (IS_RDONLY(d_inode(dentry)))
+		return -EROFS;
+	if (IS_IMMUTABLE(d_inode(dentry)))
+		return -EPERM;
+
 	if (algo <= HASH_ALGO_SHA1) {
 		offset = 1;
 		iint->ima_hash->xattr.sha1.type = IMA_XATTR_DIGEST;
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH AUTOSEL 6.18-5.15] integrity: Check for NULL returned by asymmetric_key_public_key
       [not found] <20260831133314.4125787-1-sashal@kernel.org>
  2026-08-31 13:21 ` [PATCH AUTOSEL 6.18-5.10] ima: return error early if file xattr cannot be changed Sasha Levin
@ 2026-08-31 13:21 ` Sasha Levin
  2026-08-31 13:22 ` [PATCH AUTOSEL 6.18-6.12] apparmor: propagate -ENOMEM correctly in unpack_table Sasha Levin
  2026-08-31 13:27 ` [PATCH AUTOSEL 6.18-5.10] netlabel: fix IPv6 unlabeled address add error handling Sasha Levin
  3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-08-31 13:21 UTC (permalink / raw)
  To: patches, stable
  Cc: Stefan Berger, Kamlesh Kumar, Mimi Zohar, Sasha Levin,
	roberto.sassu, dmitry.kasatkin, paul, jmorris, serge,
	linux-integrity, linux-security-module, linux-kernel

From: Stefan Berger <stefanb@linux.ibm.com>

[ Upstream commit c93a5f038ccc11ed8558ce642f62d5ede701a348 ]

Check for a NULL pointer returned by asymmetric_key_public_key and return
-ENOKEY in this case.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Tested-by: Kamlesh Kumar <kam@juniper.net>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

## Phase 1: Commit Message Forensics

### Step 1.1: Subject line
**Record:** `[integrity]` `[Check]` — Add a NULL check on the return
value of `asymmetric_key_public_key()` in asymmetric signature
verification.

### Step 1.2: Tags
**Record:**
- **Signed-off-by:** Stefan Berger `<stefanb@linux.ibm.com>` (author)
- **Tested-by:** Kamlesh Kumar `<kam@juniper.net>`
- **Signed-off-by:** Mimi Zohar `<zohar@linux.ibm.com>` (integrity
  subsystem maintainer)
- **No** Fixes:, Reported-by:, Link:, Cc: stable@vger.kernel.org,
  Reviewed-by:, Acked-by:

Notable: maintainer sign-off plus Tested-by; no syzbot or user bug
report.

### Step 1.3: Body analysis
**Record:**
- **Bug:** `asymmetric_key_public_key()` can return NULL; the code
  dereferences `pk` without checking.
- **Symptom:** NULL pointer dereference → kernel oops in
  `asymmetric_verify()`.
- **Fix:** Return `-ENOKEY` and jump to the existing `out:` cleanup
  path.
- **Series context:** Patch 1/4 of “Add support for ML-DSA signature for
  EVM and IMA”; v3 added patches 1/4 and 2/4 per Mimi Zohar’s review
  comments on v2.

### Step 1.4: Hidden bug fix?
**Record:** Yes — explicit NULL-dereference fix, not cosmetic cleanup.

---

## Phase 2: Diff Analysis

### Step 2.1: Inventory
**Record:**
- **File:** `security/integrity/digsig_asymmetric.c` (+4 / −0)
- **Function:** `asymmetric_verify()`
- **Scope:** Single-file, surgical (4 lines)

### Step 2.2: Code flow change
**Record:**
- **Before:** After `request_asymmetric_key()` succeeds, `pk =
  asymmetric_key_public_key(key)` is used immediately as
  `pk->pkey_algo`.
- **After:** If `pk` is NULL, set `ret = -ENOKEY`, `goto out` (which
  calls `key_put(key)`).
- **Path:** Error handling in IMA/EVM asymmetric signature verification
  (sig v2).

### Step 2.3: Bug mechanism
**Record:** **Category:** NULL pointer dereference.
**Mechanism:** `asymmetric_key_public_key()` is an inline accessor
returning `key->payload.data[asym_crypto]`, which can be NULL. The code
assumed it was always valid after a successful key lookup.

### Step 2.4: Fix quality
**Record:** Obviously correct; mirrors existing `!pkey` handling in
`restrict_link_by_digsig()` / `restrict_link_by_ca()`. Uses existing
`out:` label. Very low regression risk.

---

## Phase 3: Git History Investigation

### Step 3.1: Blame
**Record:** Lines 110–111 in the local tree were introduced in commit
`6bda50f4333fa` (2025-11-29) when `digsig_asymmetric.c` was added. The
missing NULL check has been present since that introduction in this
tree.

### Step 3.2: Fixes: tag
**Record:** N/A — no Fixes: tag.

### Step 3.3: Related file history
**Record:** On `stable/linux-6.18.y`, `digsig_asymmetric.c` appears from
`6bda50f4333fa`. The buggy pattern is present at merge-base
`7b923c78b50d`. Part of ML-DSA v3 series (4 patches); this commit is
standalone and does not require patches 2–4.

### Step 3.4: Author context
**Record:** Stefan Berger is a regular integrity contributor. Mimi Zohar
(maintainer) signed off. Series included in `integrity-v7.2` pull (June
2026).

### Step 3.5: Dependencies
**Record:** No prerequisites. Applies independently of ML-DSA support
(patches 3/4 and 4/4).

---

## Phase 4: Mailing List and External Research

### Step 4.1: Original discussion
**Record:** Found via spinics/openwall at [PATCH v3
1/4](https://www.spinics.net/lists/kernel/msg6157574.html). Cover
letter: [PATCH v3
0/4](https://www.spinics.net/lists/kernel/msg6157584.html). v3 added
patches 1/4 and 2/4 addressing Mimi’s v2 comments. `b4 dig -c` could not
be run (commit not in local tree); `b4 shazam` did not find message-id.
lore.kernel.org blocked by bot protection.

### Step 4.2: Reviewers
**Record:** CC’d: linux-integrity, linux-security-module, Mimi Zohar,
Roberto Sassu, Eric Biggers.

### Step 4.3: Bug report
**Record:** No external bug report, syzbot, or sanitizer report. Found
during ML-DSA series review (Mimi’s v2 feedback).

### Step 4.4: Series context
**Record:** 4-patch ML-DSA series. This patch is independently valuable;
later patches refactor and add ML-DSA sigv3 support.

### Step 4.5: Stable list
**Record:** No stable-list discussion found. Included in maintainer’s
`integrity-v7.2` pull for mainline.

---

## Phase 5: Code Semantic Analysis

### Step 5.1: Key functions
**Record:** `asymmetric_verify()` modified.

### Step 5.2: Callers
**Record:**
- `integrity_digsig_verify()` in `security/integrity/digsig.c` (sig
  types 2 and 3)
- Callers of `integrity_digsig_verify()`:
  - `security/integrity/ima/ima_appraise.c` — IMA signature appraisal
  - `security/integrity/evm/evm_main.c` — EVM signature verification

### Step 5.3: Callees
**Record:** `request_asymmetric_key()`, `asymmetric_key_public_key()`,
`verify_signature()`, `key_put()`.

### Step 5.4: Reachability
**Record:** Reachable from file access when
`CONFIG_INTEGRITY_ASYMMETRIC_KEYS` and IMA/EVM appraisal are enabled. On
this tree, IMA rejects sig version ≥ 3 before verification; sig v2
asymmetric verification is the affected path.

### Step 5.5: Similar patterns
**Record:** `crypto/asymmetric_keys/restrict.c` checks `if (!pkey)
return -ENOPKG`. `verify_signature()` checks `!key->payload.data[0]`
(same slot as `asym_crypto`) — but only after `asymmetric_verify()`
would have already crashed on NULL `pk`.

---

## Phase 6: Cross-Reference Against Local Tree

### Step 6.1: Buggy code present?
**Record:** **Yes.** Local tree is **v6.18.43** (`stable/linux-6.18.y`,
`HEAD` detached). `security/integrity/digsig_asymmetric.c` lines 110–111
lack the NULL check:

```110:111:security/integrity/digsig_asymmetric.c
        pk = asymmetric_key_public_key(key);
        pks.pkey_algo = pk->pkey_algo;
```

### Step 6.2: Backport complications
**Record:** Clean apply expected — 4 lines at a stable location. Minor
field-name difference (`pks.digest` vs `pks.m` in the submitted diff)
does not affect patch placement.

### Step 6.3: Related fixes already present?
**Record:** No — grep shows no existing NULL check at this site.

---

## Phase 7: Subsystem Context

### Step 7.1: Subsystem criticality
**Record:** **security/integrity** (IMA/EVM) — **IMPORTANT** (security-
sensitive, affects systems with integrity appraisal enabled).

### Step 7.2: Activity
**Record:** Actively maintained; recent IMA/EVM commits on this branch.

---

## Phase 8: Impact and Risk Assessment

### Step 8.1: Who is affected
**Record:** Systems with `CONFIG_INTEGRITY_ASYMMETRIC_KEYS` and IMA/EVM
digital-signature appraisal. Not universal, but important for
secured/enterprise deployments.

### Step 8.2: Trigger conditions
**Record:** A signature references a key ID that resolves to an
asymmetric key whose `asym_crypto` payload is NULL. With standard
X.509-loaded RSA/ECDSA keys this is unlikely; the subsystem already
treats `!pkey` as a valid error state elsewhere. More relevant once non-
standard key types (e.g. ML-DSA) are introduced. Trigger does not
require ML-DSA patch 4/4 on this tree, but practical likelihood on
6.18.y without ML-DSA is low.

### Step 8.3: Failure mode severity
**Record:** **Kernel oops** (NULL dereference at `pk->pkey_algo`) —
**CRITICAL** if triggered.

### Step 8.4: Risk-benefit
**Record:**
- **Benefit:** Prevents crash in security verification path; returns
  proper error instead.
- **Risk:** Very low — 4 lines, uses existing cleanup, no API change.
- **Ratio:** Favorable for stable despite rare trigger on current 6.18.y
  key types.

---

## Phase 9: Final Synthesis

### Step 9.1: Evidence summary

**FOR backport:**
- Real NULL-deref bug in security code
- Small, surgical, maintainer-signed fix with Tested-by
- Buggy code exists in v6.18.43
- Applies cleanly and standalone
- IMA/EVM verification path is security-critical
- Consistent with existing `!pkey` handling in asymmetric key code

**AGAINST backport:**
- No user report or fuzzer finding
- Added during ML-DSA series review; practical trigger on 6.18.y without
  ML-DSA may be very rare
- Standard X.509 keys normally always populate `asym_crypto`
- IMA already rejects sig v3 on this tree, limiting some future trigger
  scenarios

**Unresolved:** No confirmed production crash on 6.18.y with current key
types only.

### Step 9.2: Stable rules checklist
1. Obviously correct and tested? **PASS** (simple NULL check; Tested-by;
   maintainer SOB)
2. Fixes a real bug? **PASS** (NULL deref is a real defect, even if
   trigger is edge-case)
3. Important issue? **PASS** (kernel oops in integrity verification —
   HIGH/CRITICAL)
4. Small and contained? **PASS** (4 lines, one function)
5. No new features/APIs? **PASS**
6. Can apply to local tree? **PASS**

### Step 9.3: Exception categories
**Record:** None (not a quirk, device ID, or build fix).

### Step 9.4: Decision rationale

For **linux-6.18.y** specifically: the vulnerable code is present, the
fix is minimal and obviously correct, and a NULL dereference in the
IMA/EVM signature path is exactly the kind of security-subsystem defect
stable trees should fix. While the trigger may be uncommon with today’s
RSA/ECDSA-only keyrings, the kernel already acknowledges that asymmetric
keys can lack a public-key payload (`restrict.c`), and this path lacked
the corresponding guard. The patch is independent of the ML-DSA feature
commits and safe to backport alone.

---

## Verification

- **[Phase 1]** Parsed commit message and tags from user query and
  spinics/openwall
- **[Phase 2]** Read `security/integrity/digsig_asymmetric.c`; confirmed
  4-line NULL check at `asymmetric_key_public_key()` call site
- **[Phase 2]** Read `include/keys/asymmetric-type.h`; confirmed
  `asymmetric_key_public_key()` returns `key->payload.data[asym_crypto]`
  with no NULL guarantee
- **[Phase 3]** `git describe HEAD` → `v6.18.43`; Makefile VERSION
  6.18.43
- **[Phase 3]** `git blame -L 108,115
  security/integrity/digsig_asymmetric.c` → `6bda50f4333fa`
- **[Phase 3]** `git show 7b923c78b50d:...` → buggy code at merge-base
- **[Phase 3]** `git branch` → detached from `stable/linux-6.18.y`
- **[Phase 4]** spinics/openwall: found v3 1/4 and 0/4; v3 added patches
  per Mimi’s comments
- **[Phase 4]** `b4 shazam` — message-id not found; `b4 dig -c` not run
  (commit absent locally)
- **[Phase 4]** lore.kernel.org — blocked (bot protection)
- **[Phase 4]** Web search — commit `c93a5f0`, integrity-v7.2 pull
  confirmed
- **[Phase 5]** `grep asymmetric_verify` — callers in `digsig.c`,
  `ima_appraise.c`, `evm_main.c`
- **[Phase 5]** Read `crypto/asymmetric_keys/restrict.c` — `!pkey`
  checks at lines 141 and 183
- **[Phase 5]** Read `crypto/asymmetric_keys/signature.c` —
  `verify_signature()` checks `!key->payload.data[0]` but only after
  `asymmetric_verify()` would crash
- **[Phase 6]** Confirmed NULL check absent in current tree at lines
  110–111
- **[Phase 6]** Read `ima_appraise.c` lines 335–338 — sig v3 rejected
  before `integrity_digsig_verify()`
- **[Phase 8]** `grep mldsa` in `crypto/asymmetric_keys` — no ML-DSA
  support in this tree
- **UNVERIFIED:** Exact Mimi Zohar review text (follow-up to v2 on
  spinics not fetched); no production crash report

**YES**The background history search finished. In this
`stable/linux-6.18.y` checkout, `digsig_asymmetric.c` and the
`asymmetric_verify()` path without the NULL check both trace back to
commit `6bda50f4333fa` (Nov 29, 2025), when that file was added. The
vulnerable code is present in the current v6.18.43 tree, so the backport
analysis stands: **YES**.

 security/integrity/digsig_asymmetric.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c
index 457c0a396caf3..17219424a34dd 100644
--- a/security/integrity/digsig_asymmetric.c
+++ b/security/integrity/digsig_asymmetric.c
@@ -108,6 +108,10 @@ int asymmetric_verify(struct key *keyring, const char *sig,
 	pks.hash_algo = hash_algo_name[hdr->hash_algo];
 
 	pk = asymmetric_key_public_key(key);
+	if (!pk) {
+		ret = -ENOKEY;
+		goto out;
+	}
 	pks.pkey_algo = pk->pkey_algo;
 	if (!strcmp(pk->pkey_algo, "rsa")) {
 		pks.encoding = "pkcs1";
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH AUTOSEL 6.18-6.12] apparmor: propagate -ENOMEM correctly in unpack_table
       [not found] <20260831133314.4125787-1-sashal@kernel.org>
  2026-08-31 13:21 ` [PATCH AUTOSEL 6.18-5.10] ima: return error early if file xattr cannot be changed Sasha Levin
  2026-08-31 13:21 ` [PATCH AUTOSEL 6.18-5.15] integrity: Check for NULL returned by asymmetric_key_public_key Sasha Levin
@ 2026-08-31 13:22 ` Sasha Levin
  2026-08-31 13:27 ` [PATCH AUTOSEL 6.18-5.10] netlabel: fix IPv6 unlabeled address add error handling Sasha Levin
  3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-08-31 13:22 UTC (permalink / raw)
  To: patches, stable
  Cc: Maxime Bélair, Georgia Garcia, John Johansen, Sasha Levin,
	paul, jmorris, serge, apparmor, linux-security-module,
	linux-kernel

From: Maxime Bélair <maxime.belair@canonical.com>

[ Upstream commit 1c8a839442823ce5c627d645730d8c61d828aafa ]

Currently, if the `kvzalloc` in `unpack_table` fails, it returns NULL.
This is masked by `aa_dfa_unpack` which interprets NULL as a -EPROTO,
leading to confusing error messages in `apparmor_parser` [1].

The fixed behavior correctly propagates -ENOMEM on allocation failure.

Link: https://gitlab.com/apparmor/apparmor/-/issues/592
Reviewed-by: Georgia Garcia <georgia.garcia@canonical.com>
Signed-off-by: Maxime Bélair <maxime.belair@canonical.com>
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

# Stable Backport Analysis: `apparmor: propagate -ENOMEM correctly in
unpack_table`

**Local tree:** `v6.18.44-1-g2736c32da98b9` (kernel version **6.18.44**)

---

## PHASE 1: COMMIT MESSAGE FORENSICS

### Step 1.1: Parse the subject line
**Record:** `[apparmor]` `[propagate]` — Fix incorrect error-code
propagation in `unpack_table()` when DFA table allocation fails.

### Step 1.2: Parse all commit message tags
**Record:**
| Tag | Value |
|-----|-------|
| Link | https://gitlab.com/apparmor/apparmor/-/issues/592 |
| Reviewed-by | Georgia Garcia \<georgia.garcia@canonical.com\> |
| Signed-off-by | Maxime Bélair \<maxime.belair@canonical.com\> |
| Signed-off-by | John Johansen \<john.johansen@canonical.com\> |
| Fixes: | absent (expected for manual review) |
| Cc: stable | absent (expected) |
| Reported-by | absent |
| Tested-by | absent |

Notable: Reviewed by Canonical AppArmor developer; signed off by
subsystem maintainer (John Johansen). No syzbot report.

### Step 1.3: Analyze commit body
**Record:**
- **Bug:** `kvzalloc()` failure in `unpack_table()` returns `NULL`;
  `aa_dfa_unpack()` treats any `NULL` as protocol failure and returns
  `-EPROTO`.
- **Symptom:** Misleading "profile does not conform to protocol" errors
  in `apparmor_parser` when the real failure is OOM.
- **Root cause:** `unpack_table()` conflates allocation failure (`NULL`)
  with protocol/validation failure (`NULL` from `goto out`).
- **Version info:** None stated in commit message.

### Step 1.4: Detect hidden bug fixes
**Record:** Yes — described as error propagation, but it is a real
correctness bug in error handling. Policy load still fails, but with the
wrong errno, which misleads operators and can cause incorrect
retry/debug behavior under memory pressure.

---

## PHASE 2: DIFF ANALYSIS

### Step 2.1: Inventory the changes
**Record:**
- **File:** `security/apparmor/match.c` only (~20 lines changed)
- **Functions:** `unpack_table()`, `aa_dfa_unpack()`
- **Scope:** Single-file surgical fix

### Step 2.2: Code flow change per hunk

**Hunk 1 — `unpack_table()`:**
- **Before:** Returns `NULL` for all failures (protocol validation and
  `kvzalloc` failure).
- **After:** Initializes to `ERR_PTR(-EPROTO)`; validation failures
  return `-EPROTO`; `kvzalloc` failure returns `ERR_PTR(-ENOMEM)`;
  internal `fail` label removed in favor of explicit `ERR_PTR` returns.

**Hunk 2 — `aa_dfa_unpack()`:**
- **Before:** `if (!table) goto fail;` with `error` already set to
  `-EPROTO` at line 326.
- **After:** `if (IS_ERR(table)) { error = PTR_ERR(table); table = NULL;
  goto fail; }` — propagates the specific error from `unpack_table()`.

### Step 2.3: Bug mechanism
**Record:** **Error-path / logic correctness fix.** Allocation failure
was indistinguishable from protocol errors. The fix uses the kernel
`ERR_PTR`/`IS_ERR`/`PTR_ERR` pattern already used elsewhere in AppArmor
(e.g. `aa_dfa_unpack` itself returns `ERR_PTR(error)`).

### Step 2.4: Fix quality
**Record:** Obviously correct; minimal; uses established kernel idioms.
Low regression risk — only affects failure paths. One gap:
`remap_data16_to_data32()` still returns `NULL` on alloc failure and is
still treated as `-EPROTO` in `aa_dfa_unpack()` (lines 409–411); this
commit does not address that separate path.

---

## PHASE 3: GIT HISTORY INVESTIGATION

### Step 3.1: Blame changed lines
**Record:** `unpack_table()` introduced in `e06f75a6a2b43b` (John
Johansen, 2010). `kvzalloc` added in `a7c3e901a46ff5` (Michal Hocko, May
2017, "mm: introduce kv[mz]alloc helpers"). The ENOMEM-as-NULL bug has
existed since **v4.12** era when `kvzalloc` replaced prior allocation in
this function.

### Step 3.2: Follow Fixes: tag
**Record:** Not applicable — no `Fixes:` tag in commit message.

### Step 3.3: File history for related changes
**Record:** Recent related fixes in this tree:
- `ac8f179e5c9ee` — "return -ENOMEM in unpack_perms_table upon alloc
  failure" (identical bug class, already backported to this 6.18.y tree)
- `22dc9433d458c` — accept2 allocation failure returning success path
  (more severe, already backported)
Standalone fix; not part of a numbered series.

### Step 3.4: Author's other commits
**Record:** Maxime Bélair has at least one other AppArmor fix in tree
(`57b1bd4486d56` UAF fix). John Johansen is the AppArmor maintainer and
original author of `unpack_table()`.

### Step 3.5: Prerequisites
**Record:** No dependencies. Uses `<linux/err.h>` already included at
line 16. Applies to existing code in this tree. Standalone.

---

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

### Step 4.1: Original patch discussion
**Record:** `b4 dig -c <commit>` failed — commit not yet in this
checkout. Web search found Ubuntu AppArmor list submission at
https://www.mail-archive.com/apparmor@lists.ubuntu.com/msg12204.html
(fetch timed out). Patch content from search matches provided diff.

### Step 4.2: Reviewers
**Record:** Reviewed-by Georgia Garcia (Canonical). Signed-off-by John
Johansen (maintainer). Submitted to Ubuntu AppArmor list per mail-
archive.

### Step 4.3: Bug report
**Record:** GitLab issue #592 describes Qubes OS users seeing "Profile
does not conform to protocol" on `aa-enforce`, sometimes resolving after
multiple retries — consistent with transient failures (including OOM)
being misreported as protocol errors. Maintainer comment on related
issue #265 notes protocol errors can indicate kernel/parser bugs or
mismatched policy. Severity from reporter: operational confusion, not a
confirmed security issue.

### Step 4.4: Related patches
**Record:** `ac8f179e5c9ee` (unpack_perms_table ENOMEM fix) was
backported to stable (visible in spinics stable list for 6.19.y). Same
subsystem, same bug pattern, accepted for stable.

### Step 4.5: Stable mailing list
**Record:** No specific stable-list discussion found for this exact
patch. The sibling `unpack_perms_table` fix was included in stable
releases.

---

## PHASE 5: CODE SEMANTIC ANALYSIS

### Step 5.1: Key functions
**Record:** `unpack_table()` (static), `aa_dfa_unpack()` (exported via
`match.h`)

### Step 5.2: Callers
**Record:**
- `aa_dfa_unpack()` called from:
  - `unpack_dfa()` in `policy_unpack.c` — policy load path
  - `aa_setup_dfa_engine()` in `lsm.c` — built-in nulldfa/stacksplitdfa
    at init (static blobs, unlikely to OOM)
- `unpack_dfa()` → `unpack_pdb()` → policy unpack →
  `aa_replace_profiles()` via `apparmorfs.c`

Primary user-visible path: **policy load**
(`/sys/kernel/security/apparmor/` write or equivalent).

### Step 5.3: Callees
**Record:** `kvzalloc()`, `kvfree()`, `get_unaligned_be*()`,
`vm_unmap_aliases()`, `verify_table_headers()`, `verify_dfa()`

### Step 5.4: Call chain / reachability
**Record:** `write(apparmorfs)` → `aa_replace_profiles()` →
`aa_unpack()` → `unpack_pdb()` → `unpack_dfa()` → `aa_dfa_unpack()` →
`unpack_table()`. Reachable from **privileged userspace** loading
AppArmor policy. Trigger requires memory pressure during DFA table
unpacking.

### Step 5.5: Similar patterns
**Record:** Same NULL-on-OOM → `-EPROTO` pattern existed in
`unpack_perms_table()` (fixed by `ac8f179e5c9ee`).
`remap_data16_to_data32()` in the same file still has the unfixed
pattern.

---

## PHASE 6: CROSS-REFERENCE AGAINST LOCAL TREE (6.18.44)

### Step 6.1: Does buggy code exist?
**Record:** **Yes.** Current `security/apparmor/match.c` lines 34–91
return `NULL` on `kvzalloc` failure; `aa_dfa_unpack()` at lines 361–363
treats `!table` as `-EPROTO`. Bug present since kvzalloc adoption
(~2017).

### Step 6.2: Backport complications
**Record:** Expected **clean apply** — tree matches the "before" state
in the provided diff. No conflicting recent changes to these functions.
`git apply --check` with truncated test patch failed due to patch
formatting, not code mismatch; file content verified identical to pre-
patch state.

### Step 6.3: Related fixes already present?
**Record:** `ac8f179e5c9ee` (ENOMEM in `unpack_perms_table`) already in
tree. This specific `unpack_table` fix is **not** yet present.

---

## PHASE 7: SUBSYSTEM CONTEXT

### Step 7.1: Subsystem and criticality
**Record:** **security/apparmor** — LSM security module. **IMPORTANT**
for Ubuntu, Debian, openSUSE, and other AppArmor-enabled distributions.

### Step 7.2: Subsystem activity
**Record:** Actively maintained — multiple AppArmor stable backports
already in this 6.18.y tree (UAF, refcount, ENOMEM fixes in 2026).

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

### Step 8.1: Who is affected
**Record:** AppArmor-enabled systems where administrators load or
replace policy. Config-specific (`CONFIG_SECURITY_APPARMOR`).

### Step 8.2: Trigger conditions
**Record:** Memory pressure during `kvzalloc()` in `unpack_table()`
while unpacking a policy DFA. Uncommon but realistic on constrained
systems or under heavy memory load. Privileged operation (policy load),
not unprivileged attack surface for escalation.

### Step 8.3: Failure mode severity
**Record:** Policy load fails with **`-EPROTO`** instead of
**`-ENOMEM`**. Users see misleading protocol-conformance errors. Policy
is **not** loaded in either case — no silent security bypass. Severity:
**MEDIUM** (operational/diagnostic), not CRITICAL (no crash, corruption,
or UAF).

### Step 8.4: Risk-benefit
**Record:**
- **Benefit:** Correct errno under OOM; better diagnostics; consistent
  with already-backported `unpack_perms_table` fix; helps admins
  distinguish transient OOM from corrupt policy.
- **Risk:** Very low — ~20 lines, failure-path only, established ERR_PTR
  pattern.
- **Ratio:** Favorable, especially given subsystem precedent.

---

## PHASE 9: FINAL SYNTHESIS

### Step 9.1: Evidence summary

**FOR backport:**
- Real bug in error handling (ENOMEM masked as EPROTO)
- Bug present in this 6.18.44 tree since ~2017
- Small, surgical, maintainer-reviewed fix
- Identical bug class already backported in this tree (`ac8f179e5c9ee`)
- Affects policy-load path on AppArmor systems
- Canonical team fix with maintainer sign-off

**AGAINST backport:**
- No crash, corruption, deadlock, or security vulnerability
- Policy load fails either way — functional outcome unchanged
- GitLab #592 is not a direct ENOMEM reproducer
- `remap_data16_to_data32()` has the same unfixed pattern (incomplete
  coverage)
- Commit not yet in tree; lore fetch incomplete

**Unresolved:** Full mail-archive thread content; whether stable
maintainers will treat this identically to `unpack_perms_table`
(precedent suggests yes).

### Step 9.2: Stable rules checklist
| Criterion | Result |
|-----------|--------|
| 1. Obviously correct and tested? | **PASS** — clear ERR_PTR pattern;
Reviewed-by present |
| 2. Fixes real bug affecting users? | **PASS** — wrong errno on policy
load under OOM |
| 3. Important issue? | **PASS (borderline)** — not crash/security, but
policy-load failure misdiagnosis on production AppArmor systems; same
class already deemed stable-worthy |
| 4. Small and contained? | **PASS** — one file, ~20 lines |
| 5. No new features/APIs? | **PASS** — internal error propagation only
|
| 6. Can apply to local tree? | **PASS** — buggy code confirmed present;
clean apply expected |

### Step 9.3: Exception categories
**Record:** None (not device ID, quirk, DT, build, or docs). Pure error-
handling correctness fix.

### Step 9.4: Decision rationale
This fix belongs in stable for the same reason `ac8f179e5c9ee` was
already backported to this 6.18.y tree: AppArmor policy unpacking must
return the correct errno so administrators and userspace tools can
distinguish OOM from protocol corruption. The change is minimal,
obviously correct, and the bug has been present since `kvzalloc` was
adopted in 2017. While severity is operational rather than catastrophic,
the established precedent in this exact subsystem and tree supports
inclusion.

---

## Verification

- [Phase 1] Parsed commit message and tags from user-provided content
- [Phase 1] Link: https://gitlab.com/apparmor/apparmor/-/issues/592 —
  fetched; describes Qubes OS protocol errors on `aa-enforce`, not a
  direct ENOMEM reproducer
- [Phase 2] Read `security/apparmor/match.c` lines 34–91, 314–434 —
  confirmed buggy NULL-return pattern and `!table` → `-EPROTO` path
- [Phase 2] Confirmed `remap_data16_to_data32()` still returns NULL on
  OOM (lines 283–286, 409–411) — unfixed by this commit
- [Phase 3] `git describe HEAD` → `v6.18.44-1-g2736c32da98b9`; `make
  kernelversion` → `6.18.44`
- [Phase 3] `git blame -L 34,91 security/apparmor/match.c` → function
  from 2010; `kvzalloc` from `a7c3e901a46ff5` (2017)
- [Phase 3] `git show a7c3e901a46ff5` — confirmed kvzalloc introduction
  in `match.c`
- [Phase 3] `git log --oneline -20 -- security/apparmor/match.c` —
  recent activity, no conflicting fix
- [Phase 3] `git show ac8f179e5c9ee` — identical ENOMEM/EPROTO pattern
  fix already in this tree with stable backport marker
- [Phase 3] `git log --oneline --grep="ENOMEM" -- security/apparmor/` —
  related ENOMEM fixes present
- [Phase 4] `b4 dig -c ac8f179e5c9ee` — no lore match (stable-backport
  commit)
- [Phase 4] `b4 dig --commitish "apparmor: propagate -ENOMEM correctly
  in unpack_table"` — commit not in tree
- [Phase 4] Web search — found Ubuntu AppArmor list submission
  (msg12204); spinics stable list shows `unpack_perms_table` ENOMEM fix
  in stable batch
- [Phase 4] WebFetch gitlab #592 — success; WebFetch mail-archive —
  timed out
- [Phase 5] `grep aa_dfa_unpack security/apparmor/` — callers in
  `policy_unpack.c`, `lsm.c`
- [Phase 5] Read `unpack_pdb()` lines 765–770 — `PTR_ERR(pdb->dfa)`
  propagates error to policy unpack
- [Phase 6] Confirmed buggy code present in local 6.18.44 checkout
- [Phase 6] `ac8f179e5c9ee` confirmed in tree; this fix not yet applied
- [Phase 7] `git log --oneline -20 -- security/apparmor/` — active
  subsystem with multiple stable backports

**YES**The AppArmor `unpack_table` ENOMEM backport analysis is done for
your **6.18.44** tree.

**Verdict: YES** — backport it.

When `kvzalloc()` fails during DFA table unpacking, the kernel currently
returns `-EPROTO` instead of `-ENOMEM`, so policy load failures under
memory pressure look like protocol/corruption errors in
`apparmor_parser`. The fix is small (~20 lines in
`security/apparmor/match.c`), uses the standard `ERR_PTR`/`IS_ERR`
pattern, and matches an already-backported fix in this tree
(`ac8f179e5c9ee` for `unpack_perms_table`). The buggy code is present
here; the patch should apply cleanly.

Severity is operational (wrong errno, not a crash or security bug), but
the subsystem precedent and maintainer review support stable inclusion.

 security/apparmor/match.c | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/security/apparmor/match.c b/security/apparmor/match.c
index 0de249725efbf..b2a4bee4e3939 100644
--- a/security/apparmor/match.c
+++ b/security/apparmor/match.c
@@ -27,13 +27,13 @@
  * @blob: data to unpack (NOT NULL)
  * @bsize: size of blob
  *
- * Returns: pointer to table else NULL on failure
+ * Returns: pointer to table else ERR_PTR on failure
  *
  * NOTE: must be freed by kvfree (not kfree)
  */
 static struct table_header *unpack_table(char *blob, size_t bsize)
 {
-	struct table_header *table = NULL;
+	struct table_header *table = ERR_PTR(-EPROTO);
 	struct table_header th;
 	size_t tsize;
 
@@ -74,20 +74,21 @@ static struct table_header *unpack_table(char *blob, size_t bsize)
 		else if (th.td_flags == YYTD_DATA32)
 			UNPACK_ARRAY(table->td_data, blob, th.td_lolen,
 				     u32, __be32, get_unaligned_be32);
-		else
-			goto fail;
+		else {
+			kvfree(table);
+			table = ERR_PTR(-EPROTO);
+			goto out;
+		}
 		/* if table was vmalloced make sure the page tables are synced
 		 * before it is used, as it goes live to all cpus.
 		 */
 		if (is_vmalloc_addr(table))
 			vm_unmap_aliases();
-	}
+	} else
+		table = ERR_PTR(-ENOMEM);
 
 out:
 	return table;
-fail:
-	kvfree(table);
-	return NULL;
 }
 
 /**
@@ -359,8 +360,11 @@ struct aa_dfa *aa_dfa_unpack(void *blob, size_t size, int flags)
 
 	while (size > 0) {
 		table = unpack_table(data, size);
-		if (!table)
+		if (IS_ERR(table)) {
+			error = PTR_ERR(table);
+			table = NULL;
 			goto fail;
+		}
 
 		switch (table->td_id) {
 		case YYTD_ID_ACCEPT:
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH AUTOSEL 6.18-5.10] netlabel: fix IPv6 unlabeled address add error handling
       [not found] <20260831133314.4125787-1-sashal@kernel.org>
                   ` (2 preceding siblings ...)
  2026-08-31 13:22 ` [PATCH AUTOSEL 6.18-6.12] apparmor: propagate -ENOMEM correctly in unpack_table Sasha Levin
@ 2026-08-31 13:27 ` Sasha Levin
  3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-08-31 13:27 UTC (permalink / raw)
  To: patches, stable
  Cc: Chenguang Zhao, Paul Moore, Jakub Kicinski, Sasha Levin, davem,
	edumazet, pabeni, netdev, linux-security-module, linux-kernel

From: Chenguang Zhao <zhaochenguang@kylinos.cn>

[ Upstream commit 56872b930feee7ae07b9720ca950dd9fa65596ee ]

netlbl_unlhsh_add_addr6() always returned zero after
netlbl_af6list_add(), masking failures such as duplicate
IPv6 static label entries.

Signed-off-by: Chenguang Zhao <zhaochenguang@kylinos.cn>
Acked-by: Paul Moore <paul@paul-moore.com>
Link: https://patch.msgid.link/20260522022910.398416-1-zhaochenguang@kylinos.cn
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

# Stable Backport Analysis: `netlabel: fix IPv6 unlabeled address add
error handling`

**Local tree:** `v6.18.44` (`stable/linux-6.18.y`, `VERSION = 6`,
`PATCHLEVEL = 18`, `SUBLEVEL = 44`)

---

## PHASE 1: COMMIT MESSAGE FORENSICS

### Step 1.1: Subject line
**Record:** `[netlabel]` `[fix]` — IPv6 unlabeled address add error
handling was broken; failures from `netlbl_af6list_add()` were masked.

### Step 1.2: Tags
**Record:**
- **Signed-off-by:** Chenguang Zhao `<zhaochenguang@kylinos.cn>`
  (author)
- **Acked-by:** Paul Moore `<paul@paul-moore.com>` (netlabel maintainer
  — strong quality signal)
- **Link:** https://patch.msgid.link/20260522022910.398416-1-
  zhaochenguang@kylinos.cn
- **Signed-off-by:** Jakub Kicinski `<kuba@kernel.org>` (committer)
- No `Fixes:` tag (expected for manual review)
- No `Reported-by:` / syzbot
- No `Cc: stable@vger.kernel.org` in submission
- Ignore pipeline `Signed-off-by: Sasha Levin`

### Step 1.3: Body analysis
**Record:**
- **Bug:** `netlbl_unlhsh_add_addr6()` always returned `0` after
  `netlbl_af6list_add()`, even when that call failed.
- **Symptom:** Duplicate IPv6 static unlabeled label adds appear
  successful to callers.
- **Root cause:** Copy/paste oversight — IPv4 sibling
  `netlbl_unlhsh_add_addr4()` correctly returns `ret_val`; IPv6 path
  hard-coded `return 0`.
- **Version info:** None in message.

### Step 1.4: Hidden bug fix?
**Record:** Not disguised — explicitly labeled a fix. Error-path
`kfree(entry)` was already present; the bug is return-value propagation
and downstream effects, not a leak.

---

## PHASE 2: DIFF ANALYSIS

### Step 2.1: Inventory
**Record:**
- **Files:** `net/netlabel/netlabel_unlabeled.c` (+1 / -1)
- **Function:** `netlbl_unlhsh_add_addr6()`
- **Scope:** Single-file, single-line surgical fix

### Step 2.2: Code flow change
**Record:**
- **Before:** On `netlbl_af6list_add()` failure (e.g. `-EEXIST`), entry
  is freed, but function returns `0`.
- **After:** Returns actual `ret_val` from `netlbl_af6list_add()`.
- **Path affected:** IPv6 static unlabeled address add error path
  (admin/LSM configuration).

### Step 2.3: Bug mechanism
**Record:**
- **Category:** Logic / correctness — incorrect error propagation.
- **Mechanism:** `netlbl_af6list_add()` returns `-EEXIST` for duplicate
  address/mask (`net/netlabel/netlabel_addrlist.c:193`). IPv6 wrapper
  discarded that and reported success. IPv4 path at lines 252–254
  already does the right thing.

### Step 2.4: Fix quality
**Record:**
- Obviously correct — mirrors IPv4 and function documentation (“On
  success zero is returned, otherwise a negative value”).
- Minimal risk; no locking/API changes.
- No regression risk identified.

---

## PHASE 3: GIT HISTORY INVESTIGATION

### Step 3.1: Blame
**Record:** Buggy `return 0;` at line 298 in current tree; blame points
to `e664048784506` (file introduction in this tree). IPv4 `return
ret_val;` at line 254 present alongside it from the same introduction.

### Step 3.2: Fixes: tag
**Record:** N/A — no `Fixes:` tag.

### Step 3.3: Related file history
**Record:**
- `0c4bb32ad7fdc` — same author, separate netlabel validation fix
  already in this tree.
- Upstream fix: `56872b930feee` (mainline, May 25 2026); stable backport
  exists as `642d90c85b137` on `autosel` branch.
- Fix is **not** in current `HEAD` (`v6.18.44`).

### Step 3.4: Author context
**Record:** Chenguang Zhao has multiple netlabel fixes; Paul Moore
(maintainer) Acked this patch.

### Step 3.5: Dependencies
**Record:** Standalone one-liner; no series prerequisites. Applies
cleanly (`return 0` → `return ret_val` at line 298; upstream diff
context matches aside from unrelated `kzalloc` vs `kzalloc_obj` naming
elsewhere).

---

## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH

### Step 4.1: Original discussion
**Record:**
- `b4 dig -c 56872b930feee`: https://patch.msgid.link/20260522022910.398
  416-1-zhaochenguang@kylinos.cn
- Single v1 submission; applied to netdev/net-next by Jakub Kicinski.
- Paul Moore replied with **Acked-by** in thread.

### Step 4.2: Reviewers
**Record:** `b4 dig -w`: CC'd Paul Moore, David Miller, netdev
maintainers, `linux-security-module@vger.kernel.org`.

### Step 4.3: Bug report
**Record:** No external bug report or syzbot link — author-found logic
bug.

### Step 4.4: Series context
**Record:** Standalone 1-patch series; no dependencies.

### Step 4.5: Stable list
**Record:** No `Cc: stable` discussion found in mbox thread.

---

## PHASE 5: CODE SEMANTIC ANALYSIS

### Step 5.1: Key functions
**Record:** `netlbl_unlhsh_add_addr6()`, `netlbl_af6list_add()`,
`netlbl_unlhsh_add()`.

### Step 5.2: Callers
**Record:**
- `netlbl_unlhsh_add()` → `netlbl_unlhsh_add_addr6()` (line 423)
- `netlbl_unlhsh_add()` called from:
  - `netlbl_unlabel_staticadd()` / `netlbl_unlabel_staticadddef()`
    (Generic Netlink admin)
  - `netlbl_cfg_unlbl_static_add()` (kernel API, used e.g. from
    `security/smack/smackfs.c`)

### Step 5.3: Callees
**Record:** `kzalloc()`, `netlbl_af6list_add()` (can return `-EEXIST`),
`kfree()` on failure.

### Step 5.4: Reachability
**Record:** Reachable from userspace via Netlink (`CAP_NET_ADMIN`) and
from LSM code configuring static labels. IPv6 path requires
`CONFIG_IPV6`.

### Step 5.5: Similar patterns
**Record:** IPv4 `netlbl_unlhsh_add_addr4()` correctly returns `ret_val`
— confirms this is an IPv6-only regression/typo.

---

## PHASE 6: CROSS-REFERENCE AGAINST LOCAL TREE

### Step 6.1: Buggy code present?
**Record:** **YES** — line 298 in `/home/sasha/linux-
autosel-7.0/net/netlabel/netlabel_unlabeled.c` is `return 0;` while line
254 (IPv4) is `return ret_val;`.

### Step 6.2: Backport complications
**Record:** Clean one-line apply expected; no structural conflicts in
this file.

### Step 6.3: Related fixes already present?
**Record:** Related validation fix `0c4bb32ad7fdc` is present; this
error-handling fix is **not**.

---

## PHASE 7: SUBSYSTEM CONTEXT

### Step 7.1: Subsystem criticality
**Record:** `net/netlabel` — **IMPORTANT** (LSM integration: SELinux,
Smack; MAC labeling and audit).

### Step 7.2: Activity
**Record:** Recent activity in this tree (validation fix June 2026);
netlabel touched in 6.18.y.

---

## PHASE 8: IMPACT AND RISK ASSESSMENT

### Step 8.1: Who is affected
**Record:** Systems using NetLabel IPv6 static unlabeled labels with
SELinux/Smack (or other LSM consumers of
`netlbl_cfg_unlbl_static_add()`). Config-specific (`CONFIG_NETLABEL`,
`CONFIG_IPV6`).

### Step 8.2: Trigger conditions
**Record:** Adding a duplicate IPv6 static unlabeled label (same
address/mask). Requires admin capability. Duplicate-add is a realistic
admin/script mistake, not exotic.

### Step 8.3: Failure mode severity
**Record:**
1. Userspace receives success (`0`) instead of `-EEXIST`.
2. `netlbl_unlhsh_add()` incorrectly executes
   `atomic_inc(&netlabel_mgmt_protocount)` (lines 434–435).
3. Audit records `res=1` (success) on failure (line 444).
4. **Protocount skew:** duplicate “success” inflates count; after
   removing the real entry, `netlabel_mgmt_protocount` can remain `> 0`
   with zero entries, leaving `netlbl_enabled()` true
   (`net/netlabel/netlabel_kapi.c:960`). SELinux uses `netlbl_enabled()`
   in netfilter hooks (`security/selinux/hooks.c:6004, 6021`).

**Severity:** **MEDIUM-HIGH** for affected deployments — not a crash,
but incorrect security subsystem state and audit integrity.

### Step 8.4: Risk-benefit
**Record:**
- **Benefit:** Correct errno, accurate audit, correct
  protocount/`netlbl_enabled()` behavior.
- **Risk:** Very low (one line, maintainer-acked, mirrors working IPv4
  path).
- **Ratio:** Strong benefit, negligible risk.

---

## PHASE 9: FINAL SYNTHESIS

### Step 9.1: Evidence summary

**FOR backport:**
- Clear, real bug (IPv6-only; IPv4 correct)
- Maintainer Acked-by (Paul Moore)
- One-line, obviously correct fix
- Affects LSM/security admin path and audit logs
- Protocount inflation can leave NetLabel “enabled” after entries
  removed
- Bug confirmed present in `v6.18.44`
- Upstream already merged (`56872b930feee`)

**AGAINST backport:**
- No crash, UAF, or memory corruption
- Only failure mode from `netlbl_af6list_add()` is `-EEXIST`
  (duplicates)
- Niche subsystem (NetLabel + IPv6 static labels)
- No syzbot/user crash reports

**Unresolved:** None material to the decision.

### Step 9.2: Stable rules checklist
1. Obviously correct and tested? **PASS** — mirrors IPv4; maintainer
   ack; trivial change.
2. Fixes a real bug affecting users? **PASS** — wrong errno, audit, and
   protocount on duplicate IPv6 adds.
3. Important issue? **PASS** — security subsystem correctness and audit
   integrity (MEDIUM-HIGH).
4. Small and contained? **PASS** — 1 line, 1 file.
5. No new features/APIs? **PASS**.
6. Can apply to local tree? **PASS** — buggy code present; clean apply.

### Step 9.3: Exception categories
**Record:** N/A (not device ID, quirk, DT, build, or docs fix).

### Step 9.4: Decision rationale
This is a straightforward error-handling bug in the NetLabel IPv6 admin
path. While it does not cause a kernel oops, it corrupts security-
relevant state: callers, audit subsystem, and
`netlabel_mgmt_protocount`/`netlbl_enabled()` all behave incorrectly on
a realistic duplicate-add scenario. The fix is trivial, maintainer-
reviewed, and the bug is present in this `6.18.44` tree.

---

## Verification

- [Phase 1] `git describe HEAD` → `v6.18.44`; parsed subject, tags, body
  from provided commit message
- [Phase 1] Read current `netlbl_unlhsh_add_addr6()` — confirmed `return
  0` bug at line 298
- [Phase 2] Read diff — single line `return 0` → `return ret_val`
- [Phase 2] Read `netlbl_af6list_add()` — returns `-EEXIST` on duplicate
  (line 193)
- [Phase 2] Compared IPv4 `netlbl_unlhsh_add_addr4()` — returns
  `ret_val` (line 254)
- [Phase 3] `git blame -L 290,305` — buggy line attributed to file
  introduction
- [Phase 3] `git show 56872b930feee` — upstream fix commit confirmed
- [Phase 3] `git merge-base --is-ancestor 56872b930feee HEAD` → fix NOT
  in HEAD
- [Phase 3] `git show 642d90c85b137` — stable backport commit exists on
  `autosel` branch
- [Phase 4] `b4 dig -c 56872b930feee -w` — lore URL and recipient list
  retrieved
- [Phase 4] `b4 dig -c 56872b930feee -a` — single v1 revision
- [Phase 4] `b4 dig -m /tmp/netlabel_ipv6_fix.mbox` — Paul Moore Acked-
  by confirmed; no stable Cc in thread
- [Phase 5] `grep netlbl_unlhsh_add` — traced callers to Netlink and
  `netlbl_cfg_unlbl_static_add()`
- [Phase 5] `grep netlbl_enabled` — SELinux hooks depend on protocount
- [Phase 6] Read lines 248–299 and 364–447 — buggy code and downstream
  `atomic_inc`/audit impact verified
- [Phase 6] `git log --oneline -20 -- net/netlabel/netlabel_unlabeled.c`
  — fix not yet in tree
- [Phase 8] Traced protocount inflation scenario through add/remove
  paths (lines 434–435, 664–666, 960)

**YES**

 net/netlabel/netlabel_unlabeled.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
index 2237a5261dd2a..0dfbb63d513ce 100644
--- a/net/netlabel/netlabel_unlabeled.c
+++ b/net/netlabel/netlabel_unlabeled.c
@@ -295,7 +295,7 @@ static int netlbl_unlhsh_add_addr6(struct netlbl_unlhsh_iface *iface,
 
 	if (ret_val != 0)
 		kfree(entry);
-	return 0;
+	return ret_val;
 }
 #endif /* IPv6 */
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-08-31 13:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20260831133314.4125787-1-sashal@kernel.org>
2026-08-31 13:21 ` [PATCH AUTOSEL 6.18-5.10] ima: return error early if file xattr cannot be changed Sasha Levin
2026-08-31 13:21 ` [PATCH AUTOSEL 6.18-5.15] integrity: Check for NULL returned by asymmetric_key_public_key Sasha Levin
2026-08-31 13:22 ` [PATCH AUTOSEL 6.18-6.12] apparmor: propagate -ENOMEM correctly in unpack_table Sasha Levin
2026-08-31 13:27 ` [PATCH AUTOSEL 6.18-5.10] netlabel: fix IPv6 unlabeled address add error handling Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox