Currently, we try to ensure that we use vcnum of 0 on the first
established session on a connection and then try to use a different
vcnum on each session after that.
This is a little odd, since there's no real reason to use a different
vcnum for each SMB session. I can only assume there was some confusion
between SMB sessions and VCs. That's somewhat understandable since they
both get created during SESSION_SETUP, but the documentation indicates
that they are really orthogonal. The comment on max_vcs in particular
looks quite misguided. An SMB session is already uniquely identified
by the SMB UID value -- there's no need to again uniquely ID with a
VC.
Furthermore, a vcnum of 0 is a cue to the server that it should release
any resources that were previously held by the client. This sounds like
a good thing, until you consider that:
a) it totally ignores the fact that other programs on the box (e.g.
smbclient) might have connections established to the server. Using a
vcnum of 0 causes them to get kicked off.
b) it causes problems with NAT. If several clients are connected to the
same server via the same NAT'ed address, whenever one connects to the
server it kicks off all the others, which then reconnect and kick off
the first one...ad nauseum.
I don't see any reason to ignore the advice in "Implementing CIFS" which
has a comprehensive treatment of virtual circuits. In there, it states
"...and contrary to the specs the client should always use a VcNumber of
one, never zero."
Have the client just use a hardcoded vcnum of 1, and stop abusing the
special behavior of vcnum 0.
Reported-by: Sauron99@gmx.de <sauron99@gmx.de>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Reviewed-by: Volker Lendecke <vl@samba.org>
Signed-off-by: Steve French <smfrench@gmail.com>
by using a query reparse ioctl request.
Acked-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Pavel Shilovsky <pshilovsky@samba.org>
Signed-off-by: Steve French <smfrench@gmail.com>
Currently the trans2 ByteCount field is incorrectly left zero in
TRANS2_QUERY_FILE_INFO info_level=SMB_QUERY_FILE_ALL_INFO and
info_level=SMB_QUERY_FILE_UNIX_BASIC requests. The field should properly
reflect the FID, information_level and padding bytes carried in these
requests.
Leaving this field zero causes such requests to fail against Novell CIFS
servers. Other SMB servers (e.g. Samba) use the parameter count fields
for data length calculations instead, so do not suffer the same fate.
Signed-off-by: David Disseldorp <ddiss@suse.de>
Acked-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Steve French <smfrench@gmail.com>
Commit 9ddec56131 (cifs: move handling of signed connections into
separate function) broke signing on SMB2/3 connections. While the code
to enable signing on the connections was very similar between the two,
the bits that get set in the sec_mode are different.
Declare a couple of new smb_version_values fields and set them
appropriately for SMB1 and SMB2/3. Then change cifs_enable_signing to
use those instead.
Reported-by: Shirish Pargaonkar <shirishpargaonkar@gmail.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Tested-by: Shirish Pargaonkar <shirishpargaonkar@gmail.com>
Signed-off-by: Steve French <smfrench@gmail.com>
Now that we track what sort of NEGOTIATE response was received, stop
mandating that every session on a socket use the same type of auth.
Push that decision out into the session setup code, and make the sectype
a per-session property. This should allow us to mix multiple sectypes on
a socket as long as they are compatible with the NEGOTIATE response.
With this too, we can now eliminate the ses->secFlg field since that
info is redundant and harder to work with than a securityEnum.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Acked-by: Pavel Shilovsky <piastry@etersoft.ru>
Signed-off-by: Steve French <smfrench@gmail.com>
Currently, we determine this according to flags in the sec_mode, flags
in the global_secflags and via other methods. That makes the semantics
very hard to follow and there are corner cases where we don't handle
this correctly.
Add a new bool to the TCP_Server_Info that acts as a simple flag to tell
us whether signing is enabled on this connection or not, and fix up the
places that need to determine this to use that flag.
This is a bit weird for the SMB2 case, where signing is per-session.
SMB2 needs work in this area already though. The existing SMB2 code has
similar logic to what we're using here, so there should be no real
change in behavior. These changes should make it easier to implement
per-session signing in the future though.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Reviewed-by: Pavel Shilovsky <piastry@etersoft.ru>
Signed-off-by: Steve French <smfrench@gmail.com>
Track what sort of NEGOTIATE response we get from the server, as that
will govern what sort of authentication types this socket will support.
There are three possibilities:
LANMAN: server sent legacy LANMAN-type response
UNENCAP: server sent a newer-style response, but extended security bit
wasn't set. This socket will only support unencapsulated auth types.
EXTENDED: server sent a newer-style response with the extended security
bit set. This is necessary to support krb5 and ntlmssp auth types.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Reviewed-by: Pavel Shilovsky <piastry@etersoft.ru>
Signed-off-by: Steve French <smfrench@gmail.com>
Move the sanity checks for signed connections into a separate function.
SMB2's was a cut-and-paste job from CIFS code, so we can make them use
the same function.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Reviewed-by: Pavel Shilovsky <piastry@etersoft.ru>
Signed-off-by: Steve French <smfrench@gmail.com>
...this also gets rid of some #ifdef ugliness too.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Acked-by: Pavel Shilovsky <piastry@etersoft.ru>
Signed-off-by: Steve French <sfrench@us.ibm.com>
...cleanup.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Acked-by: Pavel Shilovsky <piastry@etersoft.ru>
Signed-off-by: Steve French <sfrench@us.ibm.com>
These look pretty cargo-culty to me, but let's be certain. Leave
them in place for now. Pop a WARN if it ever does happen. Also,
move to a more standard idiom for setting the "server" pointer.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Reviewed-by: Pavel Shilovsky <piastry@etersoft.ru>
Signed-off-by: Steve French <sfrench@us.ibm.com>
Currently, the signing routines take a pointer to a place to store the
expected sequence number for the mid response. It then stores a value
that's one below what that sequence number should be, and then adds one
to it when verifying the signature on the response.
Increment the sequence number before storing the value in the mid, and
eliminate the "+1" when checking the signature.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Steve French <sfrench@us.ibm.com>
Signed-off-by: Steve French <smfrench@gmail.com>
It's not obvious from reading the macro names that these macros
are for debugging. Convert the names to a single more typical
kernel style cifs_dbg macro.
cERROR(1, ...) -> cifs_dbg(VFS, ...)
cFYI(1, ...) -> cifs_dbg(FYI, ...)
cFYI(DBG2, ...) -> cifs_dbg(NOISY, ...)
Move the terminating format newline from the macro to the call site.
Add CONFIG_CIFS_DEBUG function cifs_vfs_err to emit the
"CIFS VFS: " prefix for VFS messages.
Size is reduced ~ 1% when CONFIG_CIFS_DEBUG is set (default y)
$ size fs/cifs/cifs.ko*
text data bss dec hex filename
265245 2525 132 267902 4167e fs/cifs/cifs.ko.new
268359 2525 132 271016 422a8 fs/cifs/cifs.ko.old
Other miscellaneous changes around these conversions:
o Miscellaneous typo fixes
o Add terminating \n's to almost all formats and remove them
from the macros to be more kernel style like. A few formats
previously had defective \n's
o Remove unnecessary OOM messages as kmalloc() calls dump_stack
o Coalesce formats to make grep easier,
added missing spaces when coalescing formats
o Use %s, __func__ instead of embedded function name
o Removed unnecessary "cifs: " prefixes
o Convert kzalloc with multiply to kcalloc
o Remove unused cifswarn macro
Signed-off-by: Joe Perches <joe@perches.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Steve French <smfrench@gmail.com>
This replaces calls to kmalloc followed by memcpy with a single call to
kmemdup. This was found via make coccicheck.
Signed-off-by: Silviu-Mihai Popescu <silviupopescu1990@gmail.com>
Signed-off-by: Steve French <sfrench@us.ibm.com>
Signed-off-by: Steve French <smfrench@gmail.com>
We had a recent fix to fix the release of pagecache pages when
cifs_writev_requeue writes fail. Unfortunately, it releases the page
before trying to unlock it. At that point, the page might be gone by the
time the unlock comes in.
Unlock the page first before checking the value of "rc", and only then
end writeback and release the pages. The page lock isn't required for
any of those operations so this should be safe.
Reported-by: Anton Altaparmakov <aia21@cam.ac.uk>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Steve French <sfrench@us.ibm.com>
Pages get the PG_writeback flag set before cifs sends its
request to SMB server in cifs_writepages(), if the SMB service
goes down, cifs may try to recommit the writing requests in
cifs_writev_requeue(). However, it does not clean its PG_writeback
flag and relaimed the pages even if it fails again in
cifs_writev_requeue(), which may lead to the hanging of the
processes accessing the cifs directory. This patch just cleans
the PG_writeback flags and reclaims the pages under that circumstances.
Steps to reproduce the bug(trying serveral times may trigger the issue):
1.Write from cifs client continuously.(e.g dd if=/dev/zero of=<cifs file>)
2.Stop SMB service from server.(e.g service smb stop)
3.Wait for two minutes， and then start SMB service from
server.(e.g service smb start)
4.The processes which are accessing cifs directory may hang up.
Signed-off-by: Ouyang Maochun <ouyang.maochun@zte.com.cn>
Signed-off-by: Jiang Yong <jian.yong5@zte.com.cn>
Tested-by: Zhang Xianwei <zhang.xianwei8@zte.com.cn>
Reviewed-by: Wang Liang <wang.liang82@zte.com.cn>
Reviewed-by: Cai Qu <cai.qu@zte.com.cn>
Reviewed-by: Jiang Biao <jiang.biao2@zte.com.cn>
Reviewed-by: Jeff Layton <jlayton@redhat.com>
Reviewed-by: Pavel Shilovsky <piastry@etersoft.ru>
Signed-off-by: Steve French <sfrench@us.ibm.com>
Use INVALID_UID and INVALID_GID instead of NO_CHANGE_64 to indicate
the value should not be changed.
In cifs_fill_unix_set_info convert from kuids and kgids into uids and
gids that will fit in FILE_UNIX_BASIC_INFO.
Cc: Steve French <smfrench@gmail.com>
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
Rebased and resending the patch.
Path based queries can fail for lack of access, especially during lookup
during open.
open itself would actually succeed becasue of back up intent bit
but queries (either path or file handle based) do not have a means to
specifiy backup intent bit.
So query the file info during lookup using
trans2 / findfirst / file_id_full_dir_info
to obtain file info as well as file_id/inode value.
Signed-off-by: Shirish Pargaonkar <shirishpargaonkar@gmail.com>
Acked-by: Jeff Layton <jlayton@samba.org>
Signed-off-by: Steve French <smfrench@gmail.com>
Replace the "marshal_iov" function with a "read_into_pages" function.
That function will copy the read data off the socket and into the
pages array, kmapping and reading pages one at a time.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
For now, none of the callers populate rq_pages. That will be done for
writes in a later patch. While we're at it, change the prototype of
setup_async_request not to need a return pointer argument. Just
return the pointer to the mid_q_entry or an ERR_PTR.
Reviewed-by: Pavel Shilovsky <pshilovsky@samba.org>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Steve French <smfrench@gmail.com>
We need a way to represent a call to be sent on the wire that does not
require having all of the page data kmapped. Behold the smb_rqst struct.
This new struct represents an array of kvecs immediately followed by an
array of pages.
Convert the signing routines to use these structs under the hood and
turn the existing functions for this into wrappers around that. For now,
we're just changing these functions to take different args. Later, we'll
teach them how to deal with arrays of pages.
Reviewed-by: Pavel Shilovsky <pshilovsky@samba.org>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Steve French <smfrench@gmail.com>
Signed-off-by: Pavel Shilovsky <pshilovsky@samba.org>
Signed-off-by: Steve French <sfrench@us.ibm.com>
Signed-off-by: Steve French <smfrench@gmail.com>
Signed-off-by: Pavel Shilovsky <pshilovsky@samba.org>
Signed-off-by: Steve French <smfrench@gmail.com>
Signed-off-by: Steve French <sfrench@us.ibm.com>
This is help us to extend the code for future protocols that can use
another fid mechanism (as SMB2 that has it divided into two parts:
persistent and violatile).
Also rename variables and refactor the code around the changes.
Reviewed-by: Jeff Layton <jlayton@samba.org>
Signed-off-by: Pavel Shilovsky <pshilovsky@samba.org>
Signed-off-by: Steve French <smfrench@gmail.com>
While trying to debug a SMB signature related issue with Windows Servers
figured out it might be easier to debug if we print the error code from
cifs_verify_signature(). Also, fix indendation while at it.
Signed-off-by: Suresh Jayaraman <sjayaraman@suse.com>
Reviewed-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Steve French <smfrench@gmail.com>
and add negotiate request type to let set_credits know that
we are only on negotiate stage and no need to make a decision
about disabling echos and oplocks.
Signed-off-by: Pavel Shilovsky <piastry@etersoft.ru>
Signed-off-by: Steve French <smfrench@gmail.com>
and rename variables around the code changes.
Reviewed-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Pavel Shilovsky <pshilovsky@samba.org>
Signed-off-by: Steve French <smfrench@gmail.com>
Split all requests to echos, oplocks and others - each group uses
its own credit slot. This is indicated by new flags
CIFS_ECHO_OP and CIFS_OBREAK_OP
that are not used now for CIFS. This change is required to support
SMB2 protocol because of different processing of these commands.
Acked-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Pavel Shilovsky <pshilovsky@samba.org>
Signed-off-by: Steve French <smfrench@gmail.com>
struct file_lock is pretty large, so we really don't want that on the
stack in a potentially long call chain. Reorganize the arguments to
CIFSSMBPosixLock to eliminate the need for that.
Eliminate the get_flag and simply use a non-NULL pLockInfo to indicate
that this is a "get" operation. In order to do that, need to add a new
loff_t argument for the start_offset.
Reported-by: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Steve French <smfrench@gmail.com>
Those macros add a newline on their own, so there's not any need to
embed one in the message itself.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Steve French <smfrench@gmail.com>
Jeff Layton