-----BEGIN PGP SIGNATURE-----
iQGzBAABCgAdFiEE6fsu8pdIjtWE/DpLiiy9cAdyT1EFAmdmM9YACgkQiiy9cAdy
T1FUVAv9GkoUno9PxDbaBEnjturb5b4EavqUKbS1QN5/RrE4ng82goUNQ7mZ7Il2
PBuTnTBfeKKg6BbwmOhJpbWKtTQ3wty1H+s9o47U5cdbTGKc2wDrHvPaDK4D4EG4
mj32OhjoqOKlpGzzNOxME3aPK7wvqz9kLgUPl9i5NovPhK8P/gZrZx1urYJVodYU
dvE+/ZQziDMwYCOXc4qjl+wWHFm1yo5cwLO4fpcRJBTP7oIeUmT+kRPdLeW+XJHh
wpR3K+3JwlcF25wf0au43jWLvhG6DuRwYzPGMROyVkVmC9nlEdmyvS5GERmdYSXa
5IQOzuAdu4b4DJLsM5PyG275C/3FZna+Whf7nwX+qjLyTR7PeuzraWuV/3zYkYWG
bRbgMBzDvWyGXkTkn2eZFsnjB2NCXXG8iJ7sU5QXvp5ElMXhVdOCfObJti72UYw4
yxu/ms3yQ2NU7ahZtisoWxJLkwk2xSoV1h9kLQul0AkTEQwCdglIvHxv31v02OPj
/mu2DDoW
=556n
-----END PGP SIGNATURE-----
Merge tag '6.13-rc3-SMB3-client-fixes' of git://git.samba.org/sfrench/cifs-2.6
Pull smb client fixes from Steve French:
- fix regression in display of write stats
- fix rmmod failure with network namespaces
- two minor cleanups
* tag '6.13-rc3-SMB3-client-fixes' of git://git.samba.org/sfrench/cifs-2.6:
smb: fix bytes written value in /proc/fs/cifs/Stats
smb: client: fix TCP timers deadlock after rmmod
smb: client: Deduplicate "select NETFS_SUPPORT" in Kconfig
smb: use macros instead of constants for leasekey size and default cifsattrs value
With recent netfs apis changes, the bytes written
value was not getting updated in /proc/fs/cifs/Stats.
Fix this by updating tcon->bytes in write operations.
Fixes: 3ee1a1fc3981 ("cifs: Cut over to using netfslib")
Signed-off-by: Bharath SM <bharathsm@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Commit ef7134c7fc48 ("smb: client: Fix use-after-free of network namespace.")
fixed a netns UAF by manually enabled socket refcounting
(sk->sk_net_refcnt=1 and sock_inuse_add(net, 1)).
The reason the patch worked for that bug was because we now hold
references to the netns (get_net_track() gets a ref internally)
and they're properly released (internally, on __sk_destruct()),
but only because sk->sk_net_refcnt was set.
Problem:
(this happens regardless of CONFIG_NET_NS_REFCNT_TRACKER and regardless
if init_net or other)
Setting sk->sk_net_refcnt=1 *manually* and *after* socket creation is not
only out of cifs scope, but also technically wrong -- it's set conditionally
based on user (=1) vs kernel (=0) sockets. And net/ implementations
seem to base their user vs kernel space operations on it.
e.g. upon TCP socket close, the TCP timers are not cleared because
sk->sk_net_refcnt=1:
(cf. commit 151c9c724d05 ("tcp: properly terminate timers for kernel sockets"))
net/ipv4/tcp.c:
void tcp_close(struct sock *sk, long timeout)
{
lock_sock(sk);
__tcp_close(sk, timeout);
release_sock(sk);
if (!sk->sk_net_refcnt)
inet_csk_clear_xmit_timers_sync(sk);
sock_put(sk);
}
Which will throw a lockdep warning and then, as expected, deadlock on
tcp_write_timer().
A way to reproduce this is by running the reproducer from ef7134c7fc48
and then 'rmmod cifs'. A few seconds later, the deadlock/lockdep
warning shows up.
Fix:
We shouldn't mess with socket internals ourselves, so do not set
sk_net_refcnt manually.
Also change __sock_create() to sock_create_kern() for explicitness.
As for non-init_net network namespaces, we deal with it the best way
we can -- hold an extra netns reference for server->ssocket and drop it
when it's released. This ensures that the netns still exists whenever
we need to create/destroy server->ssocket, but is not directly tied to
it.
Fixes: ef7134c7fc48 ("smb: client: Fix use-after-free of network namespace.")
Cc: stable@vger.kernel.org
Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de>
Signed-off-by: Steve French <stfrench@microsoft.com>
Repeating automatically selected options in Kconfig files is redundant, so
let's delete repeated "select NETFS_SUPPORT" that was added accidentally.
Fixes: 69c3c023af25 ("cifs: Implement netfslib hooks")
Signed-off-by: Dragan Simic <dsimic@manjaro.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Replace default hardcoded value for cifsAttrs with ATTR_ARCHIVE macro
Use SMB2_LEASE_KEY_SIZE macro for leasekey size in smb2_lease_break
Signed-off-by: Bharath SM <bharathsm@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
If client send parallel smb2 negotiate request on same connection,
ksmbd_conn can be racy. smb2 negotiate handling that are not
performance-related can be serialized with conn lock.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Since commit 0a77d947f599 ("ksmbd: check outstanding simultaneous SMB
operations"), ksmbd enforces a maximum number of simultaneous operations
for a connection. The problem is that reaching the limit causes ksmbd to
close the socket, and the client has no indication that it should have
slowed down.
This behaviour can be reproduced by setting "smb2 max credits = 128" (or
lower), and transferring a large file (25GB).
smbclient fails as below:
$ smbclient //192.168.1.254/testshare -U user%pass
smb: \> put file.bin
cli_push returned NT_STATUS_USER_SESSION_DELETED
putting file file.bin as \file.bin smb2cli_req_compound_submit:
Insufficient credits. 0 available, 1 needed
NT_STATUS_INTERNAL_ERROR closing remote file \file.bin
smb: \> smb2cli_req_compound_submit: Insufficient credits. 0 available,
1 needed
Windows clients fail with 0x8007003b (with smaller files even).
Fix this by delaying reading from the socket until there's room to
allocate a request. This effectively applies backpressure on the client,
so the transfer completes, albeit at a slower rate.
Fixes: 0a77d947f599 ("ksmbd: check outstanding simultaneous SMB operations")
Signed-off-by: Marios Makassikis <mmakassikis@freebox.fr>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
This changes the semantics of req_running to count all in-flight
requests on a given connection, rather than the number of elements
in the conn->request list. The latter is used only in smb2_cancel,
and the counter is not used
Signed-off-by: Marios Makassikis <mmakassikis@freebox.fr>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
-----BEGIN PGP SIGNATURE-----
iQGzBAABCgAdFiEE6fsu8pdIjtWE/DpLiiy9cAdyT1EFAmdcyoMACgkQiiy9cAdy
T1F7UQwAnMc2zmFboxx6yiVmGOyMmeBY0hvCktByu32i+7nDa/OduISJxyFpCjId
xB2RxumAnm538Kf0tNAbhZgjvNzRucQIMy8ZEMBNEEnYNfGhP75xSkAEq1/1KxAJ
7TzqAYjFdYBR6uaq6dHFSAiLwD4aX3YdCwCLEWxQNDG2FI6DLdHDALxl70DdwLcr
+xrfzXpqGPloMqVj0FtDCeZ3WIEiDWt5r3m7YA23fm2YkuozWhWXzdRb8n2grQGh
8bzy/dlx+JBS2BzfgP8UqKwrPtldlaPwK/SDK8/R5mW1hAWQ7OWz73f92d+4aFrd
W1e1fKYv9wXwPDB3t2DpTFWZ659ZVKnk5kZOnlHdI8sUjH+h0BDpZ+8dEZIKeafN
jJxQn8sPb23u0+eH//CcbwDyanLCRSPdHRyfCRdVIK9pYT2hOcT58rtlT8NuTePv
+Tttce2H38FPHvi8NVkRRcuZQkKNDak1MSykX3F8kI7MWsVt+PDIeZ/P72/SCKIL
Hyj7+I1V
=SLxt
-----END PGP SIGNATURE-----
Merge tag '6.13-rc2-smb3-client-fixes' of git://git.samba.org/sfrench/cifs-2.6
Pull smb client fixes from Steve French:
- fix rmmod leak
- two minor cleanups
- fix for unlink/rename with pending i/o
* tag '6.13-rc2-smb3-client-fixes' of git://git.samba.org/sfrench/cifs-2.6:
smb: client: destroy cfid_put_wq on module exit
cifs: Use str_yes_no() helper in cifs_ses_add_channel()
cifs: Fix rmdir failure due to ongoing I/O on deleted file
smb3: fix compiler warning in reparse code
Remove hard-coded strings by using the str_yes_no() helper function.
Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
Signed-off-by: Steve French <stfrench@microsoft.com>
The cifs_io_request struct (a wrapper around netfs_io_request) holds open
the file on the server, even beyond the local Linux file being closed.
This can cause problems with Windows-based filesystems as the file's name
still exists after deletion until the file is closed, preventing the parent
directory from being removed and causing spurious test failures in xfstests
due to inability to remove a directory. The symptom looks something like
this in the test output:
rm: cannot remove '/mnt/scratch/test/p0/d3': Directory not empty
rm: cannot remove '/mnt/scratch/test/p1/dc/dae': Directory not empty
Fix this by waiting in unlink and rename for any outstanding I/O requests
to be completed on the target file before removing that file.
Note that this doesn't prevent Linux from trying to start new requests
after deletion if it still has the file open locally - something that's
perfectly acceptable on a UNIX system.
Note also that whilst I've marked this as fixing the commit to make cifs
use netfslib, I don't know that it won't occur before that.
Fixes: 3ee1a1fc3981 ("cifs: Cut over to using netfslib")
Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
cc: Jeff Layton <jlayton@kernel.org>
cc: linux-cifs@vger.kernel.org
cc: netfs@lists.linux.dev
cc: linux-fsdevel@vger.kernel.org
Signed-off-by: Steve French <stfrench@microsoft.com>
Increment the session reference count within the lock for lookup to avoid
racy issue with session expire.
Cc: stable@vger.kernel.org
Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-25737
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Some file systems do not ensure that the single call of iterate_dir
reaches the end of the directory. For example, FUSE fetches entries from
a daemon using 4KB buffer and stops fetching if entries exceed the
buffer. And then an actor of caller, KSMBD, is used to fill the entries
from the buffer.
Thus, pattern searching on FUSE, files located after the 4KB could not
be found and STATUS_NO_SUCH_FILE was returned.
Signed-off-by: Hobin Woo <hobin.woo@samsung.com>
Reviewed-by: Sungjong Seo <sj1557.seo@samsung.com>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>
Tested-by: Yoonho Shin <yoonho.shin@samsung.com>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
utf8s_to_utf16s() specifies pwcs as a wchar_t pointer (whether big endian
or little endian is passed in as an additional parm), so to remove a
distracting compile warning it needs to be cast as (wchar_t *) in
parse_reparse_wsl_symlink() as done by other callers.
Fixes: 06a7adf318a3 ("cifs: Add support for parsing WSL-style symlinks")
Reviewed-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
-----BEGIN PGP SIGNATURE-----
iQGzBAABCgAdFiEE6fsu8pdIjtWE/DpLiiy9cAdyT1EFAmdUyQMACgkQiiy9cAdy
T1FAXAv+KNV1F2qdANnZc9sj9xSgL0uXYvUqamVeEJFmFQMb0I6C/AzBGdcxtWak
gXhCrFmlhlvQSan5Y3RGNj3ggr4UmW/+tEOEANOaZVn52T0IFrYf+wIX1jdetGLx
LL1mKodca0MfGJXQYIwqsIcAjpzCPK5Qogro9Ve8Btg+iTve350LRxDQGyCMB8uL
8kFse7eVHnPchmXcMn1O3CVTu/Iu/ZyZq4gkuUDAhP/xNM3ShL036682J3WB2FHI
VVEjIQYyIeUbxQH9IuGgRuL0NWxjdim02o5DST04klpJwYUuf5ogmNkALYfBSFrg
aKXqpUZ1ds2LEnR9++azUwnoft8fSRB+TNvLRhN1K5PqEA4nB+XJn/ZZ9OTGn92y
PuFxXk0tDxbrCuPpQYD0eFeBE2wKWCWiwtGGU6zfqRwqCVmnRdZg3bAtvGOfd9oR
PUPZtwlsCpl7a3m/cPhmkAe0WNnb9HU03lMIUVfTgGNa8sycIuNGrk7MNNI3u3FJ
2/jHA71o
=71WB
-----END PGP SIGNATURE-----
Merge tag '6.13-rc1-smb3-client-fixes' of git://git.samba.org/sfrench/cifs-2.6
Pull smb client fixes from Steve French:
- DFS fix (for race with tree disconnect and dfs cache worker)
- Four fixes for SMB3.1.1 posix extensions:
- improve special file support e.g. to Samba, retrieving the file
type earlier
- reduce roundtrips (e.g. on ls -l, in some cases)
* tag '6.13-rc1-smb3-client-fixes' of git://git.samba.org/sfrench/cifs-2.6:
smb: client: fix potential race in cifs_put_tcon()
smb3.1.1: fix posix mounts to older servers
fs/smb/client: cifs_prime_dcache() for SMB3 POSIX reparse points
fs/smb/client: Implement new SMB3 POSIX type
fs/smb/client: avoid querying SMB2_OP_QUERY_WSL_EA for SMB3 POSIX
dfs_cache_refresh() delayed worker could race with cifs_put_tcon(), so
make sure to call list_replace_init() on @tcon->dfs_ses_list after
kworker is cancelled or finished.
Fixes: 4f42a8b54b5c ("smb: client: fix DFS interlink failover")
Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Some servers which implement the SMB3.1.1 POSIX extensions did not
set the file type in the mode in the infolevel 100 response.
With the recent changes for checking the file type via the mode field,
this can cause the root directory to be reported incorrectly and
mounts (e.g. to ksmbd) to fail.
Fixes: 6a832bc8bbb2 ("fs/smb/client: Implement new SMB3 POSIX type")
Cc: stable@vger.kernel.org
Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
Cc: Ralph Boehme <slow@samba.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
The aux_payload_buf allocation in SMB2 read is performed without ensuring
alignment, which could result in out-of-bounds (OOB) reads during
cryptographic operations such as crypto_xor or ghash. This patch aligns
the allocation of aux_payload_buf to prevent these issues.
(Note that to add this patch to stable would require modifications due
to recent patch "ksmbd: use __GFP_RETRY_MAYFAIL")
Signed-off-by: Norbert Szetei <norbert@doyensec.com>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Spares an extra revalidation request
Cc: stable@vger.kernel.org
Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
Signed-off-by: Ralph Boehme <slow@samba.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Avoid extra roundtrip
Cc: stable@vger.kernel.org
Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
Signed-off-by: Ralph Boehme <slow@samba.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
An offset from client could be a negative value, It could allows
to write data outside the bounds of the allocated buffer.
Note that this issue is coming when setting
'vfs objects = streams_xattr parameter' in ksmbd.conf.
Cc: stable@vger.kernel.org # v5.15+
Reported-by: Jordy Zomer <jordyzomer@google.com>
Signed-off-by: Jordy Zomer <jordyzomer@google.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
An offset from client could be a negative value, It could lead
to an out-of-bounds read from the stream_buf.
Note that this issue is coming when setting
'vfs objects = streams_xattr parameter' in ksmbd.conf.
Cc: stable@vger.kernel.org # v5.15+
Reported-by: Jordy Zomer <jordyzomer@google.com>
Signed-off-by: Jordy Zomer <jordyzomer@google.com>
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
GCC 15 introduces -Werror=unterminated-string-initialization by default,
this results in the following build error
fs/smb/server/smb_common.c:21:35: error: initializer-string for array of 'char' is too long [-Werror=unterminated-string-ini
tialization]
21 | static const char basechars[43] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ_-!@#$%";
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
To this we are replacing char basechars[43] with a character pointer
and then using strlen to get the length.
Signed-off-by: Brahmajit Das <brahmajit.xyz@gmail.com>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
-----BEGIN PGP SIGNATURE-----
iQGzBAABCgAdFiEE6fsu8pdIjtWE/DpLiiy9cAdyT1EFAmdKXd4ACgkQiiy9cAdy
T1FjCwv/f3lb79FaVhv18LAvayqoj+iWaZg6yYepZxp2hyMTm59i5I1t7Ar92VES
2+A3XmdeK5U6y9JjMFYHY4B720JumBDTJ/FPgUPZtZqCHSnYZCm5MUQtH85VSisf
/4Ua5kv5tzCuu9UvoCp3UFcsbCCrXH8JYTBGR2EKfIHqN7ae1F80pLgBxEnBuuw9
rRx0RKxKb9CkYB8GDOHi60hF03DRlXdMa7I/aJ6dtyhZkzoDgc0PqndFQuwLMY07
f09rnXnDvHqqC+SFHprqE6V8uWl77IX6lzZ4PCz52dsm6Y/BKD22fBQA3w4zz/0s
nMvxbEaxcealSq08pTn7zWVi0tw7Ku35c0plXCtjzS4UmcouMdwI6SP986IqEH+C
0la/mDnPk36EQYdd1yYVYAcbW9VrmiYs23PvKfH4Hj6JvQeZ872RnMYZPPl+jJh6
Gazdn3yWRiKRuTcO42eC+JKaoJKG/JBqkd74WBe85Q0yVNf2m0CJEICfAcWDlK/Z
fIl3WxjN
=dfo/
-----END PGP SIGNATURE-----
Merge tag '6.13-rc-part2-smb3-client-fixes' of git://git.samba.org/sfrench/cifs-2.6
Pull smb client updates from Steve French:
- directory lease fixes
- password rotation fixes
- reconnect fix
- fix for SMB3.02 mounts
- DFS (global namespace) fixes
- fixes for special file handling (most relating to better handling
various types of symlinks)
- two minor cleanups
* tag '6.13-rc-part2-smb3-client-fixes' of git://git.samba.org/sfrench/cifs-2.6: (22 commits)
cifs: update internal version number
cifs: unlock on error in smb3_reconfigure()
cifs: during remount, make sure passwords are in sync
cifs: support mounting with alternate password to allow password rotation
smb: Initialize cfid->tcon before performing network ops
smb: During unmount, ensure all cached dir instances drop their dentry
smb: client: fix noisy message when mounting shares
smb: client: don't try following DFS links in cifs_tree_connect()
smb: client: allow reconnect when sending ioctl
smb: client: get rid of @nlsc param in cifs_tree_connect()
smb: client: allow more DFS referrals to be cached
cifs: Fix parsing reparse point with native symlink in SMB1 non-UNICODE session
cifs: Validate content of WSL reparse point buffers
cifs: Improve guard for excluding $LXDEV xattr
cifs: Add support for parsing WSL-style symlinks
cifs: Validate content of native symlink
cifs: Fix parsing native symlinks relative to the export
smb: client: fix NULL ptr deref in crypto_aead_setkey()
Update misleading comment in cifs_chan_update_iface
smb: client: change return value in open_cached_dir_by_dentry() if !cfids
...
-----BEGIN PGP SIGNATURE-----
iQGzBAABCgAdFiEE6fsu8pdIjtWE/DpLiiy9cAdyT1EFAmdImvYACgkQiiy9cAdy
T1FJ/gv+PF5zu1okBFZeJZRw0Fz4FZbHpkO3pnrGfW7guQornhveA3Jy69EnnKDh
R4UUVWJYAtNRC6GpV6idVkCrrQmFp87RI7sqPsVq4gw1O4/AUW8O0/mvzqROmqb1
AGbD2+J3QLOItvmGLqvHJ6MNOlYZQPNLz7k03eMDf3JUClsrTbKUoJNBnoAr+6ZQ
fHImvQ59gwICTVB/C/cLOK7s7EoSYo1ID07nC8CfyaRmTjLAFl+vJCedm+GECBBU
i/pRUQG8QRn0iNxV57u3UrKBR9oFMlq6lMKJkXFEDJ5w/HAtwO+grl6TM4KT2Q8J
tsINb3bMglNjleNQSzTFNsyrkXDmobDbCVQmVrDe60sixdcA5vZcU2dJjWA8qYpx
glAh86gSIw8YylNFv9m8aIfMWOk24fiPB4F8VD41THT1PA31uIyo1PBu+h3lNEnZ
3Mq8taMSEvKklK89g8O1rJwaD/8JpdtSjm3AgrqWxUZDrT0GeIeTq9l4v/Gbyzmz
2JqKasRi
=q5KR
-----END PGP SIGNATURE-----
Merge tag '6.13-rc-ksmbd-server-fixes' of git://git.samba.org/ksmbd
Pull smb server updates from Steve French:
- fix use after free due to race in ksmd workqueue handler
- debugging improvements
- fix incorrectly formatted response when client attempts SMB1
- improve memory allocation to reduce chance of OOM
- improve delays between retries when killing sessions
* tag '6.13-rc-ksmbd-server-fixes' of git://git.samba.org/ksmbd:
ksmbd: fix use-after-free in SMB request handling
ksmbd: add debug print for pending request during server shutdown
ksmbd: add netdev-up/down event debug print
ksmbd: add debug prints to know what smb2 requests were received
ksmbd: add debug print for rdma capable
ksmbd: use msleep instaed of schedule_timeout_interruptible()
ksmbd: use __GFP_RETRY_MAYFAIL
ksmbd: fix malformed unsupported smb1 negotiate response
Unlock before returning if smb3_sync_session_ctx_passwords() fails.
Fixes: 7e654ab7da03 ("cifs: during remount, make sure passwords are in sync")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
Reviewed-by: Bharath SM <bharathsm@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
This fixes scenarios where remount can overwrite the only currently
working password, breaking reconnect.
We recently introduced a password2 field in both ses and ctx structs.
This was done so as to allow the client to rotate passwords for a mount
without any downtime. However, when the client transparently handles
password rotation, it can swap the values of the two password fields
in the ses struct, but not in smb3_fs_context struct that hangs off
cifs_sb. This can lead to a situation where a remount unintentionally
overwrites a working password in the ses struct.
In order to fix this, we first get the passwords in ctx struct
in-sync with ses struct, before replacing them with what the passwords
that could be passed as a part of remount.
Also, in order to avoid race condition between smb2_reconnect and
smb3_reconfigure, we make sure to lock session_mutex before changing
password and password2 fields of the ses structure.
Fixes: 35f834265e0d ("smb3: fix broken reconnect when password changing on the server by allowing password rotation")
Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
Signed-off-by: Meetakshi Setiya <msetiya@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Fixes the case for example where the password specified on mount is a
recently expired password, but password2 is valid. Without this patch
this mount scenario would fail.
This patch introduces the following changes to support password rotation on
mount:
1. If an existing session is not found and the new session setup results in
EACCES, EKEYEXPIRED or EKEYREVOKED, swap password and password2 (if
available), and retry the mount.
2. To match the new mount with an existing session, add conditions to check
if a) password and password2 of the new mount and the existing session are
the same, or b) password of the new mount is the same as the password2 of
the existing session, and password2 of the new mount is the same as the
password of the existing session.
3. If an existing session is found, but needs reconnect, retry the session
setup after swapping password and password2 (if available), in case the
previous attempt results in EACCES, EKEYEXPIRED or EKEYREVOKED.
Cc: stable@vger.kernel.org
Signed-off-by: Meetakshi Setiya <msetiya@microsoft.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Avoid leaking a tcon ref when a lease break races with opening the
cached directory. Processing the leak break might take a reference to
the tcon in cached_dir_lease_break() and then fail to release the ref in
cached_dir_offload_close, since cfid->tcon is still NULL.
Fixes: ebe98f1447bb ("cifs: enable caching of directories for which a lease is held")
Signed-off-by: Paul Aurich <paul@darkrain42.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
The unmount process (cifs_kill_sb() calling close_all_cached_dirs()) can
race with various cached directory operations, which ultimately results
in dentries not being dropped and these kernel BUGs:
BUG: Dentry ffff88814f37e358{i=1000000000080,n=/} still in use (2) [unmount of cifs cifs]
VFS: Busy inodes after unmount of cifs (cifs)
------------[ cut here ]------------
kernel BUG at fs/super.c:661!
This happens when a cfid is in the process of being cleaned up when, and
has been removed from the cfids->entries list, including:
- Receiving a lease break from the server
- Server reconnection triggers invalidate_all_cached_dirs(), which
removes all the cfids from the list
- The laundromat thread decides to expire an old cfid.
To solve these problems, dropping the dentry is done in queued work done
in a newly-added cfid_put_wq workqueue, and close_all_cached_dirs()
flushes that workqueue after it drops all the dentries of which it's
aware. This is a global workqueue (rather than scoped to a mount), but
the queued work is minimal.
The final cleanup work for cleaning up a cfid is performed via work
queued in the serverclose_wq workqueue; this is done separate from
dropping the dentries so that close_all_cached_dirs() doesn't block on
any server operations.
Both of these queued works expect to invoked with a cfid reference and
a tcon reference to avoid those objects from being freed while the work
is ongoing.
While we're here, add proper locking to close_all_cached_dirs(), and
locking around the freeing of cfid->dentry.
Fixes: ebe98f1447bb ("cifs: enable caching of directories for which a lease is held")
Cc: stable@vger.kernel.org
Signed-off-by: Paul Aurich <paul@darkrain42.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
When the client unconditionally attempts to get an DFS referral to
check if share is DFS, some servers may return different errors that
aren't handled in smb2_get_dfs_refer(), so the following will be
logged in dmesg:
CIFS: VFS: \\srv\IPC$ smb2_get_dfs_refer: ioctl error...
which can confuse some users while mounting an SMB share.
Fix this by logging such error with FYI.
Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
We can't properly support chasing DFS links in cifs_tree_connect()
because
(1) We don't support creating new sessions while we're reconnecting,
which would be required for DFS interlinks.
(2) ->is_path_accessible() can't be called from cifs_tree_connect()
as it would deadlock with smb2_reconnect(). This is required for
checking if new DFS target is a nested DFS link.
By unconditionally trying to get an DFS referral from new DFS target
isn't correct because if the new DFS target (interlink) is an DFS
standalone namespace, then we would end up getting -ELOOP and then
potentially leaving tcon disconnected.
Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
cifs_tree_connect() no longer uses ioctl, so allow sessions to be
reconnected when sending ioctls.
Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
We can access local_nls directly from @tcon->ses, so there is no need
to pass it as parameter in cifs_tree_connect().
Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
In some DFS setups, a single DFS share may contain hundreds of DFS
links and increasing the DFS cache to allow more referrals to be
cached improves DFS failover as the client will likely find a cached
DFS referral when reconnecting and then avoiding unnecessary remounts.
Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
A race condition exists between SMB request handling in
`ksmbd_conn_handler_loop()` and the freeing of `ksmbd_conn` in the
workqueue handler `handle_ksmbd_work()`. This leads to a UAF.
- KASAN: slab-use-after-free Read in handle_ksmbd_work
- KASAN: slab-use-after-free in rtlock_slowlock_locked
This race condition arises as follows:
- `ksmbd_conn_handler_loop()` waits for `conn->r_count` to reach zero:
`wait_event(conn->r_count_q, atomic_read(&conn->r_count) == 0);`
- Meanwhile, `handle_ksmbd_work()` decrements `conn->r_count` using
`atomic_dec_return(&conn->r_count)`, and if it reaches zero, calls
`ksmbd_conn_free()`, which frees `conn`.
- However, after `handle_ksmbd_work()` decrements `conn->r_count`,
it may still access `conn->r_count_q` in the following line:
`waitqueue_active(&conn->r_count_q)` or `wake_up(&conn->r_count_q)`
This results in a UAF, as `conn` has already been freed.
The discovery of this UAF can be referenced in the following PR for
syzkaller's support for SMB requests.
Link: https://github.com/google/syzkaller/pull/5524
Fixes: ee426bfb9d09 ("ksmbd: add refcnt to ksmbd_conn struct")
Cc: linux-cifs@vger.kernel.org
Cc: stable@vger.kernel.org # v6.6.55+, v6.10.14+, v6.11.3+
Cc: syzkaller@googlegroups.com
Signed-off-by: Yunseong Kim <yskelg@gmail.com>
Acked-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
We need to know how many pending requests are left at the end of server
shutdown. That means we need to know how long the server will wait
to process pending requests in case of a server shutdown.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Add netdev-up/down event debug print to find what netdev is connected or
disconnected.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Add debug prints to know what smb2 requests were received.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Add debug print to know if netdevice is RDMA-capable network adapter.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
use msleep instaed of schedule_timeout_interruptible()
to guarantee the task delays as expected.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Prefer to report ENOMEM rather than incur the oom for allocations in
ksmbd. __GFP_NORETRY could not achieve that, It would fail the allocations
just too easily. __GFP_RETRY_MAYFAIL will keep retrying the allocation
until there is no more progress and fail the allocation instead go OOM
and let the caller to deal with it.
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
SMB1 NT_TRANSACT_IOCTL/FSCTL_GET_REPARSE_POINT even in non-UNICODE mode
returns reparse buffer in UNICODE/UTF-16 format.
This is because FSCTL_GET_REPARSE_POINT is NT-based IOCTL which does not
distinguish between 8-bit non-UNICODE and 16-bit UNICODE modes and its path
buffers are always encoded in UTF-16.
This change fixes reading of native symlinks in SMB1 when UNICODE session
is not active.
Fixes: ed3e0a149b58 ("smb: client: implement ->query_reparse_point() for SMB1")
Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
WSL socket, fifo, char and block devices have empty reparse buffer.
Validate the length of the reparse buffer.
Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
$LXDEV xattr is for storing block/char device's major and minor number.
Change guard which excludes storing $LXDEV xattr to explicitly filter
everything except block and char device. Current guard is opposite, which
is currently correct but is less-safe. This change is required for adding
support for creating WSL-style symlinks as symlinks also do not use
device's major and minor numbers.
Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
Linux CIFS client currently does not implement readlink() for WSL-style
symlinks. It is only able to detect that file is of WSL-style symlink, but
is not able to read target symlink location.
Add this missing functionality and implement support for parsing content of
WSL-style symlink.
The important note is that symlink target location stored for WSL symlink
reparse point (IO_REPARSE_TAG_LX_SYMLINK) is in UTF-8 encoding instead of
UTF-16 (which is used in whole SMB protocol and also in all other symlink
styles). So for proper locale/cp support it is needed to do conversion from
UTF-8 to local_nls.
Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
