12 Apr, 2011
15 commits
-
This is more or less the same patch as before, but with some merge
conflicts fixed up.If a process has a dirty page mapped into its page tables, then it has
the ability to change it while the client is trying to write the data
out to the server. If that happens after the signature has been
calculated then that signature will then be wrong, and the server will
likely reset the TCP connection.This patch adds a page_mkwrite handler for CIFS that simply takes the
page lock. Because the page lock is held over the life of writepage and
writepages, this prevents the page from becoming writeable until
the write call has completed.With this, we can also remove the "sign_zero_copy" module option and
always inline the pages when writing.Signed-off-by: Jeff Layton
Signed-off-by: Steve French -
Warn once if default security (ntlm) requested. We will
update the default to the stronger security mechanism
(ntlmv2) in 2.6.41. Kerberos is also stronger than
ntlm, but more servers support ntlmv2 and ntlmv2
does not require an upcall, so ntlmv2 is a better
default.Reviewed-by: Jeff Layton
CC: Suresh Jayaraman
Reviewed-by: Shirish Pargaonkar
Signed-off-by: Steve French -
When the TCP_Server_Info is first allocated and connected, tcpStatus ==
CifsGood means that the NEGOTIATE_PROTOCOL request has completed and the
socket is ready for other calls. cifs_reconnect however sets tcpStatus
to CifsGood as soon as the socket is reconnected and the optional
RFC1001 session setup is done. We have no clear way to tell the
difference between these two states, and we need to know this in order
to know whether we can send an echo or not.Resolve this by adding a new statusEnum value -- CifsNeedNegotiate. When
the socket has been connected but has not yet had a NEGOTIATE_PROTOCOL
request done, set it to this value. Once the NEGOTIATE is done,
cifs_negotiate_protocol will set tcpStatus to CifsGood.This also fixes and cleans the logic in cifs_reconnect and
cifs_reconnect_tcon. The old code checked for specific states when what
it really wants to know is whether the state has actually changed from
CifsNeedReconnect.Reported-and-Tested-by: JG
Signed-off-by: Jeff Layton
Signed-off-by: Steve French -
While testing my patchset to fix asynchronous writes, I hit a bunch
of signature problems when testing with signing on. The problem seems
to be that signature checks on receive can be running at the same
time as a process that is sending, or even that multiple receives can
be checking signatures at the same time, clobbering the same data
structures.While we're at it, clean up the comments over cifs_calculate_signature
and add a note that the srv_mutex should be held when calling this
function.This patch seems to fix the problems for me, but I'm not clear on
whether it's the best approach. If it is, then this should probably
go to stable too.Cc: stable@kernel.org
Cc: Shirish Pargaonkar
Signed-off-by: Jeff Layton
Signed-off-by: Steve French -
Minor revision to the original patch. Don't abuse the __le16 variable
on the stack by casting it to wchar_t and handing it off to char2uni.
Declare an actual wchar_t on the stack instead. This fixes a valid
sparse warning.Fix the spelling of UNI_ASTERISK. Eliminate the unneeded len_remaining
variable in cifsConvertToUCS.Also, as David Howells points out. We were better off making
cifsConvertToUCS *not* use put_unaligned_le16 since it means that we
can't optimize the mapped characters at compile time. Switch them
instead to use cpu_to_le16, and simply use put_unaligned to set them
in the string.Reported-and-acked-by: David Howells
Signed-off-by: Jeff Layton
Signed-off-by: Steve French -
Thus spake David Howells:
The code that follows this:
remaining = total_data_size - data_in_this_rsp;
if (remaining == 0)
return 0;
else if (remaining < 0) {generates better code if you drop the 'remaining' variable and compare
the values directly.Clean it up per his recommendation...
Reported-and-acked-by: David Howells
Signed-off-by: Jeff Layton
Signed-off-by: Steve French -
Commit 522440ed made cifs set backing_dev_info on the mapping attached
to new inodes. This change caused a fairly significant read performance
regression, as cifs started doing page-sized reads exclusively.By virtue of the fact that they're allocated as part of cifs_sb_info by
kzalloc, the ra_pages on cifs BDIs get set to 0, which prevents any
readahead. This forces the normal read codepaths to use readpage instead
of readpages causing a four-fold increase in the number of read calls
with the default rsize.Fix it by setting ra_pages in the BDI to the same value as that in the
default_backing_dev_info.Fixes https://bugzilla.kernel.org/show_bug.cgi?id=31662
Cc: stable@kernel.org
Reported-and-Tested-by: Till
Signed-off-by: Jeff Layton
Signed-off-by: Steve French -
The BCC is still __le16 at this point, and in any case we need to
use the get_bcc_le macro to make sure we don't hit alignment
problems.Signed-off-by: Jeff Layton
Signed-off-by: Steve French -
Currently, we skip doing the is_path_accessible check in cifs_mount if
there is no prefixpath. I have a report of at least one server however
that allows a TREE_CONNECT to a share that has a DFS referral at its
root. The reporter in this case was using a UNC that had no prefixpath,
so the is_path_accessible check was not triggered and the box later hit
a BUG() because we were chasing a DFS referral on the root dentry for
the mount.This patch fixes this by removing the check for a zero-length
prefixpath. That should make the is_path_accessible check be done in
this situation and should allow the client to chase the DFS referral at
mount time instead.Cc: stable@kernel.org
Reported-and-Tested-by: Yogesh Sharma
Signed-off-by: Jeff Layton
Signed-off-by: Steve French -
make modules C=2 M=fs/cifs CF=-D__CHECK_ENDIAN__
Found for example:
CHECK fs/cifs/cifssmb.c
fs/cifs/cifssmb.c:728:22: warning: incorrect type in assignment (different base types)
fs/cifs/cifssmb.c:728:22: expected unsigned short [unsigned] [usertype] Tid
fs/cifs/cifssmb.c:728:22: got restricted __le16 [usertype]
fs/cifs/cifssmb.c:1883:45: warning: incorrect type in assignment (different base types)
fs/cifs/cifssmb.c:1883:45: expected long long [signed] [usertype] fl_start
fs/cifs/cifssmb.c:1883:45: got restricted __le64 [usertype] start
fs/cifs/cifssmb.c:1884:54: warning: restricted __le64 degrades to integer
fs/cifs/cifssmb.c:1885:58: warning: restricted __le64 degrades to integer
fs/cifs/cifssmb.c:1886:43: warning: incorrect type in assignment (different base types)
fs/cifs/cifssmb.c:1886:43: expected unsigned int [unsigned] fl_pid
fs/cifs/cifssmb.c:1886:43: got restricted __le32 [usertype] pidIn checking new smb2 code for missing endian conversions, I noticed
some endian errors had crept in over the last few releases into the
cifs code (symlink, ntlmssp, posix lock, and also a less problematic warning
in fscache). A followon patch will address a few smb2 endian
problems.Reviewed-by: Jeff Layton
Signed-off-by: Steve French -
Ports are __be16 not unsigned short int
Eliminates the remaining fixable endian warnings:
~/cifs-2.6$ make modules C=1 M=fs/cifs CF=-D__CHECK_ENDIAN__
CHECK fs/cifs/connect.c
fs/cifs/connect.c:2408:23: warning: incorrect type in assignment (different base types)
fs/cifs/connect.c:2408:23: expected unsigned short *sport
fs/cifs/connect.c:2408:23: got restricted __be16 *
fs/cifs/connect.c:2410:23: warning: incorrect type in assignment (different base types)
fs/cifs/connect.c:2410:23: expected unsigned short *sport
fs/cifs/connect.c:2410:23: got restricted __be16 *
fs/cifs/connect.c:2416:24: warning: incorrect type in assignment (different base types)
fs/cifs/connect.c:2416:24: expected unsigned short [unsigned] [short]
fs/cifs/connect.c:2416:24: got restricted __be16 [usertype]
fs/cifs/connect.c:2423:24: warning: incorrect type in assignment (different base types)
fs/cifs/connect.c:2423:24: expected unsigned short [unsigned] [short]
fs/cifs/connect.c:2423:24: got restricted __be16 [usertype]
fs/cifs/connect.c:2326:23: warning: incorrect type in assignment (different base types)
fs/cifs/connect.c:2326:23: expected unsigned short [unsigned] sport
fs/cifs/connect.c:2326:23: got restricted __be16 [usertype] sin6_port
fs/cifs/connect.c:2330:23: warning: incorrect type in assignment (different base types)
fs/cifs/connect.c:2330:23: expected unsigned short [unsigned] sport
fs/cifs/connect.c:2330:23: got restricted __be16 [usertype] sin_port
fs/cifs/connect.c:2394:22: warning: restricted __be16 degrades to integerSigned-off-by: Steve French
-
Max share name was set to 64, and (at least for Windows)
can be 80.Signed-off-by: Steve French
-
We artificially limited the user name to 32 bytes, but modern servers handle
larger. Set the maximum length to a reasonable 256, and make the user name
string dynamically allocated rather than a fixed size in session structure.
Also clean up old checkpatch warning.Signed-off-by: Steve French
-
This flag currently only affects whether we allow "zero-copy" writes
with signing enabled. Typically we map pages in the pagecache directly
into the write request. If signing is enabled however and the contents
of the page change after the signature is calculated but before the
write is sent then the signature will be wrong. Servers typically
respond to this by closing down the socket.Still, this can provide a performance benefit so the "Experimental" flag
was overloaded to allow this. That's really not a good place for this
option however since it's not clear what that flag does.Move that flag instead to a new module parameter that better describes
its purpose. That's also better since it can be set at module insertion
time by configuring modprobe.d.Reviewed-by: Suresh Jayaraman
Signed-off-by: Jeff Layton
Signed-off-by: Steve French -
cifs_close doesn't check that the filp->private_data is non-NULL before
trying to put it. That can cause an oops in certain error conditions
that can occur on open or lookup before the private_data is set.Reported-by: Ben Greear
CC: Stable
Signed-off-by: Jeff Layton
Signed-off-by: Steve French
31 Mar, 2011
1 commit
-
Fixes generated by 'codespell' and manually reviewed.
Signed-off-by: Lucas De Marchi
10 Mar, 2011
1 commit
-
Code has been converted over to the new explicit on-stack plugging,
and delay users have been converted to use the new API for that.
So lets kill off the old plugging along with aops->sync_page().Signed-off-by: Jens Axboe
22 Feb, 2011
2 commits
-
Update version to 1.71 so we can more easily spot modules with the last two fixes
Signed-off-by: Steve French
-
LANMAN response length was changed to 16 bytes instead of 24 bytes.
Revert it back to 24 bytes.Signed-off-by: Shirish Pargaonkar
CC: stable@kernel.org
Signed-off-by: Steve French
17 Feb, 2011
1 commit
-
The code finds, the '%' sign in an ipv6 address and copies that to a
buffer allocated on the stack. It then ignores that buffer, and passes
'pct' to simple_strtoul(), which doesn't work right because we're
comparing 'endp' against a completely different string.Fix it by passing the correct pointer. While we're at it, this is a
good candidate for conversion to strict_strtoul as well.Cc: stable@kernel.org
Cc: David Howells
Reported-by: Björn JACKE
Signed-off-by: Jeff Layton
Signed-off-by: Steve French
11 Feb, 2011
1 commit
-
Slight revision to this patch...use min_t() instead of conditional
assignment. Also, remove the FIXME comment and replace it with the
explanation that Steve gave earlier.After receiving a packet, we currently check the header. If it's no
good, then we toss it out and continue the loop, leaving the caller
waiting on that response.In cases where the packet has length inconsistencies, but the MID is
valid, this leads to unneeded delays. That's especially problematic now
that the client waits indefinitely for responses.Instead, don't immediately discard the packet if checkSMB fails. Try to
find a matching mid_q_entry, mark it as having a malformed response and
issue the callback.Signed-off-by: Jeff Layton
Signed-off-by: Steve French
10 Feb, 2011
1 commit
-
Follow-on patch to 7e90d705 which is already in Steve's tree...
The check for tcpStatus == CifsGood is not meaningful since it doesn't
indicate whether the NEGOTIATE request has been done. Also, clarify
why we're checking for maxBuf == 0.Signed-off-by: Jeff Layton
Signed-off-by: Steve French
09 Feb, 2011
1 commit
-
In order to determine whether an SMBEcho request can be sent
we need to know that the socket is established (server tcpStatus == CifsGood)
AND that an SMB NegotiateProtocol has been sent (server maxBuf != 0).
Without the second check we can send an Echo request during reconnection
before the server can accept it.CC: JG
Reviewed-by: Jeff Layton
Signed-off-by: Steve French
08 Feb, 2011
1 commit
-
ses->status is never set to CifsExiting, so these checks are
always false.Tested-by: JG
Signed-off-by: Jeff Layton
Signed-off-by: Steve French
06 Feb, 2011
1 commit
-
Exit from parse_dacl if no memory returned from the call to kmalloc.
Signed-off-by: Stanislav Fomichev
Signed-off-by: Steve French
05 Feb, 2011
3 commits
-
When the socket to the server is disconnected, the client more or less
immediately calls cifs_reconnect to reconnect the socket. The NegProt
and SessSetup however are not done until an actual call needs to be
made.With the addition of the SMB echo code, it's possible that the server
will initiate a disconnect on an idle socket. The client will then
reconnect the socket but no NegotiateProtocol request is done. The
SMBEcho workqueue job will then eventually pop, and an SMBEcho will be
sent on the socket. The server will then reject it since no NegProt was
done.The ideal fix would be to either have the socket not be reconnected
until we plan to use it, or to immediately do a NegProt when the
reconnect occurs. The code is not structured for this however. For now
we must just settle for not sending any echoes until the NegProt is
done.Reported-by: JG
Signed-off-by: Jeff Layton
Signed-off-by: Steve French -
cifs_sign_smb only generates a signature if the correct Flags2 bit is
set. Make sure that it gets set correctly if we're sending an async
call.This patch fixes:
https://bugzilla.kernel.org/show_bug.cgi?id=28142
Reported-and-Tested-by: JG
Signed-off-by: Jeff Layton
Signed-off-by: Steve French -
Updating extended statistics here can cause slab memory corruption
if a callback function frees slab memory (mid_entry).Signed-off-by: Shirish Pargaonkar
Reviewed-by: Jeff Layton
Signed-off-by: Steve French
04 Feb, 2011
1 commit
-
Variable 'i' should be unsigned long as it's used in circle with num_pages,
and bytes_read/total_written should be ssize_t according to return value.Signed-off-by: Pavel Shilovsky
Reviewed-by: Shirish Pargaonkar
Signed-off-by: Steve French
02 Feb, 2011
1 commit
-
length at this point is the length returned by the last kernel_recvmsg
call. total_read is the length of all of the data read so far. length
is more or less meaningless at this point, so use total_read for
everything.Signed-off-by: Jeff Layton
Reviewed-by: Pavel Shilovsky
Signed-off-by: Steve French
01 Feb, 2011
3 commits
-
The cERROR message in checkSMB when the calculated length doesn't match
the RFC1001 length is incorrect in many cases. It always says that the
RFC1001 length is bigger than the SMB, even when it's actually the
reverse.Fix the error message to say the reverse of what it does now when the
SMB length goes beyond the end of the received data. Also, clarify the
error message when the RFC length is too big. Finally, clarify the
comments to show that the 512 byte limit on extra data at the end of
the packet is arbitrary.Signed-off-by: Jeff Layton
Signed-off-by: Steve French -
Signed-off-by: Steve French
-
Missed one change as per earlier suggestion.
Signed-off-by: Shirish Pargaonkar
Reviewed-by: Jeff Layton
Signed-off-by: Steve French
31 Jan, 2011
7 commits
-
New compiler warnings that I noticed when building a patchset based
on recent Fedora kernel:fs/cifs/cifssmb.c: In function 'CIFSSMBSetFileSize':
fs/cifs/cifssmb.c:4813:8: warning: variable 'data_offset' set but not used
[-Wunused-but-set-variable]fs/cifs/file.c: In function 'cifs_open':
fs/cifs/file.c:349:24: warning: variable 'pCifsInode' set but not used
[-Wunused-but-set-variable]
fs/cifs/file.c: In function 'cifs_partialpagewrite':
fs/cifs/file.c:1149:23: warning: variable 'cifs_sb' set but not used
[-Wunused-but-set-variable]
fs/cifs/file.c: In function 'cifs_iovec_write':
fs/cifs/file.c:1740:9: warning: passing argument 6 of 'CIFSSMBWrite2' from
incompatible pointer type [enabled by default]
fs/cifs/cifsproto.h:337:12: note: expected 'unsigned int *' but argument is
of type 'size_t *'fs/cifs/readdir.c: In function 'cifs_readdir':
fs/cifs/readdir.c:767:23: warning: variable 'cifs_sb' set but not used
[-Wunused-but-set-variable]fs/cifs/cifs_dfs_ref.c: In function 'cifs_dfs_d_automount':
fs/cifs/cifs_dfs_ref.c:342:2: warning: 'rc' may be used uninitialized in
this function [-Wuninitialized]
fs/cifs/cifs_dfs_ref.c:278:6: note: 'rc' was declared hereSigned-off-by: Jeff Layton
Reviewed-by: Pavel Shilovsky
Signed-off-by: Steve French -
Recently CIFS was changed to use the kernel crypto API for MD4 hashes,
but the Kconfig dependencies were not changed to reflect this.Signed-off-by: Jeff Layton
Reported-and-Tested-by: Suresh Jayaraman
Reviewed-by: Shirish Pargaonkar
Signed-off-by: Steve French -
Currently, we allow the pending_mid_q to grow without bound with
SIGKILL'ed processes. This could eventually be a DoS'able problem. An
unprivileged user could a process that does a long-running call and then
SIGKILL it.If he can also intercept the NT_CANCEL calls or the replies from the
server, then the pending_mid_q could grow very large, possibly even to
2^16 entries which might leave GetNextMid in an infinite loop. Fix this
by imposing a hard limit of 32k calls per server. If we cross that
limit, set the tcpStatus to CifsNeedReconnect to force cifsd to
eventually reconnect the socket and clean out the pending_mid_q.While we're at it, clean up the function a bit and eliminate an
unnecessary NULL pointer check.Signed-off-by: Jeff Layton
Reviewed-by: Shirish Pargaonkar
Signed-off-by: Steve French -
If we kill the process while it's sending on a socket then the
kernel_sendmsg will return -EINTR. This is normal. No need to spam the
ring buffer with this info.Signed-off-by: Jeff Layton
Signed-off-by: Steve French -
...just cleanup. There should be no behavior change.
Signed-off-by: Jeff Layton
Reviewed-by: Pavel Shilovsky
Signed-off-by: Steve French -
Use the new send_nt_cancel function to send an NT_CANCEL when the
process is delivered a fatal signal. This is a "best effort" enterprise
however, so don't bother to check the return code. There's nothing we
can reasonably do if it fails anyway.Reviewed-by: Pavel Shilovsky
Reviewed-by: Suresh Jayaraman
Signed-off-by: Jeff Layton
Signed-off-by: Steve French -
Currently, when a request is cancelled via signal, we delete the mid
immediately. If the request was already transmitted however, the client
is still likely to receive a response. When it does, it won't recognize
it however and will pop a printk.It's also a little dangerous to just delete the mid entry like this. We
may end up reusing that mid. If we do then we could potentially get the
response from the first request confused with the later one.Prevent the reuse of mids by marking them as cancelled and keeping them
on the pending_mid_q list. If the reply comes in, we'll delete it from
the list then. If it never comes, then we'll delete it at reconnect
or when cifsd comes down.Reviewed-by: Pavel Shilovsky
Signed-off-by: Jeff Layton
Signed-off-by: Steve French