Pull smb client fixes from Steve French:
- Fix reporting of i_blocks
- Fix Kerberos mounts with different usernames to same server
- Trivial comment cleanup
* tag 'v7.0-rc4-smb3-client-fixes' of git://git.samba.org/sfrench/cifs-2.6:
smb: client: fix generic/694 due to wrong ->i_blocks
cifs: smb1: fix comment typo
smb: client: fix krb5 mount with username option
smb2_get_ksmbd_tcon() reuses work->tcon in compound requests without
validating tcon->t_state. ksmbd_tree_conn_lookup() checks t_state ==
TREE_CONNECTED on the initial lookup path, but the compound reuse path
bypasses this check entirely.
If a prior command in the compound (SMB2_TREE_DISCONNECT) sets t_state
to TREE_DISCONNECTED and frees share_conf via ksmbd_share_config_put(),
subsequent commands dereference the freed share_conf through
work->tcon->share_conf.
KASAN report:
[ 4.144653] ==================================================================
[ 4.145059] BUG: KASAN: slab-use-after-free in smb2_write+0xc74/0xe70
[ 4.145415] Read of size 4 at addr ffff88810430c194 by task kworker/1:1/44
[ 4.145772]
[ 4.145867] CPU: 1 UID: 0 PID: 44 Comm: kworker/1:1 Not tainted 7.0.0-rc3+ #60 PREEMPTLAZY
[ 4.145871] Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[ 4.145875] Workqueue: ksmbd-io handle_ksmbd_work
[ 4.145888] Call Trace:
[ 4.145892] <TASK>
[ 4.145894] dump_stack_lvl+0x64/0x80
[ 4.145910] print_report+0xce/0x660
[ 4.145919] ? __pfx__raw_spin_lock_irqsave+0x10/0x10
[ 4.145928] ? smb2_write+0xc74/0xe70
[ 4.145931] kasan_report+0xce/0x100
[ 4.145934] ? smb2_write+0xc74/0xe70
[ 4.145937] smb2_write+0xc74/0xe70
[ 4.145939] ? __pfx_smb2_write+0x10/0x10
[ 4.145942] ? _raw_spin_unlock+0xe/0x30
[ 4.145945] ? ksmbd_smb2_check_message+0xeb2/0x24c0
[ 4.145948] ? smb2_tree_disconnect+0x31c/0x480
[ 4.145951] handle_ksmbd_work+0x40f/0x1080
[ 4.145953] process_one_work+0x5fa/0xef0
[ 4.145962] ? assign_work+0x122/0x3e0
[ 4.145964] worker_thread+0x54b/0xf70
[ 4.145967] ? __pfx_worker_thread+0x10/0x10
[ 4.145970] kthread+0x346/0x470
[ 4.145976] ? recalc_sigpending+0x19b/0x230
[ 4.145980] ? __pfx_kthread+0x10/0x10
[ 4.145984] ret_from_fork+0x4fb/0x6c0
[ 4.145992] ? __pfx_ret_from_fork+0x10/0x10
[ 4.145995] ? __switch_to+0x36c/0xbe0
[ 4.145999] ? __pfx_kthread+0x10/0x10
[ 4.146003] ret_from_fork_asm+0x1a/0x30
[ 4.146013] </TASK>
[ 4.146014]
[ 4.149858] Allocated by task 44:
[ 4.149953] kasan_save_stack+0x33/0x60
[ 4.150061] kasan_save_track+0x14/0x30
[ 4.150169] __kasan_kmalloc+0x8f/0xa0
[ 4.150274] ksmbd_share_config_get+0x1dd/0xdd0
[ 4.150401] ksmbd_tree_conn_connect+0x7e/0x600
[ 4.150529] smb2_tree_connect+0x2e6/0x1000
[ 4.150645] handle_ksmbd_work+0x40f/0x1080
[ 4.150761] process_one_work+0x5fa/0xef0
[ 4.150873] worker_thread+0x54b/0xf70
[ 4.150978] kthread+0x346/0x470
[ 4.151071] ret_from_fork+0x4fb/0x6c0
[ 4.151176] ret_from_fork_asm+0x1a/0x30
[ 4.151286]
[ 4.151332] Freed by task 44:
[ 4.151418] kasan_save_stack+0x33/0x60
[ 4.151526] kasan_save_track+0x14/0x30
[ 4.151634] kasan_save_free_info+0x3b/0x60
[ 4.151751] __kasan_slab_free+0x43/0x70
[ 4.151861] kfree+0x1ca/0x430
[ 4.151952] __ksmbd_tree_conn_disconnect+0xc8/0x190
[ 4.152088] smb2_tree_disconnect+0x1cd/0x480
[ 4.152211] handle_ksmbd_work+0x40f/0x1080
[ 4.152326] process_one_work+0x5fa/0xef0
[ 4.152438] worker_thread+0x54b/0xf70
[ 4.152545] kthread+0x346/0x470
[ 4.152638] ret_from_fork+0x4fb/0x6c0
[ 4.152743] ret_from_fork_asm+0x1a/0x30
[ 4.152853]
[ 4.152900] The buggy address belongs to the object at ffff88810430c180
[ 4.152900] which belongs to the cache kmalloc-96 of size 96
[ 4.153226] The buggy address is located 20 bytes inside of
[ 4.153226] freed 96-byte region [ffff88810430c180, ffff88810430c1e0)
[ 4.153549]
[ 4.153596] The buggy address belongs to the physical page:
[ 4.153750] page: refcount:0 mapcount:0 mapping:0000000000000000 index:0xffff88810430ce80 pfn:0x10430c
[ 4.154000] flags: 0x100000000000200(workingset|node=0|zone=2)
[ 4.154160] page_type: f5(slab)
[ 4.154251] raw: 0100000000000200 ffff888100041280 ffff888100040110 ffff888100040110
[ 4.154461] raw: ffff88810430ce80 0000000800200009 00000000f5000000 0000000000000000
[ 4.154668] page dumped because: kasan: bad access detected
[ 4.154820]
[ 4.154866] Memory state around the buggy address:
[ 4.155002] ffff88810430c080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 4.155196] ffff88810430c100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 4.155391] >ffff88810430c180: fa fb fb fb fb fb fb fb fb fb fb fb fc fc fc fc
[ 4.155587] ^
[ 4.155693] ffff88810430c200: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 4.155891] ffff88810430c280: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 4.156087] ==================================================================
Add the same t_state validation to the compound reuse path, consistent
with ksmbd_tree_conn_lookup().
Fixes: 5005bcb421 ("ksmbd: validate session id and tree id in the compound request")
Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Use sb->s_uuid for a proper volume identifier as the primary choice.
For filesystems that do not provide a UUID, fall back to stfs.f_fsid
obtained from vfs_statfs().
Cc: stable@vger.kernel.org
Reported-by: Hyunwoo Kim <imv4bel@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
When a multichannel SMB2_SESSION_SETUP request with
SMB2_SESSION_REQ_FLAG_BINDING fails ksmbd sets conn->binding = true
but never clears it on the error path. This leaves the connection in
a binding state where all subsequent ksmbd_session_lookup_all() calls
fall back to the global sessions table. This fix it by clearing
conn->binding = false in the error path.
Cc: stable@vger.kernel.org
Reported-by: Hyunwoo Kim <imv4bel@gmail.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
__ksmbd_tree_conn_disconnect() drops the share_conf reference before
checking tree_conn->refcount. When someone uses SMB3 multichannel and
binds two connections to one session, a SESSION_LOGOFF on connection A
calls ksmbd_conn_wait_idle(conn) which only drains connection A's
request counter, not connection B's. This means there's a race condition:
requests already dispatched on connection B hold tree_conn references via
work->tcon. The disconnect path frees share_conf while those requests
are still walking work->tcon->share_conf, causing a use-after-free.
This fix combines the share_conf put with the tree_conn free so it
only happens when the last reference is dropped.
Fixes: b39a1833cc ("ksmbd: fix use-after-free in ksmbd_tree_connect_put under concurrency")
Signed-off-by: Nicholas Carlini <nicholas@carlini.com>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
The file contains a spelling error in a source comment (resposne).
Typos in comments reduce readability and make text searches less reliable
for developers and maintainers.
Replace 'resposne' with 'response' in the affected comment. This is a
comment-only cleanup and does not change behavior.
[v2: Removed Fixes: and Cc: to stable tags.]
Signed-off-by: Joseph Salisbury <joseph.salisbury@oracle.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Customer reported that some of their krb5 mounts were failing against
a single server as the client was trying to mount the shares with
wrong credentials. It turned out the client was reusing SMB session
from first mount to try mounting the other shares, even though a
different username= option had been specified to the other mounts.
By using username mount option along with sec=krb5 to search for
principals from keytab is supported by cifs.upcall(8) since
cifs-utils-4.8. So fix this by matching username mount option in
match_session() even with Kerberos.
For example, the second mount below should fail with -ENOKEY as there
is no 'foobar' principal in keytab (/etc/krb5.keytab). The client
ends up reusing SMB session from first mount to perform the second
one, which is wrong.
```
$ ktutil
ktutil: add_entry -password -p testuser -k 1 -e aes256-cts
Password for testuser@ZELDA.TEST:
ktutil: write_kt /etc/krb5.keytab
ktutil: quit
$ klist -ke
Keytab name: FILE:/etc/krb5.keytab
KVNO Principal
---- ----------------------------------------------------------------
1 testuser@ZELDA.TEST (aes256-cts-hmac-sha1-96)
$ mount.cifs //w22-root2/scratch /mnt/1 -o sec=krb5,username=testuser
$ mount.cifs //w22-root2/scratch /mnt/2 -o sec=krb5,username=foobar
$ mount -t cifs | grep -Po 'username=\K\w+'
testuser
testuser
```
Reported-by: Oscar Santos <ossantos@redhat.com>
Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Cc: David Howells <dhowells@redhat.com>
Cc: linux-cifs@vger.kernel.org
Cc: stable@vger.kernel.org
Signed-off-by: Steve French <stfrench@microsoft.com>
Pull smb client fixes from Steve French:
- Fix reconnect when using non-default port
- Fix default retransmission behavior
- Fix open handle reuse in cifs_open
- Fix export for smb2-mapperror-test
- Fix potential corruption on write retry
- Fix potentially uninitialized superblock flags
- Fix missing O_DIRECT and O_SYNC flags on create
* tag 'v7.0-rc3-smb3-client-fixes' of git://git.samba.org/sfrench/cifs-2.6:
cifs: make default value of retrans as zero
smb: client: fix open handle lookup in cifs_open()
smb: client: fix iface port assignment in parse_server_interfaces
smb/client: only export symbol for 'smb2maperror-test' module
smb: client: fix in-place encryption corruption in SMB2_write()
smb: client: fix sbflags initialization
smb: client: fix atomic open with O_DIRECT & O_SYNC
When retrans mount option was introduced, the default value was set
as 1. However, in the light of some bugs that this has exposed recently
we should change it to 0 and retain the old behaviour before this option
was introduced.
Cc: <stable@vger.kernel.org>
Reviewed-by: Bharath SM <bharathsm@microsoft.com>
Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
When looking up open handles to be re-used in cifs_open(), calling
cifs_get_{writable,readable}_path() is wrong as it will look up for
the first matching open handle, and if @file->f_flags doesn't match,
it will ignore the remaining open handles in
cifsInodeInfo::openFileList that might potentially match
@file->f_flags.
For writable and readable handles, fix this by calling
__cifs_get_writable_file() and __find_readable_file(), respectively,
with FIND_OPEN_FLAGS set.
With the patch, the following program ends up with two opens instead
of three sent over the wire.
```
#define _GNU_SOURCE
#include <unistd.h>
#include <string.h>
#include <fcntl.h>
int main(int argc, char *argv[])
{
int fd;
fd = open("/mnt/1/foo", O_CREAT | O_WRONLY | O_TRUNC, 0664);
close(fd);
fd = open("/mnt/1/foo", O_DIRECT | O_WRONLY);
close(fd);
fd = open("/mnt/1/foo", O_WRONLY);
close(fd);
fd = open("/mnt/1/foo", O_DIRECT | O_WRONLY);
close(fd);
return 0;
}
```
```
$ mount.cifs //srv/share /mnt/1 -o ...
$ gcc test.c && ./a.out
```
Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Cc: David Howells <dhowells@redhat.com>
Cc: linux-cifs@vger.kernel.org
Signed-off-by: Steve French <stfrench@microsoft.com>
parse_server_interfaces() initializes interface socket addresses with
CIFS_PORT. When the mount uses a non-default port this overwrites the
configured destination port.
Later, cifs_chan_update_iface() copies this sockaddr into server->dstaddr,
causing reconnect attempts to use the wrong port after server interface
updates.
Use the existing port from server->dstaddr instead.
Cc: stable@vger.kernel.org
Fixes: fe856be475 ("CIFS: parse and store info on iface queries")
Tested-by: Dr. Thomas Orgis <thomas.orgis@uni-hamburg.de>
Reviewed-by: Enzo Matsumiya <ematsumiya@suse.de>
Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Only export smb2_get_err_map_test smb2_error_map_table_test and
smb2_error_map_num symbol for 'smb2maperror-test' module.
Fixes: 7d0bf050a5 ("smb/client: make SMB2 maperror KUnit tests a separate module")
Signed-off-by: Ye Bin <yebin10@huawei.com>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Steve French <stfrench@microsoft.com>
SMB2_write() places write payload in iov[1..n] as part of rq_iov.
smb3_init_transform_rq() pointer-shares rq_iov, so crypt_message()
encrypts iov[1] in-place, replacing the original plaintext with
ciphertext. On a replayable error, the retry sends the same iov[1]
which now contains ciphertext instead of the original data,
resulting in corruption.
The corruption is most likely to be observed when connections are
unstable, as reconnects trigger write retries that re-send the
already-encrypted data.
This affects SFU mknod, MF symlinks, etc. On kernels before
6.10 (prior to the netfs conversion), sync writes also used
this path and were similarly affected. The async write path
wasn't unaffected as it uses rq_iter which gets deep-copied.
Fix by moving the write payload into rq_iter via iov_iter_kvec(),
so smb3_init_transform_rq() deep-copies it before encryption.
Cc: stable@vger.kernel.org #6.3+
Acked-by: Henrique Carvalho <henrique.carvalho@suse.com>
Acked-by: Shyam Prasad N <sprasad@microsoft.com>
Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Signed-off-by: Bharath SM <bharathsm@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
The newly introduced variable is initialized in an #ifdef block
but used outside of it, leading to undefined behavior when
CONFIG_CIFS_ALLOW_INSECURE_LEGACY is disabled:
fs/smb/client/dir.c:417:9: error: variable 'sbflags' is uninitialized when used here [-Werror,-Wuninitialized]
417 | if (sbflags & CIFS_MOUNT_DYNPERM)
| ^~~~~~~
Move the initialization into the declaration, the same way as the
other similar function do it.
Fixes: 4fc3a433c1 ("smb: client: use atomic_t for mnt_cifs_flags")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
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: 1da177e4c3 ("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>
When KSMBD_DEBUG_AUTH logging is enabled, generate_smb3signingkey() and
generate_smb3encryptionkey() log the session, signing, encryption, and
decryption key bytes. Remove the logs to avoid exposing credentials.
Fixes: e2f34481b2 ("cifsd: add server-side procedures for SMB3")
Cc: stable@vger.kernel.org
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
opinfo pointer obtained via rcu_dereference(fp->f_opinfo) is being
accessed after rcu_read_unlock() has been called. This creates a
race condition where the memory could be freed by a concurrent
writer between the unlock and the subsequent pointer dereferences
(opinfo->is_lease, etc.), leading to a use-after-free.
Fixes: 5fb282ba4f ("ksmbd: fix possible null-deref in smb_lazy_parent_lease_break_close")
Cc: stable@vger.kernel.org
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
ksmbd currently frees oplock_info immediately using kfree(), even
though it is accessed under RCU read-side critical sections in places
like opinfo_get() and proc_show_files().
Since there is no RCU grace period delay between nullifying the pointer
and freeing the memory, a reader can still access oplock_info
structure after it has been freed. This can leads to a use-after-free
especially in opinfo_get() where atomic_inc_not_zero() is called on
already freed memory.
Fix this by switching to deferred freeing using call_rcu().
Fixes: 18b4fac5ef ("ksmbd: fix use-after-free in smb_break_all_levII_oplock()")
Cc: stable@vger.kernel.org
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
The opinfo pointer obtained via rcu_dereference(fp->f_opinfo) is
dereferenced after rcu_read_unlock(), creating a use-after-free
window. A concurrent opinfo_put() can free the opinfo between the
unlock and the subsequent access to opinfo->is_lease,
opinfo->o_lease->state, and opinfo->level.
Fix this by deferring rcu_read_unlock() until after all opinfo
field accesses are complete. The values needed (const_names, count,
level) are copied into local variables under the RCU read lock,
and the potentially-sleeping seq_printf calls happen after the
lock is released.
Found by AI-assisted code review (Claude Opus 4.6, Anthropic)
in collaboration with Ali Khaledi.
Cc: stable@vger.kernel.org
Fixes: b38f99c121 ("ksmbd: add procfs interface for runtime monitoring and statistics")
Signed-off-by: Ali Khaledi <ali.khaledi1989@gmail.com>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
If ksmbd_override_fsids() fails, we jump to err_out2. At that point, fp is
NULL because it hasn't been assigned dh_info.fp yet, so ksmbd_fd_put(work,
fp) will not be called. However, dh_info.fp was already inserted into the
session file table by ksmbd_reopen_durable_fd(), so it will leak in the
session file table until the session is closed.
Move fp = dh_info.fp; ahead of the ksmbd_override_fsids() check to fix the
problem.
Found by an experimental AI code review agent at Google.
Fixes: c8efcc7861 ("ksmbd: add support for durable handles v1/v2")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
If SMB2_open_init() or SMB2_close_init() fails (e.g. reconnect), the
iovs set @rqst will be left uninitialised, hence calling
SMB2_open_free(), SMB2_close_free() or smb2_set_related() on them will
oops.
Fix this by initialising @close_iov and @open_iov before setting them
in @rqst.
Reported-by: Thiago Becker <tbecker@redhat.com>
Fixes: 1cf9f2a6a5 ("smb: client: handle unlink(2) of files open by different clients")
Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Cc: David Howells <dhowells@redhat.com>
Cc: linux-cifs@vger.kernel.org
Cc: stable@vger.kernel.org
Signed-off-by: Steve French <stfrench@microsoft.com>
Today whenever we deal with a file, in addition to holding
a reference on the dentry, we also get a reference on the
superblock. This happens in two cases:
1. when a new cinode is allocated
2. when an oplock break is being processed
The reasoning for holding the superblock ref was to make sure
that when umount happens, if there are users of inodes and
dentries, it does not try to clean them up and wait for the
last ref to superblock to be dropped by last of such users.
But the side effect of doing that is that umount silently drops
a ref on the superblock and we could have deferred closes and
lease breaks still holding these refs.
Ideally, we should ensure that all of these users of inodes and
dentries are cleaned up at the time of umount, which is what this
code is doing.
This code change allows these code paths to use a ref on the
dentry (and hence the inode). That way, umount is
ensured to clean up SMB client resources when it's the last
ref on the superblock (For ex: when same objects are shared).
The code change also moves the call to close all the files in
deferred close list to the umount code path. It also waits for
oplock_break workers to be flushed before calling
kill_anon_super (which eventually frees up those objects).
Fixes: 24261fc23d ("cifs: delay super block destruction until all cifsFileInfo objects are gone")
Fixes: 705c79101c ("smb: client: fix use-after-free in cifs_oplock_break")
Cc: <stable@vger.kernel.org>
Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
To prevent timing attacks, MAC comparisons need to be constant-time.
Replace the memcmp() with the correct function, crypto_memneq().
Fixes: 1da177e4c3 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
SMB311_posix_query_info() is currently unused, but it may still be used in
some stable versions, so these changes are submitted as a separate patch.
Use `sizeof(struct smb311_posix_qinfo)` instead of sizeof its pointer,
so the allocated buffer matches the actual struct size.
Fixes: b1bc1874b8 ("smb311: Add support for SMB311 query info (non-compounded)")
Reported-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: ZhangGuoDong <zhangguodong@kylinos.cn>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Steve French <stfrench@microsoft.com>
Use `sizeof(struct smb311_posix_qinfo)` instead of sizeof its pointer,
so the allocated buffer matches the actual struct size.
Fixes: 6a5f6592a0 ("SMB311: Add support for query info using posix extensions (level 100)")
Reported-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: ZhangGuoDong <zhangguodong@kylinos.cn>
Reviewed-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
Signed-off-by: Steve French <stfrench@microsoft.com>
Pull smb client fixes from Steve French:
- Two multichannel fixes
- Locking fix for superblock flags
- Fix to remove debug message that could log password
- Cleanup fix for setting credentials
* tag 'v7.0rc1-smb3-client-fixes' of git://git.samba.org/sfrench/cifs-2.6:
smb: client: Use snprintf in cifs_set_cifscreds
smb: client: Don't log plaintext credentials in cifs_set_cifscreds
smb: client: fix broken multichannel with krb5+signing
smb: client: use atomic_t for mnt_cifs_flags
smb: client: fix cifs_pick_channel when channels are equally loaded
Replace unbounded sprintf() calls with the safer snprintf(). Avoid using
magic numbers and use strlen() to calculate the key descriptor buffer
size. Save the size in a local variable and reuse it for the bounded
snprintf() calls. Remove CIFSCREDS_DESC_SIZE.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
When debug logging is enabled, cifs_set_cifscreds() logs the key
payload and exposes the plaintext username and password. Remove the
debug log to avoid exposing credentials.
Fixes: 8a8798a5ff ("cifs: fetch credentials out of keyring for non-krb5 auth multiuser mounts")
Cc: stable@vger.kernel.org
Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: Steve French <stfrench@microsoft.com>
When mounting a share with 'multichannel,max_channels=n,sec=krb5i',
the client was duplicating signing key for all secondary channels,
thus making the server fail all commands sent from secondary channels
due to bad signatures.
Every channel has its own signing key, so when establishing a new
channel with krb5 auth, make sure to use the new session key as the
derived key to generate channel's signing key in SMB2_auth_kerberos().
Repro:
$ mount.cifs //srv/share /mnt -o multichannel,max_channels=4,sec=krb5i
$ sleep 5
$ umount /mnt
$ dmesg
...
CIFS: VFS: sign fail cmd 0x5 message id 0x2
CIFS: VFS: \\srv SMB signature verification returned error = -13
CIFS: VFS: sign fail cmd 0x5 message id 0x2
CIFS: VFS: \\srv SMB signature verification returned error = -13
CIFS: VFS: sign fail cmd 0x4 message id 0x2
CIFS: VFS: \\srv SMB signature verification returned error = -13
Reported-by: Xiaoli Feng <xifeng@redhat.com>
Reviewed-by: Enzo Matsumiya <ematsumiya@suse.de>
Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Cc: David Howells <dhowells@redhat.com>
Cc: linux-cifs@vger.kernel.org
Cc: stable@vger.kernel.org
Signed-off-by: Steve French <stfrench@microsoft.com>
Use atomic_t for cifs_sb_info::mnt_cifs_flags as it's currently
accessed locklessly and may be changed concurrently in mount/remount
and reconnect paths.
Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Reviewed-by: David Howells <dhowells@redhat.com>
Cc: linux-cifs@vger.kernel.org
Signed-off-by: Steve French <stfrench@microsoft.com>
smb_direct_prepare_negotiation() casts an unsigned __u32 value
from sp->max_recv_size and req->preferred_send_size to a signed
int before computing min_t(int, ...). A maliciously provided
preferred_send_size of 0x80000000 will return as smaller than
max_recv_size, and then be used to set the maximum allowed
alowed receive size for the next message.
By sending a second message with a large value (>1420 bytes)
the attacker can then achieve a heap buffer overflow.
This fix replaces min_t(int, ...) with min_t(u32)
Fixes: 0626e6641f ("cifsd: add server handler for central processing and tranport layers")
Signed-off-by: Nicholas Carlini <nicholas@carlini.com>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
Acked-by: Stefan Metzmacher <metze@samba.org>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
cifs_pick_channel uses (start % chan_count) when channels are equally
loaded, but that can return a channel that failed the eligibility
checks.
Drop the fallback and return the scan-selected channel instead. If none
is eligible, keep the existing behavior of using the primary channel.
Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com>
Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.org>
Acked-by: Meetakshi Setiya <msetiya@microsoft.com>
Reviewed-by: Shyam Prasad N <sprasad@microsoft.com>
Cc: stable@vger.kernel.org
Signed-off-by: Steve French <stfrench@microsoft.com>
This converts some of the visually simpler cases that have been split
over multiple lines. I only did the ones that are easy to verify the
resulting diff by having just that final GFP_KERNEL argument on the next
line.
Somebody should probably do a proper coccinelle script for this, but for
me the trivial script actually resulted in an assertion failure in the
middle of the script. I probably had made it a bit _too_ trivial.
So after fighting that far a while I decided to just do some of the
syntactically simpler cases with variations of the previous 'sed'
scripts.
The more syntactically complex multi-line cases would mostly really want
whitespace cleanup anyway.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This is the exact same thing as the 'alloc_obj()' version, only much
smaller because there are a lot fewer users of the *alloc_flex()
interface.
As with alloc_obj() version, this was done entirely with mindless brute
force, using the same script, except using 'flex' in the pattern rather
than 'objs*'.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This was done entirely with mindless brute force, using
git grep -l '\<k[vmz]*alloc_objs*(.*, GFP_KERNEL)' |
xargs sed -i 's/\(alloc_objs*(.*\), GFP_KERNEL)/\1)/'
to convert the new alloc_obj() users that had a simple GFP_KERNEL
argument to just drop that argument.
Note that due to the extreme simplicity of the scripting, any slightly
more complex cases spread over multiple lines would not be triggered:
they definitely exist, but this covers the vast bulk of the cases, and
the resulting diff is also then easier to check automatically.
For the same reason the 'flex' versions will be done as a separate
conversion.
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Pull kmalloc_obj conversion from Kees Cook:
"This does the tree-wide conversion to kmalloc_obj() and friends using
coccinelle, with a subsequent small manual cleanup of whitespace
alignment that coccinelle does not handle.
This uncovered a clang bug in __builtin_counted_by_ref(), so the
conversion is preceded by disabling that for current versions of
clang. The imminent clang 22.1 release has the fix.
I've done allmodconfig build tests for x86_64, arm64, i386, and arm. I
did defconfig builds for alpha, m68k, mips, parisc, powerpc, riscv,
s390, sparc, sh, arc, csky, xtensa, hexagon, and openrisc"
* tag 'kmalloc_obj-treewide-v7.0-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux:
kmalloc_obj: Clean up after treewide replacements
treewide: Replace kmalloc with kmalloc_obj for non-scalar types
compiler_types: Disable __builtin_counted_by_ref for Clang
Pull smb server fixes from Steve French:
"Two small fixes:
- fix potential deadlock
- minor cleanup"
* tag 'v7.0-rc-part2-ksmbd-server-fixes' of git://git.samba.org/ksmbd:
ksmbd: call ksmbd_vfs_kern_path_end_removing() on some error paths
smb: server: Remove duplicate include of misc.h
This is the result of running the Coccinelle script from
scripts/coccinelle/api/kmalloc_objs.cocci. The script is designed to
avoid scalar types (which need careful case-by-case checking), and
instead replace kmalloc-family calls that allocate struct or union
object instances:
Single allocations: kmalloc(sizeof(TYPE), ...)
are replaced with: kmalloc_obj(TYPE, ...)
Array allocations: kmalloc_array(COUNT, sizeof(TYPE), ...)
are replaced with: kmalloc_objs(TYPE, COUNT, ...)
Flex array allocations: kmalloc(struct_size(PTR, FAM, COUNT), ...)
are replaced with: kmalloc_flex(*PTR, FAM, COUNT, ...)
(where TYPE may also be *VAR)
The resulting allocations no longer return "void *", instead returning
"TYPE *".
Signed-off-by: Kees Cook <kees@kernel.org>
Pull smb client fixes from Steve French:
- Fix three potential double free vulnerabilities
- Fix data corruption due to racy lease checks
- Enforce SMB1 signing verification checks
- Fix invalid mount option parsing
- Remove unneeded tracepoint
- Various minor error code corrections
- Minor cleanup
* tag 'v7.0-rc-part2-smb3-client-fixes' of git://git.samba.org/sfrench/cifs-2.6:
smb: client: terminate session upon failed client required signing
cifs: some missing initializations on replay
cifs: remove unnecessary tracing after put tcon
cifs: update internal module version number
smb: client: fix data corruption due to racy lease checks
smb/client: move NT_STATUS_MORE_ENTRIES
smb/client: rename to NT_ERROR_INVALID_DATATYPE
smb/client: rename to NT_STATUS_SOME_NOT_MAPPED
smb/client: map NT_STATUS_PRIVILEGE_NOT_HELD
smb/client: map NT_STATUS_MORE_PROCESSING_REQUIRED
smb/client: map NT_STATUS_BUFFER_OVERFLOW
smb/client: map NT_STATUS_NOTIFY_ENUM_DIR
cifs: SMB1 split: Remove duplicate include of cifs_debug.h
smb: client: fix regression with mount options parsing
Pull more misc vfs updates from Christian Brauner:
"Features:
- Optimize close_range() from O(range size) to O(active FDs) by using
find_next_bit() on the open_fds bitmap instead of linearly scanning
the entire requested range. This is a significant improvement for
large-range close operations on sparse file descriptor tables.
- Add FS_XFLAG_VERITY file attribute for fs-verity files, retrievable
via FS_IOC_FSGETXATTR and file_getattr(). The flag is read-only.
Add tracepoints for fs-verity enable and verify operations,
replacing the previously removed debug printk's.
- Prevent nfsd from exporting special kernel filesystems like pidfs
and nsfs. These filesystems have custom ->open() and ->permission()
export methods that are designed for open_by_handle_at(2) only and
are incompatible with nfsd. Update the exportfs documentation
accordingly.
Fixes:
- Fix KMSAN uninit-value in ovl_fill_real() where strcmp() was used
on a non-null-terminated decrypted directory entry name from
fscrypt. This triggered on encrypted lower layers when the
decrypted name buffer contained uninitialized tail data.
The fix also adds VFS-level name_is_dot(), name_is_dotdot(), and
name_is_dot_dotdot() helpers, replacing various open-coded "." and
".." checks across the tree.
- Fix read-only fsflags not being reset together with xflags in
vfs_fileattr_set(). Currently harmless since no read-only xflags
overlap with flags, but this would cause inconsistencies for any
future shared read-only flag
- Return -EREMOTE instead of -ESRCH from PIDFD_GET_INFO when the
target process is in a different pid namespace. This lets userspace
distinguish "process exited" from "process in another namespace",
matching glibc's pidfd_getpid() behavior
Cleanups:
- Use C-string literals in the Rust seq_file bindings, replacing the
kernel::c_str!() macro (available since Rust 1.77)
- Fix typo in d_walk_ret enum comment, add porting notes for the
readlink_copy() calling convention change"
* tag 'vfs-7.0-rc1.misc.2' of git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs:
fs: add porting notes about readlink_copy()
pidfs: return -EREMOTE when PIDFD_GET_INFO is called on another ns
nfsd: do not allow exporting of special kernel filesystems
exportfs: clarify the documentation of open()/permission() expotrfs ops
fsverity: add tracepoints
fs: add FS_XFLAG_VERITY for fs-verity files
rust: seq_file: replace `kernel::c_str!` with C-Strings
fs: dcache: fix typo in enum d_walk_ret comment
ovl: use name_is_dot* helpers in readdir code
fs: add helpers name_is_dot{,dot,_dotdot}
ovl: Fix uninit-value in ovl_fill_real
fs: reset read-only fsflags together with xflags
fs/file: optimize close_range() complexity from O(N) to O(Sparse)
Currently, when smb signature verification fails, the behaviour is to log
the failure without any action to terminate the session.
Call cifs_reconnect() when client required signature verification fails.
Otherwise, log the error without reconnecting.
Signed-off-by: Aaditya Kansal <aadityakansal390@gmail.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
In several places in the code, we have a label to signify
the start of the code where a request can be replayed if
necessary. However, some of these places were missing the
necessary reinitializations of certain local variables
before replay.
This change makes sure that these variables get initialized
after the label.
Cc: stable@vger.kernel.org
Reported-by: Yuchan Nam <entropy1110@gmail.com>
Tested-by: Yuchan Nam <entropy1110@gmail.com>
Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
