Commit 0f9d140a authored by Linus Torvalds's avatar Linus Torvalds
Browse files

Merge tag '5.0-rc1-smb3-fixes' of git://git.samba.org/sfrench/cifs-2.6

Pull cifs fixes from Steve French:
 "A set of cifs/smb3 fixes, 4 for stable, most from Pavel. His patches
  fix an important set of crediting (flow control) problems, and also
  two problems in cifs_writepages, ddressing some large i/o and also
  compounding issues"

* tag '5.0-rc1-smb3-fixes' of git://git.samba.org/sfrench/cifs-2.6:
  cifs: update internal module version number
  CIFS: Fix error paths in writeback code
  CIFS: Move credit processing to mid callbacks for SMB3
  CIFS: Fix credits calculation for cancelled requests
  cifs: Fix potential OOB access of lock element array
  cifs: Limit memory used by lock request calls to a page
  cifs: move large array from stack to heap
  CIFS: Do not hide EINTR after sending network packets
  CIFS: Fix credit computation for compounded requests
  CIFS: Do not set credits to 1 if the server didn't grant anything
  CIFS: Fix adjustment of credits for MTU requests
  cifs: Fix a tiny potential memory leak
  cifs: Fix a debug message
parents e1706720 48d2ba62
......@@ -150,5 +150,5 @@ extern long cifs_ioctl(struct file *filep, unsigned int cmd, unsigned long arg);
extern const struct export_operations cifs_export_ops;
#endif /* CONFIG_CIFS_NFSD_EXPORT */
#define CIFS_VERSION "2.15"
#define CIFS_VERSION "2.16"
#endif /* _CIFSFS_H */
......@@ -1438,6 +1438,7 @@ struct mid_q_entry {
int mid_state; /* wish this were enum but can not pass to wait_event */
unsigned int mid_flags;
__le16 command; /* smb command code */
unsigned int optype; /* operation type */
bool large_buf:1; /* if valid response, is pointer to large buf */
bool multiRsp:1; /* multiple trans2 responses for one request */
bool multiEnd:1; /* both received */
......@@ -1574,6 +1575,25 @@ static inline void free_dfs_info_array(struct dfs_info3_param *param,
kfree(param);
}
static inline bool is_interrupt_error(int error)
{
switch (error) {
case -EINTR:
case -ERESTARTSYS:
case -ERESTARTNOHAND:
case -ERESTARTNOINTR:
return true;
}
return false;
}
static inline bool is_retryable_error(int error)
{
if (is_interrupt_error(error) || error == -EAGAIN)
return true;
return false;
}
#define MID_FREE 0
#define MID_REQUEST_ALLOCATED 1
#define MID_REQUEST_SUBMITTED 2
......
......@@ -128,24 +128,31 @@ static int __cifs_reconnect_tcon(const struct nls_table *nlsc,
int rc;
struct dfs_cache_tgt_list tl;
struct dfs_cache_tgt_iterator *it = NULL;
char tree[MAX_TREE_SIZE + 1];
char *tree;
const char *tcp_host;
size_t tcp_host_len;
const char *dfs_host;
size_t dfs_host_len;
tree = kzalloc(MAX_TREE_SIZE, GFP_KERNEL);
if (!tree)
return -ENOMEM;
if (tcon->ipc) {
snprintf(tree, sizeof(tree), "\\\\%s\\IPC$",
snprintf(tree, MAX_TREE_SIZE, "\\\\%s\\IPC$",
tcon->ses->server->hostname);
return CIFSTCon(0, tcon->ses, tree, tcon, nlsc);
rc = CIFSTCon(0, tcon->ses, tree, tcon, nlsc);
goto out;
}
if (!tcon->dfs_path)
return CIFSTCon(0, tcon->ses, tcon->treeName, tcon, nlsc);
if (!tcon->dfs_path) {
rc = CIFSTCon(0, tcon->ses, tcon->treeName, tcon, nlsc);
goto out;
}
rc = dfs_cache_noreq_find(tcon->dfs_path + 1, NULL, &tl);
if (rc)
return rc;
goto out;
extract_unc_hostname(tcon->ses->server->hostname, &tcp_host,
&tcp_host_len);
......@@ -165,7 +172,7 @@ static int __cifs_reconnect_tcon(const struct nls_table *nlsc,
continue;
}
snprintf(tree, sizeof(tree), "\\%s", tgt);
snprintf(tree, MAX_TREE_SIZE, "\\%s", tgt);
rc = CIFSTCon(0, tcon->ses, tree, tcon, nlsc);
if (!rc)
......@@ -182,6 +189,8 @@ static int __cifs_reconnect_tcon(const struct nls_table *nlsc,
rc = -ENOENT;
}
dfs_cache_free_tgts(&tl);
out:
kfree(tree);
return rc;
}
#else
......@@ -2114,7 +2123,7 @@ cifs_writev_requeue(struct cifs_writedata *wdata)
for (j = 0; j < nr_pages; j++) {
unlock_page(wdata2->pages[j]);
if (rc != 0 && rc != -EAGAIN) {
if (rc != 0 && !is_retryable_error(rc)) {
SetPageError(wdata2->pages[j]);
end_page_writeback(wdata2->pages[j]);
put_page(wdata2->pages[j]);
......@@ -2123,7 +2132,7 @@ cifs_writev_requeue(struct cifs_writedata *wdata)
if (rc) {
kref_put(&wdata2->refcount, cifs_writedata_release);
if (rc == -EAGAIN)
if (is_retryable_error(rc))
continue;
break;
}
......@@ -2132,7 +2141,8 @@ cifs_writev_requeue(struct cifs_writedata *wdata)
i += nr_pages;
} while (i < wdata->nr_pages);
mapping_set_error(inode->i_mapping, rc);
if (rc != 0 && !is_retryable_error(rc))
mapping_set_error(inode->i_mapping, rc);
kref_put(&wdata->refcount, cifs_writedata_release);
}
......
......@@ -433,9 +433,10 @@ static void reconn_inval_dfs_target(struct TCP_Server_Info *server,
kfree(server->hostname);
server->hostname = extract_hostname(name);
if (!server->hostname) {
cifs_dbg(FYI, "%s: failed to extract hostname from target: %d\n",
__func__, -ENOMEM);
if (IS_ERR(server->hostname)) {
cifs_dbg(FYI,
"%s: failed to extract hostname from target: %ld\n",
__func__, PTR_ERR(server->hostname));
}
}
......
......@@ -776,6 +776,7 @@ static int get_tgt_list(const struct dfs_cache_entry *ce,
it->it_name = kstrndup(t->t_name, strlen(t->t_name),
GFP_KERNEL);
if (!it->it_name) {
kfree(it);
rc = -ENOMEM;
goto err_free_it;
}
......
......@@ -733,7 +733,8 @@ reopen_success:
if (can_flush) {
rc = filemap_write_and_wait(inode->i_mapping);
mapping_set_error(inode->i_mapping, rc);
if (!is_interrupt_error(rc))
mapping_set_error(inode->i_mapping, rc);
if (tcon->unix_ext)
rc = cifs_get_inode_info_unix(&inode, full_path,
......@@ -1132,14 +1133,18 @@ cifs_push_mandatory_locks(struct cifsFileInfo *cfile)
/*
* Accessing maxBuf is racy with cifs_reconnect - need to store value
* and check it for zero before using.
* and check it before using.
*/
max_buf = tcon->ses->server->maxBuf;
if (!max_buf) {
if (max_buf < (sizeof(struct smb_hdr) + sizeof(LOCKING_ANDX_RANGE))) {
free_xid(xid);
return -EINVAL;
}
BUILD_BUG_ON(sizeof(struct smb_hdr) + sizeof(LOCKING_ANDX_RANGE) >
PAGE_SIZE);
max_buf = min_t(unsigned int, max_buf - sizeof(struct smb_hdr),
PAGE_SIZE);
max_num = (max_buf - sizeof(struct smb_hdr)) /
sizeof(LOCKING_ANDX_RANGE);
buf = kcalloc(max_num, sizeof(LOCKING_ANDX_RANGE), GFP_KERNEL);
......@@ -1472,12 +1477,16 @@ cifs_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock,
/*
* Accessing maxBuf is racy with cifs_reconnect - need to store value
* and check it for zero before using.
* and check it before using.
*/
max_buf = tcon->ses->server->maxBuf;
if (!max_buf)
if (max_buf < (sizeof(struct smb_hdr) + sizeof(LOCKING_ANDX_RANGE)))
return -EINVAL;
BUILD_BUG_ON(sizeof(struct smb_hdr) + sizeof(LOCKING_ANDX_RANGE) >
PAGE_SIZE);
max_buf = min_t(unsigned int, max_buf - sizeof(struct smb_hdr),
PAGE_SIZE);
max_num = (max_buf - sizeof(struct smb_hdr)) /
sizeof(LOCKING_ANDX_RANGE);
buf = kcalloc(max_num, sizeof(LOCKING_ANDX_RANGE), GFP_KERNEL);
......@@ -2110,6 +2119,7 @@ static int cifs_writepages(struct address_space *mapping,
pgoff_t end, index;
struct cifs_writedata *wdata;
int rc = 0;
int saved_rc = 0;
unsigned int xid;
/*
......@@ -2138,8 +2148,10 @@ retry:
rc = server->ops->wait_mtu_credits(server, cifs_sb->wsize,
&wsize, &credits);
if (rc)
if (rc != 0) {
done = true;
break;
}
tofind = min((wsize / PAGE_SIZE) - 1, end - index) + 1;
......@@ -2147,6 +2159,7 @@ retry:
&found_pages);
if (!wdata) {
rc = -ENOMEM;
done = true;
add_credits_and_wake_if(server, credits, 0);
break;
}
......@@ -2175,7 +2188,7 @@ retry:
if (rc != 0) {
add_credits_and_wake_if(server, wdata->credits, 0);
for (i = 0; i < nr_pages; ++i) {
if (rc == -EAGAIN)
if (is_retryable_error(rc))
redirty_page_for_writepage(wbc,
wdata->pages[i]);
else
......@@ -2183,7 +2196,7 @@ retry:
end_page_writeback(wdata->pages[i]);
put_page(wdata->pages[i]);
}
if (rc != -EAGAIN)
if (!is_retryable_error(rc))
mapping_set_error(mapping, rc);
}
kref_put(&wdata->refcount, cifs_writedata_release);
......@@ -2193,6 +2206,15 @@ retry:
continue;
}
/* Return immediately if we received a signal during writing */
if (is_interrupt_error(rc)) {
done = true;
break;
}
if (rc != 0 && saved_rc == 0)
saved_rc = rc;
wbc->nr_to_write -= nr_pages;
if (wbc->nr_to_write <= 0)
done = true;
......@@ -2210,6 +2232,9 @@ retry:
goto retry;
}
if (saved_rc != 0)
rc = saved_rc;
if (wbc->range_cyclic || (range_whole && wbc->nr_to_write > 0))
mapping->writeback_index = index;
......@@ -2242,8 +2267,8 @@ cifs_writepage_locked(struct page *page, struct writeback_control *wbc)
set_page_writeback(page);
retry_write:
rc = cifs_partialpagewrite(page, 0, PAGE_SIZE);
if (rc == -EAGAIN) {
if (wbc->sync_mode == WB_SYNC_ALL)
if (is_retryable_error(rc)) {
if (wbc->sync_mode == WB_SYNC_ALL && rc == -EAGAIN)
goto retry_write;
redirty_page_for_writepage(wbc, page);
} else if (rc != 0) {
......
......@@ -2257,6 +2257,11 @@ cifs_setattr_unix(struct dentry *direntry, struct iattr *attrs)
* the flush returns error?
*/
rc = filemap_write_and_wait(inode->i_mapping);
if (is_interrupt_error(rc)) {
rc = -ERESTARTSYS;
goto out;
}
mapping_set_error(inode->i_mapping, rc);
rc = 0;
......@@ -2400,6 +2405,11 @@ cifs_setattr_nounix(struct dentry *direntry, struct iattr *attrs)
* the flush returns error?
*/
rc = filemap_write_and_wait(inode->i_mapping);
if (is_interrupt_error(rc)) {
rc = -ERESTARTSYS;
goto cifs_setattr_exit;
}
mapping_set_error(inode->i_mapping, rc);
rc = 0;
......
......@@ -122,12 +122,14 @@ smb2_unlock_range(struct cifsFileInfo *cfile, struct file_lock *flock,
/*
* Accessing maxBuf is racy with cifs_reconnect - need to store value
* and check it for zero before using.
* and check it before using.
*/
max_buf = tcon->ses->server->maxBuf;
if (!max_buf)
if (max_buf < sizeof(struct smb2_lock_element))
return -EINVAL;
BUILD_BUG_ON(sizeof(struct smb2_lock_element) > PAGE_SIZE);
max_buf = min_t(unsigned int, max_buf, PAGE_SIZE);
max_num = max_buf / sizeof(struct smb2_lock_element);
buf = kcalloc(max_num, sizeof(struct smb2_lock_element), GFP_KERNEL);
if (!buf)
......@@ -264,6 +266,8 @@ smb2_push_mandatory_locks(struct cifsFileInfo *cfile)
return -EINVAL;
}
BUILD_BUG_ON(sizeof(struct smb2_lock_element) > PAGE_SIZE);
max_buf = min_t(unsigned int, max_buf, PAGE_SIZE);
max_num = max_buf / sizeof(struct smb2_lock_element);
buf = kcalloc(max_num, sizeof(struct smb2_lock_element), GFP_KERNEL);
if (!buf) {
......
......@@ -162,24 +162,31 @@ static int __smb2_reconnect(const struct nls_table *nlsc,
int rc;
struct dfs_cache_tgt_list tl;
struct dfs_cache_tgt_iterator *it = NULL;
char tree[MAX_TREE_SIZE + 1];
char *tree;
const char *tcp_host;
size_t tcp_host_len;
const char *dfs_host;
size_t dfs_host_len;
tree = kzalloc(MAX_TREE_SIZE, GFP_KERNEL);
if (!tree)
return -ENOMEM;
if (tcon->ipc) {
snprintf(tree, sizeof(tree), "\\\\%s\\IPC$",
snprintf(tree, MAX_TREE_SIZE, "\\\\%s\\IPC$",
tcon->ses->server->hostname);
return SMB2_tcon(0, tcon->ses, tree, tcon, nlsc);
rc = SMB2_tcon(0, tcon->ses, tree, tcon, nlsc);
goto out;
}
if (!tcon->dfs_path)
return SMB2_tcon(0, tcon->ses, tcon->treeName, tcon, nlsc);
if (!tcon->dfs_path) {
rc = SMB2_tcon(0, tcon->ses, tcon->treeName, tcon, nlsc);
goto out;
}
rc = dfs_cache_noreq_find(tcon->dfs_path + 1, NULL, &tl);
if (rc)
return rc;
goto out;
extract_unc_hostname(tcon->ses->server->hostname, &tcp_host,
&tcp_host_len);
......@@ -199,7 +206,7 @@ static int __smb2_reconnect(const struct nls_table *nlsc,
continue;
}
snprintf(tree, sizeof(tree), "\\%s", tgt);
snprintf(tree, MAX_TREE_SIZE, "\\%s", tgt);
rc = SMB2_tcon(0, tcon->ses, tree, tcon, nlsc);
if (!rc)
......@@ -216,6 +223,8 @@ static int __smb2_reconnect(const struct nls_table *nlsc,
rc = -ENOENT;
}
dfs_cache_free_tgts(&tl);
out:
kfree(tree);
return rc;
}
#else
......@@ -3278,12 +3287,14 @@ smb2_async_readv(struct cifs_readdata *rdata)
if (rdata->credits) {
shdr->CreditCharge = cpu_to_le16(DIV_ROUND_UP(rdata->bytes,
SMB2_MAX_BUFFER_SIZE));
shdr->CreditRequest = shdr->CreditCharge;
shdr->CreditRequest =
cpu_to_le16(le16_to_cpu(shdr->CreditCharge) + 1);
spin_lock(&server->req_lock);
server->credits += rdata->credits -
le16_to_cpu(shdr->CreditCharge);
spin_unlock(&server->req_lock);
wake_up(&server->request_q);
rdata->credits = le16_to_cpu(shdr->CreditCharge);
flags |= CIFS_HAS_CREDITS;
}
......@@ -3555,12 +3566,14 @@ smb2_async_writev(struct cifs_writedata *wdata,
if (wdata->credits) {
shdr->CreditCharge = cpu_to_le16(DIV_ROUND_UP(wdata->bytes,
SMB2_MAX_BUFFER_SIZE));
shdr->CreditRequest = shdr->CreditCharge;
shdr->CreditRequest =
cpu_to_le16(le16_to_cpu(shdr->CreditCharge) + 1);
spin_lock(&server->req_lock);
server->credits += wdata->credits -
le16_to_cpu(shdr->CreditCharge);
spin_unlock(&server->req_lock);
wake_up(&server->request_q);
wdata->credits = le16_to_cpu(shdr->CreditCharge);
flags |= CIFS_HAS_CREDITS;
}
......
......@@ -387,7 +387,7 @@ smbd_done:
if (rc < 0 && rc != -EINTR)
cifs_dbg(VFS, "Error %d sending data on socket to server\n",
rc);
else
else if (rc > 0)
rc = 0;
return rc;
......@@ -783,8 +783,34 @@ cifs_setup_request(struct cifs_ses *ses, struct smb_rqst *rqst)
}
static void
cifs_noop_callback(struct mid_q_entry *mid)
cifs_compound_callback(struct mid_q_entry *mid)
{
struct TCP_Server_Info *server = mid->server;
unsigned int optype = mid->optype;
unsigned int credits_received = 0;
if (mid->mid_state == MID_RESPONSE_RECEIVED) {
if (mid->resp_buf)
credits_received = server->ops->get_credits(mid);
else
cifs_dbg(FYI, "Bad state for cancelled MID\n");
}
add_credits(server, credits_received, optype);
}
static void
cifs_compound_last_callback(struct mid_q_entry *mid)
{
cifs_compound_callback(mid);
cifs_wake_up_task(mid);
}
static void
cifs_cancelled_callback(struct mid_q_entry *mid)
{
cifs_compound_callback(mid);
DeleteMidQEntry(mid);
}
int
......@@ -795,7 +821,8 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
int i, j, rc = 0;
int timeout, optype;
struct mid_q_entry *midQ[MAX_COMPOUND];
unsigned int credits = 0;
bool cancelled_mid[MAX_COMPOUND] = {false};
unsigned int credits[MAX_COMPOUND] = {0};
char *buf;
timeout = flags & CIFS_TIMEOUT_MASK;
......@@ -813,13 +840,31 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
return -ENOENT;
/*
* Ensure that we do not send more than 50 overlapping requests
* to the same server. We may make this configurable later or
* use ses->maxReq.
* Ensure we obtain 1 credit per request in the compound chain.
* It can be optimized further by waiting for all the credits
* at once but this can wait long enough if we don't have enough
* credits due to some heavy operations in progress or the server
* not granting us much, so a fallback to the current approach is
* needed anyway.
*/
rc = wait_for_free_request(ses->server, timeout, optype);
if (rc)
return rc;
for (i = 0; i < num_rqst; i++) {
rc = wait_for_free_request(ses->server, timeout, optype);
if (rc) {
/*
* We haven't sent an SMB packet to the server yet but
* we already obtained credits for i requests in the
* compound chain - need to return those credits back
* for future use. Note that we need to call add_credits
* multiple times to match the way we obtained credits
* in the first place and to account for in flight
* requests correctly.
*/
for (j = 0; j < i; j++)
add_credits(ses->server, 1, optype);
return rc;
}
credits[i] = 1;
}
/*
* Make sure that we sign in the same order that we send on this socket
......@@ -835,18 +880,24 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
for (j = 0; j < i; j++)
cifs_delete_mid(midQ[j]);
mutex_unlock(&ses->server->srv_mutex);
/* Update # of requests on wire to server */
add_credits(ses->server, 1, optype);
for (j = 0; j < num_rqst; j++)
add_credits(ses->server, credits[j], optype);
return PTR_ERR(midQ[i]);
}
midQ[i]->mid_state = MID_REQUEST_SUBMITTED;
midQ[i]->optype = optype;
/*
* We don't invoke the callback compounds unless it is the last
* request.
* Invoke callback for every part of the compound chain
* to calculate credits properly. Wake up this thread only when
* the last element is received.
*/
if (i < num_rqst - 1)
midQ[i]->callback = cifs_noop_callback;
midQ[i]->callback = cifs_compound_callback;
else
midQ[i]->callback = cifs_compound_last_callback;
}
cifs_in_send_inc(ses->server);
rc = smb_send_rqst(ses->server, num_rqst, rqst, flags);
......@@ -860,8 +911,20 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
mutex_unlock(&ses->server->srv_mutex);
if (rc < 0)
if (rc < 0) {
/* Sending failed for some reason - return credits back */
for (i = 0; i < num_rqst; i++)
add_credits(ses->server, credits[i], optype);
goto out;
}
/*
* At this point the request is passed to the network stack - we assume
* that any credits taken from the server structure on the client have
* been spent and we can't return them back. Once we receive responses
* we will collect credits granted by the server in the mid callbacks
* and add those credits to the server structure.
*/
/*
* Compounding is never used during session establish.
......@@ -875,36 +938,34 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
for (i = 0; i < num_rqst; i++) {
rc = wait_for_response(ses->server, midQ[i]);
if (rc != 0) {
if (rc != 0)
break;
}
if (rc != 0) {
for (; i < num_rqst; i++) {
cifs_dbg(VFS, "Cancelling wait for mid %llu cmd: %d\n",
midQ[i]->mid, le16_to_cpu(midQ[i]->command));
send_cancel(ses->server, &rqst[i], midQ[i]);
spin_lock(&GlobalMid_Lock);
if (midQ[i]->mid_state == MID_REQUEST_SUBMITTED) {
midQ[i]->mid_flags |= MID_WAIT_CANCELLED;
midQ[i]->callback = DeleteMidQEntry;
spin_unlock(&GlobalMid_Lock);
add_credits(ses->server, 1, optype);
return rc;
midQ[i]->callback = cifs_cancelled_callback;
cancelled_mid[i] = true;
credits[i] = 0;
}
spin_unlock(&GlobalMid_Lock);
}
}
for (i = 0; i < num_rqst; i++)
if (midQ[i]->resp_buf)
credits += ses->server->ops->get_credits(midQ[i]);
if (!credits)
credits = 1;
for (i = 0; i < num_rqst; i++) {
if (rc < 0)
goto out;
rc = cifs_sync_mid_result(midQ[i], ses->server);
if (rc != 0) {
add_credits(ses->server, credits, optype);
return rc;
/* mark this mid as cancelled to not free it below */
cancelled_mid[i] = true;
goto out;
}
if (!midQ[i]->resp_buf ||
......@@ -951,9 +1012,10 @@ out:
* This is prevented above by using a noop callback that will not
* wake this thread except for the very last PDU.
*/
for (i = 0; i < num_rqst; i++)
cifs_delete_mid(midQ[i]);
add_credits(ses->server, credits, optype);
for (i = 0; i < num_rqst; i++) {
if (!cancelled_mid[i])
cifs_delete_mid(midQ[i]);
}
return rc;
}
......