summaryrefslogtreecommitdiff
path: root/fs/cifs/cifssmb.c
Commit message (Collapse)AuthorAgeFilesLines
* cifs: use new enum for ses_statusShyam Prasad N2022-05-241-1/+1
| | | | | | | | | ses->status today shares statusEnum with server->tcpStatus. This has been confusing, and tcon->status has deviated to use a new enum. Follow suit and use new enum for ses_status as well. Signed-off-by: Shyam Prasad N <sprasad@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: fix signed integer overflow when fl_end is OFFSET_MAXPaulo Alcantara2022-05-191-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This fixes the following when running xfstests generic/504: [ 134.394698] CIFS: Attempting to mount \\win16.vm.test\Share [ 134.420905] CIFS: VFS: generate_smb3signingkey: dumping generated AES session keys [ 134.420911] CIFS: VFS: Session Id 05 00 00 00 00 c4 00 00 [ 134.420914] CIFS: VFS: Cipher type 1 [ 134.420917] CIFS: VFS: Session Key ea 0b d9 22 2e af 01 69 30 1b 15 74 bf 87 41 11 [ 134.420920] CIFS: VFS: Signing Key 59 28 43 5c f0 b6 b1 6f f5 7b 65 f2 9f 9e 58 7d [ 134.420923] CIFS: VFS: ServerIn Key eb aa 58 c8 95 01 9a f7 91 98 e4 fa bc d8 74 f1 [ 134.420926] CIFS: VFS: ServerOut Key 08 5b 21 e5 2e 4e 86 f6 05 c2 58 e0 af 53 83 e7 [ 134.771946] ================================================================================ [ 134.771953] UBSAN: signed-integer-overflow in fs/cifs/file.c:1706:19 [ 134.771957] 9223372036854775807 + 1 cannot be represented in type 'long long int' [ 134.771960] CPU: 4 PID: 2773 Comm: flock Not tainted 5.11.22 #1 [ 134.771964] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011 [ 134.771966] Call Trace: [ 134.771970] dump_stack+0x8d/0xb5 [ 134.771981] ubsan_epilogue+0x5/0x50 [ 134.771988] handle_overflow+0xa3/0xb0 [ 134.771997] ? lockdep_hardirqs_on_prepare+0xe8/0x1b0 [ 134.772006] cifs_setlk+0x63c/0x680 [cifs] [ 134.772085] ? _get_xid+0x5f/0xa0 [cifs] [ 134.772085] cifs_flock+0x131/0x400 [cifs] [ 134.772085] __x64_sys_flock+0xfc/0x120 [ 134.772085] do_syscall_64+0x33/0x40 [ 134.772085] entry_SYSCALL_64_after_hwframe+0x44/0xa9 [ 134.772085] RIP: 0033:0x7fea4f83b3fb [ 134.772085] Code: ff 48 8b 15 8f 1a 0d 00 f7 d8 64 89 02 b8 ff ff ff ff eb da e8 16 0b 02 00 66 0f 1f 44 00 00 f3 0f 1e fa b8 49 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 5d 1a 0d 00 f7 d8 64 89 01 48 Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz> Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* Merge tag '5.18-smb3-fixes-part2' of git://git.samba.org/sfrench/cifs-2.6Linus Torvalds2022-04-011-6/+5
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Pull more cifs updates from Steve French: - three fixes for big endian issues in how Persistent and Volatile file ids were stored - Various misc. fixes: including some for oops, 2 for ioctls, 1 for writeback - cleanup of how tcon (tree connection) status is tracked - Four changesets to move various duplicated protocol definitions (defined both in cifs.ko and ksmbd) into smbfs_common/smb2pdu.h - important performance improvement to use cached handles in some key compounding code paths (reduces numbers of opens/closes sent in some workloads) - fix to allow alternate DFS target to be used to retry on a failed i/o * tag '5.18-smb3-fixes-part2' of git://git.samba.org/sfrench/cifs-2.6: cifs: fix NULL ptr dereference in smb2_ioctl_query_info() cifs: prevent bad output lengths in smb2_ioctl_query_info() smb3: fix ksmbd bigendian bug in oplock break, and move its struct to smbfs_common smb3: cleanup and clarify status of tree connections smb3: move defines for query info and query fsinfo to smbfs_common smb3: move defines for ioctl protocol header and SMB2 sizes to smbfs_common [smb3] move more common protocol header definitions to smbfs_common cifs: fix incorrect use of list iterator after the loop ksmbd: store fids as opaque u64 integers cifs: fix bad fids sent over wire cifs: change smb2_query_info_compound to use a cached fid, if available cifs: convert the path to utf16 in smb2_query_info_compound cifs: writeback fix cifs: do not skip link targets when an I/O fails
| * smb3: cleanup and clarify status of tree connectionsSteve French2022-03-281-6/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently the way the tid (tree connection) status is tracked is confusing. The same enum is used for structs cifs_tcon and cifs_ses and TCP_Server_info, but each of these three has different states that they transition among. The current code also unnecessarily uses camelCase. Convert from use of statusEnum to a new tid_status_enum for tree connections. The valid states for a tid are: TID_NEW = 0, TID_GOOD, TID_EXITING, TID_NEED_RECON, TID_NEED_TCON, TID_IN_TCON, TID_NEED_FILES_INVALIDATE, /* unused, considering removing in future */ TID_IN_FILES_INVALIDATE It also removes CifsNeedTcon, CifsInTcon, CifsNeedFilesInvalidate and CifsInFilesInvalidate from the statusEnum used for session and TCP_Server_Info since they are not relevant for those. A follow on patch will fix the places where we use the tcon->need_reconnect flag to be more consistent with the tid->status. Also fixes a bug that was: Reported-by: kernel test robot <lkp@intel.com> Reviewed-by: Shyam Prasad N <sprasad@microsoft.com> Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* | fs: Remove ->readpages address space operationMatthew Wilcox (Oracle)2022-04-011-1/+1
|/ | | | | | | | | | | All filesystems have now been converted to use ->readahead, so remove the ->readpages operation and fix all the comments that used to refer to it. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Al Viro <viro@zeniv.linux.org.uk> Acked-by: Al Viro <viro@zeniv.linux.org.uk>
* cifs: maintain a state machine for tcp/smb/tcon sessionsShyam Prasad N2022-01-071-8/+16
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | If functions like cifs_negotiate_protocol, cifs_setup_session, cifs_tree_connect are called in parallel on different channels, each of these will be execute the requests. This maybe unnecessary in some cases, and only the first caller may need to do the work. This is achieved by having more states for the tcp/smb/tcon session status fields. And tracking the state of reconnection based on the state machine. For example: for tcp connections: CifsNew/CifsNeedReconnect -> CifsNeedNegotiate -> CifsInNegotiate -> CifsNeedSessSetup -> CifsInSessSetup -> CifsGood for smb sessions: CifsNew/CifsNeedReconnect -> CifsGood for tcon: CifsNew/CifsNeedReconnect -> CifsInFilesInvalidate -> CifsNeedTcon -> CifsInTcon -> CifsGood If any channel reconnect sees that it's in the middle of transition to CifsGood, then they can skip the function. Signed-off-by: Shyam Prasad N <sprasad@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: take cifs_tcp_ses_lock for status checksShyam Prasad N2022-01-071-1/+11
| | | | | | | | | | | | | | | | While checking/updating status for tcp ses, smb ses or tcon, we take GlobalMid_Lock. This doesn't make any sense. Replaced it with cifs_tcp_ses_lock. Ideally, we should take a spin lock per struct. But since tcp ses, smb ses and tcon objects won't add up to a lot, I think there should not be too much contention. Also, in few other places, these are checked without locking. Added locking for these. Signed-off-by: Shyam Prasad N <sprasad@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: reconnect only the connection and not smb session where possibleShyam Prasad N2022-01-021-1/+1
| | | | | | | | | | | | | | | | | With the new per-channel bitmask for reconnect, we have an option to reconnect the tcp session associated with the channel without reconnecting the smb session. i.e. if there are still channels to operate on, we can continue to use the smb session and tcon. However, there are cases where it makes sense to reconnect the smb session even when there are active channels underneath. For example for SMB session expiry. With this patch, we'll have an option to do either, and use the correct option for specific cases. Signed-off-by: Shyam Prasad N <sprasad@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: use the chans_need_reconnect bitmap for reconnect statusShyam Prasad N2022-01-021-5/+7
| | | | | | | | | | | | | | | | | | | | | | | | We use the concept of "binding" when one of the secondary channel is in the process of connecting/reconnecting to the server. Till this binding process completes, and the channel is bound to an existing session, we redirect traffic from other established channels on the binding channel, effectively blocking all traffic till individual channels get reconnected. With my last set of commits, we can get rid of this binding serialization. We now have a bitmap of connection states for each channel. We will use this bitmap instead for tracking channel status. Having a bitmap also now enables us to keep the session alive, as long as even a single channel underneath is alive. Unfortunately, this also meant that we need to supply the tcp connection info for the channel during all negotiate and session setup functions. These changes have resulted in a slightly bigger code churn. However, I expect perf and robustness improvements in the mchan scenario after this change. Signed-off-by: Shyam Prasad N <sprasad@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: track individual channel status using chans_need_reconnectShyam Prasad N2022-01-021-6/+42
| | | | | | | | | | | | | | | | We needed a way to identify the channels under the smb session which are in reconnect, so that the traffic to other channels can continue. So I replaced the bool need_reconnect with a bitmask identifying all the channels that need reconnection (named chans_need_reconnect). When a channel needs reconnection, the bit corresponding to the index of the server in ses->chans is used to set this bitmask. Checking if no channels or all the channels need reconnect then becomes very easy. Also wrote some helper macros for checking and setting the bits. Signed-off-by: Shyam Prasad N <sprasad@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: remove pathname for file from SPDX headerSteve French2021-09-131-1/+0
| | | | | | | | | | | checkpatch complains about source files with filenames (e.g. in these cases just below the SPDX header in comments at the top of various files in fs/cifs). It also is helpful to change this now so will be less confusing when the parent directory is renamed e.g. from fs/cifs to fs/smb_client (or fs/smbfs) Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: remove support for NTLM and weaker authentication algorithmsRonnie Sahlberg2021-08-251-105/+1
| | | | | | | | for SMB1. This removes the dependency to DES. Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: enable fscache usage even for files opened as rwShyam Prasad N2021-08-251-0/+1
| | | | | | | | | | | | | | | | | So far, the fscache implementation we had supports only a small set of use cases. Particularly for files opened with O_RDONLY. This commit enables it even for rw based file opens. It also enables the reuse of cached data in case of mount option (cache=singleclient) where it is guaranteed that this is the only client (and server) which operates on the files. There's also a single line change in fscache.c to get around a bug seen in fscache. Signed-off-by: Shyam Prasad N <sprasad@microsoft.com> Acked-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* CIFS: Clarify SMB1 code for POSIX delete fileSteve French2021-07-221-2/+5
| | | | | | | | | | | Coverity also complains about the way we calculate the offset (starting from the address of a 4 byte array within the header structure rather than from the beginning of the struct plus 4 bytes) for SMB1 CIFSPOSIXDelFile. This changeset doesn't change the address but makes it slightly clearer. Addresses-Coverity: 711519 ("Out of bounds write") Signed-off-by: Steve French <stfrench@microsoft.com>
* CIFS: Clarify SMB1 code for POSIX CreateSteve French2021-07-221-1/+2
| | | | | | | | | | | Coverity also complains about the way we calculate the offset (starting from the address of a 4 byte array within the header structure rather than from the beginning of the struct plus 4 bytes) for SMB1 CIFSPOSIXCreate. This changeset doesn't change the address but makes it slightly clearer. Addresses-Coverity: 711518 ("Out of bounds write") Signed-off-by: Steve French <stfrench@microsoft.com>
* CIFS: Clarify SMB1 code for POSIX LockSteve French2021-07-071-1/+2
| | | | | | | | | | | | Coverity also complains about the way we calculate the offset (starting from the address of a 4 byte array within the header structure rather than from the beginning of the struct plus 4 bytes) for SMB1 PosixLock. This changeset doesn't change the address but makes it slightly clearer. Addresses-Coverity: 711520 ("Out of bounds write") Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz> Signed-off-by: Steve French <stfrench@microsoft.com>
* CIFS: Clarify SMB1 code for rename open fileSteve French2021-07-071-1/+2
| | | | | | | | | | | | Coverity also complains about the way we calculate the offset (starting from the address of a 4 byte array within the header structure rather than from the beginning of the struct plus 4 bytes) for SMB1 RenameOpenFile. This changeset doesn't change the address but makes it slightly clearer. Addresses-Coverity: 711521 ("Out of bounds write") Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz> Signed-off-by: Steve French <stfrench@microsoft.com>
* CIFS: Clarify SMB1 code for deleteSteve French2021-07-071-1/+2
| | | | | | | | | | | | | | Coverity also complains about the way we calculate the offset (starting from the address of a 4 byte array within the header structure rather than from the beginning of the struct plus 4 bytes) for SMB1 SetFileDisposition (which is used to unlink a file by setting the delete on close flag). This changeset doesn't change the address but makes it slightly clearer. Addresses-Coverity: 711524 ("Out of bounds write") Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz> Signed-off-by: Steve French <stfrench@microsoft.com>
* CIFS: Clarify SMB1 code for SetFileSizeSteve French2021-07-071-2/+2
| | | | | | | | | | | | Coverity also complains about the way we calculate the offset (starting from the address of a 4 byte array within the header structure rather than from the beginning of the struct plus 4 bytes) for setting the file size using SMB1. This changeset doesn't change the address but makes it slightly clearer. Addresses-Coverity: 711525 ("Out of bounds write") Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz> Signed-off-by: Steve French <stfrench@microsoft.com>
* CIFS: Clarify SMB1 code for UnixSetPathInfoSteve French2021-07-021-3/+2
| | | | | | | | | | | | | Coverity also complains about the way we calculate the offset (starting from the address of a 4 byte array within the header structure rather than from the beginning of the struct plus 4 bytes) for doing SetPathInfo (setattr) when using the Unix extensions. This doesn't change the address but makes it slightly clearer. Addresses-Coverity: 711528 ("Out of bounds read") Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* CIFS: Clarify SMB1 code for UnixCreateSymLinkSteve French2021-07-021-1/+2
| | | | | | | | | | | | | Coverity also complains about the way we calculate the offset (starting from the address of a 4 byte array within the header structure rather than from the beginning of the struct plus 4 bytes) for creating SMB1 symlinks when using the Unix extensions. This doesn't change the address but makes it slightly clearer. Addresses-Coverity: 711530 ("Out of bounds read") Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: clarify SMB1 code for UnixCreateHardLinkSteve French2021-07-021-1/+2
| | | | | | | | | | | | Coverity complains about the way we calculate the offset (starting from the address of a 4 byte array within the header structure rather than from the beginning of the struct plus 4 bytes). This doesn't change the address but makes it slightly clearer. Addresses-Coverity: 711529 ("Out of bounds read") Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: use SPDX-Licence-IdentifierSteve French2021-06-201-13/+1
| | | | | | | | Add SPDX license identifier and replace license boilerplate. Corrects various checkpatch errors with the older format for noting the LGPL license. Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: remove redundant initialization of variable rcColin Ian King2021-06-201-1/+1
| | | | | | | | | | | The variable rc is being initialized with a value that is never read, the assignment is redundant and can be removed. Addresses-Coverity: ("Unused value") Signed-off-by: Colin Ian King <colin.king@canonical.com> Reviewed-by: Aurelien Aptel <aaptel@suse.com> Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: rename the *_shroot* functions to *_cached_dir*Ronnie Sahlberg2021-04-251-1/+1
| | | | | | | | These functions will eventually be used to cache any directory, not just the root so change the names. Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: remove old dead codeAurelien Aptel2021-04-251-50/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | While reviewing a patch clarifying locks and locking hierarchy I realized some locks were unused. This commit removes old data and code that isn't actually used anywhere, or hidden in ifdefs which cannot be enabled from the kernel config. * The uid/gid trees and associated locks are left-overs from when uid/sid mapping had an extra caching layer on top of the keyring and are now unused. See commit faa65f07d21e ("cifs: simplify id_to_sid and sid_to_id mapping code") from 2012. * cifs_oplock_break_ops is a left-over from when slow_work was remplaced by regular workqueue and is now unused. See commit 9b646972467f ("cifs: use workqueue instead of slow-work") from 2010. * CIFSSMBSetAttrLegacy is SMB1 cruft dealing with some legacy NT4/Win9x behaviour. * Remove CONFIG_CIFS_DNOTIFY_EXPERIMENTAL left-overs. This was already partially removed in 392e1c5dc9cc ("cifs: rename and clarify CIFS_ASYNC_OP and CIFS_NO_RESP") from 2019. Kill it completely. * Another candidate that was considered but spared is CONFIG_CIFS_NFSD_EXPORT which has an empty implementation and cannot be enabled by a config option (although it is listed but disabled with "BROKEN" as a dep). It's unclear whether this could even function today in its current form but it has it's own .c file and Kconfig entry which is a bit more involved to remove and might make a come back? Signed-off-by: Aurelien Aptel <aaptel@suse.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: use discard iterator to discard unneeded network data more efficientlyDavid Howells2021-02-251-3/+3
| | | | | | | | | | The iterator, ITER_DISCARD, that can only be used in READ mode and just discards any data copied to it, was added to allow a network filesystem to discard any unwanted data sent by a server. Convert cifs_discard_from_socket() to use this. Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* treewide: Use fallthrough pseudo-keywordGustavo A. R. Silva2020-08-231-1/+1
| | | | | | | | | | Replace the existing /* fall through */ comments and its variants with the new pseudo-keyword macro fallthrough[1]. Also, remove unnecessary fall-through markings when it is the case. [1] https://www.kernel.org/doc/html/v5.7/process/deprecated.html?highlight=fallthrough#implicit-switch-case-fall-through Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
* cifs: merge __{cifs,smb2}_reconnect[_tcon]() into cifs_tree_connect()Stefan Metzmacher2020-08-021-111/+1
| | | | | | | | | | They were identical execpt to CIFSTCon() vs. SMB2_tcon(). These are also available via ops->tree_connect(). Signed-off-by: Stefan Metzmacher <metze@samba.org> Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz> Reviewed-by: Aurelien Aptel <aaptel@suse.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: smb1: Try failing back to SetFileInfo if SetPathInfo failsRonnie Sahlberg2020-08-021-1/+38
| | | | | | | | | | | | | | RHBZ 1145308 Some very old server may not support SetPathInfo to adjust the timestamps of directories. For these servers, try to open the directory and use SetFileInfo. Minor correction to patch included that was Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com> Tested-by: Kenneth D'souza <kdsouza@redhat.com>
* cifs: minor fix to two debug messagesSteve French2020-06-011-1/+1
| | | | | | | | Joe Perches pointed out that we were missing a newline at the end of two debug messages Reported-by: Joe Perches <joe@perches.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: Standardize logging outputJoe Perches2020-06-011-12/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | Use pr_fmt to standardize all logging for fs/cifs. Some logging output had no CIFS: specific prefix. Now all output has one of three prefixes: o CIFS: o CIFS: VFS: o Root-CIFS: Miscellanea: o Convert printks to pr_<level> o Neaten macro definitions o Remove embedded CIFS: prefixes from formats o Convert "illegal" to "invalid" o Coalesce formats o Add missing '\n' format terminations o Consolidate multiple cifs_dbg continuations into single calls o More consistent use of upper case first word output logging o Multiline statement argument alignment and wrapping Signed-off-by: Joe Perches <joe@perches.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: handle hostnames that resolve to same ip in failoverPaulo Alcantara2020-06-011-18/+37
| | | | | | | | | | | | | | | | | | | | | In order to support reconnect to hostnames that resolve to same ip address, besides relying on the currently set hostname to match DFS targets, attempt to resolve the targets and then match their addresses with the reconnected server ip address. For instance, if we have two hostnames "FOO" and "BAR", and both resolve to the same ip address, we would be able to handle failover in DFS paths like \\FOO\dfs\link1 -> [ \BAZ\share2 (*), \BAR\share1 ] \\FOO\dfs\link2 -> [ \BAZ\share2 (*), \FOO\share1 ] so when "BAZ" is no longer accessible, link1 and link2 would get reconnected despite having different target hostnames. Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz> Reviewed-by: Aurelien Aptel <aaptel@suse.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: remove redundant initialization of variable rcColin Ian King2020-06-011-1/+1
| | | | | | | | | | The variable rc is being initialized with a value that is never read and it is being updated later with a new value. The initialization is redundant and can be removed. Addresses-Coverity: ("Unused value") Signed-off-by: Colin Ian King <colin.king@canonical.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: fix leaked reference on requeued writeAdam McCoy2020-05-141-1/+1
| | | | | | | | | | | | | | | | Failed async writes that are requeued may not clean up a refcount on the file, which can result in a leaked open. This scenario arises very reliably when using persistent handles and a reconnect occurs while writing. cifs_writev_requeue only releases the reference if the write fails (rc != 0). The server->ops->async_writev operation will take its own reference, so the initial reference can always be released. Signed-off-by: Adam McCoy <adam@forsedomani.com> Signed-off-by: Steve French <stfrench@microsoft.com> CC: Stable <stable@vger.kernel.org> Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>
* cifs: improve read performance for page size 64KB & cache=strict & vers=2.1+Jones Syue2020-04-151-0/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Found a read performance issue when linux kernel page size is 64KB. If linux kernel page size is 64KB and mount options cache=strict & vers=2.1+, it does not support cifs_readpages(). Instead, it is using cifs_readpage() and cifs_read() with maximum read IO size 16KB, which is much slower than read IO size 1MB when negotiated SMB 2.1+. Since modern SMB server supported SMB 2.1+ and Max Read Size can reach more than 64KB (for example 1MB ~ 8MB), this patch check max_read instead of maxBuf to determine whether server support readpages() and improve read performance for page size 64KB & cache=strict & vers=2.1+, and for SMB1 it is more cleaner to initialize server->max_read to server->maxBuf. The client is a linux box with linux kernel 4.2.8, page size 64KB (CONFIG_ARM64_64K_PAGES=y), cpu arm 1.7GHz, and use mount.cifs as smb client. The server is another linux box with linux kernel 4.2.8, share a file '10G.img' with size 10GB, and use samba-4.7.12 as smb server. The client mount a share from the server with different cache options: cache=strict and cache=none, mount -tcifs //<server_ip>/Public /cache_strict -overs=3.0,cache=strict,username=<xxx>,password=<yyy> mount -tcifs //<server_ip>/Public /cache_none -overs=3.0,cache=none,username=<xxx>,password=<yyy> The client download a 10GbE file from the server across 1GbE network, dd if=/cache_strict/10G.img of=/dev/null bs=1M count=10240 dd if=/cache_none/10G.img of=/dev/null bs=1M count=10240 Found that cache=strict (without patch) is slower read throughput and smaller read IO size than cache=none. cache=strict (without patch): read throughput 40MB/s, read IO size is 16KB cache=strict (with patch): read throughput 113MB/s, read IO size is 1MB cache=none: read throughput 109MB/s, read IO size is 1MB Looks like if page size is 64KB, cifs_set_ops() would use cifs_addr_ops_smallbuf instead of cifs_addr_ops, /* check if server can support readpages */ if (cifs_sb_master_tcon(cifs_sb)->ses->server->maxBuf < PAGE_SIZE + MAX_CIFS_HDR_SIZE) inode->i_data.a_ops = &cifs_addr_ops_smallbuf; else inode->i_data.a_ops = &cifs_addr_ops; maxBuf is came from 2 places, SMB2_negotiate() and CIFSSMBNegotiate(), (SMB2_MAX_BUFFER_SIZE is 64KB) SMB2_negotiate(): /* set it to the maximum buffer size value we can send with 1 credit */ server->maxBuf = min_t(unsigned int, le32_to_cpu(rsp->MaxTransactSize),       SMB2_MAX_BUFFER_SIZE); CIFSSMBNegotiate(): server->maxBuf = le32_to_cpu(pSMBr->MaxBufferSize); Page size 64KB and cache=strict lead to read_pages() use cifs_readpage() instead of cifs_readpages(), and then cifs_read() using maximum read IO size 16KB, which is much slower than maximum read IO size 1MB. (CIFSMaxBufSize is 16KB by default) /* FIXME: set up handlers for larger reads and/or convert to async */ rsize = min_t(unsigned int, cifs_sb->rsize, CIFSMaxBufSize); Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Jones Syue <jonessyue@qnap.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: make use of cap_unix(ses) in cifs_reconnect_tcon()Stefan Metzmacher2020-03-221-1/+1
| | | | | | | | | cap_unix(ses) defaults to false for SMB2. Signed-off-by: Stefan Metzmacher <metze@samba.org> Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com> Reviewed-by: Aurelien Aptel <aaptel@suse.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: call wake_up(&server->response_q) inside of cifs_reconnect()Stefan Metzmacher2020-03-221-1/+0
| | | | | | | | | This means it's consistently called and the callers don't need to care about it. Signed-off-by: Stefan Metzmacher <metze@samba.org> Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: handle prefix paths in reconnectPaulo Alcantara (SUSE)2020-03-221-4/+15
| | | | | | | | | | | | | | | | | | | For the case where we have a DFS path like below and we're currently connected to targetA: //dfsroot/link -> //targetA/share/foo, //targetB/share/bar after failover, we should make sure to update cifs_sb->prepath so the next operations will use the new prefix path "/bar". Besides, in order to simplify the use of different prefix paths, enforce CIFS_MOUNT_USE_PREFIX_PATH for DFS mounts so we don't have to revalidate the root dentry every time we set a new prefix path. Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz> Acked-by: Ronnie Sahlberg <lsahlber@redhat.com> Reviewed-by: Aurelien Aptel <aaptel@suse.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: fix rename() by ensuring source handle opened with DELETE bitAurelien Aptel2020-02-241-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | To rename a file in SMB2 we open it with the DELETE access and do a special SetInfo on it. If the handle is missing the DELETE bit the server will fail the SetInfo with STATUS_ACCESS_DENIED. We currently try to reuse any existing opened handle we have with cifs_get_writable_path(). That function looks for handles with WRITE access but doesn't check for DELETE, making rename() fail if it finds a handle to reuse. Simple reproducer below. To select handles with the DELETE bit, this patch adds a flag argument to cifs_get_writable_path() and find_writable_file() and the existing 'bool fsuid_only' argument is converted to a flag. The cifsFileInfo struct only stores the UNIX open mode but not the original SMB access flags. Since the DELETE bit is not mapped in that mode, this patch stores the access mask in cifs_fid on file open, which is accessible from cifsFileInfo. Simple reproducer: #include <stdio.h> #include <stdlib.h> #include <sys/types.h> #include <sys/stat.h> #include <fcntl.h> #include <unistd.h> #define E(s) perror(s), exit(1) int main(int argc, char *argv[]) { int fd, ret; if (argc != 3) { fprintf(stderr, "Usage: %s A B\n" "create&open A in write mode, " "rename A to B, close A\n", argv[0]); return 0; } fd = openat(AT_FDCWD, argv[1], O_WRONLY|O_CREAT|O_SYNC, 0666); if (fd == -1) E("openat()"); ret = rename(argv[1], argv[2]); if (ret) E("rename()"); ret = close(fd); if (ret) E("close()"); return ret; } $ gcc -o bugrename bugrename.c $ ./bugrename /mnt/a /mnt/b rename(): Permission denied Fixes: 8de9e86c67ba ("cifs: create a helper to find a writeable handle by path name") CC: Stable <stable@vger.kernel.org> Signed-off-by: Aurelien Aptel <aaptel@suse.com> Signed-off-by: Steve French <stfrench@microsoft.com> Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com> Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
* cifs: fix soft mounts hanging in the reconnect codeRonnie Sahlberg2020-02-061-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | RHBZ: 1795423 This is the SMB1 version of a patch we already have for SMB2 In recent DFS updates we have a new variable controlling how many times we will retry to reconnect the share. If DFS is not used, then this variable is initialized to 0 in: static inline int dfs_cache_get_nr_tgts(const struct dfs_cache_tgt_list *tl) { return tl ? tl->tl_numtgts : 0; } This means that in the reconnect loop in smb2_reconnect() we will immediately wrap retries to -1 and never actually get to pass this conditional: if (--retries) continue; The effect is that we no longer reach the point where we fail the commands with -EHOSTDOWN and basically the kernel threads are virtually hung and unkillable. Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com> Reviewed-by: Aurelien Aptel <aaptel@suse.com> Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
* fs/cifs/cifssmb.c: use true,false for bool variablezhengbin2020-01-261-2/+2
| | | | | | | | | | | Fixes coccicheck warning: fs/cifs/cifssmb.c:4622:3-22: WARNING: Assignment of 0/1 to bool variable fs/cifs/cifssmb.c:4756:3-22: WARNING: Assignment of 0/1 to bool variable Reported-by: Hulk Robot <hulkci@huawei.com> Signed-off-by: zhengbin <zhengbin13@huawei.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* CIFS: Close cached root handle only if it has a leasePavel Shilovsky2019-12-131-0/+3
| | | | | | | | | | | | | | SMB2_tdis() checks if a root handle is valid in order to decide whether it needs to close the handle or not. However if another thread has reference for the handle, it may end up with putting the reference twice. The extra reference that we want to put during the tree disconnect is the reference that has a directory lease. So, track the fact that we have a directory lease and close the handle only in that case. Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com> Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* smb3: pass mode bits into create callsSteve French2019-09-261-1/+2
| | | | | | | | | | | We need to populate an ACL (security descriptor open context) on file and directory correct. This patch passes in the mode. Followon patch will build the open context and the security descriptor (from the mode) that goes in the open context. Signed-off-by: Steve French <stfrench@microsoft.com> Reviewed-by: Aurelien Aptel <aaptel@suse.com>
* fs: cifs: cifsssmb: remove redundant assignment to variable retColin Ian King2019-09-161-1/+1
| | | | | | | | | | The variable ret is being initialized however this is never read and later it is being reassigned to a new value. The initialization is redundant and hence can be removed. Addresses-Coverity: ("Unused Value") Signed-off-by: Colin Ian King <colin.king@canonical.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: replace various strncpy with strscpy and similarRonnie Sahlberg2019-08-271-132/+65
| | | | | | | | | | | | | | Using strscpy is cleaner, and avoids some problems with handling maximum length strings. Linus noticed the original problem and Aurelien pointed out some additional problems. Fortunately most of this is SMB1 code (and in particular the ASCII string handling older, which is less common). Reported-by: Linus Torvalds <torvalds@linux-foundation.org> Reviewed-by: Aurelien Aptel <aaptel@suse.com> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* fs: cifs: cifsssmb: Change return type of convert_ace_to_cifs_aceHariprasad Kelam2019-07-071-11/+3
| | | | | | | | | | | | Change return from int to void of convert_ace_to_cifs_ace as it never fails. fixes below issue reported by coccicheck fs/cifs/cifssmb.c:3606:7-9: Unneeded variable: "rc". Return "0" on line 3620 Signed-off-by: Hariprasad Kelam <hariprasad.kelam@gmail.com> Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: simplify code by removing CONFIG_CIFS_ACL ifdefSteve French2019-07-071-2/+0
| | | | | | | | SMB3 ACL support is needed for many use cases now and should not be ifdeffed out, even for SMB1 (CIFS). Remove the CONFIG_CIFS_ACL ifdef so ACL support is always built into cifs.ko Signed-off-by: Steve French <stfrench@microsoft.com>
* cifs: rename and clarify CIFS_ASYNC_OP and CIFS_NO_RESPRonnie Sahlberg2019-05-071-94/+4
| | | | | | | | | | | | | | | The flags were named confusingly. CIFS_ASYNC_OP now just means that we will not block waiting for credits to become available so we thus rename this to be CIFS_NON_BLOCKING. Change CIFS_NO_RESP to CIFS_NO_RSP_BUF to clarify that we will actually get a response from the server but we will not get/do not want a response buffer. Delete CIFSSMBNotify. This is an SMB1 function that is not used. Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com> Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com>
* cifs: fix credits leak for SMB1 oplock breaksRonnie Sahlberg2019-05-071-1/+1
| | | | | | | | | | | | | | | | For SMB1 oplock breaks we would grab one credit while sending the PDU but we would never relese the credit back since we will never receive a response to this from the server. Eventuallt this would lead to a hang once all credits are leaked. Fix this by defining a new flag CIFS_NO_SRV_RSP which indicates that there is no server response to this command and thus we need to add any credits back immediately after sending the PDU. CC: Stable <stable@vger.kernel.org> #v5.0+ Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> Reviewed-by: Pavel Shilovsky <pshilov@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>