Skip to content

Commit

Permalink
[CIFS] Fix buffer overflow if server sends corrupt response to small
Browse files Browse the repository at this point in the history 
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 <[email protected]>
CC: Przemyslaw Wegrzyn <[email protected]>
Acked-by: Jeff Layton <[email protected]>
Signed-off-by: Steve French <[email protected]>
  • Loading branch information
Steve French committed Nov 13, 2007
1 parent 9418d5d commit 133672e
Show file tree
Hide file tree
Showing 7 changed files with 133 additions and 96 deletions.
11 changes: 11 additions & 0 deletions fs/cifs/cifsglob.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,17 @@ struct dir_notify_req {
#define CIFS_LARGE_BUFFER 2
#define CIFS_IOVEC 4 /* array of response buffers */

/* Type of Request to SendReceive2 */
#define CIFS_STD_OP 0 /* normal request timeout */
#define CIFS_LONG_OP 1 /* long op (up to 45 sec, oplock time) */
#define CIFS_VLONG_OP 2 /* sloow op - can take up to 180 seconds */
#define CIFS_BLOCKING_OP 4 /* operation can block */
#define CIFS_ASYNC_OP 8 /* do not wait for response */
#define CIFS_TIMEOUT_MASK 0x00F /* only one of 5 above set in req */
#define CIFS_LOG_ERROR 0x010 /* log NT STATUS if non-zero */
#define CIFS_LARGE_BUF_OP 0x020 /* large request buffer */
#define CIFS_NO_RESP 0x040 /* no response buffer required */

/* Security Flags: indicate type of session setup needed */
#define CIFSSEC_MAY_SIGN 0x00001
#define CIFSSEC_MAY_NTLM 0x00002
Expand Down
5 changes: 3 additions & 2 deletions fs/cifs/cifsproto.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,11 @@ extern int SendReceive(const unsigned int /* xid */ , struct cifsSesInfo *,
struct smb_hdr * /* input */ ,
struct smb_hdr * /* out */ ,
int * /* bytes returned */ , const int long_op);
extern int SendReceiveNoRsp(const unsigned int xid, struct cifsSesInfo *ses,
struct smb_hdr *in_buf, int flags);
extern int SendReceive2(const unsigned int /* xid */ , struct cifsSesInfo *,
struct kvec *, int /* nvec to send */,
int * /* type of buf returned */ , const int long_op,
const int logError /* whether to log status code*/ );
int * /* type of buf returned */ , const int flags);
extern int SendReceiveBlockingLock(const unsigned int /* xid */ ,
struct cifsTconInfo *,
struct smb_hdr * /* input */ ,
Expand Down
97 changes: 36 additions & 61 deletions fs/cifs/cifssmb.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -698,9 +698,7 @@ int
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"));
/*
Expand Down Expand Up @@ -737,16 +735,12 @@ CIFSSMBTDis(const int xid, struct cifsTconInfo *tcon)
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
Expand All @@ -760,10 +754,8 @@ CIFSSMBTDis(const int xid, struct cifsTconInfo *tcon)
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)
Expand All @@ -782,8 +774,6 @@ CIFSSMBLogoff(const int xid, struct cifsSesInfo *ses)
return rc;
}

smb_buffer_response = (struct smb_hdr *)pSMB; /* BB removeme BB */

if (ses->server) {
pSMB->hdr.Mid = GetNextMid(ses->server);

Expand All @@ -795,8 +785,7 @@ CIFSSMBLogoff(const int xid, struct cifsSesInfo *ses)
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) {
Expand All @@ -807,7 +796,6 @@ CIFSSMBLogoff(const int xid, struct cifsSesInfo *ses)
}
}
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
Expand Down Expand Up @@ -1255,7 +1243,7 @@ SMBLegacyOpen(const int xid, struct cifsTconInfo *tcon,
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));
Expand Down Expand Up @@ -1368,7 +1356,7 @@ CIFSSMBOpen(const int xid, struct cifsTconInfo *tcon,
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));
Expand Down Expand Up @@ -1446,7 +1434,7 @@ CIFSSMBRead(const int xid, struct cifsTconInfo *tcon, const int netfid,
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) {
Expand Down Expand Up @@ -1665,7 +1653,7 @@ CIFSSMBWrite2(const int xid, struct cifsTconInfo *tcon,


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));
Expand Down Expand Up @@ -1707,7 +1695,7 @@ CIFSSMBLock(const int xid, struct cifsTconInfo *tcon,
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)
Expand All @@ -1716,10 +1704,10 @@ CIFSSMBLock(const int xid, struct cifsTconInfo *tcon,
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;
Expand Down Expand Up @@ -1749,15 +1737,16 @@ CIFSSMBLock(const int xid, struct cifsTconInfo *tcon,
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 */
Expand All @@ -1776,7 +1765,9 @@ CIFSSMBPosixLock(const int xid, struct cifsTconInfo *tcon,
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"));

Expand Down Expand Up @@ -1818,7 +1809,7 @@ CIFSSMBPosixLock(const int xid, struct cifsTconInfo *tcon,

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
Expand All @@ -1838,8 +1829,13 @@ CIFSSMBPosixLock(const int xid, struct cifsTconInfo *tcon,
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) {
Expand Down Expand Up @@ -1874,6 +1870,11 @@ CIFSSMBPosixLock(const int xid, struct cifsTconInfo *tcon,
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 */

Expand All @@ -1886,8 +1887,6 @@ CIFSSMBClose(const int xid, struct cifsTconInfo *tcon, int smb_file_id)
{
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 */
Expand All @@ -1897,13 +1896,10 @@ CIFSSMBClose(const int xid, struct cifsTconInfo *tcon, int smb_file_id)
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) {
Expand All @@ -1912,8 +1908,6 @@ CIFSSMBClose(const int xid, struct cifsTconInfo *tcon, int smb_file_id)
}
}

cifs_small_buf_release(pSMB);

/* Since session is dead, file will be closed on server already */
if (rc == -EAGAIN)
rc = 0;
Expand Down Expand Up @@ -3102,7 +3096,7 @@ CIFSSMBGetCIFSACL(const int xid, struct cifsTconInfo *tcon, __u16 fid,
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));
Expand Down Expand Up @@ -3763,8 +3757,6 @@ CIFSFindClose(const int xid, struct cifsTconInfo *tcon,
{
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);
Expand All @@ -3776,16 +3768,13 @@ CIFSFindClose(const int xid, struct cifsTconInfo *tcon,
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)
Expand Down Expand Up @@ -4707,11 +4696,9 @@ CIFSSMBSetFileSize(const int xid, struct cifsTconInfo *tcon, __u64 size,
__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",
Expand All @@ -4721,8 +4708,6 @@ CIFSSMBSetFileSize(const int xid, struct cifsTconInfo *tcon, __u64 size,
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));

Expand Down Expand Up @@ -4773,17 +4758,13 @@ CIFSSMBSetFileSize(const int xid, struct cifsTconInfo *tcon, __u64 size,
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 */

Expand All @@ -4801,10 +4782,8 @@ CIFSSMBSetFileTimes(const int xid, struct cifsTconInfo *tcon,
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)"));
Expand All @@ -4813,8 +4792,6 @@ CIFSSMBSetFileTimes(const int xid, struct cifsTconInfo *tcon,
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) */
Expand Down Expand Up @@ -4854,14 +4831,11 @@ CIFSSMBSetFileTimes(const int xid, struct cifsTconInfo *tcon,
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 */

Expand Down Expand Up @@ -5152,7 +5126,8 @@ int CIFSSMBNotify(const int xid, struct cifsTconInfo *tcon,
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 {
Expand Down
Loading

0 comments on commit 133672e

Please sign in to comment.