* [PATCH AUTOSEL 6.18-6.6] apparmor: fix auditing of mount binary data
[not found] <20261019135441.1420968-1-sashal@kernel.org>
@ 2026-10-19 13:47 ` Sasha Levin
2026-10-19 14:26 ` sashiko-bot
0 siblings, 1 reply; 2+ messages in thread
From: Sasha Levin @ 2026-10-19 13:47 UTC (permalink / raw)
To: patches, stable
Cc: John Johansen, Sasha Levin, georgia.garcia, paul, jmorris, serge,
apparmor, linux-security-module, linux-kernel
From: John Johansen <john.johansen@canonical.com>
[ Upstream commit b9f2181b90ec047488dcb4fb1c631d417aff07e0 ]
AppArmor only mediates non-binary mount data, and should only
audit the mount data if it is non-binary.
Signed-off-by: John Johansen <john.johansen@canonical.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **6.12.111** (`pending-6.12`). The buggy
`audit_mount(..., data, ...)` call is still present, and the two-line
change applies onto the current `match_mnt_path_str()` code.
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1: PARSE THE SUBJECT LINE**
Record: **[apparmor] [fix] [do not audit mount data when it is binary]**
**Step 1.2: PARSE ALL COMMIT MESSAGE TAGS**
Upstream commit: `b9f2181b90ec047488dcb4fb1c631d417aff07e0` (John
Johansen, 2026-07-28; merged via AppArmor v7.3 pull `apparmor-
pr-2026-08-26`).
Record:
- **Fixes:** none (expected for this review pipeline)
- **Reported-by:** none
- **Tested-by:** none on the commit itself
- **Reviewed-by / Acked-by:** none on the commit
- **Link:** none
- **Cc: stable:** none (expected)
- **Signed-off-by:** John Johansen `<john.johansen@canonical.com>`
(AppArmor maintainer). Ignore any Sasha Levin SOB.
Notable pattern: no user/syzbot report; maintainer-authored correctness
fix. The v7.3 pull lists it under **Bug Fixes** (“auditing of mount
binary data”), not Features/Cleanups.
**Step 1.3: ANALYZE THE COMMIT BODY TEXT**
Record:
- **Bug:** AppArmor mediates only non-binary mount data, but still
passes binary `data` into `audit_mount()`.
- **Symptom:** AppArmor audit records can include an `options=` field
built by treating a binary blob as a C string
(`audit_log_untrustedstring()` → `strlen()`).
- **Version info:** none in the message. The pattern dates to
`2ea3ffb7782a84` (“apparmor: add mount mediation”, v4.14-rc2).
- **Root cause (author):** audit should follow the same `!binary` rule
as mediation.
**Step 1.4: DETECT HIDDEN BUG FIXES**
Record: Not a disguised cleanup. Subject and body both say **fix**. It
is a real audit-path bug, not a feature.
---
## PHASE 2: DIFF ANALYSIS
**Step 2.1: INVENTORY THE CHANGES**
Record:
- File: `security/apparmor/mount.c` only (**−2 / +2**)
- Function: `match_mnt_path_str()`
- Scope: single-file surgical fix
**Step 2.2: UNDERSTAND THE CODE FLOW CHANGE**
Before (this tree, lines 346–349):
```346:349:security/apparmor/mount.c
audit:
return audit_mount(subj_cred, profile, OP_MOUNT, mntpnt,
devname,
type, NULL,
flags, data, AA_MAY_MOUNT, &perms, info,
error);
```
After: pass `!binary ? data : NULL` instead of `data`.
`match_mnt()` only wraps `match_mnt_path_str()`, so both new-mount and
remount paths are covered. `aa_umount` / `aa_pivotroot` already pass
NULL data.
**Step 2.3: IDENTIFY THE BUG MECHANISM**
Record: **Logic / correctness in LSM audit** (binary blob treated as a
string).
Verified mechanism:
- `do_match_mnt()` already skips matching when `binary` is set (`data &&
!binary && (perms->allow & AA_MNT_CONT_MATCH)`).
- `audit_mount()` still stores `data` if `perms->audit & AA_AUDIT_DATA`.
- `audit_cb()` then does `audit_log_untrustedstring(ab, ad->mnt.data)`,
which uses `strlen()`.
`FS_BINARY_MOUNTDATA` filesystems in this tree: **NFS, NFS4, Btrfs,
Coda**.
Not an unbounded OOB on the `mount(2)` path: `path_mount()` does `((char
*)data_page)[PAGE_SIZE - 1] = 0` before `security_sb_mount()`.
`strlen()` stays inside the `copy_mount_options()` page. The bug is
wrong/garbled audit contents (and a possible hex dump of almost a full
page if the blob has no interior NUL).
**Step 2.4: ASSESS THE FIX QUALITY**
Record: Obviously correct; mirrors the existing `!binary` mediation
check. Minimal. Regression risk is low: binary `options=` simply
disappears from AppArmor mount audit records, which is the intended
policy. No API, locking, or data-structure change.
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1: BLAME THE CHANGED LINES**
Record: The `flags, data, AA_MAY_MOUNT` argument list is from
**`2ea3ffb7782a84`** (John Johansen, 2017-07-18), described as
v4.14-rc2. Wrapper cred-passing is from `90c436a64a6e20` (2022). The bug
is old, not new 7.x code.
**Step 3.2: FOLLOW THE FIXES: TAG**
Record: No `Fixes:` tag. Buggy code is the original mount-mediation
audit call. `git merge-base --is-ancestor 2ea3ffb7782a84 HEAD` → **yes,
present**.
**Step 3.3: CHECK FILE HISTORY**
Record: Recent `mount.c` commits in this tree: nosymfollow audit flags,
move_mount detached-source fix, mount mediation regression fix. No prior
fix for binary-data auditing. Next mainline commit after this one is
`apparmor: refactory mount to use check_perms` (large refactor). **This
fix is standalone and comes before that refactor.**
**Step 3.4: CHECK THE AUTHOR'S OTHER COMMITS**
Record: John Johansen is the AppArmor maintainer. Same-file history is
almost entirely his. Weight: high.
**Step 3.5: CHECK FOR DEPENDENT/PREREQUISITE COMMITS**
Record: Diff uses existing `binary` parameter and existing
`audit_mount()` signature. No new helpers. **Applies standalone.** The
following mount refactor is not required.
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1: FIND THE ORIGINAL PATCH DISCUSSION**
Record:
- `b4 dig -c b9f2181b90ec047488dcb4fb1c631d417aff07e0` (and `-w`) →
lore.kernel.org returned errors; **no thread recovered**.
- GitHub commit page confirms message/diff match the candidate.
- LKML pull: [GIT PULL] AppArmor updates for v7.3 (John Johansen,
2026-08-26) lists this under Bug Fixes and notes
merge/build/regression testing plus linux-next and Ubuntu kernel
testing.
UNVERIFIED: v1/v2 review comments, NAKs, reviewer-requested `Cc:
stable`.
**Step 4.2: CHECK WHO REVIEWED THE PATCH**
Record: No Reviewed-by on the commit. Author is the subsystem
maintainer. Recipients of the original mail **UNVERIFIED** (b4 -w
failed).
**Step 4.3: SEARCH FOR THE BUG REPORT**
Record: No Reported-by, no bugzilla/syzbot link. Likely found while
preparing the mount refactor (same-day neighbor commits on 2026-08-10).
**Step 4.4: CHECK FOR RELATED PATCHES AND SERIES**
Record: Same pull as `refactory mount to use check_perms` (cleanup, not
a dependency) and `add audit mode...` (feature, unused by this fix).
Self-contained.
**Step 4.5: CHECK STABLE MAILING LIST HISTORY**
Record: No evidence this was NAK’d for stable. Search did not find a
stable-specific discussion of this title.
---
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1: KEY FUNCTIONS**
Record: `match_mnt_path_str()` modified; `audit_mount()`, `audit_cb()`,
`do_match_mnt()`, `aa_new_mount()`, `aa_remount()`,
`apparmor_sb_mount()` involved.
**Step 5.2: TRACE CALLERS**
Record:
- `match_mnt()` → `match_mnt_path_str()`
- `aa_new_mount()` → `match_mnt` / `match_mnt_path_str` (sets `binary`
from `FS_BINARY_MOUNTDATA`)
- `aa_remount()` → `match_mnt` (same flag from superblock type)
- `apparmor_sb_mount()` → `aa_new_mount` / `aa_remount` from
`security_sb_mount()` in `path_mount()`
**Step 5.3: TRACE CALLEES**
Record: `audit_mount` → `aa_audit` → `audit_cb` →
`audit_log_untrustedstring` / `strlen`. Memory alloc/locks are
unchanged.
**Step 5.4: FOLLOW THE CALL CHAIN**
Record: Userspace `mount(2)` / remount → `path_mount` → LSM `sb_mount` →
AppArmor. Reachable from confined tasks that can mount (typically
`CAP_SYS_ADMIN`, or a user namespace). **Yes, userspace-reachable.**
This tree has no AppArmor `fs_context` hooks; the classic `sb_mount`
path is the one that matters here.
**Step 5.5: SEARCH FOR SIMILAR PATTERNS**
Record: Only this `audit_mount` call passes mount `data`. Mediation
already had the `!binary` guard; audit did not. No sibling drivers with
the same bug.
---
## PHASE 6: CROSS-REFERENCING AGAINST THE LOCAL TREE
**Step 6.1: DOES THE BUGGY CODE EXIST IN THIS TREE?**
Record: **Yes.** `make kernelversion` = **6.12.111**; `git describe
HEAD` = `p-6.12-3-gc504cf0baa3d8`. Exact before-context is in
`match_mnt_path_str()`. Introduced **v4.14**. `git merge-base --is-
ancestor b9f2181b90ec HEAD` → **not present**.
**Step 6.2: CHECK FOR BACKPORT COMPLICATIONS**
Record: **Clean apply.** Surrounding function signature (`subj_cred`,
`rules->policy`, `binary`) already matches. Confirmed `old present:
True` in the current file.
**Step 6.3: CHECK IF RELATED FIXES ARE ALREADY HERE**
Record: `git log --grep='auditing of mount binary'` on
`security/apparmor/` is empty. Closest related in-tree patch is
`33be0cfa5ba52` (“apparmor: take nosymfollow flag into account”), which
is also AppArmor **mount audit correctness** and was already taken for
this series.
---
## PHASE 7: SUBSYSTEM AND MAINTAINER CONTEXT
**Step 7.1: SUBSYSTEM AND CRITICALITY**
Record: **AppArmor LSM** (`security/apparmor/`). **IMPORTANT** (not core
mm/VFS; default MAC on Ubuntu and several LTS distros). Mount mediation
+ audit are user-visible security/audit paths.
**Step 7.2: SUBSYSTEM ACTIVITY**
Record: Mature (~2017 mount mediation) with ongoing stable backports in
this tree. Not a brand-new 7.x subsystem.
---
## PHASE 8: IMPACT AND RISK ASSESSMENT
**Step 8.1: WHO IS AFFECTED**
Record: Config-specific: `CONFIG_SECURITY_APPARMOR`, confined processes,
filesystems with `FS_BINARY_MOUNTDATA` (NFS/Btrfs/Coda), and only when
an AppArmor mount audit record is generated with `AA_AUDIT_DATA` in
`perms->audit`.
**Step 8.2: TRIGGER CONDITIONS**
Record: Confined `mount()`/`remount` of NFS/Btrfs/Coda with non-NULL
mount data, plus policy that produces a mount audit record with the
data-audit bit. Not every boot; common on Ubuntu NFS/Btrfs systems with
AppArmor. Unprivileged userns mounts can hit it if confined. Not a race.
**Step 8.3: FAILURE MODE SEVERITY**
Record:
- Failure: `options=` in AppArmor audit is a `strlen()` view of a binary
blob (garbage, early truncate at first NUL, or hex dump of up to
~PAGE_SIZE-1).
- Not a panic: VFS NUL-terminates the last byte of the options page.
- **Severity: MEDIUM** (wrong LSM audit; possible bulky hex dump). Not
crash/deadlock/corruption.
**Step 8.4: RISK-BENEFIT RATIO**
Record:
- **Benefit:** medium for AppArmor+NFS/Btrfs users (correct audit; no
binary blob in logs). Same class as the in-tree nosymfollow audit fix.
- **Risk:** very low (2 lines, existing `binary` flag, no locking).
- **Ratio:** favorable.
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1: COMPILE THE EVIDENCE**
**FOR:**
- Real bug: audit disagrees with mediation on binary mount data.
- Maintainer-authored; v7.3 pull classified it as a bug fix; tested in
linux-next/Ubuntu.
- Two-line, obviously correct, clean apply on 6.12.111.
- Buggy code present since v4.14.
- Touches a live LSM path (NFS/Btrfs).
- This tree already accepted a similar AppArmor mount-audit correctness
fix (`nosymfollow`).
- No new APIs/features; no dependency on the later mount refactor.
**AGAINST:**
- No user report, syzbot, or CVE.
- Not a crash/deadlock/corruption; gated on `AA_AUDIT_DATA`.
- Short message; no `Fixes:`/`Cc: stable`.
- Present since 2017 without reports (low “bothers people” signal).
**UNRESOLVED:**
- Original lore review thread (b4/lore 403).
- Whether reviewers asked for `Cc: stable`.
**Step 9.2: STABLE RULES CHECKLIST**
1. Obviously correct and tested? **PASS** — two-line mirror of existing
`!binary` check; maintainer + v7.3 testing.
2. Fixes a real bug that affects users? **PASS** — AppArmor mount audit
of NFS/Btrfs/Coda can emit binary `options=`.
3. Important issue? **PASS** (medium) — LSM audit correctness / “oh,
that’s not good”; not a panic. Precedent in this tree: nosymfollow
mount-audit fix.
4. Small and contained? **PASS** — 2 lines, one function.
5. No new features or APIs? **PASS**.
6. Can apply to the local tree? **PASS**.
**Step 9.3: EXCEPTION CATEGORIES**
Record: None (not a device ID, quirk, DT, build, or docs fix). Qualifies
as a normal small bug fix.
**Step 9.4: DECISION**
Backport. The tree has the bug, the patch applies cleanly, it is a
maintainer LSM audit fix with negligible regression risk, and 6.12
AppArmor users (especially Ubuntu) mount NFS/Btrfs under confinement.
What it solves for stable users: AppArmor will stop stuffing binary
mount blobs into `options=` via
`strlen()`/`audit_log_untrustedstring()`, matching the already-correct
“don’t mediate binary data” rule.
Risk vs benefit: benefit is correct MAC audit on common filesystems;
risk is essentially zero.
---
## Verification
- **[Phase 1]** Parsed tags: only author SOB; no Fixes/Reported-by/Cc
stable. Upstream SHA `b9f2181b90ec047488dcb4fb1c631d417aff07e0`.
- **[Phase 1]** v7.3 pull (openwall/LKML, 2026-08-26) lists “auditing of
mount binary data” under Bug Fixes.
- **[Phase 2]** Diff: `match_mnt_path_str()` passes `!binary ? data :
NULL`; local file still passes `data`.
- **[Phase 2]** `do_match_mnt()` already has `data && !binary &&
(perms->allow & AA_MNT_CONT_MATCH)`.
- **[Phase 2]** `audit_cb()` uses `audit_log_untrustedstring` → `strlen`
(`kernel/audit.c`).
- **[Phase 2]** `path_mount()` NUL-terminates `data_page[PAGE_SIZE-1]` —
not an OOB on this path.
- **[Phase 2]** `FS_BINARY_MOUNTDATA`: nfs, nfs4, btrfs, coda.
- **[Phase 3]** `git blame`: line from `2ea3ffb7782a84` (v4.14-rc2);
ancestor of HEAD.
- **[Phase 3]** `git merge-base --is-ancestor b9f2181b90ec HEAD` → not
in this tree.
- **[Phase 3]** No `Fixes:` tag to follow.
- **[Phase 3]** Follow-up mainline commit is a mount refactor, not a
prerequisite.
- **[Phase 4]** `b4 dig -c/-w` failed (lore 403). UNVERIFIED: review
NAKs / stable nomination in the original thread.
- **[Phase 4]** No stable-list hit for this title.
- **[Phase 5]** Call chain: `path_mount` → `apparmor_sb_mount` →
`aa_new_mount`/`aa_remount` → `match_mnt_path_str` → `audit_mount`.
- **[Phase 5]** Only one `audit_mount` site passes `data`.
- **[Phase 6]** `git describe HEAD` = `p-6.12-3-gc504cf0baa3d8`; `make
kernelversion` = 6.12.111.
- **[Phase 6]** Python check: old snippet present, new snippet absent.
- **[Phase 6]** `git log --grep='auditing of mount binary' --
security/apparmor/` empty.
- **[Phase 7]** `CONFIG_SECURITY_APPARMOR` exists; AppArmor is
IMPORTANT.
- **[Phase 8]** Failure mode: garbled/hex `options=` in AppArmor audit,
severity MEDIUM; not oops.
- **UNVERIFIED:** Original lore review comments; whether `AA_AUDIT_DATA`
is set in typical Ubuntu mount policy (code path is real; frequency of
the audit bit is policy-dependent).
The fix is small, surgical, present as a real bug in this 6.12 tree, and
meets the stable criteria for an LSM audit correctness fix.
**YES**
security/apparmor/mount.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/security/apparmor/mount.c b/security/apparmor/mount.c
index 2f5d918832c10..31f792817643c 100644
--- a/security/apparmor/mount.c
+++ b/security/apparmor/mount.c
@@ -344,8 +344,8 @@ static int match_mnt_path_str(const struct cred *subj_cred,
audit:
return audit_mount(subj_cred, profile, OP_MOUNT, mntpnt, devname,
- type, NULL,
- flags, data, AA_MAY_MOUNT, &perms, info, error);
+ type, NULL, flags, !binary ? data : NULL,
+ AA_MAY_MOUNT, &perms, info, error);
}
/**
--
2.53.0
^ permalink raw reply related [flat|nested] 2+ messages in thread