* [PATCH v2 3/7] netfs: Fix netfs_invalidate_folio() to clear dirty bit if all changes gone
From: David Howells @ 2026-04-14 8:19 UTC (permalink / raw)
To: Christian Brauner
Cc: David Howells, Paulo Alcantara, netfs, linux-afs, linux-cifs,
ceph-devel, linux-fsdevel, linux-kernel, Marc Dionne,
Paulo Alcantara, Matthew Wilcox
In-Reply-To: <20260414082004.3756080-1-dhowells@redhat.com>
If a streaming write is made, this will leave the relevant modified folio
in a not-uptodate, but dirty state with a netfs_folio struct hung off of
folio->private indicating the dirty range. Subsequently truncating the
file such that the dirty data in the folio is removed, but the first part
of the folio theoretically remains will cause the netfs_folio struct to be
discarded... but will leave the dirty flag set.
If the folio is then read via mmap(), netfs_read_folio() will see that the
page is dirty and jump to netfs_read_gaps() to fill in the missing bits.
netfs_read_gaps(), however, expects there to be a netfs_folio struct
present and can oops because truncate removed it.
Fix this by calling folio_cancel_dirty() in netfs_invalidate_folio() in the
event that all the dirty data in the folio is erased (as nfs does).
Also add some tracepoints to log modifications to a dirty page.
This can be reproduced with something like:
dd if=/dev/zero of=/xfstest.test/foo bs=1M count=1
umount /xfstest.test
mount /xfstest.test
xfs_io -c "w 0xbbbf 0xf96c" \
-c "truncate 0xbbbf" \
-c "mmap -r 0xb000 0x11000" \
-c "mr 0xb000 0x11000" \
/xfstest.test/foo
with fscaching disabled (otherwise streaming writes are suppressed) and a
change to netfs_perform_write() to disallow streaming writes if the fd is
open O_RDWR:
if (//(file->f_mode & FMODE_READ) || <--- comment this out
netfs_is_cache_enabled(ctx)) {
It should be reproducible even without this change, but if prevents the
above trivial xfs_io command from reproducing it.
Note that the initial dd is important: the file must start out sufficiently
large that the zero-point logic doesn't just clear the gaps because it
knows there's nothing in the file to read yet. Unmounting and mounting is
needed to clear the pagecache (there are other ways to do that that may
also work).
This was initially reproduced with the generic/522 xfstest on some patches
that remove the FMODE_READ restriction.
Fixes: 9ebff83e6481 ("netfs: Prep to use folio->private for write grouping and streaming write")
Reported-by: Marc Dionne <marc.dionne@auristor.com>
Signed-off-by: David Howells <dhowells@redhat.com>
cc: Paulo Alcantara <pc@manguebit.org>
cc: Matthew Wilcox <willy@infradead.org>
cc: netfs@lists.linux.dev
cc: linux-fsdevel@vger.kernel.org
---
fs/netfs/misc.c | 6 +++++-
include/trace/events/netfs.h | 4 ++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/fs/netfs/misc.c b/fs/netfs/misc.c
index 6df89c92b10b..d8e8a4b59768 100644
--- a/fs/netfs/misc.c
+++ b/fs/netfs/misc.c
@@ -256,6 +256,7 @@ void netfs_invalidate_folio(struct folio *folio, size_t offset, size_t length)
/* Move the start of the data. */
finfo->dirty_len = fend - iend;
finfo->dirty_offset = offset;
+ trace_netfs_folio(folio, netfs_folio_trace_invalidate_front);
return;
}
@@ -264,12 +265,14 @@ void netfs_invalidate_folio(struct folio *folio, size_t offset, size_t length)
*/
if (iend >= fend) {
finfo->dirty_len = offset - fstart;
+ trace_netfs_folio(folio, netfs_folio_trace_invalidate_tail);
return;
}
/* A partial write was split. The caller has already zeroed
* it, so just absorb the hole.
*/
+ trace_netfs_folio(folio, netfs_folio_trace_invalidate_middle);
}
return;
@@ -277,8 +280,9 @@ void netfs_invalidate_folio(struct folio *folio, size_t offset, size_t length)
netfs_put_group(netfs_folio_group(folio));
folio_detach_private(folio);
folio_clear_uptodate(folio);
+ folio_cancel_dirty(folio);
kfree(finfo);
- return;
+ trace_netfs_folio(folio, netfs_folio_trace_invalidate_all);
}
EXPORT_SYMBOL(netfs_invalidate_folio);
diff --git a/include/trace/events/netfs.h b/include/trace/events/netfs.h
index cbe28211106c..88d814ba1e69 100644
--- a/include/trace/events/netfs.h
+++ b/include/trace/events/netfs.h
@@ -194,6 +194,10 @@
EM(netfs_folio_trace_copy_to_cache, "mark-copy") \
EM(netfs_folio_trace_end_copy, "end-copy") \
EM(netfs_folio_trace_filled_gaps, "filled-gaps") \
+ EM(netfs_folio_trace_invalidate_all, "inval-all") \
+ EM(netfs_folio_trace_invalidate_front, "inval-front") \
+ EM(netfs_folio_trace_invalidate_middle, "inval-mid") \
+ EM(netfs_folio_trace_invalidate_tail, "inval-tail") \
EM(netfs_folio_trace_kill, "kill") \
EM(netfs_folio_trace_kill_cc, "kill-cc") \
EM(netfs_folio_trace_kill_g, "kill-g") \
^ permalink raw reply related
* [PATCH v2 2/7] netfs: fix error handling in netfs_extract_user_iter()
From: David Howells @ 2026-04-14 8:19 UTC (permalink / raw)
To: Christian Brauner
Cc: David Howells, Paulo Alcantara, netfs, linux-afs, linux-cifs,
ceph-devel, linux-fsdevel, linux-kernel, Paulo Alcantara,
Xiaoli Feng, stable
In-Reply-To: <20260414082004.3756080-1-dhowells@redhat.com>
From: Paulo Alcantara <pc@manguebit.org>
In netfs_extract_user_iter(), if iov_iter_extract_pages() failed to
extract user pages, bail out on -ENOMEM, otherwise return the error
code only if @npages == 0, allowing short DIO reads and writes to be
issued.
This fixes mmapstress02 from LTP tests against CIFS.
Fixes: 85dd2c8ff368 ("netfs: Add a function to extract a UBUF or IOVEC into a BVEC iterator")
Reported-by: Xiaoli Feng <xifeng@redhat.com>
Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Signed-off-by: David Howells <dhowells@redhat.com>
Cc: netfs@lists.linux.dev
Cc: stable@vger.kernel.org
Cc: linux-cifs@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
---
fs/netfs/iterator.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/fs/netfs/iterator.c b/fs/netfs/iterator.c
index 154a14bb2d7f..adca78747f23 100644
--- a/fs/netfs/iterator.c
+++ b/fs/netfs/iterator.c
@@ -22,7 +22,7 @@
*
* Extract the page fragments from the given amount of the source iterator and
* build up a second iterator that refers to all of those bits. This allows
- * the original iterator to disposed of.
+ * the original iterator to be disposed of.
*
* @extraction_flags can have ITER_ALLOW_P2PDMA set to request peer-to-peer DMA be
* allowed on the pages extracted.
@@ -67,8 +67,8 @@ ssize_t netfs_extract_user_iter(struct iov_iter *orig, size_t orig_len,
ret = iov_iter_extract_pages(orig, &pages, count,
max_pages - npages, extraction_flags,
&offset);
- if (ret < 0) {
- pr_err("Couldn't get user pages (rc=%zd)\n", ret);
+ if (unlikely(ret <= 0)) {
+ ret = ret ?: -EIO;
break;
}
@@ -97,6 +97,13 @@ ssize_t netfs_extract_user_iter(struct iov_iter *orig, size_t orig_len,
npages += cur_npages;
}
+ if (ret < 0 && (ret == -ENOMEM || npages == 0)) {
+ for (i = 0; i < npages; i++)
+ unpin_user_page(bv[i].bv_page);
+ kvfree(bv);
+ return ret;
+ }
+
iov_iter_bvec(new, orig->data_source, bv, npages, orig_len - count);
return npages;
}
^ permalink raw reply related
* [PATCH v2 1/7] netfs: fix VM_BUG_ON_FOLIO() issue in netfs_write_begin() call
From: David Howells @ 2026-04-14 8:19 UTC (permalink / raw)
To: Christian Brauner
Cc: David Howells, Paulo Alcantara, netfs, linux-afs, linux-cifs,
ceph-devel, linux-fsdevel, linux-kernel, Viacheslav Dubeyko,
Paulo Alcantara (Red Hat)
In-Reply-To: <20260414082004.3756080-1-dhowells@redhat.com>
From: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
The multiple runs of generic/013 test-case is capable
to reproduce a kernel BUG at mm/filemap.c:1504 with
probability of 30%.
while true; do
sudo ./check generic/013
done
[ 9849.452376] page: refcount:3 mapcount:0 mapping:00000000e58ff252 index:0x10781 pfn:0x1c322
[ 9849.452412] memcg:ffff8881a1915800
[ 9849.452417] aops:ceph_aops ino:1000058db9e dentry name(?):"f9XXXXXX"
[ 9849.452432] flags: 0x17ffffc0000000(node=0|zone=2|lastcpupid=0x1fffff)
[ 9849.452441] raw: 0017ffffc0000000 0000000000000000 dead000000000122 ffff88816110d248
[ 9849.452445] raw: 0000000000010781 0000000000000000 00000003ffffffff ffff8881a1915800
[ 9849.452447] page dumped because: VM_BUG_ON_FOLIO(!folio_test_locked(folio))
[ 9849.452474] ------------[ cut here ]------------
[ 9849.452476] kernel BUG at mm/filemap.c:1504!
[ 9849.478635] Oops: invalid opcode: 0000 [#1] SMP KASAN NOPTI
[ 9849.481772] CPU: 2 UID: 0 PID: 84223 Comm: fsstress Not tainted 7.0.0-rc1+ #18 PREEMPT(full)
[ 9849.482881] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-9.fc43 06/1
0/2025
[ 9849.484539] RIP: 0010:folio_unlock+0x85/0xa0
[ 9849.485076] Code: 89 df 31 f6 e8 1c f3 ff ff 48 8b 5d f8 c9 31 c0 31 d2 31 f6 31 ff c3 cc
cc cc cc 48 c7 c6 80 6c d9 a7 48 89 df e8 4b b3 10 00 <0f> 0b 48 89 df e8 21 e6 2c 00 eb 9d 0f 1f 40 00 66 66 2e 0f 1f 84
[ 9849.493818] RSP: 0018:ffff8881bb8076b0 EFLAGS: 00010246
[ 9849.495740] RAX: 0000000000000000 RBX: ffffea00070c8980 RCX: 0000000000000000
[ 9849.498678] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
[ 9849.500559] RBP: ffff8881bb8076b8 R08: 0000000000000000 R09: 0000000000000000
[ 9849.501097] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000010782000
[ 9849.502108] R13: ffff8881935de738 R14: ffff88816110d010 R15: 0000000000001000
[ 9849.502516] FS: 00007e36cbe94740(0000) GS:ffff88824a899000(0000) knlGS:0000000000000000
[ 9849.502996] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 9849.503810] CR2: 000000c0002b0000 CR3: 000000011bbf6004 CR4: 0000000000772ef0
[ 9849.504459] PKRU: 55555554
[ 9849.504626] Call Trace:
[ 9849.505242] <TASK>
[ 9849.505379] netfs_write_begin+0x7c8/0x10a0
[ 9849.505877] ? __kasan_check_read+0x11/0x20
[ 9849.506384] ? __pfx_netfs_write_begin+0x10/0x10
[ 9849.507178] ceph_write_begin+0x8c/0x1c0
[ 9849.507934] generic_perform_write+0x391/0x8f0
[ 9849.508503] ? __pfx_generic_perform_write+0x10/0x10
[ 9849.509062] ? file_update_time_flags+0x19a/0x4b0
[ 9849.509581] ? ceph_get_caps+0x63/0xf0
[ 9849.510259] ? ceph_get_caps+0x63/0xf0
[ 9849.510530] ceph_write_iter+0xe79/0x1ae0
[ 9849.511282] ? __pfx_ceph_write_iter+0x10/0x10
[ 9849.511839] ? lock_acquire+0x1ad/0x310
[ 9849.512334] ? ksys_write+0xf9/0x230
[ 9849.512582] ? lock_is_held_type+0xaa/0x140
[ 9849.513128] vfs_write+0x512/0x1110
[ 9849.513634] ? __fget_files+0x33/0x350
[ 9849.513893] ? __pfx_vfs_write+0x10/0x10
[ 9849.514143] ? mutex_lock_nested+0x1b/0x30
[ 9849.514394] ksys_write+0xf9/0x230
[ 9849.514621] ? __pfx_ksys_write+0x10/0x10
[ 9849.514887] ? do_syscall_64+0x25e/0x1520
[ 9849.515122] ? __kasan_check_read+0x11/0x20
[ 9849.515366] ? trace_hardirqs_on_prepare+0x178/0x1c0
[ 9849.515655] __x64_sys_write+0x72/0xd0
[ 9849.515885] ? trace_hardirqs_on+0x24/0x1c0
[ 9849.516130] x64_sys_call+0x22f/0x2390
[ 9849.516341] do_syscall_64+0x12b/0x1520
[ 9849.516545] ? do_syscall_64+0x27c/0x1520
[ 9849.516783] ? do_syscall_64+0x27c/0x1520
[ 9849.517003] ? lock_release+0x318/0x480
[ 9849.517220] ? __x64_sys_io_getevents+0x143/0x2d0
[ 9849.517479] ? percpu_ref_put_many.constprop.0+0x8f/0x210
[ 9849.517779] ? entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 9849.518073] ? do_syscall_64+0x25e/0x1520
[ 9849.518291] ? __kasan_check_read+0x11/0x20
[ 9849.518519] ? trace_hardirqs_on_prepare+0x178/0x1c0
[ 9849.518799] ? do_syscall_64+0x27c/0x1520
[ 9849.519024] ? local_clock_noinstr+0xf/0x120
[ 9849.519262] ? entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 9849.519544] ? do_syscall_64+0x25e/0x1520
[ 9849.519781] ? __kasan_check_read+0x11/0x20
[ 9849.520008] ? trace_hardirqs_on_prepare+0x178/0x1c0
[ 9849.520273] ? do_syscall_64+0x27c/0x1520
[ 9849.520491] ? trace_hardirqs_on_prepare+0x178/0x1c0
[ 9849.520767] ? irqentry_exit+0x10c/0x6c0
[ 9849.520984] ? trace_hardirqs_off+0x86/0x1b0
[ 9849.521224] ? exc_page_fault+0xab/0x130
[ 9849.521472] entry_SYSCALL_64_after_hwframe+0x76/0x7e
[ 9849.521766] RIP: 0033:0x7e36cbd14907
[ 9849.521989] Code: 10 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 00 f3 0f 1e fa 64 8b 04 25 18 00 00 00 85 c0 75 10 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 51 c3 48 83 ec 28 48 89 54 24 18 48 89 74 24
[ 9849.523057] RSP: 002b:00007ffff2d2a968 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
[ 9849.523484] RAX: ffffffffffffffda RBX: 000000000000e549 RCX: 00007e36cbd14907
[ 9849.523885] RDX: 000000000000e549 RSI: 00005bd797ec6370 RDI: 0000000000000004
[ 9849.524277] RBP: 0000000000000004 R08: 0000000000000047 R09: 00005bd797ec6370
[ 9849.524652] R10: 0000000000000078 R11: 0000000000000246 R12: 0000000000000049
[ 9849.525062] R13: 0000000010781a37 R14: 00005bd797ec6370 R15: 0000000000000000
[ 9849.525447] </TASK>
[ 9849.525574] Modules linked in: intel_rapl_msr intel_rapl_common intel_uncore_frequency_common intel_pmc_core pmt_telemetry pmt_discovery pmt_class intel_pmc_ssram_telemetry intel_vsec kvm_intel joydev kvm irqbypass ghash_clmulni_intel aesni_intel input_leds rapl mac_hid psmouse vga16fb serio_raw vgastate floppy i2c_piix4 bochs qemu_fw_cfg i2c_smbus pata_acpi sch_fq_codel rbd msr parport_pc ppdev lp parport efi_pstore
[ 9849.529150] ---[ end trace 0000000000000000 ]---
[ 9849.529502] RIP: 0010:folio_unlock+0x85/0xa0
[ 9849.530813] Code: 89 df 31 f6 e8 1c f3 ff ff 48 8b 5d f8 c9 31 c0 31 d2 31 f6 31 ff c3 cc cc cc cc 48 c7 c6 80 6c d9 a7 48 89 df e8 4b b3 10 00 <0f> 0b 48 89 df e8 21 e6 2c 00 eb 9d 0f 1f 40 00 66 66 2e 0f 1f 84
[ 9849.534986] RSP: 0018:ffff8881bb8076b0 EFLAGS: 00010246
[ 9849.536198] RAX: 0000000000000000 RBX: ffffea00070c8980 RCX: 0000000000000000
[ 9849.537718] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000
[ 9849.539321] RBP: ffff8881bb8076b8 R08: 0000000000000000 R09: 0000000000000000
[ 9849.540862] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000010782000
[ 9849.542438] R13: ffff8881935de738 R14: ffff88816110d010 R15: 0000000000001000
[ 9849.543996] FS: 00007e36cbe94740(0000) GS:ffff88824b899000(0000) knlGS:0000000000000000
[ 9849.545854] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 9849.547092] CR2: 00007e36cb3ff000 CR3: 000000011bbf6006 CR4: 0000000000772ef0
[ 9849.548679] PKRU: 55555554
The race sequence:
1. Read completes -> netfs_read_collection() runs
2. netfs_wake_rreq_flag(rreq, NETFS_RREQ_IN_PROGRESS, ...)
3. netfs_wait_for_read() returns -EFAULT to netfs_write_begin()
4. The netfs_unlock_abandoned_read_pages() unlocks the folio
5. netfs_write_begin() calls folio_unlock(folio) -> VM_BUG_ON_FOLIO()
The key reason of the issue that netfs_unlock_abandoned_read_pages()
doesn't check the flag NETFS_RREQ_NO_UNLOCK_FOLIO and executes
folio_unlock() unconditionally. This patch implements in
netfs_unlock_abandoned_read_pages() logic similar to
netfs_unlock_read_folio().
Fixes: ee4cdf7ba857 ("netfs: Speed up buffered reading")
Signed-off-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
cc: netfs@lists.linux.dev
cc: linux-fsdevel@vger.kernel.org
cc: Ceph Development <ceph-devel@vger.kernel.org>
---
fs/netfs/read_retry.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/fs/netfs/read_retry.c b/fs/netfs/read_retry.c
index cca9ac43c077..68fc869513ef 100644
--- a/fs/netfs/read_retry.c
+++ b/fs/netfs/read_retry.c
@@ -288,8 +288,15 @@ void netfs_unlock_abandoned_read_pages(struct netfs_io_request *rreq)
struct folio *folio = folioq_folio(p, slot);
if (folio && !folioq_is_marked2(p, slot)) {
- trace_netfs_folio(folio, netfs_folio_trace_abandon);
- folio_unlock(folio);
+ if (folio->index == rreq->no_unlock_folio &&
+ test_bit(NETFS_RREQ_NO_UNLOCK_FOLIO,
+ &rreq->flags)) {
+ _debug("no unlock");
+ } else {
+ trace_netfs_folio(folio,
+ netfs_folio_trace_abandon);
+ folio_unlock(folio);
+ }
}
}
}
^ permalink raw reply related
* [PATCH v2 0/7] netfs: Miscellaneous fixes
From: David Howells @ 2026-04-14 8:19 UTC (permalink / raw)
To: Christian Brauner
Cc: David Howells, Paulo Alcantara, netfs, linux-afs, linux-cifs,
ceph-devel, linux-fsdevel, linux-kernel
Hi Christian,
Here are some miscellaneous fixes for netfslib:
(1) Fix triggering of a VM_BUG_ON_FOLIO() in netfs_write_begin().
(2) Fix error handling in netfs_extract_user_iter().
(3) Fix netfs_invalidate_folio() to clear the folio dirty bit if all dirty
data removed.
(4) Fix the handling of a partially failed copy (ie. EFAULT) into a
streaming write folio. Also remove the netfs_folio if a streaming
write folio is entirely overwritten.
(5) Fix netfs_read_gaps() to remove the netfs_folio from a filled folio.
(6) Fix the calculation of zero_point in netfs_release_folio() to limit it
to ->remote_i_size, not ->i_size.
(7) Fix netfs_perform_read() to not disable streaming writes when writing
to an fd that's open O_RDWR.
The patches can also be found here:
https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=netfs-fixes
Thanks,
David
Changes
=======
ver #2)
- Rebase on v7.0
- Drop the cachefiles dentry count fix as that's now upstream.
- Add four new patches from testing the generic/522 xfstest.
David Howells (5):
netfs: Fix netfs_invalidate_folio() to clear dirty bit if all changes
gone
netfs: Fix streaming write being overwritten
netfs: Fix read-gaps to remove netfs_folio from filled folio
netfs: Fix zeropoint update where i_size > remote_i_size
netfs: Fix write streaming disablement if fd open O_RDWR
Paulo Alcantara (1):
netfs: fix error handling in netfs_extract_user_iter()
Viacheslav Dubeyko (1):
netfs: fix VM_BUG_ON_FOLIO() issue in netfs_write_begin() call
fs/netfs/buffered_read.c | 9 ++++---
fs/netfs/buffered_write.c | 51 +++++++++++++++++++++++-------------
fs/netfs/iterator.c | 13 ++++++---
fs/netfs/misc.c | 8 ++++--
fs/netfs/read_retry.c | 11 ++++++--
include/trace/events/netfs.h | 5 ++++
6 files changed, 69 insertions(+), 28 deletions(-)
^ permalink raw reply
* Re: [PATCH 00/26] netfs: Keep track of folios in a segmented bio_vec[] chain
From: David Howells @ 2026-04-14 8:02 UTC (permalink / raw)
To: Stefan Metzmacher
Cc: dhowells, Christian Brauner, Matthew Wilcox, Christoph Hellwig,
Steve French, Namjae Jeon, Paulo Alcantara, Jens Axboe,
Leon Romanovsky, Steve French, ChenXiaoSong, Marc Dionne,
Eric Van Hensbergen, Dominique Martinet, Ilya Dryomov,
Trond Myklebust, netfs, linux-afs, linux-cifs, linux-nfs,
ceph-devel, v9fs, linux-erofs, linux-fsdevel, linux-kernel
In-Reply-To: <9639cd12-5e9d-4eb4-9d9c-f5047ccad7b7@samba.org>
Stefan Metzmacher <metze@samba.org> wrote:
> Any idea on how to resolve the actual conflict?
I didn't manage to beat the bugs out in time (turned out several were
preexisting).
However, if you look here:
https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=cifs-next
I've stacked my patchset on top of yours and modified smbdirect_connection.c
appropriately.
David
^ permalink raw reply
* Re: [GIT PULL] smb3 client fixes
From: Paulo Alcantara @ 2026-04-14 4:39 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Steve French, LKML, CIFS
In-Reply-To: <CAHk-=wjTfaTPf+Vp=XGmW1z9YPZcx8dhPjaGfhfYett9=-jHMA@mail.gmail.com>
Thanks for the review, Linus.
What about below changes now?
diff --git a/fs/dcache.c b/fs/dcache.c
index df11bbba0342..dbcbd0affb26 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -3196,15 +3196,18 @@ void d_mark_tmpfile(struct file *file, struct inode *inode)
}
EXPORT_SYMBOL(d_mark_tmpfile);
-void d_mark_tmpfile_name(struct file *file, const struct qstr *name)
+int d_mark_tmpfile_name(struct file *file, const struct qstr *name)
{
struct dentry *dentry = file->f_path.dentry;
char *dname = dentry->d_shortname.string;
- BUG_ON(dname_external(dentry));
- BUG_ON(d_really_is_positive(dentry));
- BUG_ON(!d_unlinked(dentry));
- BUG_ON(name->len > DNAME_INLINE_LEN - 1);
+ if (unlikely(dname_external(dentry) ||
+ d_really_is_positive(dentry) ||
+ !d_unlinked(dentry)))
+ return -EINVAL;
+ if (unlikely(name->len > DNAME_INLINE_LEN - 1))
+ return -ENAMETOOLONG;
+
spin_lock(&dentry->d_parent->d_lock);
spin_lock_nested(&dentry->d_lock, DENTRY_D_LOCK_NESTED);
dentry->__d_name.len = name->len;
@@ -3212,6 +3215,7 @@ void d_mark_tmpfile_name(struct file *file, const struct qstr *name)
dname[name->len] = '\0';
spin_unlock(&dentry->d_lock);
spin_unlock(&dentry->d_parent->d_lock);
+ return 0;
}
EXPORT_SYMBOL(d_mark_tmpfile_name);
diff --git a/fs/smb/client/cifsfs.h b/fs/smb/client/cifsfs.h
index 18f9f93a01b4..7370b38da938 100644
--- a/fs/smb/client/cifsfs.h
+++ b/fs/smb/client/cifsfs.h
@@ -10,6 +10,7 @@
#define _CIFSFS_H
#include <linux/hash.h>
+#include <linux/dcache.h>
#define ROOT_I 2
@@ -149,17 +150,11 @@ struct dentry *cifs_smb3_do_mount(struct file_system_type *fs_type, int flags,
char *cifs_silly_fullpath(struct dentry *dentry);
-#define CIFS_TMPNAME_PREFIX ".__smbfile_tmp"
-#define CIFS_TMPNAME_PREFIX_LEN ((int)sizeof(CIFS_TMPNAME_PREFIX) - 1)
-#define CIFS_TMPNAME_COUNTER_LEN ((int)sizeof(cifs_tmpcounter) * 2)
-#define CIFS_TMPNAME_LEN \
- (CIFS_TMPNAME_PREFIX_LEN + CIFS_TMPNAME_COUNTER_LEN)
+#define CIFS_TMPNAME_PREFIX ".__smbfile_tmp"
+#define CIFS_TMPNAME_LEN (DNAME_INLINE_LEN - 1)
-#define CIFS_SILLYNAME_PREFIX ".__smbfile_silly"
-#define CIFS_SILLYNAME_PREFIX_LEN ((int)sizeof(CIFS_SILLYNAME_PREFIX) - 1)
-#define CIFS_SILLYNAME_COUNTER_LEN ((int)sizeof(cifs_sillycounter) * 2)
-#define CIFS_SILLYNAME_LEN \
- (CIFS_SILLYNAME_PREFIX_LEN + CIFS_SILLYNAME_COUNTER_LEN)
+#define CIFS_SILLYNAME_PREFIX ".__smbfile_silly"
+#define CIFS_SILLYNAME_LEN (DNAME_INLINE_LEN - 1)
#ifdef CONFIG_CIFS_NFSD_EXPORT
extern const struct export_operations cifs_export_ops;
diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
index 6ea1ae7f7a46..07b9d2369787 100644
--- a/fs/smb/client/dir.c
+++ b/fs/smb/client/dir.c
@@ -1056,9 +1056,9 @@ int cifs_tmpfile(struct mnt_idmap *idmap, struct inode *dir,
{
struct dentry *dentry = file->f_path.dentry;
struct cifs_sb_info *cifs_sb = CIFS_SB(dir);
+ size_t namesize = CIFS_TMPNAME_LEN + 1;
char *path __free(kfree) = NULL, *name;
unsigned int oflags = file->f_flags;
- size_t size = CIFS_TMPNAME_LEN + 1;
int retries = 0, max_retries = 16;
struct TCP_Server_Info *server;
struct cifs_pending_open open;
@@ -1070,6 +1070,7 @@ int cifs_tmpfile(struct mnt_idmap *idmap, struct inode *dir,
struct inode *inode;
unsigned int xid;
__u32 oplock;
+ int namelen;
int rc;
if (unlikely(cifs_forced_shutdown(cifs_sb)))
@@ -1093,7 +1094,7 @@ int cifs_tmpfile(struct mnt_idmap *idmap, struct inode *dir,
server->ops->new_lease_key(&fid);
cifs_add_pending_open(&fid, tlink, &open);
- path = alloc_parent_path(dentry, size - 1);
+ path = alloc_parent_path(dentry, namesize - 1);
if (IS_ERR(path)) {
cifs_del_pending_open(&open);
rc = PTR_ERR(path);
@@ -1103,16 +1104,24 @@ int cifs_tmpfile(struct mnt_idmap *idmap, struct inode *dir,
name = path + strlen(path);
do {
- scnprintf(name, size,
- CIFS_TMPNAME_PREFIX "%0*x",
- CIFS_TMPNAME_COUNTER_LEN,
- atomic_inc_return(&cifs_tmpcounter));
+ /* Append tmpfile name to @path */
+ namelen = scnprintf(name, namesize,
+ CIFS_TMPNAME_PREFIX "%0*x",
+ (int)sizeof(cifs_tmpcounter) * 2,
+ atomic_inc_return(&cifs_tmpcounter));
rc = __cifs_do_create(dir, dentry, path, xid, tlink, oflags,
mode, &oplock, &fid, NULL, &inode);
if (!rc) {
+ rc = d_mark_tmpfile_name(file, &QSTR_LEN(name, namelen));
+ if (rc) {
+ cifs_dbg(VFS | ONCE, "%s: failed to set filename in dentry: %d\n",
+ __func__, rc);
+ rc = -EISDIR;
+ iput(inode);
+ goto err_open;
+ }
set_nlink(inode, 0);
mark_inode_dirty(inode);
- d_mark_tmpfile_name(file, &QSTR_LEN(name, size - 1));
d_instantiate(dentry, inode);
break;
}
@@ -1170,7 +1179,7 @@ char *cifs_silly_fullpath(struct dentry *dentry)
dput(sdentry);
scnprintf(name, namesize,
CIFS_SILLYNAME_PREFIX "%0*x",
- CIFS_SILLYNAME_COUNTER_LEN,
+ (int)sizeof(cifs_sillycounter) * 2,
atomic_inc_return(&cifs_sillycounter));
sdentry = lookup_noperm(&QSTR(name), dentry->d_parent);
if (IS_ERR(sdentry))
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index f60819dcfebd..c5bd5a74baba 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -264,7 +264,7 @@ extern void d_invalidate(struct dentry *);
extern struct dentry * d_make_root(struct inode *);
extern void d_mark_tmpfile(struct file *, struct inode *);
-void d_mark_tmpfile_name(struct file *file, const struct qstr *name);
+int d_mark_tmpfile_name(struct file *file, const struct qstr *name);
extern void d_tmpfile(struct file *, struct inode *);
extern struct dentry *d_find_alias(struct inode *);
^ permalink raw reply related
* Re: [GIT PULL] smb3 client fixes
From: Linus Torvalds @ 2026-04-14 3:26 UTC (permalink / raw)
To: Paulo Alcantara; +Cc: Steve French, LKML, CIFS
In-Reply-To: <ab9457074b5852e73e84f0209bb58279@manguebit.org>
On Mon, 13 Apr 2026 at 20:13, Paulo Alcantara <pc@manguebit.org> wrote:
>
> Do the below changes look any better?
Let me suggest:
> -void d_mark_tmpfile_name(struct file *file, const struct qstr *name)
> +int d_mark_tmpfile_name(struct file *file, const struct qstr *name)
> {
> struct dentry *dentry = file->f_path.dentry;
> char *dname = dentry->d_shortname.string;
>
> - BUG_ON(dname_external(dentry));
> - BUG_ON(d_really_is_positive(dentry));
> - BUG_ON(!d_unlinked(dentry));
> - BUG_ON(name->len > DNAME_INLINE_LEN - 1);
> + if (WARN_ON_ONCE(dname_external(dentry)))
> + return -EINVAL;
> + if (WARN_ON_ONCE(d_really_is_positive(dentry)))
> + return -EINVAL;
> + if (WARN_ON_ONCE(!d_unlinked(dentry)))
> + return -EINVAL;
> + if (WARN_ON_ONCE(name->len > DNAME_INLINE_LEN - 1))
> + return -ENAMETOOLONG;
Let's just return errors from this, and put the WARN_ON() in the caller.
It's not like the WARN_ON() is ever going to trigger anyway, unless
there's somethingwrong in the caller, so let's not make this helper
infrastructure any uglier than it needs to be. Adding the warnings
there only makes it less obvious.
You want the warnings to be where the *problem* is. Not in the helper
that did everything right, but was passed the wrong values.
See what I'm saying?
Also, this is a bit of a mixed bag:
> - scnprintf(name, size,
> - CIFS_TMPNAME_PREFIX "%0*x",
> - CIFS_TMPNAME_COUNTER_LEN,
> - atomic_inc_return(&cifs_tmpcounter));
> + /* Append tmpfile name to @path */
> + namelen = scnprintf(name, namesize, ".__smbfile_tmp%0*x",
> + (int)sizeof(cifs_tmpcounter) * 2,
> + atomic_inc_return(&cifs_tmpcounter));
the 'namelen = ' part is better because it makes it much more obvious
where the numbers come from,. but I certainly didn't mind having a
CIFS_TMPNAME_PREFIX define to help abstract things out a bit.
What I minded was seeing *three* different #defines where one was a
magic nmumber that was very tighly tied to the others, but they didn't
help make that obvious.
So it's not that I minded the CIFS_TMPNAME_PREFIX thing,
I minded ther *magic* of it all that made it so obvious that somebody
knew a-priori what they were going to get, and then made the code
match the result they wanted, but without making the code show how
they got there.
That whole "namelen = scnprintf()" is a really cheap way to show "this
is how I got the name length" - because that's what the function
returns anyway.
And the other place you had that scnprintf() you still had that "I
know what the end result is, so I'm just hardcoding it as a random
value".
Linus
^ permalink raw reply
* Re: [GIT PULL] smb3 client fixes
From: Paulo Alcantara @ 2026-04-14 3:13 UTC (permalink / raw)
To: Linus Torvalds, Steve French; +Cc: LKML, CIFS
In-Reply-To: <CAHk-=wgerpUKCDhdzKH0FEdLyfhj3doc9t+kO9Yb6rSsTp7hdQ@mail.gmail.com>
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Mon, 13 Apr 2026 at 15:06, Steve French <smfrench@gmail.com> wrote:
>>
>> git://git.samba.org/sfrench/cifs-2.6.git tags/v7.1-rc1-part1-smb3-client-fixes
>
> I've pulled this, but then looking at the dcache changes I noted the
> big forest of BUG_ON() which really isn't valid. Error handling is a
> thing - BUG_ON() is *not* error handling.
>
> And then looking at verifying the length of the name - one of the
> things checked for in that forest of BUG_ON() calls - the call site is
> an unreadable mess.
>
> You have this:
>
> size_t size = CIFS_TMPNAME_LEN + 1;
>
> fifty lines earlier, and then you do
>
> d_mark_tmpfile_name(file, &QSTR_LEN(name, size - 1));
>
> which is not just illegible, it's also illogical. That "size" is just
> voodoo. The string is generated by
>
> scnprintf(name, size,
> CIFS_TMPNAME_PREFIX "%0*x",
> CIFS_TMPNAME_COUNTER_LEN,
> atomic_inc_return(&cifs_tmpcounter));
>
> which uses several other magic #define's, and yes, I'm sure it all
> adds up to CIFS_TMPNAME_LEN in the end, but this is basically all just
> line noise.
>
> PLEASE write this legibly instead, and make that new dentry helper
> actually do error handling, not BUG_ON().
>
> Because this kind of mess is simply not acceptable.
>
> I don't even understand why you use a variable for an insane constant.
> The code *could* have done something like this:
>
> namelen = scnprintf(name, size,
> CIFS_TMPNAME_PREFIX "%0*x",
> CIFS_TMPNAME_COUNTER_LEN,
> atomic_inc_return(&cifs_tmpcounter));
>
> and it would all have actually made sense. But stating the final size
> like that really doesn't - not without at least a big comment on how
> those random things are interrelated.
>
> So this is in my tree now, but I expect it to be cleaned up and made sensible.
ACK.
Do the below changes look any better? I wasn't sure what exact error
values to return from d_mark_tmpfile_name(), hopefully it's fine.
diff --git a/fs/dcache.c b/fs/dcache.c
index df11bbba0342..151f83f0a0e5 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -3196,15 +3196,20 @@ void d_mark_tmpfile(struct file *file, struct inode *inode)
}
EXPORT_SYMBOL(d_mark_tmpfile);
-void d_mark_tmpfile_name(struct file *file, const struct qstr *name)
+int d_mark_tmpfile_name(struct file *file, const struct qstr *name)
{
struct dentry *dentry = file->f_path.dentry;
char *dname = dentry->d_shortname.string;
- BUG_ON(dname_external(dentry));
- BUG_ON(d_really_is_positive(dentry));
- BUG_ON(!d_unlinked(dentry));
- BUG_ON(name->len > DNAME_INLINE_LEN - 1);
+ if (WARN_ON_ONCE(dname_external(dentry)))
+ return -EINVAL;
+ if (WARN_ON_ONCE(d_really_is_positive(dentry)))
+ return -EINVAL;
+ if (WARN_ON_ONCE(!d_unlinked(dentry)))
+ return -EINVAL;
+ if (WARN_ON_ONCE(name->len > DNAME_INLINE_LEN - 1))
+ return -ENAMETOOLONG;
+
spin_lock(&dentry->d_parent->d_lock);
spin_lock_nested(&dentry->d_lock, DENTRY_D_LOCK_NESTED);
dentry->__d_name.len = name->len;
@@ -3212,6 +3217,7 @@ void d_mark_tmpfile_name(struct file *file, const struct qstr *name)
dname[name->len] = '\0';
spin_unlock(&dentry->d_lock);
spin_unlock(&dentry->d_parent->d_lock);
+ return 0;
}
EXPORT_SYMBOL(d_mark_tmpfile_name);
diff --git a/fs/smb/client/cifsfs.h b/fs/smb/client/cifsfs.h
index 18f9f93a01b4..804b57595ab8 100644
--- a/fs/smb/client/cifsfs.h
+++ b/fs/smb/client/cifsfs.h
@@ -10,6 +10,7 @@
#define _CIFSFS_H
#include <linux/hash.h>
+#include <linux/dcache.h>
#define ROOT_I 2
@@ -149,17 +150,8 @@ struct dentry *cifs_smb3_do_mount(struct file_system_type *fs_type, int flags,
char *cifs_silly_fullpath(struct dentry *dentry);
-#define CIFS_TMPNAME_PREFIX ".__smbfile_tmp"
-#define CIFS_TMPNAME_PREFIX_LEN ((int)sizeof(CIFS_TMPNAME_PREFIX) - 1)
-#define CIFS_TMPNAME_COUNTER_LEN ((int)sizeof(cifs_tmpcounter) * 2)
-#define CIFS_TMPNAME_LEN \
- (CIFS_TMPNAME_PREFIX_LEN + CIFS_TMPNAME_COUNTER_LEN)
-
-#define CIFS_SILLYNAME_PREFIX ".__smbfile_silly"
-#define CIFS_SILLYNAME_PREFIX_LEN ((int)sizeof(CIFS_SILLYNAME_PREFIX) - 1)
-#define CIFS_SILLYNAME_COUNTER_LEN ((int)sizeof(cifs_sillycounter) * 2)
-#define CIFS_SILLYNAME_LEN \
- (CIFS_SILLYNAME_PREFIX_LEN + CIFS_SILLYNAME_COUNTER_LEN)
+#define CIFS_TMPNAME_LEN (DNAME_INLINE_LEN - 1)
+#define CIFS_SILLYNAME_LEN (DNAME_INLINE_LEN - 1)
#ifdef CONFIG_CIFS_NFSD_EXPORT
extern const struct export_operations cifs_export_ops;
diff --git a/fs/smb/client/dir.c b/fs/smb/client/dir.c
index 6ea1ae7f7a46..2abe76a7cec0 100644
--- a/fs/smb/client/dir.c
+++ b/fs/smb/client/dir.c
@@ -1056,9 +1056,9 @@ int cifs_tmpfile(struct mnt_idmap *idmap, struct inode *dir,
{
struct dentry *dentry = file->f_path.dentry;
struct cifs_sb_info *cifs_sb = CIFS_SB(dir);
+ size_t namesize = CIFS_TMPNAME_LEN + 1;
char *path __free(kfree) = NULL, *name;
unsigned int oflags = file->f_flags;
- size_t size = CIFS_TMPNAME_LEN + 1;
int retries = 0, max_retries = 16;
struct TCP_Server_Info *server;
struct cifs_pending_open open;
@@ -1070,6 +1070,7 @@ int cifs_tmpfile(struct mnt_idmap *idmap, struct inode *dir,
struct inode *inode;
unsigned int xid;
__u32 oplock;
+ int namelen;
int rc;
if (unlikely(cifs_forced_shutdown(cifs_sb)))
@@ -1093,7 +1094,7 @@ int cifs_tmpfile(struct mnt_idmap *idmap, struct inode *dir,
server->ops->new_lease_key(&fid);
cifs_add_pending_open(&fid, tlink, &open);
- path = alloc_parent_path(dentry, size - 1);
+ path = alloc_parent_path(dentry, namesize - 1);
if (IS_ERR(path)) {
cifs_del_pending_open(&open);
rc = PTR_ERR(path);
@@ -1103,16 +1104,21 @@ int cifs_tmpfile(struct mnt_idmap *idmap, struct inode *dir,
name = path + strlen(path);
do {
- scnprintf(name, size,
- CIFS_TMPNAME_PREFIX "%0*x",
- CIFS_TMPNAME_COUNTER_LEN,
- atomic_inc_return(&cifs_tmpcounter));
+ /* Append tmpfile name to @path */
+ namelen = scnprintf(name, namesize, ".__smbfile_tmp%0*x",
+ (int)sizeof(cifs_tmpcounter) * 2,
+ atomic_inc_return(&cifs_tmpcounter));
rc = __cifs_do_create(dir, dentry, path, xid, tlink, oflags,
mode, &oplock, &fid, NULL, &inode);
if (!rc) {
+ rc = d_mark_tmpfile_name(file, &QSTR_LEN(name, namelen));
+ if (rc) {
+ rc = -EISDIR;
+ iput(inode);
+ goto err_open;
+ }
set_nlink(inode, 0);
mark_inode_dirty(inode);
- d_mark_tmpfile_name(file, &QSTR_LEN(name, size - 1));
d_instantiate(dentry, inode);
break;
}
@@ -1168,9 +1174,8 @@ char *cifs_silly_fullpath(struct dentry *dentry)
do {
dput(sdentry);
- scnprintf(name, namesize,
- CIFS_SILLYNAME_PREFIX "%0*x",
- CIFS_SILLYNAME_COUNTER_LEN,
+ scnprintf(name, namesize, ".__smbfile_silly%0*x",
+ (int)sizeof(cifs_sillycounter) * 2,
atomic_inc_return(&cifs_sillycounter));
sdentry = lookup_noperm(&QSTR(name), dentry->d_parent);
if (IS_ERR(sdentry))
diff --git a/include/linux/dcache.h b/include/linux/dcache.h
index f60819dcfebd..c5bd5a74baba 100644
--- a/include/linux/dcache.h
+++ b/include/linux/dcache.h
@@ -264,7 +264,7 @@ extern void d_invalidate(struct dentry *);
extern struct dentry * d_make_root(struct inode *);
extern void d_mark_tmpfile(struct file *, struct inode *);
-void d_mark_tmpfile_name(struct file *file, const struct qstr *name);
+int d_mark_tmpfile_name(struct file *file, const struct qstr *name);
extern void d_tmpfile(struct file *, struct inode *);
extern struct dentry *d_find_alias(struct inode *);
^ permalink raw reply related
* Re: [GIT PULL] smb3 client fixes
From: Steve French @ 2026-04-14 1:22 UTC (permalink / raw)
To: Linus Torvalds, Paulo Alcantara; +Cc: LKML, CIFS
In-Reply-To: <CAHk-=wgerpUKCDhdzKH0FEdLyfhj3doc9t+kO9Yb6rSsTp7hdQ@mail.gmail.com>
On Mon, Apr 13, 2026 at 7:28 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Mon, 13 Apr 2026 at 15:06, Steve French <smfrench@gmail.com> wrote:
> >
> > git://git.samba.org/sfrench/cifs-2.6.git tags/v7.1-rc1-part1-smb3-client-fixes
>
> I've pulled this, but then looking at the dcache changes I noted the
> big forest of BUG_ON() which really isn't valid. Error handling is a
> thing - BUG_ON() is *not* error handling.
>
> And then looking at verifying the length of the name - one of the
> things checked for in that forest of BUG_ON() calls - the call site is
> an unreadable mess.
>
> You have this:
>
> size_t size = CIFS_TMPNAME_LEN + 1;
>
> fifty lines earlier, and then you do
>
> d_mark_tmpfile_name(file, &QSTR_LEN(name, size - 1));
>
> which is not just illegible, it's also illogical. That "size" is just
> voodoo. The string is generated by
>
> scnprintf(name, size,
> CIFS_TMPNAME_PREFIX "%0*x",
> CIFS_TMPNAME_COUNTER_LEN,
> atomic_inc_return(&cifs_tmpcounter));
>
> which uses several other magic #define's, and yes, I'm sure it all
> adds up to CIFS_TMPNAME_LEN in the end, but this is basically all just
> line noise.
>
> PLEASE write this legibly instead, and make that new dentry helper
> actually do error handling, not BUG_ON().
>
> Because this kind of mess is simply not acceptable.
>
> I don't even understand why you use a variable for an insane constant.
> The code *could* have done something like this:
>
> namelen = scnprintf(name, size,
> CIFS_TMPNAME_PREFIX "%0*x",
> CIFS_TMPNAME_COUNTER_LEN,
> atomic_inc_return(&cifs_tmpcounter));
>
> and it would all have actually made sense. But stating the final size
> like that really doesn't - not without at least a big comment on how
> those random things are interrelated.
>
> So this is in my tree now, but I expect it to be cleaned up and made sensible.
Will do or will have Paulo do.
--
Thanks,
Steve
^ permalink raw reply
* Re: [GIT PULL] smb3 client fixes
From: pr-tracker-bot @ 2026-04-14 0:29 UTC (permalink / raw)
To: Steve French; +Cc: Linus Torvalds, LKML, CIFS
In-Reply-To: <CAH2r5muW_D8eDN0iN7kzFV0km+QBVZn087+AGHTeSHm0E-KqYw@mail.gmail.com>
The pull request you sent on Mon, 13 Apr 2026 17:06:16 -0500:
> git://git.samba.org/sfrench/cifs-2.6.git tags/v7.1-rc1-part1-smb3-client-fixes
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/81dc1e4d32b064ac47abc60b0acbf49b66a34d52
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [GIT PULL] smb3 client fixes
From: Linus Torvalds @ 2026-04-14 0:28 UTC (permalink / raw)
To: Steve French; +Cc: LKML, CIFS
In-Reply-To: <CAH2r5muW_D8eDN0iN7kzFV0km+QBVZn087+AGHTeSHm0E-KqYw@mail.gmail.com>
On Mon, 13 Apr 2026 at 15:06, Steve French <smfrench@gmail.com> wrote:
>
> git://git.samba.org/sfrench/cifs-2.6.git tags/v7.1-rc1-part1-smb3-client-fixes
I've pulled this, but then looking at the dcache changes I noted the
big forest of BUG_ON() which really isn't valid. Error handling is a
thing - BUG_ON() is *not* error handling.
And then looking at verifying the length of the name - one of the
things checked for in that forest of BUG_ON() calls - the call site is
an unreadable mess.
You have this:
size_t size = CIFS_TMPNAME_LEN + 1;
fifty lines earlier, and then you do
d_mark_tmpfile_name(file, &QSTR_LEN(name, size - 1));
which is not just illegible, it's also illogical. That "size" is just
voodoo. The string is generated by
scnprintf(name, size,
CIFS_TMPNAME_PREFIX "%0*x",
CIFS_TMPNAME_COUNTER_LEN,
atomic_inc_return(&cifs_tmpcounter));
which uses several other magic #define's, and yes, I'm sure it all
adds up to CIFS_TMPNAME_LEN in the end, but this is basically all just
line noise.
PLEASE write this legibly instead, and make that new dentry helper
actually do error handling, not BUG_ON().
Because this kind of mess is simply not acceptable.
I don't even understand why you use a variable for an insane constant.
The code *could* have done something like this:
namelen = scnprintf(name, size,
CIFS_TMPNAME_PREFIX "%0*x",
CIFS_TMPNAME_COUNTER_LEN,
atomic_inc_return(&cifs_tmpcounter));
and it would all have actually made sense. But stating the final size
like that really doesn't - not without at least a big comment on how
those random things are interrelated.
So this is in my tree now, but I expect it to be cleaned up and made sensible.
Linus
^ permalink raw reply
* [GIT PULL] smb3 client fixes
From: Steve French @ 2026-04-13 22:06 UTC (permalink / raw)
To: Linus Torvalds; +Cc: LKML, CIFS
Please pull the following changes since commit
591cd656a1bf5ea94a222af5ef2ee76df029c1d2:
Linux 7.0-rc7 (2026-04-05 15:26:23 -0700)
are available in the Git repository at:
git://git.samba.org/sfrench/cifs-2.6.git tags/v7.1-rc1-part1-smb3-client-fixes
for you to fetch changes up to 4248ed1013816f97f4029d06b16c67a6e43d0668:
smb: client: allow both 'lease' and 'nolease' mount options
(2026-04-13 09:14:54 -0500)
----------------------------------------------------------------
Twenty six smb3 client fixes
- Fix EAs bounds check
- Fix OOB read in symlink response parsing
- Add support for creating tmpfiles
- Minor debug improvement for mount failure
- Minor crypto cleanup
- Add missing module description
- mount fix for lease vs. nolease
- Add Metze as maintainer for smbdirect
- Minor error mapping header cleanup
- Improve search speed of SMB1 maperror
- Fix potential null ptr ref in smb2 map error tests
----------------------------------------------------------------
Eric Biggers (1):
smb: client: Remove unnecessary selection of CRYPTO_ECB
Fredric Cover (1):
fs/smb/client: add verbose error logging for UNC parsing
Greg Kroah-Hartman (2):
smb: client: fix off-by-8 bounds check in check_wsl_eas()
smb: client: fix OOB reads parsing symlink error response
Huiwen He (9):
smb/client: annotate nterr.h with DOS error codes
smb/client: autogenerate SMB1 NT status to DOS error mapping
smb/client: replace nt_errs with ntstatus_to_dos_map
smb/client: refactor ntstatus_to_dos() to return mapping entry
smb/client: use binary search for NT status to DOS mapping
smb/client: move ERRnetlogonNotStarted to DOS error class
smb/client: annotate smberr.h with POSIX error codes
smb/client: autogenerate SMB1 DOS/SRV to POSIX error mapping
smb/client: use binary search for SMB1 DOS/SRV error mapping
Paulo Alcantara (4):
vfs: introduce d_mark_tmpfile_name()
smb: client: add support for O_TMPFILE
smb: client: set ATTR_TEMPORARY with O_TMPFILE | O_EXCL
smb: client: get rid of d_drop()+d_add()
Rajasi Mandal (1):
smb: client: allow both 'lease' and 'nolease' mount options
Steve French (1):
MAINTAINERS: create entry for smbdirect
SunJianHao (1):
smb/client: avoid null-ptr-deref when tests fail in test_cmp_map()
Venkat Rao Bagalkote (1):
smb: client: add missing MODULE_DESCRIPTION() to smb1maperror_test
Youling Tang (4):
smb/client: check if ntstatus_to_dos_map is sorted
smb/client: introduce KUnit test to check ntstatus_to_dos_map search
smb/client: check if SMB1 DOS/SRV error mapping arrays are sorted
smb/client: introduce KUnit tests to check DOS/SRV err mapping search
ZhangGuoDong (1):
smb/client: move smb2maperror declarations to smb2proto.h
MAINTAINERS | 14 +
fs/dcache.c | 19 +
fs/smb/client/.gitignore | 3 +
fs/smb/client/Kconfig | 12 +-
fs/smb/client/Makefile | 25 +-
fs/smb/client/cifsfs.c | 11 +-
fs/smb/client/cifsfs.h | 23 +-
fs/smb/client/cifsglob.h | 23 +-
fs/smb/client/cifsproto.h | 3 +-
fs/smb/client/dir.c | 352 +++++++++---
fs/smb/client/file.c | 53 +-
fs/smb/client/fs_context.c | 18 +-
fs/smb/client/fs_context.h | 2 +-
fs/smb/client/gen_smb1_mapping | 124 +++++
fs/smb/client/inode.c | 3 +-
fs/smb/client/link.c | 1 +
fs/smb/client/nterr.c | 704 ------------------------
fs/smb/client/nterr.h | 1082 +++++++++++++++++++------------------
fs/smb/client/smb1maperror.c | 873 ++++++------------------------
fs/smb/client/smb1maperror_test.c | 77 +++
fs/smb/client/smb1proto.h | 15 +
fs/smb/client/smb2file.c | 20 +-
fs/smb/client/smb2inode.c | 402 ++++++--------
fs/smb/client/smb2maperror.c | 3 -
fs/smb/client/smb2maperror_test.c | 8 +-
fs/smb/client/smb2proto.h | 7 +-
fs/smb/client/smberr.h | 415 +++++++++-----
include/linux/dcache.h | 1 +
28 files changed, 1835 insertions(+), 2458 deletions(-)
create mode 100644 fs/smb/client/gen_smb1_mapping
delete mode 100644 fs/smb/client/nterr.c
create mode 100644 fs/smb/client/smb1maperror_test.c
--
Thanks,
Steve
^ permalink raw reply
* [PATCH 2/2] smb: client: pass correct from_reconnect to cifs_put_tcp_session()
From: Henrique Carvalho @ 2026-04-13 19:11 UTC (permalink / raw)
To: sfrench
Cc: pc, ronniesahlberg, sprasad, tom, bharathsm, ematsumiya,
linux-cifs, stable
In-Reply-To: <20260413191110.1508848-1-henrique.carvalho@suse.com>
cifs_decrease_secondary_channels() tore down removed channels with
from_reconnect=false, even when the shrink was triggered from reconnect
context, which could synchronously wait on reconnect work and break the
reconnect-side teardown path.
Pass down the from_reconnect argument to cifs_put_tcp_session() so
reconnect-driven channel removal uses the same non-blocking teardown
semantics as the rest of the reconnect path.
This is a minor fix. I believe this bug cannot be triggered in the
current state of cifs.
Fixes: ee1d21794e55 ("cifs: handle when server stops supporting multichannel")
Cc: stable@vger.kernel.org
Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com>
---
fs/smb/client/cifsproto.h | 1 +
fs/smb/client/sess.c | 4 ++--
fs/smb/client/smb2pdu.c | 2 +-
3 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
index 884bfa1cf0b4..b00b2e070ada 100644
--- a/fs/smb/client/cifsproto.h
+++ b/fs/smb/client/cifsproto.h
@@ -374,6 +374,7 @@ bool cifs_chan_needs_reconnect(struct cifs_ses *ses,
bool cifs_chan_is_iface_active(struct cifs_ses *ses,
struct TCP_Server_Info *server);
void cifs_decrease_secondary_channels(struct cifs_ses *ses,
+ bool from_reconnect,
bool disable_mchan);
void cifs_chan_update_iface(struct cifs_ses *ses,
struct TCP_Server_Info *server);
diff --git a/fs/smb/client/sess.c b/fs/smb/client/sess.c
index 698bd27119ae..47bb566c8731 100644
--- a/fs/smb/client/sess.c
+++ b/fs/smb/client/sess.c
@@ -273,7 +273,7 @@ int cifs_try_adding_channels(struct cifs_ses *ses)
* Otherwise, it disables all but the primary channel.
*/
void
-cifs_decrease_secondary_channels(struct cifs_ses *ses, bool disable_mchan)
+cifs_decrease_secondary_channels(struct cifs_ses *ses, bool from_reconnect, bool disable_mchan)
{
int i, chan_count;
struct TCP_Server_Info *server;
@@ -319,7 +319,7 @@ cifs_decrease_secondary_channels(struct cifs_ses *ses, bool disable_mchan)
server->terminate = true;
cifs_signal_cifsd_for_reconnect(server, false);
}
- cifs_put_tcp_session(server, false);
+ cifs_put_tcp_session(server, from_reconnect);
}
spin_lock(&ses->chan_lock);
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 2eb13b2665a4..cf7b74a2b9b0 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -206,7 +206,7 @@ cifs_chan_skip_or_disable(struct cifs_ses *ses,
return -EHOSTDOWN;
}
- cifs_decrease_secondary_channels(ses, disable_mchan);
+ cifs_decrease_secondary_channels(ses, from_reconnect, disable_mchan);
return 0;
}
--
2.53.0
^ permalink raw reply related
* [PATCH 1/2] smb: client: serialize channel scaling path
From: Henrique Carvalho @ 2026-04-13 19:11 UTC (permalink / raw)
To: sfrench
Cc: pc, ronniesahlberg, sprasad, tom, bharathsm, ematsumiya,
linux-cifs, stable
Channel scaling serialization was coded at several call sites with plain
flag values in ses->flags, which duplicated the same bookkeeping in
reconnect and remount paths, and was missing in the mount path.
Move the CIFS_SES_FLAG_SCALE_CHANNELS acquisition and release inside
smb3_update_ses_channels(), and convert the session flags to bit indices
and their operations to bitops.
Make smb3_update_ses_channels return -EBUSY if there is already an
ongoing channel scaling operation.
Fixes: 556bb341f9f2e ("smb: client: introduce multichannel async work during mount")
Cc: stable@vger.kernel.org
Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com>
---
fs/smb/client/cifsglob.h | 6 +++---
fs/smb/client/fs_context.c | 15 ---------------
fs/smb/client/smb2pdu.c | 24 ++++++++----------------
3 files changed, 11 insertions(+), 34 deletions(-)
diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index 709e96e07791..7b1323927711 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -1049,8 +1049,8 @@ struct cifs_chan {
__u8 signkey[SMB3_SIGN_KEY_SIZE];
};
-#define CIFS_SES_FLAG_SCALE_CHANNELS (0x1)
-#define CIFS_SES_FLAGS_PENDING_QUERY_INTERFACES (0x2)
+#define CIFS_SES_FLAG_SCALE_CHANNELS 0
+#define CIFS_SES_FLAGS_PENDING_QUERY_INTERFACES 1
/*
* Session structure. One of these for each uid session with a particular host
@@ -1089,7 +1089,7 @@ struct cifs_ses {
bool domainAuto:1;
bool expired_pwd; /* track if access denied or expired pwd so can know if need to update */
int unicode;
- unsigned int flags;
+ unsigned long flags;
__u16 session_flags;
__u8 smb3signingkey[SMB3_SIGN_KEY_SIZE];
__u8 smb3encryptionkey[SMB3_ENC_DEC_KEY_SIZE];
diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
index a46764c24710..e0e13c22e159 100644
--- a/fs/smb/client/fs_context.c
+++ b/fs/smb/client/fs_context.c
@@ -1166,27 +1166,12 @@ static int smb3_reconfigure(struct fs_context *fc)
/* Synchronize ses->chan_max with the new mount context */
smb3_sync_ses_chan_max(ses, ctx->max_channels);
- /* Now update the session's channels to match the new configuration */
- /* Prevent concurrent scaling operations */
- spin_lock(&ses->ses_lock);
- if (ses->flags & CIFS_SES_FLAG_SCALE_CHANNELS) {
- spin_unlock(&ses->ses_lock);
- mutex_unlock(&ses->session_mutex);
- return -EINVAL;
- }
- ses->flags |= CIFS_SES_FLAG_SCALE_CHANNELS;
- spin_unlock(&ses->ses_lock);
mutex_unlock(&ses->session_mutex);
rc = smb3_update_ses_channels(ses, ses->server,
false /* from_reconnect */,
false /* disable_mchan */);
-
- /* Clear scaling flag after operation */
- spin_lock(&ses->ses_lock);
- ses->flags &= ~CIFS_SES_FLAG_SCALE_CHANNELS;
- spin_unlock(&ses->ses_lock);
} else {
mutex_unlock(&ses->session_mutex);
}
diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
index 5188218c25be..2eb13b2665a4 100644
--- a/fs/smb/client/smb2pdu.c
+++ b/fs/smb/client/smb2pdu.c
@@ -228,6 +228,10 @@ int smb3_update_ses_channels(struct cifs_ses *ses, struct TCP_Server_Info *serve
bool from_reconnect, bool disable_mchan)
{
int rc = 0;
+
+ if (test_and_set_bit(CIFS_SES_FLAG_SCALE_CHANNELS, &ses->flags))
+ return -EBUSY;
+
/*
* Manage session channels based on current count vs max:
* - If disable requested, skip or disable the channel
@@ -243,6 +247,7 @@ int smb3_update_ses_channels(struct cifs_ses *ses, struct TCP_Server_Info *serve
rc = cifs_chan_skip_or_disable(ses, server, from_reconnect, disable_mchan);
}
+ clear_bit(CIFS_SES_FLAG_SCALE_CHANNELS, &ses->flags);
return rc;
}
@@ -432,15 +437,6 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
goto out;
}
- spin_lock(&ses->ses_lock);
- if (ses->flags & CIFS_SES_FLAG_SCALE_CHANNELS) {
- spin_unlock(&ses->ses_lock);
- mutex_unlock(&ses->session_mutex);
- goto skip_add_channels;
- }
- ses->flags |= CIFS_SES_FLAG_SCALE_CHANNELS;
- spin_unlock(&ses->ses_lock);
-
if (!rc &&
(server->capabilities & SMB2_GLOBAL_CAP_MULTI_CHANNEL) &&
server->ops->query_server_interfaces) {
@@ -450,11 +446,11 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
* is in progress. This will be used to avoid calling
* smb2_reconnect recursively.
*/
- ses->flags |= CIFS_SES_FLAGS_PENDING_QUERY_INTERFACES;
+ set_bit(CIFS_SES_FLAGS_PENDING_QUERY_INTERFACES, &ses->flags);
xid = get_xid();
rc = server->ops->query_server_interfaces(xid, tcon, false);
free_xid(xid);
- ses->flags &= ~CIFS_SES_FLAGS_PENDING_QUERY_INTERFACES;
+ clear_bit(CIFS_SES_FLAGS_PENDING_QUERY_INTERFACES, &ses->flags);
if (!tcon->ipc && !tcon->dummy)
queue_delayed_work(cifsiod_wq, &tcon->query_interfaces,
@@ -492,10 +488,6 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
}
skip_add_channels:
- spin_lock(&ses->ses_lock);
- ses->flags &= ~CIFS_SES_FLAG_SCALE_CHANNELS;
- spin_unlock(&ses->ses_lock);
-
if (smb2_command != SMB2_INTERNAL_CMD)
cifs_queue_server_reconn(server);
@@ -609,7 +601,7 @@ static int smb2_ioctl_req_init(u32 opcode, struct cifs_tcon *tcon,
*/
if (opcode == FSCTL_VALIDATE_NEGOTIATE_INFO ||
(opcode == FSCTL_QUERY_NETWORK_INTERFACE_INFO &&
- (tcon->ses->flags & CIFS_SES_FLAGS_PENDING_QUERY_INTERFACES)))
+ test_bit(CIFS_SES_FLAGS_PENDING_QUERY_INTERFACES, &tcon->ses->flags)))
return __smb2_plain_req_init(SMB2_IOCTL, tcon, server,
request_buf, total_len);
--
2.53.0
^ permalink raw reply related
* [PATCH 8/8] smb: common: add SMB3_COMPRESS_MAX_ALGS
From: Enzo Matsumiya @ 2026-04-13 19:07 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
In-Reply-To: <20260413190713.283939-1-ematsumiya@suse.de>
Set it to number of currently defined algorithms (6 as of now).
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/common/smb2pdu.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/smb/common/smb2pdu.h b/fs/smb/common/smb2pdu.h
index e482c86ceb00..cda71964ed01 100644
--- a/fs/smb/common/smb2pdu.h
+++ b/fs/smb/common/smb2pdu.h
@@ -510,6 +510,8 @@ struct smb2_encryption_neg_context {
/* Pattern scanning algorithm See MS-SMB2 3.1.4.4.1 */
#define SMB3_COMPRESS_PATTERN cpu_to_le16(0x0004) /* Pattern_V1 */
#define SMB3_COMPRESS_LZ4 cpu_to_le16(0x0005)
+/* Account for NONE for easier array indexing */
+#define SMB3_COMPRESS_MAX_ALGS 6
/* Compression Flags */
#define SMB2_COMPRESSION_CAPABILITIES_FLAG_NONE cpu_to_le32(0x00000000)
--
2.53.0
^ permalink raw reply related
* [PATCH 7/8] smb: client: compress: add compress/common.h
From: Enzo Matsumiya @ 2026-04-13 19:07 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
In-Reply-To: <20260413190713.283939-1-ematsumiya@suse.de>
Add compress/common.h to aggregate helpers and definitions that will be
shared with other compression algorithms.
Also add a few build time checks for proper support.
Changes:
- update affected call paths in compress/lz77.c
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/compress/common.h | 152 ++++++++++++++++++++++++++++++++
fs/smb/client/compress/lz77.c | 122 +++++--------------------
2 files changed, 175 insertions(+), 99 deletions(-)
create mode 100644 fs/smb/client/compress/common.h
diff --git a/fs/smb/client/compress/common.h b/fs/smb/client/compress/common.h
new file mode 100644
index 000000000000..b5ccf5debd22
--- /dev/null
+++ b/fs/smb/client/compress/common.h
@@ -0,0 +1,152 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2026, SUSE LLC
+ *
+ * Authors: Enzo Matsumiya <ematsumiya@suse.de>
+ *
+ * Common helpers and definitions for compression/decompression.
+ *
+ * Naming convention:
+ * - smb_compress_*: MS-SMB2 related API
+ * - compress_*: Compression internals
+ * - <alg>_*: Compression algorithm specifics
+ *
+ * Upper case for macros and constants, lower case for everything else.
+ */
+#ifndef _COMPRESS_COMMON_H
+#define _COMPRESS_COMMON_H
+
+#include <linux/kernel.h>
+#include <linux/count_zeros.h>
+#include <linux/string.h>
+#include <linux/sizes.h>
+#include <linux/slab.h>
+
+/*
+ * Build time checks/asserts.
+ * These are assumptions/expectations for all algorithms implemented.
+ */
+#ifndef __LITTLE_ENDIAN /* TODO */
+# error "SMB3 compression is only supported on little endian architectures"
+#endif /* !__LITTLE_ENDIAN */
+
+#if BITS_PER_LONG < 64 /* TODO */
+# error "SMB3 compression is only supported on 64-bit architectures"
+#endif /* BITS_PER_LONG < 64 */
+
+/* Build time double checks (probably unnecessary) */
+static_assert(sizeof(u16) == 2);
+static_assert(sizeof(u32) == 4);
+static_assert(sizeof(u64) == 8);
+static_assert(sizeof(size_t) == 8);
+
+#ifdef CONFIG_CIFS_DEBUG2
+# define COMPRESS_DEBUG
+#endif /* CONFIG_CIFS_DEBUG2 */
+
+#define COMPRESS_LOG_FMT "CIFS: %s:%d %s(): "
+
+#ifdef COMPRESS_DEBUG
+# define compress_log(fmt, ...) \
+ pr_err(COMPRESS_LOG_FMT fmt, __FILE__, __LINE__, __func__, ## __VA_ARGS__)
+
+/* Make sure to handle assertion failures! */
+# define COMPRESS_ASSERT(cond) \
+ !WARN_ONCE(!(cond), COMPRESS_LOG_FMT "assertion failed: %s\n", \
+ __FILE__, __LINE__, __func__, #cond)
+#else /* COMPRESS_DEBUG */
+# define compress_log(...)
+/* XXX: should not fail silently on non-debug? */
+# define COMPRESS_ASSERT(cond) likely(cond)
+#endif /* !COMPRESS_DEBUG */
+
+/*
+ * Memory ops helpers.
+ */
+#undef MEM_UNALIGNED_READ
+#undef MEM_UNALIGNED_WRITE
+
+/* Read prefetch */
+#define MEM_PREFETCH(ptr) __builtin_prefetch((ptr), 0, 3)
+
+#ifdef CONFIG_X86
+# define MEM_UNALIGNED_READ(ptr, t) (*(const t *)(ptr))
+# define MEM_UNALIGNED_WRITE(ptr, v, t) (*(t *)(ptr) = (t)(v))
+#else
+# include <linux/unaligned.h>
+# define MEM_UNALIGNED_READ(ptr, t) get_unaligned((const t *)(ptr))
+# define MEM_UNALIGNED_WRITE(ptr, v, t) put_unaligned((v), (t *)(ptr))
+#endif /* !CONFIG_X86 */
+
+#define mem_read8(ptr) MEM_UNALIGNED_READ(ptr, u8)
+#define mem_read16(ptr) MEM_UNALIGNED_READ(ptr, u16)
+#define mem_read32(ptr) MEM_UNALIGNED_READ(ptr, u32)
+#define mem_read64(ptr) MEM_UNALIGNED_READ(ptr, u64)
+#define mem_write8(ptr, v) MEM_UNALIGNED_WRITE(ptr, v, u8)
+#define mem_write16(ptr, v) MEM_UNALIGNED_WRITE(ptr, v, u16)
+#define mem_write32(ptr, v) MEM_UNALIGNED_WRITE(ptr, v, u32)
+/* mem_write64() not implemented because it's not used anywhere. */
+
+/*
+ * COMPRESS_RSTEP_SIZE: Number of bytes to read from input buffer for hashing and initial
+ * match check (default 4 bytes).
+ * COMPRESS_MSTEP_SIZE: Number of bytes to extend-compare a found match (default 8 bytes).
+ */
+#define COMPRESS_RSTEP_SIZE sizeof(u32)
+#define COMPRESS_MSTEP_SIZE sizeof(u64)
+
+static __always_inline size_t mem_match_len(const void *match, const void *cur, const void *end)
+{
+ const void *start = cur;
+
+ /* Callers must ensure @cur + COMPRESS_MSTEP_SIZE < @end. */
+ do {
+ const u64 diff = mem_read64(cur) ^ mem_read64(match);
+
+ if (!diff) {
+ cur += COMPRESS_MSTEP_SIZE;
+ match += COMPRESS_MSTEP_SIZE;
+
+ continue;
+ }
+
+ /* This computes the number of common bytes in @diff. */
+ cur += count_trailing_zeros(diff) >> 3;
+
+ return (cur - start);
+ } while (likely(cur + COMPRESS_MSTEP_SIZE <= end));
+
+ /* Fallback to byte-by-byte comparison for last bytes (< COMPRESS_MSTEP_SIZE). */
+ while (cur < end && mem_read8(match) == mem_read8(cur)) {
+ cur++;
+ match++;
+ }
+
+ return (cur - start);
+}
+
+/*
+ * Hashing
+ *
+ * Same for all algorithms.
+ *
+ * XXX: these are fixed for now, might make them tunables in the future.
+ */
+
+/*
+ * COMPRESS_HASH_LOG: ilog2 hash size (recommended to be 13 - 18, default 15)
+ * COMPRESS_HASH_SIZE: Hashtable size (default is 32k (1 << COMPRESS_HASH_LOG))).
+ */
+#define COMPRESS_HASH_LOG 15
+#define COMPRESS_HASH_SIZE (1U << COMPRESS_HASH_LOG)
+
+static __always_inline u32 compress_hash(const u32 v)
+{
+ return ((v ^ 0x9E3779B9U) * 0x85EBCA6BU) >> (32 - COMPRESS_HASH_LOG);
+}
+
+static __always_inline u32 compress_hash_ptr(const void *ptr)
+{
+ return compress_hash(mem_read32(ptr));
+}
+#endif /* _COMPRESS_COMMON_H */
diff --git a/fs/smb/client/compress/lz77.c b/fs/smb/client/compress/lz77.c
index 7365d0f97396..d8c73f7f7483 100644
--- a/fs/smb/client/compress/lz77.c
+++ b/fs/smb/client/compress/lz77.c
@@ -11,19 +11,13 @@
#include <linux/count_zeros.h>
#include <linux/unaligned.h>
+#include "common.h"
#include "lz77.h"
/*
* Compression parameters.
*
* LZ77_MATCH_MAX_DIST: Farthest back a match can be from current position (can be 1 - 8K).
- * LZ77_HASH_LOG:
- * LZ77_HASH_SIZE: ilog2 hash size (recommended to be 13 - 18, default 15 (hash size
- * 32k)).
- * LZ77_RSTEP_SIZE: Number of bytes to read from input buffer for hashing and initial
- * match check (default 4 bytes, this effectivelly makes this the min
- * match len).
- * LZ77_MSTEP_SIZE: Number of bytes to extend-compare a found match (default 8 bytes).
* LZ77_SKIP_TRIGGER: ilog2 value for adaptive skipping, i.e. to progressively skip input
* bytes when we can't find matches. Default is 4.
* Higher values (>0) will decrease compression time, but will result
@@ -31,75 +25,10 @@
* compression ratio (more matches found), but will increase time.
*/
#define LZ77_MATCH_MAX_DIST SZ_8K
-#define LZ77_HASH_LOG 15
-#define LZ77_HASH_SIZE (1 << LZ77_HASH_LOG)
-#define LZ77_RSTEP_SIZE sizeof(u32)
-#define LZ77_MSTEP_SIZE sizeof(u64)
#define LZ77_SKIP_TRIGGER 4
-#define LZ77_PREFETCH(ptr) __builtin_prefetch((ptr), 0, 3)
#define LZ77_FLAG_MAX 32
-static __always_inline u8 lz77_read8(const u8 *ptr)
-{
- return get_unaligned(ptr);
-}
-
-static __always_inline u32 lz77_read32(const u32 *ptr)
-{
- return get_unaligned(ptr);
-}
-
-static __always_inline u64 lz77_read64(const u64 *ptr)
-{
- return get_unaligned(ptr);
-}
-
-static __always_inline void lz77_write8(u8 *ptr, u8 v)
-{
- put_unaligned(v, ptr);
-}
-
-static __always_inline void lz77_write16(u16 *ptr, u16 v)
-{
- put_unaligned_le16(v, ptr);
-}
-
-static __always_inline void lz77_write32(u32 *ptr, u32 v)
-{
- put_unaligned_le32(v, ptr);
-}
-
-static __always_inline u32 lz77_match_len(const void *match, const void *cur, const void *end)
-{
- const void *start = cur;
-
- /* Safe for a do/while because otherwise we wouldn't reach here from the main loop. */
- do {
- const u64 diff = lz77_read64(cur) ^ lz77_read64(match);
-
- if (!diff) {
- cur += LZ77_MSTEP_SIZE;
- match += LZ77_MSTEP_SIZE;
-
- continue;
- }
-
- /* This computes the number of common bytes in @diff. */
- cur += count_trailing_zeros(diff) >> 3;
-
- return (cur - start);
- } while (likely(cur + LZ77_MSTEP_SIZE <= end));
-
- /* Fallback to byte-by-byte comparison for last <8 bytes. */
- while (cur < end && lz77_read8(cur) == lz77_read8(match)) {
- cur++;
- match++;
- }
-
- return (cur - start);
-}
-
/**
* lz77_encode_match() - Match encoding.
* @dst: compressed buffer
@@ -120,24 +49,24 @@ static __always_inline void *lz77_encode_match(void *dst, void **nib, u16 dist,
dist <<= 3;
if (len < 7) {
- lz77_write16(dst, dist + len);
+ mem_write16(dst, dist + len);
return dst + sizeof(u16);
}
dist |= 7;
- lz77_write16(dst, dist);
+ mem_write16(dst, dist);
dst += sizeof(u16);
len -= 7;
if (!*nib) {
- lz77_write8(dst, umin(len, 15));
+ mem_write8(dst, umin(len, 15));
*nib = dst;
dst++;
} else {
u8 *b = *nib;
- lz77_write8(b, *b | umin(len, 15) << 4);
+ mem_write8(b, *b | umin(len, 15) << 4);
*nib = NULL;
}
@@ -146,23 +75,23 @@ static __always_inline void *lz77_encode_match(void *dst, void **nib, u16 dist,
len -= 15;
if (len < 255) {
- lz77_write8(dst, len);
+ mem_write8(dst, len);
return dst + 1;
}
- lz77_write8(dst, 0xff);
+ mem_write8(dst, 0xff);
dst++;
len += 7 + 15;
if (len <= 0xffff) {
- lz77_write16(dst, len);
+ mem_write16(dst, len);
return dst + sizeof(u16);
}
- lz77_write16(dst, 0);
+ mem_write16(dst, 0);
dst += sizeof(u16);
- lz77_write32(dst, len);
+ mem_write32(dst, len);
return dst + sizeof(u32);
}
@@ -200,7 +129,7 @@ static __always_inline void *lz77_encode_literals(const void *start, const void
*f <<= len;
*fc += len;
if (*fc == LZ77_FLAG_MAX) {
- lz77_write32(*fp, *f);
+ mem_write32(*fp, *f);
*fc = 0;
*fp = dst;
dst += sizeof(u32);
@@ -210,11 +139,6 @@ static __always_inline void *lz77_encode_literals(const void *start, const void
return dst;
}
-static __always_inline u32 lz77_hash(const u32 v)
-{
- return ((v ^ 0x9E3779B9) * 0x85EBCA6B) >> (32 - LZ77_HASH_LOG);
-}
-
noinline int lz77_compress(const void *src, const u32 slen, void *dst, u32 *dlen)
{
const void *srcp, *rlim, *end, *anchor;
@@ -228,25 +152,25 @@ noinline int lz77_compress(const void *src, const u32 slen, void *dst, u32 *dlen
srcp = anchor = src;
end = srcp + slen; /* absolute end */
- rlim = end - LZ77_MSTEP_SIZE; /* read limit (for lz77_match_len()) */
+ rlim = end - COMPRESS_MSTEP_SIZE; /* read limit (for mem_match_len()) */
dstp = dst;
flag_pos = dstp;
dstp += sizeof(u32);
nib = NULL;
- htable = kvcalloc(LZ77_HASH_SIZE, sizeof(*htable), GFP_KERNEL);
+ htable = kvcalloc(COMPRESS_HASH_SIZE, sizeof(*htable), GFP_KERNEL);
if (!htable)
return -ENOMEM;
- LZ77_PREFETCH(srcp + LZ77_RSTEP_SIZE);
+ MEM_PREFETCH(srcp + COMPRESS_RSTEP_SIZE);
/*
* Adjust @srcp so we don't get a false positive match on first iteration.
* Then prepare hash for first loop iteration (don't advance @srcp again).
*/
- hash = lz77_hash(lz77_read32(srcp++));
+ hash = compress_hash_ptr(srcp++);
htable[hash] = 0;
- hash = lz77_hash(lz77_read32(srcp));
+ hash = compress_hash_ptr(srcp);
/*
* Main loop.
@@ -278,11 +202,11 @@ noinline int lz77_compress(const void *src, const u32 slen, void *dst, u32 *dlen
if (unlikely(next > rlim))
goto out;
- hash = lz77_hash(lz77_read32(next));
+ hash = compress_hash_ptr(next);
match = src + htable[cur_hash];
htable[cur_hash] = srcp - src;
} while (likely(match + LZ77_MATCH_MAX_DIST < srcp) ||
- lz77_read32(match) != lz77_read32(srcp));
+ mem_read32(match) != mem_read32(srcp));
/*
* Match found. Warm/cold path; begin parsing @srcp and writing to @dstp:
@@ -295,17 +219,17 @@ noinline int lz77_compress(const void *src, const u32 slen, void *dst, u32 *dlen
* redundantly compute it again in lz77_match_len() than to adjust pointers/len.
*/
dstp = lz77_encode_literals(anchor, srcp, dstp, &flag, &flag_count, &flag_pos);
- len = lz77_match_len(match, srcp, end);
+ len = mem_match_len(match, srcp, end);
dstp = lz77_encode_match(dstp, &nib, srcp - match, len);
srcp += len;
anchor = srcp;
- LZ77_PREFETCH(srcp);
+ MEM_PREFETCH(srcp);
flag = (flag << 1) | 1;
flag_count++;
if (flag_count == LZ77_FLAG_MAX) {
- lz77_write32(flag_pos, flag);
+ mem_write32(flag_pos, flag);
flag_count = 0;
flag_pos = dstp;
dstp += sizeof(u32);
@@ -315,7 +239,7 @@ noinline int lz77_compress(const void *src, const u32 slen, void *dst, u32 *dlen
break;
/* Prepare for next loop. */
- hash = lz77_hash(lz77_read32(srcp));
+ hash = compress_hash_ptr(srcp);
} while (srcp < end);
out:
dstp = lz77_encode_literals(anchor, end, dstp, &flag, &flag_count, &flag_pos);
@@ -323,7 +247,7 @@ noinline int lz77_compress(const void *src, const u32 slen, void *dst, u32 *dlen
flag_count = LZ77_FLAG_MAX - flag_count;
flag <<= flag_count;
flag |= (1UL << flag_count) - 1;
- lz77_write32(flag_pos, flag);
+ mem_write32(flag_pos, flag);
*dlen = dstp - dst;
kvfree(htable);
--
2.53.0
^ permalink raw reply related
* [PATCH 6/8] smb: client: compress: add code docs to lz77.c
From: Enzo Matsumiya @ 2026-04-13 19:07 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
In-Reply-To: <20260413190713.283939-1-ematsumiya@suse.de>
Document parts of the code, especially the apparently
non-sense parts.
Other:
- change pointer increment constants to sizeof() values
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/compress/lz77.c | 81 +++++++++++++++++++++++++++++++----
1 file changed, 73 insertions(+), 8 deletions(-)
diff --git a/fs/smb/client/compress/lz77.c b/fs/smb/client/compress/lz77.c
index 96744f52e364..7365d0f97396 100644
--- a/fs/smb/client/compress/lz77.c
+++ b/fs/smb/client/compress/lz77.c
@@ -15,6 +15,20 @@
/*
* Compression parameters.
+ *
+ * LZ77_MATCH_MAX_DIST: Farthest back a match can be from current position (can be 1 - 8K).
+ * LZ77_HASH_LOG:
+ * LZ77_HASH_SIZE: ilog2 hash size (recommended to be 13 - 18, default 15 (hash size
+ * 32k)).
+ * LZ77_RSTEP_SIZE: Number of bytes to read from input buffer for hashing and initial
+ * match check (default 4 bytes, this effectivelly makes this the min
+ * match len).
+ * LZ77_MSTEP_SIZE: Number of bytes to extend-compare a found match (default 8 bytes).
+ * LZ77_SKIP_TRIGGER: ilog2 value for adaptive skipping, i.e. to progressively skip input
+ * bytes when we can't find matches. Default is 4.
+ * Higher values (>0) will decrease compression time, but will result
+ * in worse compression ratio. Lower values will give better
+ * compression ratio (more matches found), but will increase time.
*/
#define LZ77_MATCH_MAX_DIST SZ_8K
#define LZ77_HASH_LOG 15
@@ -86,6 +100,19 @@ static __always_inline u32 lz77_match_len(const void *match, const void *cur, co
return (cur - start);
}
+/**
+ * lz77_encode_match() - Match encoding.
+ * @dst: compressed buffer
+ * @nib: pointer to an address in @dst
+ * @dist: match distance
+ * @len: match length
+ *
+ * Assumes all args were previously checked.
+ *
+ * Return: @dst advanced to new position
+ *
+ * Ref: MS-XCA 2.3.4 "Plain LZ77 Compression Algorithm Details" - "Processing"
+ */
static __always_inline void *lz77_encode_match(void *dst, void **nib, u16 dist, u32 len)
{
len -= 3;
@@ -95,12 +122,12 @@ static __always_inline void *lz77_encode_match(void *dst, void **nib, u16 dist,
if (len < 7) {
lz77_write16(dst, dist + len);
- return dst + 2;
+ return dst + sizeof(u16);
}
dist |= 7;
lz77_write16(dst, dist);
- dst += 2;
+ dst += sizeof(u16);
len -= 7;
if (!*nib) {
@@ -130,16 +157,32 @@ static __always_inline void *lz77_encode_match(void *dst, void **nib, u16 dist,
if (len <= 0xffff) {
lz77_write16(dst, len);
- return dst + 2;
+ return dst + sizeof(u16);
}
lz77_write16(dst, 0);
- dst += 2;
+ dst += sizeof(u16);
lz77_write32(dst, len);
- return dst + 4;
+ return dst + sizeof(u32);
}
+/**
+ * lz77_encode_literals() - Literals encoding.
+ * @start: where to start copying literals (uncompressed buffer)
+ * @end: when to stop copying (uncompressed buffer)
+ * @dst: compressed buffer
+ * @f: pointer to current flag value
+ * @fc: pointer to current flag count
+ * @fp: pointer to current flag address
+ *
+ * Batch copy literals from @start to @dst, updating flag values accordingly.
+ * Assumes all args were previously checked.
+ *
+ * Return: @dst advanced to new position
+ *
+ * MS-XCA 2.3.4 "Plain LZ77 Compression Algorithm Details" - "Processing"
+ */
static __always_inline void *lz77_encode_literals(const void *start, const void *end, void *dst,
long *f, u32 *fc, void **fp)
{
@@ -160,7 +203,7 @@ static __always_inline void *lz77_encode_literals(const void *start, const void
lz77_write32(*fp, *f);
*fc = 0;
*fp = dst;
- dst += 4;
+ dst += sizeof(u32);
}
} while (start < end);
@@ -188,7 +231,7 @@ noinline int lz77_compress(const void *src, const u32 slen, void *dst, u32 *dlen
rlim = end - LZ77_MSTEP_SIZE; /* read limit (for lz77_match_len()) */
dstp = dst;
flag_pos = dstp;
- dstp += 4;
+ dstp += sizeof(u32);
nib = NULL;
htable = kvcalloc(LZ77_HASH_SIZE, sizeof(*htable), GFP_KERNEL);
@@ -197,6 +240,10 @@ noinline int lz77_compress(const void *src, const u32 slen, void *dst, u32 *dlen
LZ77_PREFETCH(srcp + LZ77_RSTEP_SIZE);
+ /*
+ * Adjust @srcp so we don't get a false positive match on first iteration.
+ * Then prepare hash for first loop iteration (don't advance @srcp again).
+ */
hash = lz77_hash(lz77_read32(srcp++));
htable[hash] = 0;
hash = lz77_hash(lz77_read32(srcp));
@@ -219,6 +266,14 @@ noinline int lz77_compress(const void *src, const u32 slen, void *dst, u32 *dlen
srcp = next;
next += step;
+
+ /*
+ * Adaptive skipping.
+ *
+ * Increment @step every (1 << LZ77_SKIP_TRIGGER, 16 in our case) bytes
+ * without a match.
+ * Reset to 1 when a match is found.
+ */
step = (skip++ >> LZ77_SKIP_TRIGGER);
if (unlikely(next > rlim))
goto out;
@@ -229,6 +284,16 @@ noinline int lz77_compress(const void *src, const u32 slen, void *dst, u32 *dlen
} while (likely(match + LZ77_MATCH_MAX_DIST < srcp) ||
lz77_read32(match) != lz77_read32(srcp));
+ /*
+ * Match found. Warm/cold path; begin parsing @srcp and writing to @dstp:
+ * - flush literals
+ * - compute match length (*)
+ * - encode match
+ *
+ * (*) Current minimum match length is defined by the memory read size above, so
+ * here we already know that we have 4 matching bytes, but it's just faster to
+ * redundantly compute it again in lz77_match_len() than to adjust pointers/len.
+ */
dstp = lz77_encode_literals(anchor, srcp, dstp, &flag, &flag_count, &flag_pos);
len = lz77_match_len(match, srcp, end);
dstp = lz77_encode_match(dstp, &nib, srcp - match, len);
@@ -243,7 +308,7 @@ noinline int lz77_compress(const void *src, const u32 slen, void *dst, u32 *dlen
lz77_write32(flag_pos, flag);
flag_count = 0;
flag_pos = dstp;
- dstp += 4;
+ dstp += sizeof(u32);
}
if (unlikely(srcp > rlim))
--
2.53.0
^ permalink raw reply related
* [PATCH 5/8] smb: client: compress: LZ77 optimizations
From: Enzo Matsumiya @ 2026-04-13 19:07 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
In-Reply-To: <20260413190713.283939-1-ematsumiya@suse.de>
This patch implements several micro-optimizations on lz77_compress()
with the goal of reducing the number of instructions per [input]
byte (a.k.a. IPB).
Changes:
- change hashtable to be u32 (instead of u64) -- change the hash
function to reflect that (adds lz77_hash() and lz77_read32() helpers)
- batch-write literals instead of 1 by 1 -- now that we have a well
defined hot path (match finding) and a cold path (encode literals +
match), batch writing makes a significant difference
- implement adaptive skipping of input bytes -- skip input bytes more
aggressively if too few matches are being found
- name some constants for more meaningful context
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/compress/lz77.c | 173 +++++++++++++++++++++-------------
fs/smb/client/compress/lz77.h | 4 +-
2 files changed, 108 insertions(+), 69 deletions(-)
diff --git a/fs/smb/client/compress/lz77.c b/fs/smb/client/compress/lz77.c
index 480927dcd4c6..96744f52e364 100644
--- a/fs/smb/client/compress/lz77.c
+++ b/fs/smb/client/compress/lz77.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
- * Copyright (C) 2024, SUSE LLC
+ * Copyright (C) 2024-2026, SUSE LLC
*
* Authors: Enzo Matsumiya <ematsumiya@suse.de>
*
@@ -16,17 +16,26 @@
/*
* Compression parameters.
*/
-#define LZ77_MATCH_MIN_LEN 4
#define LZ77_MATCH_MAX_DIST SZ_8K
#define LZ77_HASH_LOG 15
#define LZ77_HASH_SIZE (1 << LZ77_HASH_LOG)
-#define LZ77_STEP_SIZE sizeof(u64)
+#define LZ77_RSTEP_SIZE sizeof(u32)
+#define LZ77_MSTEP_SIZE sizeof(u64)
+#define LZ77_SKIP_TRIGGER 4
+
+#define LZ77_PREFETCH(ptr) __builtin_prefetch((ptr), 0, 3)
+#define LZ77_FLAG_MAX 32
static __always_inline u8 lz77_read8(const u8 *ptr)
{
return get_unaligned(ptr);
}
+static __always_inline u32 lz77_read32(const u32 *ptr)
+{
+ return get_unaligned(ptr);
+}
+
static __always_inline u64 lz77_read64(const u64 *ptr)
{
return get_unaligned(ptr);
@@ -50,14 +59,14 @@ static __always_inline void lz77_write32(u32 *ptr, u32 v)
static __always_inline u32 lz77_match_len(const void *match, const void *cur, const void *end)
{
const void *start = cur;
- u64 diff;
/* Safe for a do/while because otherwise we wouldn't reach here from the main loop. */
do {
- diff = lz77_read64(cur) ^ lz77_read64(match);
+ const u64 diff = lz77_read64(cur) ^ lz77_read64(match);
+
if (!diff) {
- cur += LZ77_STEP_SIZE;
- match += LZ77_STEP_SIZE;
+ cur += LZ77_MSTEP_SIZE;
+ match += LZ77_MSTEP_SIZE;
continue;
}
@@ -66,7 +75,7 @@ static __always_inline u32 lz77_match_len(const void *match, const void *cur, co
cur += count_trailing_zeros(diff) >> 3;
return (cur - start);
- } while (likely(cur + LZ77_STEP_SIZE <= end));
+ } while (likely(cur + LZ77_MSTEP_SIZE <= end));
/* Fallback to byte-by-byte comparison for last <8 bytes. */
while (cur < end && lz77_read8(cur) == lz77_read8(match)) {
@@ -77,7 +86,7 @@ static __always_inline u32 lz77_match_len(const void *match, const void *cur, co
return (cur - start);
}
-static __always_inline void *lz77_write_match(void *dst, void **nib, u32 dist, u32 len)
+static __always_inline void *lz77_encode_match(void *dst, void **nib, u16 dist, u32 len)
{
len -= 3;
dist--;
@@ -131,94 +140,124 @@ static __always_inline void *lz77_write_match(void *dst, void **nib, u32 dist, u
return dst + 4;
}
-noinline int lz77_compress(const void *src, u32 slen, void *dst, u32 *dlen)
+static __always_inline void *lz77_encode_literals(const void *start, const void *end, void *dst,
+ long *f, u32 *fc, void **fp)
+{
+ if (start >= end)
+ return dst;
+
+ do {
+ const u32 len = umin(end - start, LZ77_FLAG_MAX - *fc);
+
+ memcpy(dst, start, len);
+
+ dst += len;
+ start += len;
+
+ *f <<= len;
+ *fc += len;
+ if (*fc == LZ77_FLAG_MAX) {
+ lz77_write32(*fp, *f);
+ *fc = 0;
+ *fp = dst;
+ dst += 4;
+ }
+ } while (start < end);
+
+ return dst;
+}
+
+static __always_inline u32 lz77_hash(const u32 v)
+{
+ return ((v ^ 0x9E3779B9) * 0x85EBCA6B) >> (32 - LZ77_HASH_LOG);
+}
+
+noinline int lz77_compress(const void *src, const u32 slen, void *dst, u32 *dlen)
{
- const void *srcp, *end;
+ const void *srcp, *rlim, *end, *anchor;
+ u32 *htable, hash, flag_count = 0;
void *dstp, *nib, *flag_pos;
- u32 flag_count = 0;
long flag = 0;
- u64 *htable;
/* This is probably a bug, so throw a warning. */
if (WARN_ON_ONCE(*dlen < lz77_compressed_alloc_size(slen)))
return -EINVAL;
- srcp = src;
- end = src + slen;
+ srcp = anchor = src;
+ end = srcp + slen; /* absolute end */
+ rlim = end - LZ77_MSTEP_SIZE; /* read limit (for lz77_match_len()) */
dstp = dst;
- nib = NULL;
flag_pos = dstp;
dstp += 4;
+ nib = NULL;
htable = kvcalloc(LZ77_HASH_SIZE, sizeof(*htable), GFP_KERNEL);
if (!htable)
return -ENOMEM;
- /* Main loop. */
- do {
- u32 dist, len = 0;
- const void *wnd;
- u64 hash;
-
- hash = ((lz77_read64(srcp) << 24) * 889523592379ULL) >> (64 - LZ77_HASH_LOG);
- wnd = src + htable[hash];
- htable[hash] = srcp - src;
- dist = srcp - wnd;
-
- if (dist && dist < LZ77_MATCH_MAX_DIST)
- len = lz77_match_len(wnd, srcp, end);
+ LZ77_PREFETCH(srcp + LZ77_RSTEP_SIZE);
- if (len < LZ77_MATCH_MIN_LEN) {
- lz77_write8(dstp, lz77_read8(srcp));
-
- dstp++;
- srcp++;
-
- flag <<= 1;
- flag_count++;
- if (flag_count == 32) {
- lz77_write32(flag_pos, flag);
- flag_count = 0;
- flag_pos = dstp;
- dstp += 4;
- }
-
- continue;
- }
+ hash = lz77_hash(lz77_read32(srcp++));
+ htable[hash] = 0;
+ hash = lz77_hash(lz77_read32(srcp));
- dstp = lz77_write_match(dstp, &nib, dist, len);
+ /*
+ * Main loop.
+ *
+ * @dlen is >= lz77_compressed_alloc_size(), so run without bound-checking @dstp.
+ *
+ * This code was crafted in a way to best utilise fetch-decode-execute CPU flow.
+ * Any attempt to optimize it, or even organize it, can lead to huge performance loss.
+ */
+ do {
+ const void *match, *next = srcp;
+ u32 len, step = 1, skip = 1U << LZ77_SKIP_TRIGGER;
+
+ /* Match finding (hot path -- don't change the read/check/write order). */
+ do {
+ const u32 cur_hash = hash;
+
+ srcp = next;
+ next += step;
+ step = (skip++ >> LZ77_SKIP_TRIGGER);
+ if (unlikely(next > rlim))
+ goto out;
+
+ hash = lz77_hash(lz77_read32(next));
+ match = src + htable[cur_hash];
+ htable[cur_hash] = srcp - src;
+ } while (likely(match + LZ77_MATCH_MAX_DIST < srcp) ||
+ lz77_read32(match) != lz77_read32(srcp));
+
+ dstp = lz77_encode_literals(anchor, srcp, dstp, &flag, &flag_count, &flag_pos);
+ len = lz77_match_len(match, srcp, end);
+ dstp = lz77_encode_match(dstp, &nib, srcp - match, len);
srcp += len;
+ anchor = srcp;
+
+ LZ77_PREFETCH(srcp);
flag = (flag << 1) | 1;
flag_count++;
- if (flag_count == 32) {
+ if (flag_count == LZ77_FLAG_MAX) {
lz77_write32(flag_pos, flag);
flag_count = 0;
flag_pos = dstp;
dstp += 4;
}
- } while (likely(srcp + LZ77_STEP_SIZE <= end));
-
- while (srcp < end) {
- u32 c = umin(end - srcp, 32 - flag_count);
- memcpy(dstp, srcp, c);
+ if (unlikely(srcp > rlim))
+ break;
- dstp += c;
- srcp += c;
-
- flag <<= c;
- flag_count += c;
- if (flag_count == 32) {
- lz77_write32(flag_pos, flag);
- flag_count = 0;
- flag_pos = dstp;
- dstp += 4;
- }
- }
+ /* Prepare for next loop. */
+ hash = lz77_hash(lz77_read32(srcp));
+ } while (srcp < end);
+out:
+ dstp = lz77_encode_literals(anchor, end, dstp, &flag, &flag_count, &flag_pos);
- flag <<= (32 - flag_count);
- flag |= (1UL << (32 - flag_count)) - 1;
+ flag_count = LZ77_FLAG_MAX - flag_count;
+ flag <<= flag_count;
+ flag |= (1UL << flag_count) - 1;
lz77_write32(flag_pos, flag);
*dlen = dstp - dst;
diff --git a/fs/smb/client/compress/lz77.h b/fs/smb/client/compress/lz77.h
index 2603eab9e071..4e570846aefa 100644
--- a/fs/smb/client/compress/lz77.h
+++ b/fs/smb/client/compress/lz77.h
@@ -1,6 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0-only */
/*
- * Copyright (C) 2024, SUSE LLC
+ * Copyright (C) 2024-2026, SUSE LLC
*
* Authors: Enzo Matsumiya <ematsumiya@suse.de>
*
@@ -39,5 +39,5 @@ static __always_inline u32 lz77_compressed_alloc_size(const u32 size)
return size + (size >> 3) + 8;
}
-int lz77_compress(const void *src, u32 slen, void *dst, u32 *dlen);
+int lz77_compress(const void *src, const u32 slen, void *dst, u32 *dlen);
#endif /* _SMB_COMPRESS_LZ77_H */
--
2.53.0
^ permalink raw reply related
* [PATCH 4/8] smb: client: compress: increase LZ77_MATCH_MAX_DIST
From: Enzo Matsumiya @ 2026-04-13 19:07 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
In-Reply-To: <20260413190713.283939-1-ematsumiya@suse.de>
Increase max distance (i.e. window size) from 1k to 8k.
This allows better compression and is just as fast.
Other:
- drop LZ77_MATCH_MIN_DIST as it's nused -- main loop
already checks if dist > 0
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/compress/lz77.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/smb/client/compress/lz77.c b/fs/smb/client/compress/lz77.c
index 61cdf1c14612..480927dcd4c6 100644
--- a/fs/smb/client/compress/lz77.c
+++ b/fs/smb/client/compress/lz77.c
@@ -17,8 +17,7 @@
* Compression parameters.
*/
#define LZ77_MATCH_MIN_LEN 4
-#define LZ77_MATCH_MIN_DIST 1
-#define LZ77_MATCH_MAX_DIST SZ_1K
+#define LZ77_MATCH_MAX_DIST SZ_8K
#define LZ77_HASH_LOG 15
#define LZ77_HASH_SIZE (1 << LZ77_HASH_LOG)
#define LZ77_STEP_SIZE sizeof(u64)
--
2.53.0
^ permalink raw reply related
* [PATCH 3/8] smb: client: compress: fix counting in LZ77 match finding
From: Enzo Matsumiya @ 2026-04-13 19:07 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
In-Reply-To: <20260413190713.283939-1-ematsumiya@suse.de>
- lz77_match_len() increments @cur before checking for equality,
leading to off-by-one match len in some cases.
Fix by moving pointers increment to inside the loop.
Also rename @wnd arg to @match (more accurate name).
- both lz77_match_len() and lz77_compress() checked for
"buf + step < end" when the correct is "<=" for such cases.
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/compress/lz77.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/fs/smb/client/compress/lz77.c b/fs/smb/client/compress/lz77.c
index c1e7fada6e61..61cdf1c14612 100644
--- a/fs/smb/client/compress/lz77.c
+++ b/fs/smb/client/compress/lz77.c
@@ -48,17 +48,17 @@ static __always_inline void lz77_write32(u32 *ptr, u32 v)
put_unaligned_le32(v, ptr);
}
-static __always_inline u32 lz77_match_len(const void *wnd, const void *cur, const void *end)
+static __always_inline u32 lz77_match_len(const void *match, const void *cur, const void *end)
{
const void *start = cur;
u64 diff;
/* Safe for a do/while because otherwise we wouldn't reach here from the main loop. */
do {
- diff = lz77_read64(cur) ^ lz77_read64(wnd);
+ diff = lz77_read64(cur) ^ lz77_read64(match);
if (!diff) {
cur += LZ77_STEP_SIZE;
- wnd += LZ77_STEP_SIZE;
+ match += LZ77_STEP_SIZE;
continue;
}
@@ -67,10 +67,13 @@ static __always_inline u32 lz77_match_len(const void *wnd, const void *cur, cons
cur += count_trailing_zeros(diff) >> 3;
return (cur - start);
- } while (likely(cur + LZ77_STEP_SIZE < end));
+ } while (likely(cur + LZ77_STEP_SIZE <= end));
- while (cur < end && lz77_read8(cur++) == lz77_read8(wnd++))
- ;
+ /* Fallback to byte-by-byte comparison for last <8 bytes. */
+ while (cur < end && lz77_read8(cur) == lz77_read8(match)) {
+ cur++;
+ match++;
+ }
return (cur - start);
}
@@ -195,7 +198,7 @@ noinline int lz77_compress(const void *src, u32 slen, void *dst, u32 *dlen)
flag_pos = dstp;
dstp += 4;
}
- } while (likely(srcp + LZ77_STEP_SIZE < end));
+ } while (likely(srcp + LZ77_STEP_SIZE <= end));
while (srcp < end) {
u32 c = umin(end - srcp, 32 - flag_count);
--
2.53.0
^ permalink raw reply related
* [PATCH 2/8] smb: client: compress: fix bad encoding on last LZ77 flag
From: Enzo Matsumiya @ 2026-04-13 19:07 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
In-Reply-To: <20260413190713.283939-1-ematsumiya@suse.de>
End-of-stream flag could lead to UB because of int promotion
(overwriting signed bit).
Fix it by changing operand from '1' to '1UL'.
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/compress/lz77.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/smb/client/compress/lz77.c b/fs/smb/client/compress/lz77.c
index 16c7d8f3ef17..c1e7fada6e61 100644
--- a/fs/smb/client/compress/lz77.c
+++ b/fs/smb/client/compress/lz77.c
@@ -216,7 +216,7 @@ noinline int lz77_compress(const void *src, u32 slen, void *dst, u32 *dlen)
}
flag <<= (32 - flag_count);
- flag |= (1 << (32 - flag_count)) - 1;
+ flag |= (1UL << (32 - flag_count)) - 1;
lz77_write32(flag_pos, flag);
*dlen = dstp - dst;
--
2.53.0
^ permalink raw reply related
* [PATCH 1/8] smb: client: compress: fix buffer overrun in lz77_compress()
From: Enzo Matsumiya @ 2026-04-13 19:07 UTC (permalink / raw)
To: linux-cifs
Cc: smfrench, pc, ronniesahlberg, sprasad, tom, bharathsm,
henrique.carvalho
@dst buffer is allocated with same size as @src, which, for good
compression cases, works fine.
However, when compression goes bad (e.g. random bytes payloads), the
compressed size can increase significantly, and even by stopping the
main loop at 7/8 of @slen, writing leftover literals could write past
the end of @dst because of LZ77 metadata.
To fix this, add lz77_compressed_alloc_size() helper to compute the
correct allocation size for @dst, accounting for metadata and worst
cast scenario (all literals).
While this is overprovisioning memory, it's not only correct, but also
allows lz77_compress() main loop to run without ever checking @dst
limits (i.e. a perf improvement).
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
---
fs/smb/client/compress.c | 6 +-----
fs/smb/client/compress/lz77.c | 14 ++++----------
fs/smb/client/compress/lz77.h | 28 ++++++++++++++++++++++++++++
3 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/fs/smb/client/compress.c b/fs/smb/client/compress.c
index 3d1e73f5d9af..be9023f841e6 100644
--- a/fs/smb/client/compress.c
+++ b/fs/smb/client/compress.c
@@ -329,11 +329,7 @@ int smb_compress(struct TCP_Server_Info *server, struct smb_rqst *rq, compress_s
goto err_free;
}
- /*
- * This is just overprovisioning, as the algorithm will error out if @dst reaches 7/8
- * of @slen.
- */
- dlen = slen;
+ dlen = lz77_compressed_alloc_size(slen);
dst = kvzalloc(dlen, GFP_KERNEL);
if (!dst) {
ret = -ENOMEM;
diff --git a/fs/smb/client/compress/lz77.c b/fs/smb/client/compress/lz77.c
index 96e8a8057a77..16c7d8f3ef17 100644
--- a/fs/smb/client/compress/lz77.c
+++ b/fs/smb/client/compress/lz77.c
@@ -137,6 +137,10 @@ noinline int lz77_compress(const void *src, u32 slen, void *dst, u32 *dlen)
long flag = 0;
u64 *htable;
+ /* This is probably a bug, so throw a warning. */
+ if (WARN_ON_ONCE(*dlen < lz77_compressed_alloc_size(slen)))
+ return -EINVAL;
+
srcp = src;
end = src + slen;
dstp = dst;
@@ -180,15 +184,6 @@ noinline int lz77_compress(const void *src, u32 slen, void *dst, u32 *dlen)
continue;
}
- /*
- * Bail out if @dstp reached >= 7/8 of @slen -- already compressed badly, not worth
- * going further.
- */
- if (unlikely(dstp - dst >= slen - (slen >> 3))) {
- *dlen = slen;
- goto out;
- }
-
dstp = lz77_write_match(dstp, &nib, dist, len);
srcp += len;
@@ -225,7 +220,6 @@ noinline int lz77_compress(const void *src, u32 slen, void *dst, u32 *dlen)
lz77_write32(flag_pos, flag);
*dlen = dstp - dst;
-out:
kvfree(htable);
if (*dlen < slen)
diff --git a/fs/smb/client/compress/lz77.h b/fs/smb/client/compress/lz77.h
index cdcb191b48a2..2603eab9e071 100644
--- a/fs/smb/client/compress/lz77.h
+++ b/fs/smb/client/compress/lz77.h
@@ -11,5 +11,33 @@
#include <linux/kernel.h>
+/**
+ * lz77_compressed_alloc_size() - Compute compressed buffer size.
+ * @size: uncompressed (src) size
+ *
+ * Compute allocation size for the compressed buffer based on uncompressed size.
+ * Accounts for metadata and overprovision for the worst case scenario.
+ *
+ * LZ77 metadata is a 4-byte flag that is written:
+ * - on dst begin (pos 0)
+ * - every 32 literals or matches
+ * - on end-of-stream (possibly, if last write was another flag)
+ *
+ * Worst case scenario is an all-literal compression, which means:
+ * metadata bytes = 4 + ((@size / 32) * 4) + 4, or, simplified, (@size >> 3) + 8
+ *
+ * The worst case scenario rarely happens, but such overprovisioning also allows lz77_compress()
+ * main loop to run without ever bound checking dst, which is a huge perf improvement, while also
+ * being safe when compression goes bad.
+ *
+ * Return: required (*) allocation size for compressed buffer.
+ *
+ * (*) checked once in the beginning of lz77_compress()
+ */
+static __always_inline u32 lz77_compressed_alloc_size(const u32 size)
+{
+ return size + (size >> 3) + 8;
+}
+
int lz77_compress(const void *src, u32 slen, void *dst, u32 *dlen);
#endif /* _SMB_COMPRESS_LZ77_H */
--
2.53.0
^ permalink raw reply related
* [PATCH 5.10 174/491] smb: client: fix atomic open with O_DIRECT & O_SYNC
From: Greg Kroah-Hartman @ 2026-04-13 15:56 UTC (permalink / raw)
To: stable
Cc: Greg Kroah-Hartman, patches, Paulo Alcantara (Red Hat),
David Howells, Henrique Carvalho, Tom Talpey, linux-cifs,
Steve French, Sasha Levin
In-Reply-To: <20260413155819.042779211@linuxfoundation.org>
5.10-stable review patch. If anyone has any objections, please let me know.
------------------
From: Paulo Alcantara <pc@manguebit.org>
[ Upstream commit 4a7d2729dc99437dbb880a64c47828c0d191b308 ]
When user application requests O_DIRECT|O_SYNC along with O_CREAT on
open(2), CREATE_NO_BUFFER and CREATE_WRITE_THROUGH bits were missed in
CREATE request when performing an atomic open, thus leading to
potentially data integrity issues.
Fix this by setting those missing bits in CREATE request when
O_DIRECT|O_SYNC has been specified in cifs_do_create().
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Acked-by: Henrique Carvalho <henrique.carvalho@suse.com>
Cc: Tom Talpey <tom@talpey.com>
Cc: linux-cifs@vger.kernel.org
Cc: stable@vger.kernel.org
Signed-off-by: Steve French <stfrench@microsoft.com>
[ adapted file paths from fs/smb/client/ to fs/cifs/ ]
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
fs/cifs/cifsglob.h | 11 +++++++++++
fs/cifs/dir.c | 1 +
fs/cifs/file.c | 17 +++--------------
3 files changed, 15 insertions(+), 14 deletions(-)
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -26,6 +26,7 @@
#include <linux/mm.h>
#include <linux/mempool.h>
#include <linux/workqueue.h>
+#include <linux/fcntl.h>
#include "cifs_fs_sb.h"
#include "cifsacl.h"
#include <crypto/internal/hash.h>
@@ -2124,4 +2125,14 @@ static inline bool cifs_ses_exiting(stru
return ret;
}
+static inline int cifs_open_create_options(unsigned int oflags, int opts)
+{
+ /* O_SYNC also has bit for O_DSYNC so following check picks up either */
+ if (oflags & O_SYNC)
+ opts |= CREATE_WRITE_THROUGH;
+ if (oflags & O_DIRECT)
+ opts |= CREATE_NO_BUFFER;
+ return opts;
+}
+
#endif /* _CIFS_GLOB_H */
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -348,6 +348,7 @@ cifs_do_create(struct inode *inode, stru
goto out;
}
+ create_options |= cifs_open_create_options(oflags, create_options);
/*
* if we're not using unix extensions, see if we need to set
* ATTR_READONLY on the create call
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -216,19 +216,13 @@ cifs_nt_open(char *full_path, struct ino
*********************************************************************/
disposition = cifs_get_disposition(f_flags);
-
/* BB pass O_SYNC flag through on file attributes .. BB */
buf = kmalloc(sizeof(FILE_ALL_INFO), GFP_KERNEL);
if (!buf)
return -ENOMEM;
- /* O_SYNC also has bit for O_DSYNC so following check picks up either */
- if (f_flags & O_SYNC)
- create_options |= CREATE_WRITE_THROUGH;
-
- if (f_flags & O_DIRECT)
- create_options |= CREATE_NO_BUFFER;
+ create_options |= cifs_open_create_options(f_flags, create_options);
oparms.tcon = tcon;
oparms.cifs_sb = cifs_sb;
@@ -750,13 +744,8 @@ cifs_reopen_file(struct cifsFileInfo *cf
}
desired_access = cifs_convert_flags(cfile->f_flags);
-
- /* O_SYNC also has bit for O_DSYNC so following check picks up either */
- if (cfile->f_flags & O_SYNC)
- create_options |= CREATE_WRITE_THROUGH;
-
- if (cfile->f_flags & O_DIRECT)
- create_options |= CREATE_NO_BUFFER;
+ create_options |= cifs_open_create_options(cfile->f_flags,
+ create_options);
if (server->ops->get_lease_key)
server->ops->get_lease_key(inode, &cfile->fid);
^ permalink raw reply
* Re: [PATCH v2 1/3] cifs.upcall: add option to enable debug logs
From: Steve French @ 2026-04-13 14:21 UTC (permalink / raw)
To: Pierguido Lambri; +Cc: Linux CIFS Mailing list
In-Reply-To: <20260129140948.621754-1-plambri@redhat.com>
merged the three patches into cifs-utils for-next
On Thu, Jan 29, 2026 at 8:09 AM Pierguido Lambri <plambri@redhat.com> wrote:
>
> cifs.upcall uses two levels of logs, DEBUG and ERR.
> However, when using systemd, these logs will always be recorded.
> When the system does a lot of upcalls, the journal could be filled
> with debug logs, which may not be useful at that time.
> Added then a new option '-d' to enable debug logs only when needed.
> This will set a logmask up to LOG_DEBUG instead of the default
> of LOG_ERR, thus reducing the amount of logs when no debug is needed.
>
> Signed-off-by: Pierguido Lambri <plambri@redhat.com>
> Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
> ---
> cifs.upcall.c | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/cifs.upcall.c b/cifs.upcall.c
> index 69e27a3..9c2843d 100644
> --- a/cifs.upcall.c
> +++ b/cifs.upcall.c
> @@ -1356,7 +1356,7 @@ lowercase_string(char *c)
>
> static void usage(void)
> {
> - fprintf(stderr, "Usage: %s [ -K /path/to/keytab] [-k /path/to/krb5.conf] [-E] [-t] [-v] [-l] [-e nsecs] key_serial\n", prog);
> + fprintf(stderr, "Usage: %s [ -K /path/to/keytab] [-k /path/to/krb5.conf] [-d] [-E] [-t] [-v] [-l] [-e nsecs] key_serial\n", prog);
> }
>
> static const struct option long_options[] = {
> @@ -1379,6 +1379,7 @@ int main(const int argc, char *const argv[])
> size_t datalen;
> long rc = 1;
> int c;
> + int mask;
> bool try_dns = false, legacy_uid = false , env_probe = true;
> char *buf;
> char hostbuf[NI_MAXHOST], *host;
> @@ -1395,12 +1396,19 @@ int main(const int argc, char *const argv[])
> hostbuf[0] = '\0';
>
> openlog(prog, 0, LOG_DAEMON);
> + mask = LOG_UPTO(LOG_ERR);
> + setlogmask(mask);
>
> - while ((c = getopt_long(argc, argv, "cEk:K:ltve:", long_options, NULL)) != -1) {
> + while ((c = getopt_long(argc, argv, "cdEk:K:ltve:", long_options, NULL)) != -1) {
> switch (c) {
> case 'c':
> /* legacy option -- skip it */
> break;
> + case 'd':
> + /* enable debug logs */
> + mask = LOG_UPTO(LOG_DEBUG);
> + setlogmask(mask);
> + break;
> case 'E':
> /* skip probing initiating process env */
> env_probe = false;
> --
> 2.52.0
>
>
--
Thanks,
Steve
^ permalink raw reply
* Re: [PATCH 7/9] smb: client: block cache=ro and cache=singleclient on remount
From: Meetakshi Setiya @ 2026-04-13 13:40 UTC (permalink / raw)
To: rajasimandalos
Cc: sfrench, linux-cifs, pc, ronniesahlberg, sprasad, tom, metze,
bharathsm, samba-technical, linux-kernel, Rajasi Mandal
In-Reply-To: <20260409095926.905020-7-rajasimandalos@gmail.com>
Hey Rajasi,
I guess this is not handling remounts where cache=singleclient/ro is
not specified i.e. preserving previous behaviour.
To reproduce, mount with cache=ro and remount without providing cache=ro.
sudo mount -t cifs //server/winshare /mnt/mainshare -o
remount,username=demouser
mount error(22): Invalid argument
On Thu, Apr 9, 2026 at 3:07 AM <rajasimandalos@gmail.com> wrote:
>
> From: Rajasi Mandal <rajasimandal@microsoft.com>
>
> cache=ro and cache=singleclient are mount-time environment declarations
> where the admin promises that the share is read-only or exclusively
> accessed. The client bypasses server-based coherency (oplocks/leases)
> and caches aggressively based on this promise.
>
> These modes were intentionally excluded from smb3_update_mnt_flags()
> when it was introduced in commit 2d39f50c2b15 ("cifs: move update of
> flags into a separate function") — only cache=strict, cache=none and
> cache=loose were made reconfigurable. However, remount currently
> silently accepts cache=ro and cache=singleclient without actually
> applying them, which is confusing.
>
> Add explicit checks in smb3_verify_reconfigure_ctx() to reject
> attempts to change these options during remount with a clear error
> message.
>
> Signed-off-by: Rajasi Mandal <rajasimandal@microsoft.com>
> ---
> fs/smb/client/fs_context.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c
> index ce4842e778c4..2be72733ef2e 100644
> --- a/fs/smb/client/fs_context.c
> +++ b/fs/smb/client/fs_context.c
> @@ -1180,6 +1180,14 @@ static int smb3_verify_reconfigure_ctx(struct fs_context *fc,
> cifs_errorf(fc, "can not change rdma during remount\n");
> return -EINVAL;
> }
> + if (new_ctx->cache_ro != old_ctx->cache_ro) {
> + cifs_errorf(fc, "can not change cache=ro during remount\n");
> + return -EINVAL;
> + }
> + if (new_ctx->cache_rw != old_ctx->cache_rw) {
> + cifs_errorf(fc, "can not change cache=singleclient during remount\n");
> + return -EINVAL;
> + }
>
> return 0;
> }
> --
> 2.43.0
>
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox