07 Feb, 2019
2 commits
-
commit 28eb24ff75c5ac130eb326b3b4d0dcecfc0f427d upstream.
In case a hostname resolves to a different IP address (e.g. long
running mounts), make sure to resolve it every time prior to calling
generic_ip_connect() in reconnect.Suggested-by: Steve French
Signed-off-by: Paulo Alcantara
Signed-off-by: Steve French
Signed-off-by: Pavel Shilovsky
Signed-off-by: Greg Kroah-Hartman -
commit 8e6e72aeceaaed5aeeb1cb43d3085de7ceb14f79 upstream.
Signed-off-by: Pavel Shilovsky
Signed-off-by: Steve French
CC: Stable
Signed-off-by: Greg Kroah-Hartman
31 Jan, 2019
4 commits
-
commit ef68e831840c40c7d01b328b3c0f5d8c4796c232 upstream.
When executing add_credits() we currently call cifs_reconnect()
if the number of credits is zero and there are no requests in
flight. In this case we may call cifs_reconnect() recursively
twice and cause memory corruption given the following sequence
of functions:mid1.callback() -> add_credits() -> cifs_reconnect() ->
-> mid2.callback() -> add_credits() -> cifs_reconnect().Fix this by avoiding to call cifs_reconnect() in add_credits()
and checking for zero credits in the demultiplex thread.Cc:
Signed-off-by: Pavel Shilovsky
Reviewed-by: Ronnie Sahlberg
Signed-off-by: Steve French
Signed-off-by: Greg Kroah-Hartman -
commit ec678eae746dd25766a61c4095e2b649d3b20b09 upstream.
We do need to account for credits received in error responses
to read requests on encrypted sessions.Cc:
Signed-off-by: Pavel Shilovsky
Reviewed-by: Ronnie Sahlberg
Signed-off-by: Steve French
Signed-off-by: Greg Kroah-Hartman -
commit 8004c78c68e894e4fd5ac3c22cc22eb7dc24cabc upstream.
Currently we mark MID as malformed if we get an error from server
in a read response. This leads to not properly processing credits
in the readv callback. Fix this by marking such a response as
normal received response and process it appropriately.Cc:
Signed-off-by: Pavel Shilovsky
Reviewed-by: Ronnie Sahlberg
Signed-off-by: Steve French
Signed-off-by: Greg Kroah-Hartman -
commit acc58d0bab55a50e02c25f00bd6a210ee121595f upstream.
When doing MTU i/o we need to leave some credits for
possible reopen requests and other operations happening
in parallel. Currently we leave 1 credit which is not
enough even for reopen only: we need at least 2 credits
if durable handle reconnect fails. Also there may be
other operations at the same time including compounding
ones which require 3 credits at a time each. Fix this
by leaving 8 credits which is big enough to cover most
scenarios.Was able to reproduce this when server was configured
to give out fewer credits than usual.The proper fix would be to reconnect a file handle first
and then obtain credits for an MTU request but this leads
to bigger code changes and should happen in other patches.Cc:
Signed-off-by: Pavel Shilovsky
Signed-off-by: Steve French
Signed-off-by: Greg Kroah-Hartman
26 Jan, 2019
1 commit
-
commit 7420451f6a109f7f8f1bf283f34d08eba3259fb3 upstream.
allow disabling cifs (SMB1 ie vers=1.0) and vers=2.0 in the
config for the build of cifs.ko if want to always prevent mounting
with these less secure dialects.Signed-off-by: Steve French
Reviewed-by: Aurelien Aptel
Reviewed-by: Jeremy Allison
Cc: Alakesh Haloi
Signed-off-by: Greg Kroah-Hartman
17 Jan, 2019
3 commits
-
commit b9a74cde94957d82003fb9f7ab4777938ca851cd upstream.
If maxBuf is small but non-zero, it could result in a zero sized lock
element array which we would then try and access OOB.Signed-off-by: Ross Lagerwall
Signed-off-by: Steve French
CC: Stable
Signed-off-by: Greg Kroah-Hartman -
commit ee13919c2e8d1f904e035ad4b4239029a8994131 upstream.
Currently we hide EINTR code returned from sock_sendmsg()
and return 0 instead. This makes a caller think that we
successfully completed the network operation which is not
true. Fix this by properly returning EINTR to callers.Cc:
Signed-off-by: Pavel Shilovsky
Reviewed-by: Jeff Layton
Signed-off-by: Steve French
Signed-off-by: Greg Kroah-Hartman -
commit b983f7e92348d7e7d091db1b78b7915e9dd3d63a upstream.
Currently for MTU requests we allocate maximum possible credits
in advance and then adjust them according to the request size.
While we were adjusting the number of credits belonging to the
server, we were skipping adjustment of credits belonging to the
request. This patch fixes it by setting request credits to
CreditCharge field value of SMB2 packet header.Also ask 1 credit more for async read and write operations to
increase parallelism and match the behavior of other operations.Signed-off-by: Pavel Shilovsky
Signed-off-by: Steve French
CC: Stable
Signed-off-by: Greg Kroah-Hartman
10 Jan, 2019
1 commit
-
commit 9a596f5b39593414c0ec80f71b94a226286f084e upstream.
While resolving a bug with locks on samba shares found a strange behavior.
When a file locked by one node and we trying to lock it from another node
it fail with errno 5 (EIO) but in that case errno must be set to
(EACCES | EAGAIN).
This isn't happening when we try to lock file second time on same node.
In this case it returns EACCES as expected.
Also this issue not reproduces when we use SMB1 protocol (vers=1.0 in
mount options).Further investigation showed that the mapping from status_to_posix_error
is different for SMB1 and SMB2+ implementations.
For SMB1 mapping is [NT_STATUS_LOCK_NOT_GRANTED to ERRlock]
(See fs/cifs/netmisc.c line 66)
but for SMB2+ mapping is [STATUS_LOCK_NOT_GRANTED to -EIO]
(see fs/cifs/smb2maperror.c line 383)Quick changes in SMB2+ mapping from EIO to EACCES has fixed issue.
BUG: https://bugzilla.kernel.org/show_bug.cgi?id=201971
Signed-off-by: Georgy A Bystrenin
Reviewed-by: Pavel Shilovsky
CC: Stable
Signed-off-by: Steve French
Signed-off-by: Greg Kroah-Hartman
29 Dec, 2018
1 commit
-
commit 2d204ee9d671327915260071c19350d84344e096 upstream
The "le32_to_cpu(rsp->OutputOffset) + *plen" addition can overflow and
wrap around to a smaller value which looks like it would lead to an
information leak.Fixes: 4a72dafa19ba ("SMB2 FSCTL and IOCTL worker function")
Signed-off-by: Dan Carpenter
Signed-off-by: Steve French
Reviewed-by: Aurelien Aptel
CC: Stable
Signed-off-by: Sudip Mukherjee
Signed-off-by: Sasha Levin
21 Dec, 2018
1 commit
-
[ Upstream commit 6e785302dad32228819d8066e5376acd15d0e6ba ]
Missing a dependency. Shouldn't show cifs posix extensions
in Kconfig if CONFIG_CIFS_ALLOW_INSECURE_DIALECTS (ie SMB1
protocol) is disabled.Signed-off-by: Steve French
Reviewed-by: Pavel Shilovsky
Signed-off-by: Sasha Levin
13 Dec, 2018
1 commit
-
commit c988de29ca161823db6a7125e803d597ef75b49c upstream.
Make sure to use the CIFS_DIR_SEP(cifs_sb) as path separator for
prefixpath too. Fixes a bug with smb1 UNIX extensions.Fixes: a6b5058fafdf ("fs/cifs: make share unaccessible at root level mountable")
Signed-off-by: Paulo Alcantara
Reviewed-by: Aurelien Aptel
Signed-off-by: Steve French
CC: Stable
Signed-off-by: Greg Kroah-Hartman
27 Nov, 2018
2 commits
-
[ Upstream commit 0c5d6cb6643f48ad3775322f3ebab6c7eb67484e ]
If the application buffer was too small to fit all the names
we would still count the number of bytes and return this for
listxattr. This would then trigger a BUG in usercopy.cFix the computation of the size so that we return -ERANGE
correctly when the buffer is too small.This fixes the kernel BUG for xfstest generic/377
Signed-off-by: Ronnie Sahlberg
Signed-off-by: Steve French
Reviewed-by: Aurelien Aptel
Signed-off-by: Sasha Levin -
[ Upstream commit 8c6c9bed8773375b1d54ccca2911ec892c59db5d ]
There is a null check on dst_file->private data which suggests
it can be potentially null. However, before this check, pointer
smb_file_target is derived from dst_file->private and dereferenced
in the call to tlink_tcon, hence there is a potential null pointer
deference.Fix this by assigning smb_file_target and target_tcon after the
null pointer sanity checks.Detected by CoverityScan, CID#1475302 ("Dereference before null check")
Fixes: 04b38d601239 ("vfs: pull btrfs clone API to vfs layer")
Signed-off-by: Colin Ian King
Signed-off-by: Steve French
Signed-off-by: Sasha Levin
14 Nov, 2018
3 commits
-
commit 926674de6705f0f1dbf29a62fd758d0977f535d6 upstream.
Some servers (e.g. Azure) do not include a spnego blob in the SMB3
negotiate protocol response, so on kerberos mounts ("sec=krb5")
we can fail, as we expected the server to list its supported
auth types (OIDs in the spnego blob in the negprot response).
Change this so that on krb5 mounts we default to trying krb5 if the
server doesn't list its supported protocol mechanisms.Signed-off-by: Steve French
Reviewed-by: Ronnie Sahlberg
CC: Stable
Signed-off-by: Greg Kroah-Hartman -
commit 1e77a8c204c9d1b655c61751b8ad0fde22421dbb upstream.
If backupuid mount option is sent, we can incorrectly retry
(on access denied on query info) with a cifs (FindFirst) operation
on an smb3 mount which causes the server to force the session close.We set backup intent on open so no need for this fallback.
See kernel bugzilla 201435
Signed-off-by: Steve French
CC: Stable
Reviewed-by: Ronnie Sahlberg
Signed-off-by: Greg Kroah-Hartman -
commit 2c887635cd6ab3af619dc2be94e5bf8f2e172b78 upstream.
Currently, "echo 0 > /proc/fs/cifs/Stats" resets all of the stats
except the session and share reconnect counts. Fix it to
reset those as well.CC: Stable
Signed-off-by: Steve French
Reviewed-by: Aurelien Aptel
Signed-off-by: Greg Kroah-Hartman
04 Nov, 2018
1 commit
-
[ Upstream commit 3995bbf53bd2047f2720c6fdd4bf38f6d942a0c0 ]
On 32-bit (e.g. with m68k-linux-gnu-gcc-4.1):
fs/cifs/inode.c: In function ‘simple_hashstr’:
fs/cifs/inode.c:713: warning: integer constant is too large for ‘long’ typeFixes: 7ea884c77e5c97f1 ("smb3: Fix root directory when server returns inode number of zero")
Signed-off-by: Geert Uytterhoeven
Signed-off-by: Steve French
Reviewed-by: Aurelien Aptel
Signed-off-by: Sasha Levin
10 Oct, 2018
4 commits
-
commit 0595751f267994c3c7027377058e4185b3a28e75 upstream.
When mounting a Windows share that is the root of a drive (eg. C$)
the server does not return . and .. directory entries. This results in
the smb2 code path erroneously skipping the 2 first entries.Pseudo-code of the readdir() code path:
cifs_readdir(struct file, struct dir_context)
initiate_cifs_search ops->query_dir_firstdir_emit_dots
dir_emit < (pos_in_buf)) && (cur_ent != NULL); i++) {
/* go entry by entry figuring out which is first */
cur_ent = nxt_dir_entry(cur_ent, end_of_smb,
cfile->srch_inf.info_level);
}C) cifs_filldir() skips . and .. so we can safely ignore them for now.
Sample program:
int main(int argc, char **argv)
{
const char *path = argc >= 2 ? argv[1] : ".";
DIR *dh;
struct dirent *de;printf("listing path \n", path);
dh = opendir(path);
if (!dh) {
printf("opendir error %d\n", errno);
return 1;
}while (1) {
de = readdir(dh);
if (!de) {
if (errno) {
printf("readdir error %d\n", errno);
return 1;
}
printf("end of listing\n");
break;
}
printf("off=%lu \n", de->d_off, de->d_name);
}return 0;
}Before the fix with SMB1 on root shares:
off=1
off=2
off=3
off=4and on non-root shares:
off=1
off=4 off=5 we skipped . and .. from response buffer (C)
off=6 but still incremented pos
off=7
off=8Therefore the fix for smb2 is to mimic smb1 behaviour and offset the
index_of_last_entry by 2.Test results comparing smb1 and smb2 before/after the fix on root
share, non-root shares and on large directories (ie. multi-response
dir listing):PRE FIX
=======
pre-1-root VS pre-2-root:
ERR pre-2-root is missing [bootmgr, $Recycle.Bin]
pre-1-nonroot VS pre-2-nonroot:
OK~ same files, same order, different offsets
pre-1-nonroot-large VS pre-2-nonroot-large:
OK~ same files, same order, different offsetsPOST FIX
========
post-1-root VS post-2-root:
OK same files, same order, same offsets
post-1-nonroot VS post-2-nonroot:
OK same files, same order, same offsets
post-1-nonroot-large VS post-2-nonroot-large:
OK same files, same order, same offsetsREGRESSION?
===========
pre-1-root VS post-1-root:
OK same files, same order, same offsets
pre-1-nonroot VS post-1-nonroot:
OK same files, same order, same offsetsBugLink: https://bugzilla.samba.org/show_bug.cgi?id=13107
Signed-off-by: Aurelien Aptel
Signed-off-by: Paulo Alcantara
Reviewed-by: Ronnie Sahlberg
Signed-off-by: Steve French
CC: Stable
Signed-off-by: Greg Kroah-Hartman -
[ Upstream commit 097f5863b1a0c9901f180bbd56ae7d630655faaa ]
We need to verify that the "data_offset" is within bounds.
Reported-by: Dr Silvio Cesare of InfoSect
Signed-off-by: Dan Carpenter
Signed-off-by: Steve French
Reviewed-by: Aurelien Aptel
Signed-off-by: Sasha Levin
Signed-off-by: Greg Kroah-Hartman -
[ Upstream commit bcfb84a996f6fa90b5e6e2954b2accb7a4711097 ]
A powerpc build of cifs with gcc v8.2.0 produces this warning:
fs/cifs/cifssmb.c: In function ‘CIFSSMBNegotiate’:
fs/cifs/cifssmb.c:605:3: warning: ‘strncpy’ writing 16 bytes into a region of size 1 overflows the destination [-Wstringop-overflow=]
strncpy(pSMB->DialectsArray+count, protocols[i].name, 16);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~Since we are already doing a strlen() on the source, change the strncpy
to a memcpy().Signed-off-by: Stephen Rothwell
Signed-off-by: Steve French
Signed-off-by: Sasha Levin
Signed-off-by: Greg Kroah-Hartman -
[ Upstream commit c15e3f19a6d5c89b1209dc94b40e568177cb0921 ]
When a Mac client saves an item containing a backslash to a file server
the backslash is represented in the CIFS/SMB protocol as as U+F026.
Before this change, listing a directory containing an item with a
backslash in its name will return that item with the backslash
represented with a true backslash character (U+005C) because
convert_sfm_character mapped U+F026 to U+005C when interpretting the
CIFS/SMB protocol response. However, attempting to open or stat the
path using a true backslash will result in an error because
convert_to_sfm_char does not map U+005C back to U+F026 causing the
CIFS/SMB request to be made with the backslash represented as U+005C.This change simply prevents the U+F026 to U+005C conversion from
happenning. This is analogous to how the code does not do any
translation of UNI_SLASH (U+F000).Signed-off-by: Jon Kuhn
Signed-off-by: Steve French
Signed-off-by: Sasha Levin
Signed-off-by: Greg Kroah-Hartman
26 Sep, 2018
2 commits
-
commit 56446f218af1133c802dad8e9e116f07f381846c upstream.
The problem is that "entryptr + next_offset" and "entryptr + len + size"
can wrap. I ended up changing the type of "entryptr" because it makes
the math easier when we don't have to do so much casting.Signed-off-by: Dan Carpenter
Signed-off-by: Steve French
Reviewed-by: Aurelien Aptel
Reviewed-by: Pavel Shilovsky
CC: Stable
Signed-off-by: Greg Kroah-Hartman -
commit 8ad8aa353524d89fa2e09522f3078166ff78ec42 upstream.
The "old_entry + le32_to_cpu(pDirInfo->NextEntryOffset)" can wrap
around so I have added a check for integer overflow.Reported-by: Dr Silvio Cesare of InfoSect
Reviewed-by: Ronnie Sahlberg
Reviewed-by: Aurelien Aptel
Signed-off-by: Dan Carpenter
Signed-off-by: Steve French
CC: Stable
Signed-off-by: Greg Kroah-Hartman
20 Sep, 2018
2 commits
-
commit f801568332321e2b1e7a8bd26c3e4913a312a2ec upstream.
Although servers will typically ignore unsupported features,
we should advertise the support for directory leases (as
Windows e.g. does) in the negotiate protocol capabilities we
pass to the server, and should check for the server capability
(CAP_DIRECTORY_LEASING) before sending a lease request for an
open of a directory. This will prevent us from accidentally
sending directory leases to SMB2.1 or SMB2 server for example.Signed-off-by: Steve French
CC: Stable
Reviewed-by: Ronnie Sahlberg
Signed-off-by: Greg Kroah-Hartman -
commit 5e19697b56a64004e2d0ff1bb952ea05493c088f upstream.
When "backup intent" is requested on the mount (e.g. backupuid or
backupgid mount options), the corresponding flag needs to be set
on opens of directories (and files) but was missing in some
places causing access denied trying to enumerate and backup
servers.Fixes kernel bugzilla #200953
https://bugzilla.kernel.org/show_bug.cgi?id=200953Reported-and-tested-by:
Signed-off-by: Steve French
CC: Stable
Reviewed-by: Pavel Shilovsky
Signed-off-by: Greg Kroah-Hartman
15 Sep, 2018
3 commits
-
[ Upstream commit 289131e1f1e6ad8c661ec05e176b8f0915672059 ]
For SMB2/SMB3 the number of requests sent was not displayed
in /proc/fs/cifs/Stats unless CONFIG_CIFS_STATS2 was
enabled (only number of failed requests displayed). As
with earlier dialects, we should be displaying these
counters if CONFIG_CIFS_STATS is enabled. They
are important for debugging.e.g. when you cat /proc/fs/cifs/Stats (before the patch)
Resources in use
CIFS Session: 1
Share (unique mount targets): 2
SMB Request/Response Buffer: 1 Pool size: 5
SMB Small Req/Resp Buffer: 1 Pool size: 30
Operations (MIDs): 00 session 0 share reconnects
Total vfs operations: 690 maximum at one time: 21) \\localhost\test
SMBs: 975
Negotiates: 0 sent 0 failed
SessionSetups: 0 sent 0 failed
Logoffs: 0 sent 0 failed
TreeConnects: 0 sent 0 failed
TreeDisconnects: 0 sent 0 failed
Creates: 0 sent 2 failed
Closes: 0 sent 0 failed
Flushes: 0 sent 0 failed
Reads: 0 sent 0 failed
Writes: 0 sent 0 failed
Locks: 0 sent 0 failed
IOCTLs: 0 sent 1 failed
Cancels: 0 sent 0 failed
Echos: 0 sent 0 failed
QueryDirectories: 0 sent 63 failedSigned-off-by: Steve French
Reviewed-by: Aurelien Aptel
Reviewed-by: Pavel Shilovsky
Signed-off-by: Sasha Levin
Signed-off-by: Greg Kroah-Hartman -
[ Upstream commit c281bc0c7412308c7ec0888904f7c99353da4796 ]
echo 0 > /proc/fs/cifs/Stats is supposed to reset the stats
but there were four (see example below) that were not reset
(bytes read and witten, total vfs ops and max ops
at one time)....
0 session 0 share reconnects
Total vfs operations: 100 maximum at one time: 21) \\localhost\test
SMBs: 0
Bytes read: 502092 Bytes written: 31457286
TreeConnects: 0 total 0 failed
TreeDisconnects: 0 total 0 failed
...This patch fixes cifs_stats_proc_write to properly reset
those four.Signed-off-by: Steve French
Reviewed-by: Aurelien Aptel
Signed-off-by: Sasha Levin
Signed-off-by: Greg Kroah-Hartman -
[ Upstream commit e6c47dd0da1e3a484e778046fc10da0b20606a86 ]
Some SMB2/3 servers, Win2016 but possibly others too, adds padding
not only between PDUs in a compound but also to the final PDU.
This padding extends the PDU to a multiple of 8 bytes.Check if the unexpected length looks like this might be the case
and avoid triggering the log messages for :"SMB2 server sent bad RFC1001 len %d not %d\n"
Signed-off-by: Ronnie Sahlberg
Signed-off-by: Steve French
Signed-off-by: Sasha Levin
Signed-off-by: Greg Kroah-Hartman
05 Sep, 2018
6 commits
-
commit 21ba3845b59c733a79ed4fe1c4f3732e7ece9df7 upstream.
Fil in the correct namelen (typically 255 not 4096) in the
statfs response and also fill in a reasonably unique fsid
(in this case taken from the volume id, and the creation time
of the volume).In the case of the POSIX statfs all fields are now filled in,
and in the case of non-POSIX mounts, all fields are filled
in which can be.Signed-off-by: Steve French
CC: Stable
Reviewed-by: Aurelien Aptel
Signed-off-by: Greg Kroah-Hartman -
commit 22783155f4bf956c346a81624ec9258930a6fe06 upstream.
Fixes problem pointed out by Pavel in discussions about commit
729c0c9dd55204f0c9a823ac8a7bfa83d36c7e78Signed-off-by: Pavel Shilovsky
Signed-off-by: Steve French
Reviewed-by: Ronnie Sahlberg
CC: Stable # 3.18.x+
Signed-off-by: Greg Kroah-Hartman -
commit fd09b7d3b352105f08b8e02f7afecf7e816380ef upstream.
An earlier commit had a typo which prevented the
optimization from working:commit 18dd8e1a65dd ("Do not send SMB3 SET_INFO request if nothing is changing")
Thank you to Metze for noticing this. Also clear a
reserved field in the FILE_BASIC_INFO struct we send
that should be zero (all the other fields in that
struct were set or cleared explicitly already in
cifs_set_file_info).Reviewed-by: Pavel Shilovsky
CC: Stable # 4.9.x+
Reported-by: Stefan Metzmacher
Signed-off-by: Steve French
Signed-off-by: Greg Kroah-Hartman -
commit e02789a53d71334b067ad72eee5d4e88a0158083 upstream.
When enumerating snapshots, the last few bytes of the final
snapshot could be left off since we were miscalculating the
length returned (leaving off the sizeof struct SRV_SNAPSHOT_ARRAY)
See MS-SMB2 section 2.2.32.2. In addition fixup the length used
to allow smaller buffer to be passed in, in order to allow
returning the size of the whole snapshot array more easily.Sample userspace output with a kernel patched with this
(mounted to a Windows volume with two snapshots).
Before this patch, the second snapshot would be missing a
few bytes at the end.~/cifs-2.6# ~/enum-snapshots /mnt/file
press enter to issue the ioctl to retrieve snapshot information ...size of snapshot array = 102
Num snapshots: 2 Num returned: 2 Array Size: 102Snapshot 0:@GMT-2018.06.30-19.34.17
Snapshot 1:@GMT-2018.06.30-19.33.37CC: Stable
Signed-off-by: Steve French
Reviewed-by: Pavel Shilovsky
Signed-off-by: Greg Kroah-Hartman -
commit 126c97f4d0d1b5b956e8b0740c81a2b2a2ae548c upstream.
The kmalloc was not being checked - if it fails issue a warning
and return -ENOMEM to the caller.Signed-off-by: Nicholas Mc Guire
Fixes: b8da344b74c8 ("cifs: dynamic allocation of ntlmssp blob")
Signed-off-by: Steve French
Reviewed-by: Pavel Shilovsky
cc: Stable `
Signed-off-by: Greg Kroah-Hartman -
commit 950132afd59385caf6e2b84e5235d069fa10681d upstream.
/proc/fs/cifs/DebugData displays the features (Kconfig options)
used to build cifs.ko but it was missing some, and needed comma
separator. These can be useful in debugging certain problems
so we know which optional features were enabled in the user's build.
Also clarify them, by making them more closely match the
corresponding CONFIG_CIFS_* parm.Old format:
Features: dfs fscache posix spnego xattr aclNew format:
Features: DFS,FSCACHE,SMB_DIRECT,STATS,DEBUG2,ALLOW_INSECURE_LEGACY,CIFS_POSIX,UPCALL(SPNEGO),XATTR,ACLSigned-off-by: Steve French
Reviewed-by: Ronnie Sahlberg
Reviewed-by: Pavel Shilovsky
Reviewed-by: Paulo Alcantara
CC: Stable
Signed-off-by: Greg Kroah-Hartman
28 Jul, 2018
1 commit
-
This reverts commit 748144f35514aef14c4fdef5bcaa0db99cb9367a which is
commit f46ecbd97f508e68a7806291a139499794874f3d upstream.Philip reports:
seems adding "cifs: Fix slab-out-of-bounds in send_set_info() on SMB2
ACE setting" (commit 748144f) [1] created a regression within linux
v4.14 kernel series. Writing to a mounted cifs either freezes on writing
or crashes the PC. A more detailed explanation you may find in our
forums [2]. Reverting the patch, seems to "fix" it. Thoughts?[2] https://forum.manjaro.org/t/53250
Reported-by: Philip Müller
Cc: Jianhong Yin
Cc: Stefano Brivio
Cc: Aurelien Aptel
Cc: Steve French
Signed-off-by: Greg Kroah-Hartman
11 Jul, 2018
2 commits
-
commit f46ecbd97f508e68a7806291a139499794874f3d upstream.
A "small" CIFS buffer is not big enough in general to hold a
setacl request for SMB2, and we end up overflowing the buffer in
send_set_info(). For instance:# mount.cifs //127.0.0.1/test /mnt/test -o username=test,password=test,nounix,cifsacl
# touch /mnt/test/acltest
# getcifsacl /mnt/test/acltest
REVISION:0x1
CONTROL:0x9004
OWNER:S-1-5-21-2926364953-924364008-418108241-1000
GROUP:S-1-22-2-1001
ACL:S-1-5-21-2926364953-924364008-418108241-1000:ALLOWED/0x0/0x1e01ff
ACL:S-1-22-2-1001:ALLOWED/0x0/R
ACL:S-1-22-2-1001:ALLOWED/0x0/R
ACL:S-1-5-21-2926364953-924364008-418108241-1000:ALLOWED/0x0/0x1e01ff
ACL:S-1-1-0:ALLOWED/0x0/R
# setcifsacl -a "ACL:S-1-22-2-1004:ALLOWED/0x0/R" /mnt/test/acltestthis setacl will cause the following KASAN splat:
[ 330.777927] BUG: KASAN: slab-out-of-bounds in send_set_info+0x4dd/0xc20 [cifs]
[ 330.779696] Write of size 696 at addr ffff88010d5e2860 by task setcifsacl/1012[ 330.781882] CPU: 1 PID: 1012 Comm: setcifsacl Not tainted 4.18.0-rc2+ #2
[ 330.783140] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
[ 330.784395] Call Trace:
[ 330.784789] dump_stack+0xc2/0x16b
[ 330.786777] print_address_description+0x6a/0x270
[ 330.787520] kasan_report+0x258/0x380
[ 330.788845] memcpy+0x34/0x50
[ 330.789369] send_set_info+0x4dd/0xc20 [cifs]
[ 330.799511] SMB2_set_acl+0x76/0xa0 [cifs]
[ 330.801395] set_smb2_acl+0x7ac/0xf30 [cifs]
[ 330.830888] cifs_xattr_set+0x963/0xe40 [cifs]
[ 330.840367] __vfs_setxattr+0x84/0xb0
[ 330.842060] __vfs_setxattr_noperm+0xe6/0x370
[ 330.843848] vfs_setxattr+0xc2/0xd0
[ 330.845519] setxattr+0x258/0x320
[ 330.859211] path_setxattr+0x15b/0x1b0
[ 330.864392] __x64_sys_setxattr+0xc0/0x160
[ 330.866133] do_syscall_64+0x14e/0x4b0
[ 330.876631] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 330.878503] RIP: 0033:0x7ff2e507db0a
[ 330.880151] Code: 48 8b 0d 89 93 2c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 49 89 ca b8 bc 00 00 00 0f 05 3d 01 f0 ff ff 73 01 c3 48 8b 0d 56 93 2c 00 f7 d8 64 89 01 48
[ 330.885358] RSP: 002b:00007ffdc4903c18 EFLAGS: 00000246 ORIG_RAX: 00000000000000bc
[ 330.887733] RAX: ffffffffffffffda RBX: 000055d1170de140 RCX: 00007ff2e507db0a
[ 330.890067] RDX: 000055d1170de7d0 RSI: 000055d115b39184 RDI: 00007ffdc4904818
[ 330.892410] RBP: 0000000000000001 R08: 0000000000000000 R09: 000055d1170de7e4
[ 330.894785] R10: 00000000000002b8 R11: 0000000000000246 R12: 0000000000000007
[ 330.897148] R13: 000055d1170de0c0 R14: 0000000000000008 R15: 000055d1170de550[ 330.901057] Allocated by task 1012:
[ 330.902888] kasan_kmalloc+0xa0/0xd0
[ 330.904714] kmem_cache_alloc+0xc8/0x1d0
[ 330.906615] mempool_alloc+0x11e/0x380
[ 330.908496] cifs_small_buf_get+0x35/0x60 [cifs]
[ 330.910510] smb2_plain_req_init+0x4a/0xd60 [cifs]
[ 330.912551] send_set_info+0x198/0xc20 [cifs]
[ 330.914535] SMB2_set_acl+0x76/0xa0 [cifs]
[ 330.916465] set_smb2_acl+0x7ac/0xf30 [cifs]
[ 330.918453] cifs_xattr_set+0x963/0xe40 [cifs]
[ 330.920426] __vfs_setxattr+0x84/0xb0
[ 330.922284] __vfs_setxattr_noperm+0xe6/0x370
[ 330.924213] vfs_setxattr+0xc2/0xd0
[ 330.926008] setxattr+0x258/0x320
[ 330.927762] path_setxattr+0x15b/0x1b0
[ 330.929592] __x64_sys_setxattr+0xc0/0x160
[ 330.931459] do_syscall_64+0x14e/0x4b0
[ 330.933314] entry_SYSCALL_64_after_hwframe+0x44/0xa9[ 330.936843] Freed by task 0:
[ 330.938588] (stack is not available)[ 330.941886] The buggy address belongs to the object at ffff88010d5e2800
which belongs to the cache cifs_small_rq of size 448
[ 330.946362] The buggy address is located 96 bytes inside of
448-byte region [ffff88010d5e2800, ffff88010d5e29c0)
[ 330.950722] The buggy address belongs to the page:
[ 330.952789] page:ffffea0004357880 count:1 mapcount:0 mapping:ffff880108fdca80 index:0x0 compound_mapcount: 0
[ 330.955665] flags: 0x17ffffc0008100(slab|head)
[ 330.957760] raw: 0017ffffc0008100 dead000000000100 dead000000000200 ffff880108fdca80
[ 330.960356] raw: 0000000000000000 0000000080100010 00000001ffffffff 0000000000000000
[ 330.963005] page dumped because: kasan: bad access detected[ 330.967039] Memory state around the buggy address:
[ 330.969255] ffff88010d5e2880: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 330.971833] ffff88010d5e2900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 330.974397] >ffff88010d5e2980: 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc
[ 330.976956] ^
[ 330.979226] ffff88010d5e2a00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 330.981755] ffff88010d5e2a80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 330.984225] ==================================================================Fix this by allocating a regular CIFS buffer in
smb2_plain_req_init() if the request command is SMB2_SET_INFO.Reported-by: Jianhong Yin
Fixes: 366ed846df60 ("cifs: Use smb 2 - 3 and cifsacl mount options setacl function")
CC: Stable
Signed-off-by: Stefano Brivio
Reviewed-and-tested-by: Aurelien Aptel
Signed-off-by: Steve French
Signed-off-by: Greg Kroah-Hartman -
commit 7ffbe65578b44fafdef577a360eb0583929f7c6e upstream.
For every request we send, whether it is SMB1 or SMB2+, we attempt to
reconnect tcon (cifs_reconnect_tcon or smb2_reconnect) before carrying
out the request.So, while server->tcpStatus != CifsNeedReconnect, we wait for the
reconnection to succeed on wait_event_interruptible_timeout(). If it
returns, that means that either the condition was evaluated to true, or
timeout elapsed, or it was interrupted by a signal.Since we're not handling the case where the process woke up due to a
received signal (-ERESTARTSYS), the next call to
wait_event_interruptible_timeout() will _always_ fail and we end up
looping forever inside either cifs_reconnect_tcon() or smb2_reconnect().Here's an example of how to trigger that:
$ mount.cifs //foo/share /mnt/test -o
username=foo,password=foo,vers=1.0,hard(break connection to server before executing bellow cmd)
$ stat -f /mnt/test & sleep 140
[1] 2511$ ps -aux -q 2511
USER PID %CPU %MEM VSZ RSS TTY STAT START TIME COMMAND
root 2511 0.0 0.0 12892 1008 pts/0 S 12:24 0:00 stat -f
/mnt/test$ kill -9 2511
(wait for a while; process is stuck in the kernel)
$ ps -aux -q 2511
USER PID %CPU %MEM VSZ RSS TTY STAT START TIME COMMAND
root 2511 83.2 0.0 12892 1008 pts/0 R 12:24 30:01 stat -f
/mnt/testBy using 'hard' mount point means that cifs.ko will keep retrying
indefinitely, however we must allow the process to be killed otherwise
it would hang the system.Signed-off-by: Paulo Alcantara
Cc: stable@vger.kernel.org
Reviewed-by: Aurelien Aptel
Signed-off-by: Steve French
Signed-off-by: Greg Kroah-Hartman
