Skip to content

Commit

Permalink
cifs: keep BCC in little-endian format
Browse files Browse the repository at this point in the history 
This is the same patch as originally posted, just with some merge
conflicts fixed up...

Currently, the ByteCount is usually converted to host-endian on receive.
This is confusing however, as we need to keep two sets of routines for
accessing it, and keep track of when to use each routine. Munging
received packets like this also limits when the signature can be
calulated.

Simplify the code by keeping the received ByteCount in little-endian
format. This allows us to eliminate a set of routines for accessing it
and we can now drop the *_le suffixes from the accessor functions since
that's now implied.

While we're at it, switch all of the places that read the ByteCount
directly to use the get_bcc inline which should also clean up some
unaligned accesses.

Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Steve French <sfrench@us.ibm.com>
  • Loading branch information
Jeff Layton authored and Steve French committed May 19, 2011
1 parent 0e6e37a commit 820a803
Show file tree
Hide file tree
Showing 9 changed files with 41 additions and 82 deletions.
2 changes: 1 addition & 1 deletion fs/cifs/cifs_debug.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ void cifs_dump_detail(struct smb_hdr *smb)
cERROR(1, "Cmd: %d Err: 0x%x Flags: 0x%x Flgs2: 0x%x Mid: %d Pid: %d",
smb->Command, smb->Status.CifsError,
smb->Flags, smb->Flags2, smb->Mid, smb->Pid);
cERROR(1, "smb buf %p len %d", smb, smbCalcSize_LE(smb));
cERROR(1, "smb buf %p len %d", smb, smbCalcSize(smb));
}


