From: Sasha Levin <sashal@kernel.org>
To: patches@lists.linux.dev, stable@vger.kernel.org
Cc: Gael Blivet <gael.blivet@gmail.com>,
Namjae Jeon <linkinjeon@kernel.org>,
Sasha Levin <sashal@kernel.org>,
linux-cifs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH AUTOSEL 6.18-5.15] ksmbd: skip fallocate for SMB2_CREATE_ALLOCATION_SIZE on a stream handle
Date: Mon, 19 Oct 2026 09:45:01 -0400 [thread overview]
Message-ID: <20261019135441.1420968-10-sashal@kernel.org> (raw)
In-Reply-To: <20261019135441.1420968-1-sashal@kernel.org>
From: Gael Blivet <gael.blivet@gmail.com>
[ Upstream commit bfdf81c62f4f52a3626ea62db0744def6778e83f ]
smb2_open() calls vfs_fallocate(fp->filp, ...) unconditionally when a
client's CREATE request includes an AllocationSize create context.
For a stream handle, fp->filp refers to the base file's data fork
(streams are xattr-backed on the same underlying file, not separate
files), so this pre-allocates storage on the base file's actual data
instead of doing anything meaningful for the stream -- fallocate has
no applicability to an xattr-backed stream at all.
Skip the fallocate call for stream handles.
Assisted-by: Claude:claude-sonnet-5
Signed-off-by: Gael Blivet <gael.blivet@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The local tree is **6.12.111**. The buggy `vfs_fallocate()` on stream
CREATEs is present here, and the skip is a small, self-contained
correctness fix that applies to this checkout.
## Phase 1: Commit message forensics
**Step 1.1: Subject line**
- Record: **[ksmbd] [skip] skip fallocate for
SMB2_CREATE_ALLOCATION_SIZE on a stream handle**
- Subsystem: ksmbd (in-kernel SMB3 server). Verb “skip” here means “do
not run a VFS operation that does not apply.”
**Step 1.2: Tags**
- Record:
- `Assisted-by: Claude:claude-sonnet-5`
- `Signed-off-by: Gael Blivet <gael.blivet@gmail.com>`
- `Acked-by: Namjae Jeon <linkinjeon@kernel.org>` (on `e360be3d03f77`;
the candidate copy uses `Signed-off-by: Namjae Jeon`)
- `Signed-off-by: Steve French` (on the smb.git copy)
- No `Fixes:`, `Reported-by:`, `Tested-by:`, `Cc: stable`, or `Link:`
- Absence of `Fixes:`/`Cc: stable` is expected for this review
pipeline
**Step 1.3: Body**
- Record:
- **Bug:** `smb2_open()` always calls `vfs_fallocate(fp->filp,
FALLOC_FL_KEEP_SIZE, …)` when CREATE has an AllocationSize context.
- **Root cause:** For a named-stream handle, `fp->filp` is the **base
file**. Streams are xattr-backed, not separate files.
- **Symptom:** Preallocation (and a level-II oplock break) hits the
base file’s data fork instead of the stream.
- **Failure mode:** Wrong-file disk preallocation; unexpected oplock
break on the base file.
- No stack trace, no version list, no user report.
**Step 1.4: Hidden bug fix?**
- Record: **Yes, this is an explicit bug fix**, not cleanup. Cover
letter for series 8/8: opening a stream with AllocationSize
“fallocates storage on the underlying base file’s actual data.”
## Phase 2: Diff analysis
**Step 2.1: Inventory**
- Record:
- File: `fs/smb/server/smb2pdu.c` only
- Scope: **+16 / −7** (net +9), one function: `smb2_open()`
- Classification: **single-file surgical fix**
**Step 2.2: Code-flow change**
- Record:
- **Before:** If CREATE has `SMB2_CREATE_ALLOCATION_SIZE`, always
`smb_break_all_levII_oplock(work, fp, 1)` then
`vfs_fallocate(fp->filp, FALLOC_FL_KEEP_SIZE, 0, alloc_size)`.
Failures are only debug-logged.
- **After:** Same path, but only when `!ksmbd_stream_fd(fp)`. Stream
handles skip both the oplock break and fallocate.
- Affected path: SMB2 CREATE with AllocationSize on a stream, after
the file is already opened.
**Step 2.3: Bug mechanism**
- Record:
- **Category:** logic / correctness (wrong object for a VFS call),
with resource side effects
- **Mechanism:** Stream fds share the base `struct file`.
`ksmbd_stream_fd()` is `fp->stream.name != NULL`. Stream I/O already
goes through xattrs (`ksmbd_vfs_stream_read`/`_write`); fallocate
has no stream equivalent.
- `FALLOC_FL_KEEP_SIZE` does not change `i_size`, but it **does**
allocate blocks on the base inode and can fill the filesystem.
`vfs_fallocate()` also takes `file_start_write()` and may run for a
long time on a large client-supplied size (bounded by `s_maxbytes`).
**Step 2.4: Fix quality**
- Record:
- Obviously correct: skip a VFS op that cannot apply to an xattr
stream.
- Matches existing stream guards (read/write/copy-chunk).
- Regression risk is **very low**: regular-file CREATE is unchanged;
stream CREATE only stops a harmful no-op-for-the-stream.
- Incomplete vs SET_INFO: `set_file_allocation_info()` in this tree
still fallocates `fp->filp` with no stream check. That does not make
this CREATE skip wrong.
## Phase 3: Git history
**Step 3.1: Blame**
- Record:
- Unconditional fallocate in `smb2_open()` comes from `e8c061917133dd`
(“ksmbd: opencode to avoid trivial wrappers”, 2021-06-22), replacing
`ksmbd_vfs_alloc_size()`.
- That commit **is an ancestor of this HEAD**.
- `ksmbd_stream_fd()` is from `f44158485826c0` (2021-03-16) and is
also an ancestor.
- Bug has been in ksmbd for the lifetime of this 6.12 line.
**Step 3.2: Fixes: tag**
- Record: **N/A** (no `Fixes:`). Introducer is the original CREATE
AllocationSize handling, present since cifsd/`e8c0619`.
**Step 3.3: Related file history**
- Record:
- `a9f106c765c12` — “fix wrong allocation size update in smb2_open()”
(response timing, not this bug).
- `2a4179c99bc6b` — overflow in `set_file_allocation_info()` (already
in this tree).
- `3b41d8b05dc89` — “return requested create allocation size” is
**not** in this tree (`merge-base --is-ancestor` failed). Candidate
parent uses outer-scope `alloc_size`; 6.12 still has a local `loff_t
alloc_size`. Functional hunk still matches.
**Step 3.4: Author**
- Record: Gael Blivet, stream/AAPL/Time Machine work; maintainer Namjae
Jeon acked and applied this patch. Not a drive-by.
**Step 3.5: Dependencies**
- Record:
- Cover: **“None of this depends on or touches the AAPL/Time Machine
feature series.”**
- Needs only `ksmbd_stream_fd()`, which exists here.
- Series is **[PATCH 8/8]**; this hunk does not need patches 1–7.
- `git apply --check` of `e360be3d03f77` on this tree: **Hunk #1
succeeded at 3593 (offset -504 lines).**
## Phase 4: Mailing list / external
**Step 4.1: Original discussion**
- Record:
- `b4 dig -c e360be3d03f77` →
https://patch.msgid.link/20260709000548.66445-9-g.blivet@me.com
- Series: **[PATCH 0/8] ksmbd: follow-up fixes to previously-merged
patches** (2026-07-09)
- Only **v1** found (`b4 dig -a`).
- Namjae Jeon: **“Applied them except 0002, 0005 patch”** — this is
patch 8, so it was applied. No NAK on patch 8. No `Cc: stable` in
review.
**Step 4.2: Recipients (`b4 dig -w`)**
- Record: Gaël Blivet-Bailly, Namjae Jeon, `linux-cifs@vger.kernel.org`.
Maintainer was on To/Cc and acked.
**Step 4.3: Bug report**
- Record: No syzbot/bugzilla. Found by **Sashiko (LLM review)** of
stream-handling code. Related real-world class: namjaejeon/ksmbd#507
(SetInfo/EOF on a stream truncated the **base** file because
`fp->filp` is the base file). Same object-confusion pattern.
**Step 4.4: Series context**
- Record: Eight independent follow-ups. Patch 8 is standalone. Also
listed in the 7.3-rc1 ksmbd pull under alternate-data-stream handling
(`e360be3d03f77`).
**Step 4.5: Stable list**
- Record: lore.kernel.org/stable returned **403**. **UNVERIFIED**
whether stable@ already discussed this; that does not change the
technical verdict.
## Phase 5: Semantic / call-graph
**Step 5.1: Functions**
- Record: `smb2_open()` only. Helper used: `ksmbd_stream_fd()`.
**Step 5.2: Callers**
- Record: `fs/smb/server/smb2ops.c` maps `SMB2_CREATE_HE` → `smb2_open`.
Reachable from any authenticated SMB2 CREATE.
**Step 5.3: Callees**
- Record: `smb2_find_context_vals()`, `smb_break_all_levII_oplock()`,
`vfs_fallocate()`. Stream CREATE still sets `fp->stream.name` via
`smb2_set_stream_name_xattr()` **before** this block (around line
3485).
**Step 5.4: Reachability**
- Record: **Yes, from the network.** Requires:
1. `CONFIG_SMB_SERVER`
2. Share with `KSMBD_SHARE_FLAG_STREAMS`
3. CREATE of `name:stream` plus AllocationSize create context (`AlSi`)
- Windows ADS and macOS resource-fork/Finder streams are the usual
clients. Unprivileged local syscall is not the path; an authenticated
SMB client is.
**Step 5.5: Similar patterns**
- Record: `ksmbd_vfs_read`/`write` already branch on
`ksmbd_stream_fd()`. `ksmbd_vfs_copy_file_ranges()` returns `-EBADF`
for streams. This CREATE fallocate path was the missing guard.
`set_file_allocation_info()` still unguarded in **this** tree.
## Phase 6: This tree (6.12.111)
**Step 6.1: Buggy code present?**
- Record: **Yes.** `fs/smb/server/smb2pdu.c` ~3596–3602:
```3592:3602:fs/smb/server/smb2pdu.c
alloc_size =
le64_to_cpu(az_req->AllocationSize);
ksmbd_debug(SMB,
"request smb2 create allocate size :
%llu\n",
alloc_size);
smb_break_all_levII_oplock(work, fp, 1);
err = vfs_fallocate(fp->filp,
FALLOC_FL_KEEP_SIZE, 0,
alloc_size);
if (err < 0)
ksmbd_debug(SMB,
"vfs_fallocate is failed :
%d\n",
err);
```
- `ksmbd_stream_fd()` is in `fs/smb/server/vfs_cache.h`.
- Candidate SHAs `e360be3d03f77` / `bfdf81c62f4f` are **not** ancestors
of HEAD.
**Step 6.2: Backport difficulty**
- Record: **Clean apply with offset** (`git apply --check` succeeded at
line 3593). Local `loff_t alloc_size` vs later outer-scope
`alloc_size` does not conflict with the wrapped lines.
**Step 6.3: Related fixes already here?**
- Record: No skip of CREATE fallocate for streams. Overflow check in
SET_INFO allocation is a different bug.
## Phase 7: Subsystem context
**Step 7.1**
- Record: **ksmbd (`fs/smb/server`)**, **IMPORTANT** (in-kernel SMB3
server for NAS/shares). Not mm/core, but network-facing and used in
production.
**Step 7.2**
- Record: Actively backported on this branch (many ksmbd fixes in recent
`git log -- fs/smb/server`).
## Phase 8: Impact and risk
**Step 8.1: Who is affected**
- Record: ksmbd users with **streams enabled**, when a client CREATEs a
named stream with AllocationSize. Typical Windows/macOS
interoperability.
**Step 8.2: Trigger**
- Record: Authenticated SMB CREATE of `file:stream` + AllocationSize.
Not every open; common enough for ADS-aware clients. Same client could
already write the file; unique harm is **wrong object** (base file)
and a large fallocate without intending to preallocate the data fork.
**Step 8.3: Failure mode**
- Record:
- Preallocates blocks on the **base file** (`FALLOC_FL_KEEP_SIZE` → no
size change, but real space use).
- Breaks level-II oplocks on the base file (other clients can lose
caching).
- Large `AllocationSize` can run a long `fallocate` under
`file_start_write()` (filesystem write exclusion).
- **Severity: HIGH** for disk/oplock side effects on a NAS; not a
panic/UAF. Not content corruption of existing data.
**Step 8.4: Risk–benefit**
- Record:
- **Benefit:** Stops wrong-file preallocation and spurious oplock
breaks; one `if` around existing code.
- **Risk:** Very low. Stream CREATE AllocationSize currently does the
wrong thing; skipping is the correct xattr-stream behavior.
- **Ratio:** Favorable.
## Phase 9: Synthesis
**Step 9.1: Evidence**
**For:**
- Real wrong-file VFS operation, present since 2021, present in 6.12.111
- User-visible: disk on the base file, oplock disruption
- Small, obviously correct, applies with offset
- `ksmbd_stream_fd()` already here; no AAPL/Time Machine dependency
- Maintainer applied it (patch 8 of 8)
- Same stream/`fp->filp` confusion class as documented user bugs
**Against:**
- No crash/UAF/CVE; found by automated review, not a reporter
- SET_INFO allocation path still unfixed here
- Needs streams share flag
- lore stable search unverified (HTTP 403)
**Step 9.2: Stable rules**
1. Obviously correct and tested? **PASS** — skip is correct; maintainer
applied it.
2. Real bug affecting users? **PASS** — wrong-file fallocate on stream
CREATE.
3. Important (crash/security/corruption/deadlock/etc.)? **PASS** —
unexpected allocation and oplock break on the base file of a network
file server; can consume substantial disk.
4. Small and contained? **PASS** — ~9 lines, one function.
5. No new features/APIs? **PASS**
6. Applies to this tree? **PASS** — `git apply --check` succeeded.
**Step 9.3: Exceptions**
- Record: Not a device-ID/quirk/DT/build/doc exception. Qualifies as a
normal small bugfix.
**Step 9.4: Decision**
Backport to **this** 6.12.111 tree. The bug is here, the helper is here,
the patch applies, and the change only disables a harmful path.
---
### What this solves for 6.12 users
On ksmbd shares with named streams, an SMB2 CREATE of an alternate data
stream that includes AllocationSize currently calls `vfs_fallocate()` on
the **base file**. That can allocate disk and break oplocks on the wrong
object. Streams are xattr-backed; fallocate cannot apply to them.
Skipping it is the correct, minimal fix.
### Verification
- **[Phase 1]** Parsed tags on `e360be3d03f77` / `bfdf81c62f4f` (same
patch-id `8e46981f5116…`); no Fixes/Reported-by/Cc:stable
- **[Phase 2]** Diff: wrap fallocate + oplock break in `if
(!ksmbd_stream_fd(fp))` in `smb2_open()`
- **[Phase 3]** `git blame`: fallocate from `e8c061917133dd` (ancestor);
`ksmbd_stream_fd` from `f44158485826c0` (ancestor)
- **[Phase 3]** `git merge-base --is-ancestor e360be3d03f77 HEAD` → not
in this tree
- **[Phase 3]** `git merge-base --is-ancestor 3b41d8b05dc89 HEAD` →
later alloc_size-field commit not here
- **[Phase 4]** `b4 dig -c e360be3d03f77` → lore msgid
`20260709000548.66445-9-g.blivet@me.com`; series 8/8
- **[Phase 4]** `b4 dig -a`: v1 only; `b4 dig -w`: linux-cifs + Namjae
Jeon
- **[Phase 4]** mbox: Namjae “Applied them except 0002, 0005”; patch 8
applied; no NAK
- **[Phase 4]** Cover: independent of Time Machine/AAPL
- **[Phase 5]** `SMB2_CREATE_HE` → `smb2_open`; streams gated by
`KSMBD_SHARE_FLAG_STREAMS`
- **[Phase 6]** `git describe HEAD` → `p-6.12-3-gc504cf0baa3d8`; `make
kernelversion` → **6.12.111**
- **[Phase 6]** Buggy lines at `smb2pdu.c:3596–3602`; `git apply
--check` succeeded at 3593
- **[Phase 8]** Failure mode: wrong-file preallocation + oplock break;
severity HIGH for ksmbd/NAS, not a panic
- **UNVERIFIED:** stable@ lore search (HTTP 403); whether Windows always
sends AllocationSize on ADS CREATE (bug is still real when it does)
**YES**
fs/smb/server/smb2pdu.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index f383d1e5e5cb7..9c5275d3640cc 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -3633,13 +3633,22 @@ int smb2_open(struct ksmbd_work *work)
ksmbd_debug(SMB,
"request smb2 create allocate size : %llu\n",
alloc_size);
- smb_break_all_levII_oplock(work, fp, 1);
- err = vfs_fallocate(fp->filp, FALLOC_FL_KEEP_SIZE, 0,
- alloc_size);
- if (err < 0)
- ksmbd_debug(SMB,
- "vfs_fallocate is failed : %d\n",
- err);
+ /*
+ * fp->filp is the base file's data fork for a stream
+ * handle (streams are xattr-backed on the same
+ * underlying file) -- fallocate has no meaning for a
+ * stream and would otherwise pre-allocate storage on
+ * the base file's data instead.
+ */
+ if (!ksmbd_stream_fd(fp)) {
+ smb_break_all_levII_oplock(work, fp, 1);
+ err = vfs_fallocate(fp->filp, FALLOC_FL_KEEP_SIZE, 0,
+ alloc_size);
+ if (err < 0)
+ ksmbd_debug(SMB,
+ "vfs_fallocate is failed : %d\n",
+ err);
+ }
}
context = smb2_find_context_vals(req, SMB2_CREATE_QUERY_ON_DISK_ID, 4);
--
2.53.0
next parent reply other threads:[~2026-10-19 13:54 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20261019135441.1420968-1-sashal@kernel.org>
2026-10-19 13:45 ` Sasha Levin [this message]
2026-10-19 13:45 ` [PATCH AUTOSEL 6.18-6.12] ksmbd: fix off-by-one rejecting minimal COPYCHUNK query-limits request Sasha Levin
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-5.15] ksmbd: handle empty QUERY_ALLOCATED_RANGES output Sasha Levin
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-5.15] ksmbd: notify parent directory leases on child create Sasha Levin
2026-10-19 13:46 ` [PATCH AUTOSEL 6.18-6.1] ksmbd: handle allocated range queries on dense files Sasha Levin
2026-10-19 13:47 ` [PATCH AUTOSEL 6.18-6.6] ksmbd: report actual xattr value length in stream enumeration Sasha Levin
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-5.15] smb/server: cancel async requests when closing connection Sasha Levin
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-5.15] ksmbd: allow FSCTL_SET_SPARSE without input buffer Sasha Levin
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-5.15] ksmbd: protect private extended attributes Sasha Levin
2026-10-19 13:48 ` [PATCH AUTOSEL 6.18-6.1] ksmbd: allow I/O on directory named streams Sasha Levin
2026-10-19 13:49 ` [PATCH AUTOSEL 6.18-5.15] smb: server: Free session data in user_session.c with kfree_sensitive() Sasha Levin
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-6.6] ksmbd: return buffer too small for short security queries Sasha Levin
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.15] ksmbd: honor client signing-required in all modes Sasha Levin
2026-10-19 13:50 ` [PATCH AUTOSEL 6.18-5.15] ksmbd: preserve access denied status for copychunk Sasha Levin
2026-10-19 13:51 ` [PATCH AUTOSEL 6.18-5.15] ksmbd: fix heap out-of-bounds write in krb5_authenticate() Sasha Levin
2026-10-19 13:51 ` [PATCH AUTOSEL 6.18-6.6] smb: server: Make sure that passkey is not leaked on the heap in user_config.c Sasha Levin
2026-10-19 13:51 ` [PATCH AUTOSEL 6.18-6.1] ksmbd: honor byte-range locks for zero data Sasha Levin
2026-10-19 13:52 ` [PATCH AUTOSEL 6.18-5.15] ksmbd: reject blocking compound lock requests Sasha Levin
2026-10-19 13:52 ` [PATCH AUTOSEL 6.18-6.12] ksmbd: reject SMB3.1.1 binding with mismatched cipher Sasha Levin
2026-10-19 13:52 ` [PATCH AUTOSEL 6.18-6.1] ksmbd: fix AsyncId zeroed before use in smb2_lock() cancel response Sasha Levin
2026-10-19 13:52 ` [PATCH AUTOSEL 6.18-6.6] ksmbd: fix durable handle v2 default timeout units (60 -> 60000) Sasha Levin
2026-10-19 13:53 ` [PATCH AUTOSEL 6.18-5.15] ksmbd: enable TCP keepalive for accepted connections Sasha Levin
2026-10-19 13:53 ` [PATCH AUTOSEL 6.18-5.15] ksmbd: fix SMB2 byte-range lock end offset 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=20261019135441.1420968-10-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=gael.blivet@gmail.com \
--cc=linkinjeon@kernel.org \
--cc=linux-cifs@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=patches@lists.linux.dev \
--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