[CIFS] Fix buffer overflow if server sends corrupt response to small
request
In SendReceive() function in transport.c - it memcpy's
message payload into a buffer passed via out_buf param. The function
assumes that all buffers are of size (CIFSMaxBufSize +
MAX_CIFS_HDR_SIZE) , unfortunately it is also called with smaller
(MAX_CIFS_SMALL_BUFFER_SIZE) buffers. There are eight callers
(SMB worker functions) which are primarily affected by this change:
TreeDisconnect, uLogoff, Close, findClose, SetFileSize, SetFileTimes,
Lock and PosixLock
CC: Dave Kleikamp <shaggy@austin.ibm.com>
CC: Przemyslaw Wegrzyn <czajnik@czajsoft.pl>
Acked-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Steve French <sfrench@us.ibm.com>
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 59d7b7c..9e8a6be 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -698,9 +698,7 @@
CIFSSMBTDis(const int xid, struct cifsTconInfo *tcon)
{
struct smb_hdr *smb_buffer;
- struct smb_hdr *smb_buffer_response; /* BB removeme BB */
int rc = 0;
- int length;
cFYI(1, ("In tree disconnect"));
/*
@@ -737,16 +735,12 @@
if (rc) {
up(&tcon->tconSem);
return rc;
- } else {
- smb_buffer_response = smb_buffer; /* BB removeme BB */
}
- rc = SendReceive(xid, tcon->ses, smb_buffer, smb_buffer_response,
- &length, 0);
+
+ rc = SendReceiveNoRsp(xid, tcon->ses, smb_buffer, 0);
if (rc)
cFYI(1, ("Tree disconnect failed %d", rc));
- if (smb_buffer)
- cifs_small_buf_release(smb_buffer);
up(&tcon->tconSem);
/* No need to return error on this operation if tid invalidated and
@@ -760,10 +754,8 @@
int
CIFSSMBLogoff(const int xid, struct cifsSesInfo *ses)
{
- struct smb_hdr *smb_buffer_response;
LOGOFF_ANDX_REQ *pSMB;
int rc = 0;
- int length;
cFYI(1, ("In SMBLogoff for session disconnect"));
if (ses)
@@ -782,8 +774,6 @@
return rc;
}
- smb_buffer_response = (struct smb_hdr *)pSMB; /* BB removeme BB */
-
if (ses->server) {
pSMB->hdr.Mid = GetNextMid(ses->server);
@@ -795,8 +785,7 @@
pSMB->hdr.Uid = ses->Suid;
pSMB->AndXCommand = 0xFF;
- rc = SendReceive(xid, ses, (struct smb_hdr *) pSMB,
- smb_buffer_response, &length, 0);
+ rc = SendReceiveNoRsp(xid, ses, (struct smb_hdr *) pSMB, 0);
if (ses->server) {
atomic_dec(&ses->server->socketUseCount);
if (atomic_read(&ses->server->socketUseCount) == 0) {
@@ -807,7 +796,6 @@
}
}
up(&ses->sesSem);
- cifs_small_buf_release(pSMB);
/* if session dead then we do not need to do ulogoff,
since server closed smb session, no sense reporting
@@ -1255,7 +1243,7 @@
pSMB->ByteCount = cpu_to_le16(count);
/* long_op set to 1 to allow for oplock break timeouts */
rc = SendReceive(xid, tcon->ses, (struct smb_hdr *) pSMB,
- (struct smb_hdr *) pSMBr, &bytes_returned, 1);
+ (struct smb_hdr *)pSMBr, &bytes_returned, CIFS_LONG_OP);
cifs_stats_inc(&tcon->num_opens);
if (rc) {
cFYI(1, ("Error in Open = %d", rc));
@@ -1368,7 +1356,7 @@
pSMB->ByteCount = cpu_to_le16(count);
/* long_op set to 1 to allow for oplock break timeouts */
rc = SendReceive(xid, tcon->ses, (struct smb_hdr *) pSMB,
- (struct smb_hdr *) pSMBr, &bytes_returned, 1);
+ (struct smb_hdr *)pSMBr, &bytes_returned, CIFS_LONG_OP);
cifs_stats_inc(&tcon->num_opens);
if (rc) {
cFYI(1, ("Error in Open = %d", rc));
@@ -1446,7 +1434,7 @@
iov[0].iov_base = (char *)pSMB;
iov[0].iov_len = pSMB->hdr.smb_buf_length + 4;
rc = SendReceive2(xid, tcon->ses, iov, 1 /* num iovecs */,
- &resp_buf_type, 0 /* not long op */, 1 /* log err */ );
+ &resp_buf_type, CIFS_STD_OP | CIFS_LOG_ERROR);
cifs_stats_inc(&tcon->num_reads);
pSMBr = (READ_RSP *)iov[0].iov_base;
if (rc) {
@@ -1665,7 +1653,7 @@
rc = SendReceive2(xid, tcon->ses, iov, n_vec + 1, &resp_buf_type,
- long_op, 0 /* do not log STATUS code */ );
+ long_op);
cifs_stats_inc(&tcon->num_writes);
if (rc) {
cFYI(1, ("Send error Write2 = %d", rc));
@@ -1707,7 +1695,7 @@
int timeout = 0;
__u16 count;
- cFYI(1, ("In CIFSSMBLock - timeout %d numLock %d", waitFlag, numLock));
+ cFYI(1, ("CIFSSMBLock timeout %d numLock %d", waitFlag, numLock));
rc = small_smb_init(SMB_COM_LOCKING_ANDX, 8, tcon, (void **) &pSMB);
if (rc)
@@ -1716,10 +1704,10 @@
pSMBr = (LOCK_RSP *)pSMB; /* BB removeme BB */
if (lockType == LOCKING_ANDX_OPLOCK_RELEASE) {
- timeout = -1; /* no response expected */
+ timeout = CIFS_ASYNC_OP; /* no response expected */
pSMB->Timeout = 0;
} else if (waitFlag == TRUE) {
- timeout = 3; /* blocking operation, no timeout */
+ timeout = CIFS_BLOCKING_OP; /* blocking operation, no timeout */
pSMB->Timeout = cpu_to_le32(-1);/* blocking - do not time out */
} else {
pSMB->Timeout = 0;
@@ -1749,15 +1737,16 @@
if (waitFlag) {
rc = SendReceiveBlockingLock(xid, tcon, (struct smb_hdr *) pSMB,
(struct smb_hdr *) pSMBr, &bytes_returned);
+ cifs_small_buf_release(pSMB);
} else {
- rc = SendReceive(xid, tcon->ses, (struct smb_hdr *) pSMB,
- (struct smb_hdr *) pSMBr, &bytes_returned, timeout);
+ rc = SendReceiveNoRsp(xid, tcon->ses, (struct smb_hdr *)pSMB,
+ timeout);
+ /* SMB buffer freed by function above */
}
cifs_stats_inc(&tcon->num_locks);
if (rc) {
cFYI(1, ("Send error in Lock = %d", rc));
}
- cifs_small_buf_release(pSMB);
/* Note: On -EAGAIN error only caller can retry on handle based calls
since file handle passed in no longer valid */
@@ -1776,7 +1765,9 @@
int rc = 0;
int timeout = 0;
int bytes_returned = 0;
+ int resp_buf_type = 0;
__u16 params, param_offset, offset, byte_count, count;
+ struct kvec iov[1];
cFYI(1, ("Posix Lock"));
@@ -1818,7 +1809,7 @@
parm_data->lock_type = cpu_to_le16(lock_type);
if (waitFlag) {
- timeout = 3; /* blocking operation, no timeout */
+ timeout = CIFS_BLOCKING_OP; /* blocking operation, no timeout */
parm_data->lock_flags = cpu_to_le16(1);
pSMB->Timeout = cpu_to_le32(-1);
} else
@@ -1838,8 +1829,13 @@
rc = SendReceiveBlockingLock(xid, tcon, (struct smb_hdr *) pSMB,
(struct smb_hdr *) pSMBr, &bytes_returned);
} else {
- rc = SendReceive(xid, tcon->ses, (struct smb_hdr *) pSMB,
- (struct smb_hdr *) pSMBr, &bytes_returned, timeout);
+ iov[0].iov_base = (char *)pSMB;
+ iov[0].iov_len = pSMB->hdr.smb_buf_length + 4;
+ rc = SendReceive2(xid, tcon->ses, iov, 1 /* num iovecs */,
+ &resp_buf_type, timeout);
+ pSMB = NULL; /* request buf already freed by SendReceive2. Do
+ not try to free it twice below on exit */
+ pSMBr = (struct smb_com_transaction2_sfi_rsp *)iov[0].iov_base;
}
if (rc) {
@@ -1874,6 +1870,11 @@
if (pSMB)
cifs_small_buf_release(pSMB);
+ if (resp_buf_type == CIFS_SMALL_BUFFER)
+ cifs_small_buf_release(iov[0].iov_base);
+ else if (resp_buf_type == CIFS_LARGE_BUFFER)
+ cifs_buf_release(iov[0].iov_base);
+
/* Note: On -EAGAIN error only caller can retry on handle based calls
since file handle passed in no longer valid */
@@ -1886,8 +1887,6 @@
{
int rc = 0;
CLOSE_REQ *pSMB = NULL;
- CLOSE_RSP *pSMBr = NULL;
- int bytes_returned;
cFYI(1, ("In CIFSSMBClose"));
/* do not retry on dead session on close */
@@ -1897,13 +1896,10 @@
if (rc)
return rc;
- pSMBr = (CLOSE_RSP *)pSMB; /* BB removeme BB */
-
pSMB->FileID = (__u16) smb_file_id;
pSMB->LastWriteTime = 0xFFFFFFFF;
pSMB->ByteCount = 0;
- rc = SendReceive(xid, tcon->ses, (struct smb_hdr *) pSMB,
- (struct smb_hdr *) pSMBr, &bytes_returned, 0);
+ rc = SendReceiveNoRsp(xid, tcon->ses, (struct smb_hdr *) pSMB, 0);
cifs_stats_inc(&tcon->num_closes);
if (rc) {
if (rc != -EINTR) {
@@ -1912,8 +1908,6 @@
}
}
- cifs_small_buf_release(pSMB);
-
/* Since session is dead, file will be closed on server already */
if (rc == -EAGAIN)
rc = 0;
@@ -3102,7 +3096,7 @@
iov[0].iov_len = pSMB->hdr.smb_buf_length + 4;
rc = SendReceive2(xid, tcon->ses, iov, 1 /* num iovec */, &buf_type,
- 0 /* not long op */, 0 /* do not log STATUS codes */ );
+ CIFS_STD_OP);
cifs_stats_inc(&tcon->num_acl_get);
if (rc) {
cFYI(1, ("Send error in QuerySecDesc = %d", rc));
@@ -3763,8 +3757,6 @@
{
int rc = 0;
FINDCLOSE_REQ *pSMB = NULL;
- CLOSE_RSP *pSMBr = NULL; /* BB removeme BB */
- int bytes_returned;
cFYI(1, ("In CIFSSMBFindClose"));
rc = small_smb_init(SMB_COM_FIND_CLOSE2, 1, tcon, (void **)&pSMB);
@@ -3776,16 +3768,13 @@
if (rc)
return rc;
- pSMBr = (CLOSE_RSP *)pSMB; /* BB removeme BB */
pSMB->FileID = searchHandle;
pSMB->ByteCount = 0;
- rc = SendReceive(xid, tcon->ses, (struct smb_hdr *) pSMB,
- (struct smb_hdr *) pSMBr, &bytes_returned, 0);
+ rc = SendReceiveNoRsp(xid, tcon->ses, (struct smb_hdr *) pSMB, 0);
if (rc) {
cERROR(1, ("Send error in FindClose = %d", rc));
}
cifs_stats_inc(&tcon->num_fclose);
- cifs_small_buf_release(pSMB);
/* Since session is dead, search handle closed on server already */
if (rc == -EAGAIN)
@@ -4707,11 +4696,9 @@
__u16 fid, __u32 pid_of_opener, int SetAllocation)
{
struct smb_com_transaction2_sfi_req *pSMB = NULL;
- struct smb_com_transaction2_sfi_rsp *pSMBr = NULL;
char *data_offset;
struct file_end_of_file_info *parm_data;
int rc = 0;
- int bytes_returned = 0;
__u16 params, param_offset, offset, byte_count, count;
cFYI(1, ("SetFileSize (via SetFileInfo) %lld",
@@ -4721,8 +4708,6 @@
if (rc)
return rc;
- pSMBr = (struct smb_com_transaction2_sfi_rsp *)pSMB;
-
pSMB->hdr.Pid = cpu_to_le16((__u16)pid_of_opener);
pSMB->hdr.PidHigh = cpu_to_le16((__u16)(pid_of_opener >> 16));
@@ -4773,17 +4758,13 @@
pSMB->Reserved4 = 0;
pSMB->hdr.smb_buf_length += byte_count;
pSMB->ByteCount = cpu_to_le16(byte_count);
- rc = SendReceive(xid, tcon->ses, (struct smb_hdr *) pSMB,
- (struct smb_hdr *) pSMBr, &bytes_returned, 0);
+ rc = SendReceiveNoRsp(xid, tcon->ses, (struct smb_hdr *) pSMB, 0);
if (rc) {
cFYI(1,
("Send error in SetFileInfo (SetFileSize) = %d",
rc));
}
- if (pSMB)
- cifs_small_buf_release(pSMB);
-
/* Note: On -EAGAIN error only caller can retry on handle based calls
since file handle passed in no longer valid */
@@ -4801,10 +4782,8 @@
const FILE_BASIC_INFO *data, __u16 fid)
{
struct smb_com_transaction2_sfi_req *pSMB = NULL;
- struct smb_com_transaction2_sfi_rsp *pSMBr = NULL;
char *data_offset;
int rc = 0;
- int bytes_returned = 0;
__u16 params, param_offset, offset, byte_count, count;
cFYI(1, ("Set Times (via SetFileInfo)"));
@@ -4813,8 +4792,6 @@
if (rc)
return rc;
- pSMBr = (struct smb_com_transaction2_sfi_rsp *)pSMB;
-
/* At this point there is no need to override the current pid
with the pid of the opener, but that could change if we someday
use an existing handle (rather than opening one on the fly) */
@@ -4854,14 +4831,11 @@
pSMB->hdr.smb_buf_length += byte_count;
pSMB->ByteCount = cpu_to_le16(byte_count);
memcpy(data_offset, data, sizeof(FILE_BASIC_INFO));
- rc = SendReceive(xid, tcon->ses, (struct smb_hdr *) pSMB,
- (struct smb_hdr *) pSMBr, &bytes_returned, 0);
+ rc = SendReceiveNoRsp(xid, tcon->ses, (struct smb_hdr *) pSMB, 0);
if (rc) {
cFYI(1, ("Send error in Set Time (SetFileInfo) = %d", rc));
}
- cifs_small_buf_release(pSMB);
-
/* Note: On -EAGAIN error only caller can retry on handle based calls
since file handle passed in no longer valid */
@@ -5152,7 +5126,8 @@
pSMB->ByteCount = 0;
rc = SendReceive(xid, tcon->ses, (struct smb_hdr *) pSMB,
- (struct smb_hdr *) pSMBr, &bytes_returned, -1);
+ (struct smb_hdr *)pSMBr, &bytes_returned,
+ CIFS_ASYNC_OP);
if (rc) {
cFYI(1, ("Error in Notify = %d", rc));
} else {