Expand Down
22 changes: 2 additions & 20 deletions fs/cifs/cifspdu.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -435,36 +435,18 @@ struct smb_hdr {
/* given a pointer to an smb_hdr retrieve the pointer to the byte area */
#define pByteArea(smb_var) (BCC(smb_var) + 2)

/* get the converted ByteCount for a SMB packet and return it */
static inline __u16
get_bcc(struct smb_hdr *hdr)
{
__u16 *bc_ptr = (__u16 *)BCC(hdr);

return get_unaligned(bc_ptr);
}

/* get the unconverted ByteCount for a SMB packet and return it */
static inline __u16
get_bcc_le(struct smb_hdr *hdr)
get_bcc(struct smb_hdr *hdr)
{
__le16 *bc_ptr = (__le16 *)BCC(hdr);

return get_unaligned_le16(bc_ptr);
}

/* set the ByteCount for a SMB packet in host-byte order */
static inline void
put_bcc(__u16 count, struct smb_hdr *hdr)
{
__u16 *bc_ptr = (__u16 *)BCC(hdr);

put_unaligned(count, bc_ptr);
}

/* set the ByteCount for a SMB packet in little-endian */
static inline void
put_bcc_le(__u16 count, struct smb_hdr *hdr)
put_bcc(__u16 count, struct smb_hdr *hdr)
{
__le16 *bc_ptr = (__le16 *)BCC(hdr);

Expand Down
1 change: 0 additions & 1 deletion fs/cifs/cifsproto.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ extern void cifs_update_eof(struct cifsInodeInfo *cifsi, loff_t offset,
extern struct cifsFileInfo *find_writable_file(struct cifsInodeInfo *, bool);
extern struct cifsFileInfo *find_readable_file(struct cifsInodeInfo *, bool);
extern unsigned int smbCalcSize(struct smb_hdr *ptr);
extern unsigned int smbCalcSize_LE(struct smb_hdr *ptr);
extern int decode_negTokenInit(unsigned char *security_blob, int length,
struct TCP_Server_Info *server);
extern int cifs_convert_address(struct sockaddr *dst, const char *src, int len);
Expand Down
62 changes: 32 additions & 30 deletions fs/cifs/cifssmb.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,7 @@ CIFSSMBNegotiate(unsigned int xid, struct cifsSesInfo *ses)

if ((pSMBr->hdr.Flags2 & SMBFLG2_EXT_SEC) &&
(server->capabilities & CAP_EXTENDED_SECURITY)) {
count = pSMBr->ByteCount;
count = get_bcc(&pSMBr->hdr);
if (count < 16) {
rc = -EIO;
goto neg_err_exit;
Expand Down Expand Up @@ -736,7 +736,7 @@ CIFSSMBEcho(struct TCP_Server_Info *server)
smb->hdr.Tid = 0xffff;
smb->hdr.WordCount = 1;
put_unaligned_le16(1, &smb->EchoCount);
put_bcc_le(1, &smb->hdr);
put_bcc(1, &smb->hdr);
smb->Data[0] = 'a';
inc_rfc1001_len(smb, 3);

Expand Down Expand Up @@ -1079,7 +1079,7 @@ CIFSPOSIXCreate(const int xid, struct cifsTconInfo *tcon, __u32 posix_flags,
cFYI(1, "copying inode info");
rc = validate_t2((struct smb_t2_rsp *)pSMBr);

if (rc || (pSMBr->ByteCount < sizeof(OPEN_PSX_RSP))) {
if (rc || get_bcc(&pSMBr->hdr) < sizeof(OPEN_PSX_RSP)) {
rc = -EIO; /* bad smb */
goto psx_create_err;
}
Expand All @@ -1100,7 +1100,7 @@ CIFSPOSIXCreate(const int xid, struct cifsTconInfo *tcon, __u32 posix_flags,
pRetData->Type = cpu_to_le32(-1); /* unknown */
cFYI(DBG2, "unknown type");
} else {
if (pSMBr->ByteCount < sizeof(OPEN_PSX_RSP)
if (get_bcc(&pSMBr->hdr) < sizeof(OPEN_PSX_RSP)
+ sizeof(FILE_UNIX_BASIC_INFO)) {
cERROR(1, "Open response data too small");
pRetData->Type = cpu_to_le32(-1);
Expand Down Expand Up @@ -1867,7 +1867,7 @@ CIFSSMBPosixLock(const int xid, struct cifsTconInfo *tcon,
__u16 data_count;
rc = validate_t2((struct smb_t2_rsp *)pSMBr);

if (rc || (pSMBr->ByteCount < sizeof(struct cifs_posix_lock))) {
if (rc || get_bcc(&pSMBr->hdr) < sizeof(*parm_data)) {
rc = -EIO; /* bad smb */
goto plk_err_exit;
}
Expand Down Expand Up @@ -2494,7 +2494,7 @@ CIFSSMBUnixQuerySymLink(const int xid, struct cifsTconInfo *tcon,

rc = validate_t2((struct smb_t2_rsp *)pSMBr);
/* BB also check enough total bytes returned */
if (rc || (pSMBr->ByteCount < 2))
if (rc || get_bcc(&pSMBr->hdr) < 2)
rc = -EIO;
else {
bool is_unicode;
Expand Down Expand Up @@ -2576,14 +2576,14 @@ CIFSSMBQueryReparseLinkInfo(const int xid, struct cifsTconInfo *tcon,
} else { /* decode response */
__u32 data_offset = le32_to_cpu(pSMBr->DataOffset);
__u32 data_count = le32_to_cpu(pSMBr->DataCount);
if ((pSMBr->ByteCount < 2) || (data_offset > 512)) {
/* BB also check enough total bytes returned */
if (get_bcc(&pSMBr->hdr) < 2 || data_offset > 512) {
/* BB also check enough total bytes returned */
rc = -EIO; /* bad smb */
goto qreparse_out;
}
if (data_count && (data_count < 2048)) {
char *end_of_smb = 2 /* sizeof byte count */ +
pSMBr->ByteCount + (char *)&pSMBr->ByteCount;
get_bcc(&pSMBr->hdr) + (char *)&pSMBr->ByteCount;

struct reparse_data *reparse_buf =
(struct reparse_data *)
Expand Down Expand Up @@ -2841,8 +2841,8 @@ CIFSSMBGetPosixACL(const int xid, struct cifsTconInfo *tcon,
/* decode response */

rc = validate_t2((struct smb_t2_rsp *)pSMBr);
if (rc || (pSMBr->ByteCount < 2))
/* BB also check enough total bytes returned */
if (rc || get_bcc(&pSMBr->hdr) < 2)
rc = -EIO; /* bad smb */
else {
__u16 data_offset = le16_to_cpu(pSMBr->t2.DataOffset);
Expand Down Expand Up @@ -2991,8 +2991,8 @@ CIFSGetExtAttr(const int xid, struct cifsTconInfo *tcon,
} else {
/* decode response */
rc = validate_t2((struct smb_t2_rsp *)pSMBr);
if (rc || (pSMBr->ByteCount < 2))
/* BB also check enough total bytes returned */
if (rc || get_bcc(&pSMBr->hdr) < 2)
/* If rc should we check for EOPNOSUPP and
disable the srvino flag? or in caller? */
rc = -EIO; /* bad smb */
Expand Down Expand Up @@ -3067,6 +3067,7 @@ validate_ntransact(char *buf, char **ppparm, char **ppdata,
char *end_of_smb;
__u32 data_count, data_offset, parm_count, parm_offset;
struct smb_com_ntransact_rsp *pSMBr;
u16 bcc;

*pdatalen = 0;
*pparmlen = 0;
Expand All @@ -3076,8 +3077,8 @@ validate_ntransact(char *buf, char **ppparm, char **ppdata,

pSMBr = (struct smb_com_ntransact_rsp *)buf;

/* ByteCount was converted from little endian in SendReceive */
end_of_smb = 2 /* sizeof byte count */ + pSMBr->ByteCount +
bcc = get_bcc(&pSMBr->hdr);
end_of_smb = 2 /* sizeof byte count */ + bcc +
(char *)&pSMBr->ByteCount;

data_offset = le32_to_cpu(pSMBr->DataOffset);
Expand All @@ -3103,7 +3104,7 @@ validate_ntransact(char *buf, char **ppparm, char **ppdata,
*ppdata, data_count, (data_count + *ppdata),
end_of_smb, pSMBr);
return -EINVAL;
} else if (parm_count + data_count > pSMBr->ByteCount) {
} else if (parm_count + data_count > bcc) {
cFYI(1, "parm count and data count larger than SMB");
return -EINVAL;
}
Expand Down Expand Up @@ -3389,7 +3390,7 @@ CIFSSMBQFileInfo(const int xid, struct cifsTconInfo *tcon,

if (rc) /* BB add auto retry on EOPNOTSUPP? */
rc = -EIO;
else if (pSMBr->ByteCount < 40)
else if (get_bcc(&pSMBr->hdr) < 40)
rc = -EIO; /* bad smb */
else if (pFindData) {
__u16 data_offset = le16_to_cpu(pSMBr->t2.DataOffset);
Expand Down Expand Up @@ -3477,9 +3478,9 @@ CIFSSMBQPathInfo(const int xid, struct cifsTconInfo *tcon,

if (rc) /* BB add auto retry on EOPNOTSUPP? */
rc = -EIO;
else if (!legacy && (pSMBr->ByteCount < 40))
else if (!legacy && get_bcc(&pSMBr->hdr) < 40)
rc = -EIO; /* bad smb */
else if (legacy && (pSMBr->ByteCount < 24))
else if (legacy && get_bcc(&pSMBr->hdr) < 24)
rc = -EIO; /* 24 or 26 expected but we do not read
last field */
else if (pFindData) {
Expand Down Expand Up @@ -3555,7 +3556,7 @@ CIFSSMBUnixQFileInfo(const int xid, struct cifsTconInfo *tcon,
} else { /* decode response */
rc = validate_t2((struct smb_t2_rsp *)pSMBr);

if (rc || (pSMBr->ByteCount < sizeof(FILE_UNIX_BASIC_INFO))) {
if (rc || get_bcc(&pSMBr->hdr) < sizeof(FILE_UNIX_BASIC_INFO)) {
cERROR(1, "Malformed FILE_UNIX_BASIC_INFO response.\n"
"Unix Extensions can be disabled on mount "
"by specifying the nosfu mount option.");
Expand Down Expand Up @@ -3641,7 +3642,7 @@ CIFSSMBUnixQPathInfo(const int xid, struct cifsTconInfo *tcon,
} else { /* decode response */
rc = validate_t2((struct smb_t2_rsp *)pSMBr);

if (rc || (pSMBr->ByteCount < sizeof(FILE_UNIX_BASIC_INFO))) {
if (rc || get_bcc(&pSMBr->hdr) < sizeof(FILE_UNIX_BASIC_INFO)) {
cERROR(1, "Malformed FILE_UNIX_BASIC_INFO response.\n"
"Unix Extensions can be disabled on mount "
"by specifying the nosfu mount option.");
Expand Down Expand Up @@ -4046,8 +4047,8 @@ CIFSGetSrvInodeNumber(const int xid, struct cifsTconInfo *tcon,
} else {
/* decode response */
rc = validate_t2((struct smb_t2_rsp *)pSMBr);
if (rc || (pSMBr->ByteCount < 2))
/* BB also check enough total bytes returned */
if (rc || get_bcc(&pSMBr->hdr) < 2)
/* If rc should we check for EOPNOSUPP and
disable the srvino flag? or in caller? */
rc = -EIO; /* bad smb */
Expand Down Expand Up @@ -4272,13 +4273,13 @@ CIFSGetDFSRefer(const int xid, struct cifsSesInfo *ses,
rc = validate_t2((struct smb_t2_rsp *)pSMBr);

/* BB Also check if enough total bytes returned? */
if (rc || (pSMBr->ByteCount < 17)) {
if (rc || get_bcc(&pSMBr->hdr) < 17) {
rc = -EIO; /* bad smb */
goto GetDFSRefExit;
}

cFYI(1, "Decoding GetDFSRefer response BCC: %d Offset %d",
pSMBr->ByteCount,
get_bcc(&pSMBr->hdr),
le16_to_cpu(pSMBr->t2.DataOffset));

/* parse returned result into more usable form */
Expand Down Expand Up @@ -4344,12 +4345,12 @@ SMBOldQFSInfo(const int xid, struct cifsTconInfo *tcon, struct kstatfs *FSData)
} else { /* decode response */
rc = validate_t2((struct smb_t2_rsp *)pSMBr);

if (rc || (pSMBr->ByteCount < 18))
if (rc || get_bcc(&pSMBr->hdr) < 18)
rc = -EIO; /* bad smb */
else {
__u16 data_offset = le16_to_cpu(pSMBr->t2.DataOffset);
cFYI(1, "qfsinf resp BCC: %d Offset %d",
pSMBr->ByteCount, data_offset);
get_bcc(&pSMBr->hdr), data_offset);

response_data = (FILE_SYSTEM_ALLOC_INFO *)
(((char *) &pSMBr->hdr.Protocol) + data_offset);
Expand Down Expand Up @@ -4423,7 +4424,7 @@ CIFSSMBQFSInfo(const int xid, struct cifsTconInfo *tcon, struct kstatfs *FSData)
} else { /* decode response */
rc = validate_t2((struct smb_t2_rsp *)pSMBr);

if (rc || (pSMBr->ByteCount < 24))
if (rc || get_bcc(&pSMBr->hdr) < 24)
rc = -EIO; /* bad smb */
else {
__u16 data_offset = le16_to_cpu(pSMBr->t2.DataOffset);
Expand Down Expand Up @@ -4503,7 +4504,7 @@ CIFSSMBQFSAttributeInfo(const int xid, struct cifsTconInfo *tcon)
} else { /* decode response */
rc = validate_t2((struct smb_t2_rsp *)pSMBr);

if (rc || (pSMBr->ByteCount < 13)) {
if (rc || get_bcc(&pSMBr->hdr) < 13) {
/* BB also check if enough bytes returned */
rc = -EIO; /* bad smb */
} else {
Expand Down Expand Up @@ -4574,7 +4575,8 @@ CIFSSMBQFSDeviceInfo(const int xid, struct cifsTconInfo *tcon)
} else { /* decode response */
rc = validate_t2((struct smb_t2_rsp *)pSMBr);

if (rc || (pSMBr->ByteCount < sizeof(FILE_SYSTEM_DEVICE_INFO)))
if (rc || get_bcc(&pSMBr->hdr) <
sizeof(FILE_SYSTEM_DEVICE_INFO))
rc = -EIO; /* bad smb */
else {
__u16 data_offset = le16_to_cpu(pSMBr->t2.DataOffset);
Expand Down Expand Up @@ -4643,7 +4645,7 @@ CIFSSMBQFSUnixInfo(const int xid, struct cifsTconInfo *tcon)
} else { /* decode response */
rc = validate_t2((struct smb_t2_rsp *)pSMBr);

if (rc || (pSMBr->ByteCount < 13)) {
if (rc || get_bcc(&pSMBr->hdr) < 13) {
rc = -EIO; /* bad smb */
} else {
__u16 data_offset = le16_to_cpu(pSMBr->t2.DataOffset);
Expand Down Expand Up @@ -4788,7 +4790,7 @@ CIFSSMBQFSPosixInfo(const int xid, struct cifsTconInfo *tcon,
} else { /* decode response */
rc = validate_t2((struct smb_t2_rsp *)pSMBr);

if (rc || (pSMBr->ByteCount < 13)) {
if (rc || get_bcc(&pSMBr->hdr) < 13) {
rc = -EIO; /* bad smb */
} else {
__u16 data_offset = le16_to_cpu(pSMBr->t2.DataOffset);
Expand Down Expand Up @@ -5517,7 +5519,7 @@ CIFSSMBQAllEAs(const int xid, struct cifsTconInfo *tcon,
of these trans2 responses */

rc = validate_t2((struct smb_t2_rsp *)pSMBr);
if (rc || (pSMBr->ByteCount < 4)) {
if (rc || get_bcc(&pSMBr->hdr) < 4) {
rc = -EIO; /* bad smb */
goto QAllEAsOut;
}
Expand Down
4 changes: 2 additions & 2 deletions fs/cifs/connect.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -317,12 +317,12 @@ static int coalesce_t2(struct smb_hdr *psecond, struct smb_hdr *pTargetSMB)
put_unaligned_le16(total_in_buf, &pSMBt->t2_rsp.DataCount);

/* fix up the BCC */
byte_count = get_bcc_le(pTargetSMB);
byte_count = get_bcc(pTargetSMB);
byte_count += total_in_buf2;
/* is the result too big for the field? */
if (byte_count > USHRT_MAX)
return -EPROTO;
put_bcc_le(byte_count, pTargetSMB);
put_bcc(byte_count, pTargetSMB);

byte_count = be32_to_cpu(pTargetSMB->smb_buf_length);
byte_count += total_in_buf2;
Expand Down
4 changes: 2 additions & 2 deletions fs/cifs/misc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ checkSMB(struct smb_hdr *smb, __u16 mid, unsigned int length)

if (check_smb_hdr(smb, mid))
return 1;
clc_len = smbCalcSize_LE(smb);
clc_len = smbCalcSize(smb);

if (4 + len != length) {
cERROR(1, "Length read does not match RFC1001 length %d",
Expand Down Expand Up @@ -519,7 +519,7 @@ is_valid_oplock_break(struct smb_hdr *buf, struct TCP_Server_Info *srv)
(struct smb_com_transaction_change_notify_rsp *)buf;
struct file_notify_information *pnotify;
__u32 data_offset = 0;
if (get_bcc_le(buf) > sizeof(struct file_notify_information)) {
if (get_bcc(buf) > sizeof(struct file_notify_information)) {
data_offset = le32_to_cpu(pSMBr->DataOffset);

pnotify = (struct file_notify_information *)
Expand Down
7 changes: 0 additions & 7 deletions fs/cifs/netmisc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -919,13 +919,6 @@ smbCalcSize(struct smb_hdr *ptr)
2 /* size of the bcc field */ + get_bcc(ptr));
}

unsigned int
smbCalcSize_LE(struct smb_hdr *ptr)
{
return (sizeof(struct smb_hdr) + (2 * ptr->WordCount) +
2 /* size of the bcc field */ + get_bcc_le(ptr));
}

/* The following are taken from fs/ntfs/util.c */

#define NTFS_TIME_OFFSET ((u64)(369*365 + 89) * 24 * 3600 * 10000000)
Expand Down
2 changes: 1 addition & 1 deletion fs/cifs/sess.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -862,7 +862,7 @@ CIFS_SessSetup(unsigned int xid, struct cifsSesInfo *ses,
smb_buf->smb_buf_length =
cpu_to_be32(be32_to_cpu(smb_buf->smb_buf_length) + count);

put_bcc_le(count, smb_buf);
put_bcc(count, smb_buf);

rc = SendReceive2(xid, ses, iov, 3 /* num_iovecs */, &resp_buf_type,
CIFS_LOG_ERROR);
Expand Down
Loading

0 comments on commit 820a803

Please sign in to comment.