Re: [PATCH] ksmbd: fix integer overflow in set_file_allocation_info()
Namjae Jeon <[email protected]>
| Newsgroups | org.kernel.vger.linux-cifs,org.kernel.vger.linux-kernel,org.kernel.vger.stable |
|---|---|
| Message-ID | <CAKYAXd9SuA0BcYd=m_FN4sN_ByE7knmkh7Qbsd4Y5Wt6hY0Dgg@mail.gmail.com> |
On Fri, Jul 10, 2026 at 9:32 AM Ibrahim Hashimov <[email protected]> wrote: > > set_file_allocation_info() converts the client-supplied > FILE_ALLOCATION_INFORMATION::AllocationSize into a 512-byte block > count with: > > alloc_blks = (le64_to_cpu(file_alloc_info->AllocationSize) + 511) >> 9; > > AllocationSize is a fully client-controlled __le64 field; the only > validation performed by the caller (smb2_set_info_file(), case > FILE_ALLOCATION_INFORMATION) is that the fixed buffer is at least > sizeof(struct smb2_file_alloc_info) == 8 bytes. The value itself is > never range-checked before this arithmetic. > > When AllocationSize is close to U64_MAX (e.g. 0xffffffffffffffff), > "AllocationSize + 511" wraps around mod 2^64 to a small number > (0xffffffffffffffff + 511 = 510), so alloc_blks becomes 0. Since any > existing regular file has stat.blocks > 0, the function then takes > the "shrink" branch and calls: > > ksmbd_vfs_truncate(work, fp, alloc_blks * 512); /* == 0 */ > > silently truncating the file to size 0, even though the client asked > to grow the allocation to (what looks like) the maximum possible > size. The trailing "if (size < alloc_blks * 512) i_size_write(inode, > size);" restore is guarded by a comparison that is never true once > alloc_blks == 0, so the truncation is not undone. This lets an > authenticated SMB client that already holds an open handle with > FILE_WRITE_DATA on a file silently truncate that same file to size 0 > via a single crafted SET_INFO(FILE_ALLOCATION_INFORMATION) request > advertising a near-U64_MAX AllocationSize, even though the request > asks to grow the file's allocation rather than shrink it. This is a > functional/data-loss bug, not a privilege-boundary > violation: the same client could already truncate the file via > FILE_END_OF_FILE_INFORMATION or a plain write. > > Fix it by validating AllocationSize against MAX_LFS_FILESIZE, the > same upper bound the VFS itself uses to reject unrepresentable file > sizes, before doing the "+511" rounding, and rejecting oversized > values with -EINVAL. Bounding AllocationSize to > MAX_LFS_FILESIZE - 511 guarantees the "+511" addition cannot wrap, > and that the subsequent "alloc_blks * 512" values passed to > vfs_fallocate() and ksmbd_vfs_truncate() stay within a representable > loff_t as well. > > No legitimate SMB client asks for an allocation size anywhere near > 2^64 bytes, so this only rejects a value that was previously > silently misinterpreted as zero. > > Runtime-verified on a v6.19 KASAN test stand: sending SET_INFO > (FILE_ALLOCATION_INFORMATION) with AllocationSize = 0xffffffffffffffff > against ksmbd now returns -EINVAL and leaves the target file's size > unchanged, where the unpatched kernel truncated it from 4096 to 0 > bytes. > > Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3") > Cc: [email protected] > Signed-off-by: Ibrahim Hashimov <[email protected]> > Assisted-by: AuditCode-AI:2026.07 Applied it to #ksmbd-for-next-next. Thanks!