* [PATCH 0/5] cifs: clean up some unaligned memory accesses
@ 2011-01-18 20:33 Jeff Layton
[not found] ` <1295382799-7902-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Jeff Layton @ 2011-01-18 20:33 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
When the CIFS client is marshalling a call or parsing a response from
the server, it often will access fields within the packets directly.
It's easily possible however that those fields will not be aligned
properly. Many CPUs handle this transparently. Some CPUs (such as ia64)
throw a warning and then use a slow-path mechanism to deal with it.
Other CPUs (mostly the embedded ones, it seems) actually throw an
exception and just don't work.
For more background on the problem, see this file in the kernel source
tree:
Documentation/unaligned-memory-access.txt
This was originally reported quite some time ago here:
https://bugzilla.kernel.org/show_bug.cgi?id=11115
I've also had a report of the same problem on ia64 against RHEL5:
https://bugzilla.redhat.com/show_bug.cgi?id=659715
My patchset is based on the one originally by John Voltz. His proposed
patch also patched up the generic NLS code for unaligned access, but I
took a different approach by making sure that we just never called into
those routines with an unaligned buffer.
I've tested this patchset on x86_64 and it seems to be fine. I've also
tested a version of this patchset backported to RHEL5 on ia64. Certain
tests would make that arch pop these sorts of printks:
kernel unaligned access to 0xe0000040ed16807f, ip=0xa0000002029b8530
...with this set, those are eliminated. I suspect that this may also
help CIFS to work on some embedded arches as well (such as avr32).
Note that this is likely not a comprehensive fix for CIFS though. It
seems like there are a lot of places in cifssmb.c that access fields in
the request or response directly. Any of them are probably also fair
game for unaligned access fixes.
I think this patchset is pretty safe, so we should consider getting it
into 2.6.38 if possible. If not, definitely for 2.6.39.
Jeff Layton (5):
cifs: use get/put_unaligned functions to access ByteCount
cifs: clean up unaligned accesses in validate_t2
cifs: fix unaligned access in check2ndT2 and coalesce_t2
cifs: clean up unaligned accesses in cifs_unicode.c
cifs: fix unaligned accesses in cifsConvertToUCS
fs/cifs/cifs_unicode.c | 125 +++++++++++++++++++++++++++++++++++++++--------
fs/cifs/cifspdu.h | 47 ++++++++++++++++--
fs/cifs/cifssmb.c | 52 ++++++++++----------
fs/cifs/connect.c | 43 +++++++---------
fs/cifs/misc.c | 71 ---------------------------
fs/cifs/netmisc.c | 4 +-
fs/cifs/sess.c | 13 ++---
fs/cifs/transport.c | 9 ++--
8 files changed, 202 insertions(+), 162 deletions(-)
--
1.7.3.4
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH 1/5] cifs: use get/put_unaligned functions to access ByteCount
[not found] ` <1295382799-7902-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2011-01-18 20:33 ` Jeff Layton
[not found] ` <1295382799-7902-2-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-01-18 20:33 ` [PATCH 2/5] cifs: clean up unaligned accesses in validate_t2 Jeff Layton
` (4 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Jeff Layton @ 2011-01-18 20:33 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
It's possible that when we access the ByteCount that the alignment
will be off. Most CPUs deal with that transparently, but there's
usually some performance impact. Some CPUs raise an exception on
unaligned accesses.
Fix this by accessing the byte count using the get_unaligned and
put_unaligned inlined functions. While we're at it, fix the types
of some of the variables that end up getting returns from these
functions.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/cifs/cifspdu.h | 47 +++++++++++++++++++++++++++++++++++++++++++----
fs/cifs/cifssmb.c | 14 +++++---------
fs/cifs/connect.c | 10 +++++-----
fs/cifs/netmisc.c | 4 ++--
fs/cifs/sess.c | 13 ++++++-------
fs/cifs/transport.c | 9 ++++-----
6 files changed, 65 insertions(+), 32 deletions(-)
diff --git a/fs/cifs/cifspdu.h b/fs/cifs/cifspdu.h
index ea205b4..b5c8cc5 100644
--- a/fs/cifs/cifspdu.h
+++ b/fs/cifs/cifspdu.h
@@ -23,6 +23,7 @@
#define _CIFSPDU_H
#include <net/sock.h>
+#include <asm/unaligned.h>
#include "smbfsctl.h"
#ifdef CONFIG_CIFS_WEAK_PW_HASH
@@ -426,11 +427,49 @@ struct smb_hdr {
__u16 Mid;
__u8 WordCount;
} __attribute__((packed));
-/* given a pointer to an smb_hdr retrieve the value of byte count */
-#define BCC(smb_var) (*(__u16 *)((char *)(smb_var) + sizeof(struct smb_hdr) + (2 * (smb_var)->WordCount)))
-#define BCC_LE(smb_var) (*(__le16 *)((char *)(smb_var) + sizeof(struct smb_hdr) + (2 * (smb_var)->WordCount)))
+
+/* given a pointer to an smb_hdr retrieve a char pointer to the byte count */
+#define BCC(smb_var) ((unsigned char *)(smb_var) + sizeof(struct smb_hdr) + \
+ (2 * (smb_var)->WordCount))
+
/* given a pointer to an smb_hdr retrieve the pointer to the byte area */
-#define pByteArea(smb_var) ((unsigned char *)(smb_var) + sizeof(struct smb_hdr) + (2 * (smb_var)->WordCount) + 2)
+#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)
+{
+ __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)
+{
+ __le16 *bc_ptr = (__le16 *)BCC(hdr);
+
+ put_unaligned_le16(count, bc_ptr);
+}
/*
* Computer Name Length (since Netbios name was length 16 with last byte 0x20)
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index c5b2014..3b3072c 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -333,7 +333,6 @@ static int validate_t2(struct smb_t2_rsp *pSMB)
{
int rc = -EINVAL;
int total_size;
- char *pBCC;
/* check for plausible wct, bcc and t2 data and parm sizes */
/* check for parm and data offset going beyond end of smb */
@@ -346,13 +345,9 @@ static int validate_t2(struct smb_t2_rsp *pSMB)
if (total_size < 512) {
total_size +=
le16_to_cpu(pSMB->t2_rsp.DataCount);
- /* BCC le converted in SendReceive */
- pBCC = (pSMB->hdr.WordCount * 2) +
- sizeof(struct smb_hdr) +
- (char *)pSMB;
- if ((total_size <= (*(u16 *)pBCC)) &&
- (total_size <
- CIFSMaxBufSize+MAX_CIFS_HDR_SIZE)) {
+ if (total_size <= get_bcc(&pSMB->hdr) &&
+ total_size <
+ CIFSMaxBufSize + MAX_CIFS_HDR_SIZE) {
return 0;
}
}
@@ -362,6 +357,7 @@ static int validate_t2(struct smb_t2_rsp *pSMB)
sizeof(struct smb_t2_rsp) + 16);
return rc;
}
+
int
CIFSSMBNegotiate(unsigned int xid, struct cifsSesInfo *ses)
{
@@ -5430,7 +5426,7 @@ QAllEAsRetry:
}
/* make sure list_len doesn't go past end of SMB */
- end_of_smb = (char *)pByteArea(&pSMBr->hdr) + BCC(&pSMBr->hdr);
+ end_of_smb = (char *)pByteArea(&pSMBr->hdr) + get_bcc(&pSMBr->hdr);
if ((char *)ea_response_data + list_len > end_of_smb) {
cFYI(1, "EA list appears to go beyond SMB");
rc = -EIO;
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 32c2f55..9bcdf2b 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -318,9 +318,9 @@ static int coalesce_t2(struct smb_hdr *psecond, struct smb_hdr *pTargetSMB)
memcpy(data_area_of_target, data_area_of_buf2, total_in_buf2);
total_in_buf += total_in_buf2;
pSMBt->t2_rsp.DataCount = cpu_to_le16(total_in_buf);
- byte_count = le16_to_cpu(BCC_LE(pTargetSMB));
+ byte_count = get_bcc_le(pTargetSMB);
byte_count += total_in_buf2;
- BCC_LE(pTargetSMB) = cpu_to_le16(byte_count);
+ put_bcc_le(byte_count, pTargetSMB);
byte_count = pTargetSMB->smb_buf_length;
byte_count += total_in_buf2;
@@ -2937,8 +2937,8 @@ CIFSTCon(unsigned int xid, struct cifsSesInfo *ses,
TCONX_RSP *pSMBr;
unsigned char *bcc_ptr;
int rc = 0;
- int length, bytes_left;
- __u16 count;
+ int length;
+ __u16 bytes_left, count;
if (ses == NULL)
return -EIO;
@@ -3032,7 +3032,7 @@ CIFSTCon(unsigned int xid, struct cifsSesInfo *ses,
tcon->need_reconnect = false;
tcon->tid = smb_buffer_response->Tid;
bcc_ptr = pByteArea(smb_buffer_response);
- bytes_left = BCC(smb_buffer_response);
+ bytes_left = get_bcc(smb_buffer_response);
length = strnlen(bcc_ptr, bytes_left - 2);
if (smb_buffer->Flags2 & SMBFLG2_UNICODE)
is_unicode = true;
diff --git a/fs/cifs/netmisc.c b/fs/cifs/netmisc.c
index 6783ce6..8d9189f 100644
--- a/fs/cifs/netmisc.c
+++ b/fs/cifs/netmisc.c
@@ -916,14 +916,14 @@ unsigned int
smbCalcSize(struct smb_hdr *ptr)
{
return (sizeof(struct smb_hdr) + (2 * ptr->WordCount) +
- 2 /* size of the bcc field */ + BCC(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 */ + le16_to_cpu(BCC_LE(ptr)));
+ 2 /* size of the bcc field */ + get_bcc_le(ptr));
}
/* The following are taken from fs/ntfs/util.c */
diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
index 1cffd82..1adc962 100644
--- a/fs/cifs/sess.c
+++ b/fs/cifs/sess.c
@@ -277,7 +277,7 @@ static void ascii_ssetup_strings(char **pbcc_area, struct cifsSesInfo *ses,
}
static void
-decode_unicode_ssetup(char **pbcc_area, int bleft, struct cifsSesInfo *ses,
+decode_unicode_ssetup(char **pbcc_area, __u16 bleft, struct cifsSesInfo *ses,
const struct nls_table *nls_cp)
{
int len;
@@ -323,7 +323,7 @@ decode_unicode_ssetup(char **pbcc_area, int bleft, struct cifsSesInfo *ses,
return;
}
-static int decode_ascii_ssetup(char **pbcc_area, int bleft,
+static int decode_ascii_ssetup(char **pbcc_area, __u16 bleft,
struct cifsSesInfo *ses,
const struct nls_table *nls_cp)
{
@@ -575,12 +575,11 @@ CIFS_SessSetup(unsigned int xid, struct cifsSesInfo *ses,
char *str_area;
SESSION_SETUP_ANDX *pSMB;
__u32 capabilities;
- int count;
+ __u16 count;
int resp_buf_type;
struct kvec iov[3];
enum securityEnum type;
- __u16 action;
- int bytes_remaining;
+ __u16 action, bytes_remaining;
struct key *spnego_key = NULL;
__le32 phase = NtLmNegotiate; /* NTLMSSP, if needed, is multistage */
u16 blob_len;
@@ -876,7 +875,7 @@ ssetup_ntlmssp_authenticate:
count = iov[1].iov_len + iov[2].iov_len;
smb_buf->smb_buf_length += count;
- BCC_LE(smb_buf) = cpu_to_le16(count);
+ put_bcc_le(count, smb_buf);
rc = SendReceive2(xid, ses, iov, 3 /* num_iovecs */, &resp_buf_type,
CIFS_LOG_ERROR);
@@ -910,7 +909,7 @@ ssetup_ntlmssp_authenticate:
cFYI(1, "UID = %d ", ses->Suid);
/* response can have either 3 or 4 word count - Samba sends 3 */
/* and lanman response is 3 */
- bytes_remaining = BCC(smb_buf);
+ bytes_remaining = get_bcc(smb_buf);
bcc_ptr = pByteArea(smb_buf);
if (smb_buf->WordCount == 4) {
diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
index e2e66f3..fbeaee8 100644
--- a/fs/cifs/transport.c
+++ b/fs/cifs/transport.c
@@ -484,7 +484,7 @@ send_nt_cancel(struct TCP_Server_Info *server, struct smb_hdr *in_buf,
in_buf->smb_buf_length = sizeof(struct smb_hdr) - 4 + 2;
in_buf->Command = SMB_COM_NT_CANCEL;
in_buf->WordCount = 0;
- BCC_LE(in_buf) = 0;
+ put_bcc_le(0, in_buf);
mutex_lock(&server->srv_mutex);
rc = cifs_sign_smb(in_buf, server, &mid->sequence_number);
@@ -648,8 +648,7 @@ SendReceive2(const unsigned int xid, struct cifsSesInfo *ses,
if (receive_len >= sizeof(struct smb_hdr) - 4
/* do not count RFC1001 header */ +
(2 * midQ->resp_buf->WordCount) + 2 /* bcc */ )
- BCC(midQ->resp_buf) =
- le16_to_cpu(BCC_LE(midQ->resp_buf));
+ put_bcc(get_bcc_le(midQ->resp_buf), midQ->resp_buf);
if ((flags & CIFS_NO_RESP) == 0)
midQ->resp_buf = NULL; /* mark it so buf will
not be freed by
@@ -803,7 +802,7 @@ SendReceive(const unsigned int xid, struct cifsSesInfo *ses,
if (receive_len >= sizeof(struct smb_hdr) - 4
/* do not count RFC1001 header */ +
(2 * out_buf->WordCount) + 2 /* bcc */ )
- BCC(out_buf) = le16_to_cpu(BCC_LE(out_buf));
+ put_bcc(get_bcc_le(midQ->resp_buf), midQ->resp_buf);
} else {
rc = -EIO;
cERROR(1, "Bad MID state?");
@@ -1015,7 +1014,7 @@ SendReceiveBlockingLock(const unsigned int xid, struct cifsTconInfo *tcon,
if (receive_len >= sizeof(struct smb_hdr) - 4
/* do not count RFC1001 header */ +
(2 * out_buf->WordCount) + 2 /* bcc */ )
- BCC(out_buf) = le16_to_cpu(BCC_LE(out_buf));
+ put_bcc(get_bcc_le(out_buf), out_buf);
out:
delete_mid(midQ);
--
1.7.3.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 2/5] cifs: clean up unaligned accesses in validate_t2
[not found] ` <1295382799-7902-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-01-18 20:33 ` [PATCH 1/5] cifs: use get/put_unaligned functions to access ByteCount Jeff Layton
@ 2011-01-18 20:33 ` Jeff Layton
[not found] ` <1295382799-7902-3-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-01-18 20:33 ` [PATCH 3/5] cifs: fix unaligned access in check2ndT2 and coalesce_t2 Jeff Layton
` (3 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Jeff Layton @ 2011-01-18 20:33 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
...and clean up function to reduce indentation.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/cifs/cifssmb.c | 44 +++++++++++++++++++++++---------------------
1 files changed, 23 insertions(+), 21 deletions(-)
diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
index 3b3072c..75a1559 100644
--- a/fs/cifs/cifssmb.c
+++ b/fs/cifs/cifssmb.c
@@ -331,31 +331,33 @@ smb_init_no_reconnect(int smb_command, int wct, struct cifsTconInfo *tcon,
static int validate_t2(struct smb_t2_rsp *pSMB)
{
- int rc = -EINVAL;
- int total_size;
+ unsigned int total_size;
+
+ /* check for plausible wct */
+ if (pSMB->hdr.WordCount < 10)
+ goto vt2_err;
- /* check for plausible wct, bcc and t2 data and parm sizes */
/* check for parm and data offset going beyond end of smb */
- if (pSMB->hdr.WordCount >= 10) {
- if ((le16_to_cpu(pSMB->t2_rsp.ParameterOffset) <= 1024) &&
- (le16_to_cpu(pSMB->t2_rsp.DataOffset) <= 1024)) {
- /* check that bcc is at least as big as parms + data */
- /* check that bcc is less than negotiated smb buffer */
- total_size = le16_to_cpu(pSMB->t2_rsp.ParameterCount);
- if (total_size < 512) {
- total_size +=
- le16_to_cpu(pSMB->t2_rsp.DataCount);
- if (total_size <= get_bcc(&pSMB->hdr) &&
- total_size <
- CIFSMaxBufSize + MAX_CIFS_HDR_SIZE) {
- return 0;
- }
- }
- }
- }
+ if (get_unaligned_le16(&pSMB->t2_rsp.ParameterOffset) > 1024 ||
+ get_unaligned_le16(&pSMB->t2_rsp.DataOffset) > 1024)
+ goto vt2_err;
+
+ /* check that bcc is at least as big as parms + data */
+ /* check that bcc is less than negotiated smb buffer */
+ total_size = get_unaligned_le16(&pSMB->t2_rsp.ParameterCount);
+ if (total_size >= 512)
+ goto vt2_err;
+
+ total_size += get_unaligned_le16(&pSMB->t2_rsp.DataCount);
+ if (total_size > get_bcc(&pSMB->hdr) ||
+ total_size >= CIFSMaxBufSize + MAX_CIFS_HDR_SIZE)
+ goto vt2_err;
+
+ return 0;
+vt2_err:
cifs_dump_mem("Invalid transact2 SMB: ", (char *)pSMB,
sizeof(struct smb_t2_rsp) + 16);
- return rc;
+ return -EINVAL;
}
int
--
1.7.3.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 3/5] cifs: fix unaligned access in check2ndT2 and coalesce_t2
[not found] ` <1295382799-7902-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-01-18 20:33 ` [PATCH 1/5] cifs: use get/put_unaligned functions to access ByteCount Jeff Layton
2011-01-18 20:33 ` [PATCH 2/5] cifs: clean up unaligned accesses in validate_t2 Jeff Layton
@ 2011-01-18 20:33 ` Jeff Layton
[not found] ` <1295382799-7902-4-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-01-18 20:33 ` [PATCH 4/5] cifs: clean up unaligned accesses in cifs_unicode.c Jeff Layton
` (2 subsequent siblings)
5 siblings, 1 reply; 17+ messages in thread
From: Jeff Layton @ 2011-01-18 20:33 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/cifs/connect.c | 33 ++++++++++++++-------------------
1 files changed, 14 insertions(+), 19 deletions(-)
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index 9bcdf2b..d5b779e 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -232,9 +232,8 @@ cifs_reconnect(struct TCP_Server_Info *server)
static int check2ndT2(struct smb_hdr *pSMB, unsigned int maxBufSize)
{
struct smb_t2_rsp *pSMBt;
- int total_data_size;
- int data_in_this_rsp;
int remaining;
+ __u16 total_data_size, data_in_this_rsp;
if (pSMB->Command != SMB_COM_TRANSACTION2)
return 0;
@@ -248,8 +247,8 @@ static int check2ndT2(struct smb_hdr *pSMB, unsigned int maxBufSize)
pSMBt = (struct smb_t2_rsp *)pSMB;
- total_data_size = le16_to_cpu(pSMBt->t2_rsp.TotalDataCount);
- data_in_this_rsp = le16_to_cpu(pSMBt->t2_rsp.DataCount);
+ total_data_size = get_unaligned_le16(&pSMBt->t2_rsp.TotalDataCount);
+ data_in_this_rsp = get_unaligned_le16(&pSMBt->t2_rsp.DataCount);
remaining = total_data_size - data_in_this_rsp;
@@ -275,21 +274,18 @@ static int coalesce_t2(struct smb_hdr *psecond, struct smb_hdr *pTargetSMB)
{
struct smb_t2_rsp *pSMB2 = (struct smb_t2_rsp *)psecond;
struct smb_t2_rsp *pSMBt = (struct smb_t2_rsp *)pTargetSMB;
- int total_data_size;
- int total_in_buf;
- int remaining;
- int total_in_buf2;
char *data_area_of_target;
char *data_area_of_buf2;
- __u16 byte_count;
+ int remaining;
+ __u16 byte_count, total_data_size, total_in_buf, total_in_buf2;
- total_data_size = le16_to_cpu(pSMBt->t2_rsp.TotalDataCount);
+ total_data_size = get_unaligned_le16(&pSMBt->t2_rsp.TotalDataCount);
- if (total_data_size != le16_to_cpu(pSMB2->t2_rsp.TotalDataCount)) {
+ if (total_data_size !=
+ get_unaligned_le16(&pSMB2->t2_rsp.TotalDataCount))
cFYI(1, "total data size of primary and secondary t2 differ");
- }
- total_in_buf = le16_to_cpu(pSMBt->t2_rsp.DataCount);
+ total_in_buf = get_unaligned_le16(&pSMBt->t2_rsp.DataCount);
remaining = total_data_size - total_in_buf;
@@ -299,25 +295,25 @@ static int coalesce_t2(struct smb_hdr *psecond, struct smb_hdr *pTargetSMB)
if (remaining == 0) /* nothing to do, ignore */
return 0;
- total_in_buf2 = le16_to_cpu(pSMB2->t2_rsp.DataCount);
+ total_in_buf2 = get_unaligned_le16(&pSMB2->t2_rsp.DataCount);
if (remaining < total_in_buf2) {
cFYI(1, "transact2 2nd response contains too much data");
}
/* find end of first SMB data area */
data_area_of_target = (char *)&pSMBt->hdr.Protocol +
- le16_to_cpu(pSMBt->t2_rsp.DataOffset);
+ get_unaligned_le16(&pSMBt->t2_rsp.DataOffset);
/* validate target area */
- data_area_of_buf2 = (char *) &pSMB2->hdr.Protocol +
- le16_to_cpu(pSMB2->t2_rsp.DataOffset);
+ data_area_of_buf2 = (char *)&pSMB2->hdr.Protocol +
+ get_unaligned_le16(&pSMB2->t2_rsp.DataOffset);
data_area_of_target += total_in_buf;
/* copy second buffer into end of first buffer */
memcpy(data_area_of_target, data_area_of_buf2, total_in_buf2);
total_in_buf += total_in_buf2;
- pSMBt->t2_rsp.DataCount = cpu_to_le16(total_in_buf);
+ put_unaligned_le16(total_in_buf, &pSMBt->t2_rsp.DataCount);
byte_count = get_bcc_le(pTargetSMB);
byte_count += total_in_buf2;
put_bcc_le(byte_count, pTargetSMB);
@@ -334,7 +330,6 @@ static int coalesce_t2(struct smb_hdr *psecond, struct smb_hdr *pTargetSMB)
return 0; /* we are done */
} else /* more responses to go */
return 1;
-
}
static void
--
1.7.3.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 4/5] cifs: clean up unaligned accesses in cifs_unicode.c
[not found] ` <1295382799-7902-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (2 preceding siblings ...)
2011-01-18 20:33 ` [PATCH 3/5] cifs: fix unaligned access in check2ndT2 and coalesce_t2 Jeff Layton
@ 2011-01-18 20:33 ` Jeff Layton
[not found] ` <1295382799-7902-5-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-01-18 20:33 ` [PATCH 5/5] cifs: fix unaligned accesses in cifsConvertToUCS Jeff Layton
2011-01-19 14:20 ` [PATCH 0/5] cifs: clean up some unaligned memory accesses Pavel Shilovsky
5 siblings, 1 reply; 17+ messages in thread
From: Jeff Layton @ 2011-01-18 20:33 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
Make sure we use get/put_unaligned routines when accessing wide
character strings.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/cifs/cifs_unicode.c | 49 ++++++++++++++++++++++++++---------------------
1 files changed, 27 insertions(+), 22 deletions(-)
diff --git a/fs/cifs/cifs_unicode.c b/fs/cifs/cifs_unicode.c
index 430f510..8134443 100644
--- a/fs/cifs/cifs_unicode.c
+++ b/fs/cifs/cifs_unicode.c
@@ -44,10 +44,14 @@ cifs_ucs2_bytes(const __le16 *from, int maxbytes,
int charlen, outlen = 0;
int maxwords = maxbytes / 2;
char tmp[NLS_MAX_CHARSET_SIZE];
+ __u16 ftmp;
- for (i = 0; i < maxwords && from[i]; i++) {
- charlen = codepage->uni2char(le16_to_cpu(from[i]), tmp,
- NLS_MAX_CHARSET_SIZE);
+ for (i = 0; i < maxwords; i++) {
+ ftmp = get_unaligned_le16(&from[i]);
+ if (ftmp == 0)
+ break;
+
+ charlen = codepage->uni2char(ftmp, tmp, NLS_MAX_CHARSET_SIZE);
if (charlen > 0)
outlen += charlen;
else
@@ -60,7 +64,7 @@ cifs_ucs2_bytes(const __le16 *from, int maxbytes,
/*
* cifs_mapchar - convert a little-endian char to proper char in codepage
* @target - where converted character should be copied
- * @src_char - 2 byte little-endian source character
+ * @src_char - 2 byte host-endian source character
* @cp - codepage to which character should be converted
* @mapchar - should character be mapped according to mapchars mount option?
*
@@ -69,7 +73,7 @@ cifs_ucs2_bytes(const __le16 *from, int maxbytes,
* enough to hold the result of the conversion (at least NLS_MAX_CHARSET_SIZE).
*/
static int
-cifs_mapchar(char *target, const __le16 src_char, const struct nls_table *cp,
+cifs_mapchar(char *target, const __u16 src_char, const struct nls_table *cp,
bool mapchar)
{
int len = 1;
@@ -82,7 +86,7 @@ cifs_mapchar(char *target, const __le16 src_char, const struct nls_table *cp,
* build_path_from_dentry are modified, as they use slash as
* separator.
*/
- switch (le16_to_cpu(src_char)) {
+ switch (src_char) {
case UNI_COLON:
*target = ':';
break;
@@ -109,8 +113,7 @@ out:
return len;
cp_convert:
- len = cp->uni2char(le16_to_cpu(src_char), target,
- NLS_MAX_CHARSET_SIZE);
+ len = cp->uni2char(src_char, target, NLS_MAX_CHARSET_SIZE);
if (len <= 0) {
*target = '?';
len = 1;
@@ -149,6 +152,7 @@ cifs_from_ucs2(char *to, const __le16 *from, int tolen, int fromlen,
int nullsize = nls_nullsize(codepage);
int fromwords = fromlen / 2;
char tmp[NLS_MAX_CHARSET_SIZE];
+ __u16 ftmp;
/*
* because the chars can be of varying widths, we need to take care
@@ -158,19 +162,23 @@ cifs_from_ucs2(char *to, const __le16 *from, int tolen, int fromlen,
*/
safelen = tolen - (NLS_MAX_CHARSET_SIZE + nullsize);
- for (i = 0; i < fromwords && from[i]; i++) {
+ for (i = 0; i < fromwords; i++) {
+ ftmp = get_unaligned_le16(&from[i]);
+ if (ftmp == 0)
+ break;
+
/*
* check to see if converting this character might make the
* conversion bleed into the null terminator
*/
if (outlen >= safelen) {
- charlen = cifs_mapchar(tmp, from[i], codepage, mapchar);
+ charlen = cifs_mapchar(tmp, ftmp, codepage, mapchar);
if ((outlen + charlen) > (tolen - nullsize))
break;
}
/* put converted char into 'to' buffer */
- charlen = cifs_mapchar(&to[outlen], from[i], codepage, mapchar);
+ charlen = cifs_mapchar(&to[outlen], ftmp, codepage, mapchar);
outlen += charlen;
}
@@ -193,24 +201,21 @@ cifs_strtoUCS(__le16 *to, const char *from, int len,
{
int charlen;
int i;
- wchar_t *wchar_to = (wchar_t *)to; /* needed to quiet sparse */
+ wchar_t wchar_to; /* needed to quiet sparse */
for (i = 0; len && *from; i++, from += charlen, len -= charlen) {
-
- /* works for 2.4.0 kernel or later */
- charlen = codepage->char2uni(from, len, &wchar_to[i]);
+ charlen = codepage->char2uni(from, len, &wchar_to);
if (charlen < 1) {
- cERROR(1, "strtoUCS: char2uni of %d returned %d",
- (int)*from, charlen);
+ cERROR(1, "strtoUCS: char2uni of 0x%x returned %d",
+ *from, charlen);
/* A question mark */
- to[i] = cpu_to_le16(0x003f);
+ wchar_to = 0x003f;
charlen = 1;
- } else
- to[i] = cpu_to_le16(wchar_to[i]);
-
+ }
+ put_unaligned_le16(wchar_to, &to[i]);
}
- to[i] = 0;
+ put_unaligned_le16(0, &to[i]);
return i;
}
--
1.7.3.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH 5/5] cifs: fix unaligned accesses in cifsConvertToUCS
[not found] ` <1295382799-7902-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (3 preceding siblings ...)
2011-01-18 20:33 ` [PATCH 4/5] cifs: clean up unaligned accesses in cifs_unicode.c Jeff Layton
@ 2011-01-18 20:33 ` Jeff Layton
[not found] ` <1295382799-7902-6-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-01-19 14:20 ` [PATCH 0/5] cifs: clean up some unaligned memory accesses Pavel Shilovsky
5 siblings, 1 reply; 17+ messages in thread
From: Jeff Layton @ 2011-01-18 20:33 UTC (permalink / raw)
To: smfrench-Re5JQEeQqe8AvxtiuMwx3w; +Cc: linux-cifs-u79uwXL29TY76Z2rM5mHXA
Move cifsConvertToUCS to cifs_unicode.c where all of the other unicode
related functions live. Have it store mapped characters in 'temp' and
then use put_unaligned_le16 to copy it to the target buffer. Also fix
the comments to match kernel coding style.
Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
fs/cifs/cifs_unicode.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++
fs/cifs/misc.c | 71 --------------------------------------------
2 files changed, 76 insertions(+), 71 deletions(-)
diff --git a/fs/cifs/cifs_unicode.c b/fs/cifs/cifs_unicode.c
index 8134443..6a5090d 100644
--- a/fs/cifs/cifs_unicode.c
+++ b/fs/cifs/cifs_unicode.c
@@ -257,3 +257,79 @@ cifs_strndup_from_ucs(const char *src, const int maxlen, const bool is_unicode,
return dst;
}
+/*
+ * Convert 16 bit Unicode pathname to wire format from string in current code
+ * page. Conversion may involve remapping up the six characters that are
+ * only legal in POSIX-like OS (if they are present in the string). Path
+ * names are little endian 16 bit Unicode on the wire
+ */
+int
+cifsConvertToUCS(__le16 *target, const char *source, int maxlen,
+ const struct nls_table *cp, int mapChars)
+{
+ int i, j, charlen;
+ int len_remaining = maxlen;
+ char src_char;
+ __u16 temp;
+
+ if (!mapChars)
+ return cifs_strtoUCS(target, source, PATH_MAX, cp);
+
+ for (i = 0, j = 0; i < maxlen; j++) {
+ src_char = source[i];
+ switch (src_char) {
+ case 0:
+ put_unaligned_le16(0, &target[j]);
+ goto ctoUCS_out;
+ case ':':
+ temp = UNI_COLON;
+ break;
+ case '*':
+ temp = UNI_ASTERIK;
+ break;
+ case '?':
+ temp = UNI_QUESTION;
+ break;
+ case '<':
+ temp = UNI_LESSTHAN;
+ break;
+ case '>':
+ temp = UNI_GRTRTHAN;
+ break;
+ case '|':
+ temp = UNI_PIPE;
+ break;
+ /*
+ * FIXME: We can not handle remapping backslash (UNI_SLASH)
+ * until all the calls to build_path_from_dentry are modified,
+ * as they use backslash as separator.
+ */
+ default:
+ charlen = cp->char2uni(source+i, len_remaining,
+ &temp);
+ /*
+ * if no match, use question mark, which at least in
+ * some cases serves as wild card
+ */
+ if (charlen < 1) {
+ temp = 0x003f;
+ charlen = 1;
+ }
+ len_remaining -= charlen;
+ /*
+ * character may take more than one byte in the source
+ * string, but will take exactly two bytes in the
+ * target string
+ */
+ i += charlen;
+ continue;
+ }
+ put_unaligned_le16(temp, &target[j]);
+ i++; /* move to next char in source string */
+ len_remaining--;
+ }
+
+ctoUCS_out:
+ return i;
+}
+
diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
index c1f2a51..2036757 100644
--- a/fs/cifs/misc.c
+++ b/fs/cifs/misc.c
@@ -626,77 +626,6 @@ dump_smb(struct smb_hdr *smb_buf, int smb_buf_length)
return;
}
-/* Convert 16 bit Unicode pathname to wire format from string in current code
- page. Conversion may involve remapping up the seven characters that are
- only legal in POSIX-like OS (if they are present in the string). Path
- names are little endian 16 bit Unicode on the wire */
-int
-cifsConvertToUCS(__le16 *target, const char *source, int maxlen,
- const struct nls_table *cp, int mapChars)
-{
- int i, j, charlen;
- int len_remaining = maxlen;
- char src_char;
- __u16 temp;
-
- if (!mapChars)
- return cifs_strtoUCS(target, source, PATH_MAX, cp);
-
- for (i = 0, j = 0; i < maxlen; j++) {
- src_char = source[i];
- switch (src_char) {
- case 0:
- target[j] = 0;
- goto ctoUCS_out;
- case ':':
- target[j] = cpu_to_le16(UNI_COLON);
- break;
- case '*':
- target[j] = cpu_to_le16(UNI_ASTERIK);
- break;
- case '?':
- target[j] = cpu_to_le16(UNI_QUESTION);
- break;
- case '<':
- target[j] = cpu_to_le16(UNI_LESSTHAN);
- break;
- case '>':
- target[j] = cpu_to_le16(UNI_GRTRTHAN);
- break;
- case '|':
- target[j] = cpu_to_le16(UNI_PIPE);
- break;
- /* BB We can not handle remapping slash until
- all the calls to build_path_from_dentry
- are modified, as they use slash as separator BB */
- /* case '\\':
- target[j] = cpu_to_le16(UNI_SLASH);
- break;*/
- default:
- charlen = cp->char2uni(source+i,
- len_remaining, &temp);
- /* if no match, use question mark, which
- at least in some cases servers as wild card */
- if (charlen < 1) {
- target[j] = cpu_to_le16(0x003f);
- charlen = 1;
- } else
- target[j] = cpu_to_le16(temp);
- len_remaining -= charlen;
- /* character may take more than one byte in the
- the source string, but will take exactly two
- bytes in the target string */
- i += charlen;
- continue;
- }
- i++; /* move to next char in source string */
- len_remaining--;
- }
-
-ctoUCS_out:
- return i;
-}
-
void
cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb)
{
--
1.7.3.4
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH 0/5] cifs: clean up some unaligned memory accesses
[not found] ` <1295382799-7902-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
` (4 preceding siblings ...)
2011-01-18 20:33 ` [PATCH 5/5] cifs: fix unaligned accesses in cifsConvertToUCS Jeff Layton
@ 2011-01-19 14:20 ` Pavel Shilovsky
5 siblings, 0 replies; 17+ messages in thread
From: Pavel Shilovsky @ 2011-01-19 14:20 UTC (permalink / raw)
To: Jeff Layton
Cc: smfrench-Re5JQEeQqe8AvxtiuMwx3w, linux-cifs-u79uwXL29TY76Z2rM5mHXA
2011/1/18 Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
> When the CIFS client is marshalling a call or parsing a response from
> the server, it often will access fields within the packets directly.
> It's easily possible however that those fields will not be aligned
> properly. Many CPUs handle this transparently. Some CPUs (such as ia64)
> throw a warning and then use a slow-path mechanism to deal with it.
> Other CPUs (mostly the embedded ones, it seems) actually throw an
> exception and just don't work.
>
> For more background on the problem, see this file in the kernel source
> tree:
>
> Documentation/unaligned-memory-access.txt
>
> This was originally reported quite some time ago here:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=11115
>
> I've also had a report of the same problem on ia64 against RHEL5:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=659715
>
> My patchset is based on the one originally by John Voltz. His proposed
> patch also patched up the generic NLS code for unaligned access, but I
> took a different approach by making sure that we just never called into
> those routines with an unaligned buffer.
>
> I've tested this patchset on x86_64 and it seems to be fine. I've also
> tested a version of this patchset backported to RHEL5 on ia64. Certain
> tests would make that arch pop these sorts of printks:
>
> kernel unaligned access to 0xe0000040ed16807f, ip=0xa0000002029b8530
>
> ...with this set, those are eliminated. I suspect that this may also
> help CIFS to work on some embedded arches as well (such as avr32).
>
> Note that this is likely not a comprehensive fix for CIFS though. It
> seems like there are a lot of places in cifssmb.c that access fields in
> the request or response directly. Any of them are probably also fair
> game for unaligned access fixes.
>
> I think this patchset is pretty safe, so we should consider getting it
> into 2.6.38 if possible. If not, definitely for 2.6.39.
>
> Jeff Layton (5):
> cifs: use get/put_unaligned functions to access ByteCount
> cifs: clean up unaligned accesses in validate_t2
> cifs: fix unaligned access in check2ndT2 and coalesce_t2
> cifs: clean up unaligned accesses in cifs_unicode.c
> cifs: fix unaligned accesses in cifsConvertToUCS
>
> fs/cifs/cifs_unicode.c | 125 +++++++++++++++++++++++++++++++++++++++--------
> fs/cifs/cifspdu.h | 47 ++++++++++++++++--
> fs/cifs/cifssmb.c | 52 ++++++++++----------
> fs/cifs/connect.c | 43 +++++++---------
> fs/cifs/misc.c | 71 ---------------------------
> fs/cifs/netmisc.c | 4 +-
> fs/cifs/sess.c | 13 ++---
> fs/cifs/transport.c | 9 ++--
> 8 files changed, 202 insertions(+), 162 deletions(-)
>
> --
> 1.7.3.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Jeff, I am not very comfortable in this area, but after investigating
the problem a little your patchset looks good to me. You can add my
this tag to all patches of the set: Acked-by: Pavel Shilovsky
<piastryyy-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
--
Best regards,
Pavel Shilovsky.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/5] cifs: use get/put_unaligned functions to access ByteCount
[not found] ` <1295382799-7902-2-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2011-01-19 15:33 ` Shirish Pargaonkar
[not found] ` <AANLkTi=XsEtjps=hq897Rt+akT9F-Toe1bpRuT=aMqPO-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Shirish Pargaonkar @ 2011-01-19 15:33 UTC (permalink / raw)
To: Jeff Layton
Cc: smfrench-Re5JQEeQqe8AvxtiuMwx3w, linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Tue, Jan 18, 2011 at 2:33 PM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> It's possible that when we access the ByteCount that the alignment
> will be off. Most CPUs deal with that transparently, but there's
> usually some performance impact. Some CPUs raise an exception on
> unaligned accesses.
>
> Fix this by accessing the byte count using the get_unaligned and
> put_unaligned inlined functions. While we're at it, fix the types
> of some of the variables that end up getting returns from these
> functions.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> fs/cifs/cifspdu.h | 47 +++++++++++++++++++++++++++++++++++++++++++----
> fs/cifs/cifssmb.c | 14 +++++---------
> fs/cifs/connect.c | 10 +++++-----
> fs/cifs/netmisc.c | 4 ++--
> fs/cifs/sess.c | 13 ++++++-------
> fs/cifs/transport.c | 9 ++++-----
> 6 files changed, 65 insertions(+), 32 deletions(-)
>
> diff --git a/fs/cifs/cifspdu.h b/fs/cifs/cifspdu.h
> index ea205b4..b5c8cc5 100644
> --- a/fs/cifs/cifspdu.h
> +++ b/fs/cifs/cifspdu.h
> @@ -23,6 +23,7 @@
> #define _CIFSPDU_H
>
> #include <net/sock.h>
> +#include <asm/unaligned.h>
> #include "smbfsctl.h"
>
> #ifdef CONFIG_CIFS_WEAK_PW_HASH
> @@ -426,11 +427,49 @@ struct smb_hdr {
> __u16 Mid;
> __u8 WordCount;
> } __attribute__((packed));
> -/* given a pointer to an smb_hdr retrieve the value of byte count */
> -#define BCC(smb_var) (*(__u16 *)((char *)(smb_var) + sizeof(struct smb_hdr) + (2 * (smb_var)->WordCount)))
> -#define BCC_LE(smb_var) (*(__le16 *)((char *)(smb_var) + sizeof(struct smb_hdr) + (2 * (smb_var)->WordCount)))
> +
> +/* given a pointer to an smb_hdr retrieve a char pointer to the byte count */
> +#define BCC(smb_var) ((unsigned char *)(smb_var) + sizeof(struct smb_hdr) + \
> + (2 * (smb_var)->WordCount))
> +
> /* given a pointer to an smb_hdr retrieve the pointer to the byte area */
> -#define pByteArea(smb_var) ((unsigned char *)(smb_var) + sizeof(struct smb_hdr) + (2 * (smb_var)->WordCount) + 2)
> +#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)
> +{
> + __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)
> +{
> + __le16 *bc_ptr = (__le16 *)BCC(hdr);
> +
> + put_unaligned_le16(count, bc_ptr);
> +}
>
> /*
> * Computer Name Length (since Netbios name was length 16 with last byte 0x20)
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index c5b2014..3b3072c 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -333,7 +333,6 @@ static int validate_t2(struct smb_t2_rsp *pSMB)
> {
> int rc = -EINVAL;
> int total_size;
> - char *pBCC;
>
> /* check for plausible wct, bcc and t2 data and parm sizes */
> /* check for parm and data offset going beyond end of smb */
> @@ -346,13 +345,9 @@ static int validate_t2(struct smb_t2_rsp *pSMB)
> if (total_size < 512) {
> total_size +=
> le16_to_cpu(pSMB->t2_rsp.DataCount);
> - /* BCC le converted in SendReceive */
> - pBCC = (pSMB->hdr.WordCount * 2) +
> - sizeof(struct smb_hdr) +
> - (char *)pSMB;
> - if ((total_size <= (*(u16 *)pBCC)) &&
> - (total_size <
> - CIFSMaxBufSize+MAX_CIFS_HDR_SIZE)) {
> + if (total_size <= get_bcc(&pSMB->hdr) &&
> + total_size <
> + CIFSMaxBufSize + MAX_CIFS_HDR_SIZE) {
> return 0;
> }
> }
> @@ -362,6 +357,7 @@ static int validate_t2(struct smb_t2_rsp *pSMB)
> sizeof(struct smb_t2_rsp) + 16);
> return rc;
> }
> +
> int
> CIFSSMBNegotiate(unsigned int xid, struct cifsSesInfo *ses)
> {
> @@ -5430,7 +5426,7 @@ QAllEAsRetry:
> }
>
> /* make sure list_len doesn't go past end of SMB */
> - end_of_smb = (char *)pByteArea(&pSMBr->hdr) + BCC(&pSMBr->hdr);
> + end_of_smb = (char *)pByteArea(&pSMBr->hdr) + get_bcc(&pSMBr->hdr);
> if ((char *)ea_response_data + list_len > end_of_smb) {
> cFYI(1, "EA list appears to go beyond SMB");
> rc = -EIO;
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 32c2f55..9bcdf2b 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -318,9 +318,9 @@ static int coalesce_t2(struct smb_hdr *psecond, struct smb_hdr *pTargetSMB)
> memcpy(data_area_of_target, data_area_of_buf2, total_in_buf2);
> total_in_buf += total_in_buf2;
> pSMBt->t2_rsp.DataCount = cpu_to_le16(total_in_buf);
> - byte_count = le16_to_cpu(BCC_LE(pTargetSMB));
> + byte_count = get_bcc_le(pTargetSMB);
> byte_count += total_in_buf2;
> - BCC_LE(pTargetSMB) = cpu_to_le16(byte_count);
> + put_bcc_le(byte_count, pTargetSMB);
>
> byte_count = pTargetSMB->smb_buf_length;
> byte_count += total_in_buf2;
> @@ -2937,8 +2937,8 @@ CIFSTCon(unsigned int xid, struct cifsSesInfo *ses,
> TCONX_RSP *pSMBr;
> unsigned char *bcc_ptr;
> int rc = 0;
> - int length, bytes_left;
> - __u16 count;
> + int length;
> + __u16 bytes_left, count;
>
> if (ses == NULL)
> return -EIO;
> @@ -3032,7 +3032,7 @@ CIFSTCon(unsigned int xid, struct cifsSesInfo *ses,
> tcon->need_reconnect = false;
> tcon->tid = smb_buffer_response->Tid;
> bcc_ptr = pByteArea(smb_buffer_response);
> - bytes_left = BCC(smb_buffer_response);
> + bytes_left = get_bcc(smb_buffer_response);
> length = strnlen(bcc_ptr, bytes_left - 2);
> if (smb_buffer->Flags2 & SMBFLG2_UNICODE)
> is_unicode = true;
> diff --git a/fs/cifs/netmisc.c b/fs/cifs/netmisc.c
> index 6783ce6..8d9189f 100644
> --- a/fs/cifs/netmisc.c
> +++ b/fs/cifs/netmisc.c
> @@ -916,14 +916,14 @@ unsigned int
> smbCalcSize(struct smb_hdr *ptr)
> {
> return (sizeof(struct smb_hdr) + (2 * ptr->WordCount) +
> - 2 /* size of the bcc field */ + BCC(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 */ + le16_to_cpu(BCC_LE(ptr)));
> + 2 /* size of the bcc field */ + get_bcc_le(ptr));
> }
>
> /* The following are taken from fs/ntfs/util.c */
> diff --git a/fs/cifs/sess.c b/fs/cifs/sess.c
> index 1cffd82..1adc962 100644
> --- a/fs/cifs/sess.c
> +++ b/fs/cifs/sess.c
> @@ -277,7 +277,7 @@ static void ascii_ssetup_strings(char **pbcc_area, struct cifsSesInfo *ses,
> }
>
> static void
> -decode_unicode_ssetup(char **pbcc_area, int bleft, struct cifsSesInfo *ses,
> +decode_unicode_ssetup(char **pbcc_area, __u16 bleft, struct cifsSesInfo *ses,
> const struct nls_table *nls_cp)
> {
> int len;
> @@ -323,7 +323,7 @@ decode_unicode_ssetup(char **pbcc_area, int bleft, struct cifsSesInfo *ses,
> return;
> }
>
> -static int decode_ascii_ssetup(char **pbcc_area, int bleft,
> +static int decode_ascii_ssetup(char **pbcc_area, __u16 bleft,
> struct cifsSesInfo *ses,
> const struct nls_table *nls_cp)
> {
> @@ -575,12 +575,11 @@ CIFS_SessSetup(unsigned int xid, struct cifsSesInfo *ses,
> char *str_area;
> SESSION_SETUP_ANDX *pSMB;
> __u32 capabilities;
> - int count;
> + __u16 count;
> int resp_buf_type;
> struct kvec iov[3];
> enum securityEnum type;
> - __u16 action;
> - int bytes_remaining;
> + __u16 action, bytes_remaining;
> struct key *spnego_key = NULL;
> __le32 phase = NtLmNegotiate; /* NTLMSSP, if needed, is multistage */
> u16 blob_len;
> @@ -876,7 +875,7 @@ ssetup_ntlmssp_authenticate:
> count = iov[1].iov_len + iov[2].iov_len;
> smb_buf->smb_buf_length += count;
>
> - BCC_LE(smb_buf) = cpu_to_le16(count);
> + put_bcc_le(count, smb_buf);
>
> rc = SendReceive2(xid, ses, iov, 3 /* num_iovecs */, &resp_buf_type,
> CIFS_LOG_ERROR);
> @@ -910,7 +909,7 @@ ssetup_ntlmssp_authenticate:
> cFYI(1, "UID = %d ", ses->Suid);
> /* response can have either 3 or 4 word count - Samba sends 3 */
> /* and lanman response is 3 */
> - bytes_remaining = BCC(smb_buf);
> + bytes_remaining = get_bcc(smb_buf);
> bcc_ptr = pByteArea(smb_buf);
>
> if (smb_buf->WordCount == 4) {
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index e2e66f3..fbeaee8 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -484,7 +484,7 @@ send_nt_cancel(struct TCP_Server_Info *server, struct smb_hdr *in_buf,
> in_buf->smb_buf_length = sizeof(struct smb_hdr) - 4 + 2;
> in_buf->Command = SMB_COM_NT_CANCEL;
> in_buf->WordCount = 0;
> - BCC_LE(in_buf) = 0;
> + put_bcc_le(0, in_buf);
>
> mutex_lock(&server->srv_mutex);
> rc = cifs_sign_smb(in_buf, server, &mid->sequence_number);
> @@ -648,8 +648,7 @@ SendReceive2(const unsigned int xid, struct cifsSesInfo *ses,
> if (receive_len >= sizeof(struct smb_hdr) - 4
> /* do not count RFC1001 header */ +
> (2 * midQ->resp_buf->WordCount) + 2 /* bcc */ )
> - BCC(midQ->resp_buf) =
> - le16_to_cpu(BCC_LE(midQ->resp_buf));
> + put_bcc(get_bcc_le(midQ->resp_buf), midQ->resp_buf);
> if ((flags & CIFS_NO_RESP) == 0)
> midQ->resp_buf = NULL; /* mark it so buf will
> not be freed by
> @@ -803,7 +802,7 @@ SendReceive(const unsigned int xid, struct cifsSesInfo *ses,
> if (receive_len >= sizeof(struct smb_hdr) - 4
> /* do not count RFC1001 header */ +
> (2 * out_buf->WordCount) + 2 /* bcc */ )
> - BCC(out_buf) = le16_to_cpu(BCC_LE(out_buf));
> + put_bcc(get_bcc_le(midQ->resp_buf), midQ->resp_buf);
> } else {
> rc = -EIO;
> cERROR(1, "Bad MID state?");
> @@ -1015,7 +1014,7 @@ SendReceiveBlockingLock(const unsigned int xid, struct cifsTconInfo *tcon,
> if (receive_len >= sizeof(struct smb_hdr) - 4
> /* do not count RFC1001 header */ +
> (2 * out_buf->WordCount) + 2 /* bcc */ )
> - BCC(out_buf) = le16_to_cpu(BCC_LE(out_buf));
> + put_bcc(get_bcc_le(out_buf), out_buf);
>
> out:
> delete_mid(midQ);
> --
> 1.7.3.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Should get_bcc* and put_bcc* functions be renamed to respective
get_bcc16* and put_bcc16* respectively (or something like that)
in case there is a need to have similar functions for four bytes fields?
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 2/5] cifs: clean up unaligned accesses in validate_t2
[not found] ` <1295382799-7902-3-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2011-01-19 15:39 ` Shirish Pargaonkar
0 siblings, 0 replies; 17+ messages in thread
From: Shirish Pargaonkar @ 2011-01-19 15:39 UTC (permalink / raw)
To: Jeff Layton
Cc: smfrench-Re5JQEeQqe8AvxtiuMwx3w, linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Tue, Jan 18, 2011 at 2:33 PM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> ...and clean up function to reduce indentation.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> fs/cifs/cifssmb.c | 44 +++++++++++++++++++++++---------------------
> 1 files changed, 23 insertions(+), 21 deletions(-)
>
> diff --git a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c
> index 3b3072c..75a1559 100644
> --- a/fs/cifs/cifssmb.c
> +++ b/fs/cifs/cifssmb.c
> @@ -331,31 +331,33 @@ smb_init_no_reconnect(int smb_command, int wct, struct cifsTconInfo *tcon,
>
> static int validate_t2(struct smb_t2_rsp *pSMB)
> {
> - int rc = -EINVAL;
> - int total_size;
> + unsigned int total_size;
> +
> + /* check for plausible wct */
> + if (pSMB->hdr.WordCount < 10)
> + goto vt2_err;
>
> - /* check for plausible wct, bcc and t2 data and parm sizes */
> /* check for parm and data offset going beyond end of smb */
> - if (pSMB->hdr.WordCount >= 10) {
> - if ((le16_to_cpu(pSMB->t2_rsp.ParameterOffset) <= 1024) &&
> - (le16_to_cpu(pSMB->t2_rsp.DataOffset) <= 1024)) {
> - /* check that bcc is at least as big as parms + data */
> - /* check that bcc is less than negotiated smb buffer */
> - total_size = le16_to_cpu(pSMB->t2_rsp.ParameterCount);
> - if (total_size < 512) {
> - total_size +=
> - le16_to_cpu(pSMB->t2_rsp.DataCount);
> - if (total_size <= get_bcc(&pSMB->hdr) &&
> - total_size <
> - CIFSMaxBufSize + MAX_CIFS_HDR_SIZE) {
> - return 0;
> - }
> - }
> - }
> - }
> + if (get_unaligned_le16(&pSMB->t2_rsp.ParameterOffset) > 1024 ||
> + get_unaligned_le16(&pSMB->t2_rsp.DataOffset) > 1024)
> + goto vt2_err;
> +
> + /* check that bcc is at least as big as parms + data */
> + /* check that bcc is less than negotiated smb buffer */
> + total_size = get_unaligned_le16(&pSMB->t2_rsp.ParameterCount);
> + if (total_size >= 512)
> + goto vt2_err;
> +
> + total_size += get_unaligned_le16(&pSMB->t2_rsp.DataCount);
> + if (total_size > get_bcc(&pSMB->hdr) ||
> + total_size >= CIFSMaxBufSize + MAX_CIFS_HDR_SIZE)
> + goto vt2_err;
> +
> + return 0;
> +vt2_err:
> cifs_dump_mem("Invalid transact2 SMB: ", (char *)pSMB,
> sizeof(struct smb_t2_rsp) + 16);
> - return rc;
> + return -EINVAL;
> }
>
> int
> --
> 1.7.3.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Looks correct.
Reviewed-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 1/5] cifs: use get/put_unaligned functions to access ByteCount
[not found] ` <AANLkTi=XsEtjps=hq897Rt+akT9F-Toe1bpRuT=aMqPO-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2011-01-19 15:40 ` Jeff Layton
0 siblings, 0 replies; 17+ messages in thread
From: Jeff Layton @ 2011-01-19 15:40 UTC (permalink / raw)
To: Shirish Pargaonkar
Cc: smfrench-Re5JQEeQqe8AvxtiuMwx3w, linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Wed, 19 Jan 2011 09:33:47 -0600
Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
> Should get_bcc* and put_bcc* functions be renamed to respective
> get_bcc16* and put_bcc16* respectively (or something like that)
> in case there is a need to have similar functions for four bytes fields?
get/put_bcc are for the ByteCount field specifically and that's always
2 bytes.
--
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 3/5] cifs: fix unaligned access in check2ndT2 and coalesce_t2
[not found] ` <1295382799-7902-4-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2011-01-19 15:42 ` Shirish Pargaonkar
0 siblings, 0 replies; 17+ messages in thread
From: Shirish Pargaonkar @ 2011-01-19 15:42 UTC (permalink / raw)
To: Jeff Layton
Cc: smfrench-Re5JQEeQqe8AvxtiuMwx3w, linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Tue, Jan 18, 2011 at 2:33 PM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> fs/cifs/connect.c | 33 ++++++++++++++-------------------
> 1 files changed, 14 insertions(+), 19 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 9bcdf2b..d5b779e 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -232,9 +232,8 @@ cifs_reconnect(struct TCP_Server_Info *server)
> static int check2ndT2(struct smb_hdr *pSMB, unsigned int maxBufSize)
> {
> struct smb_t2_rsp *pSMBt;
> - int total_data_size;
> - int data_in_this_rsp;
> int remaining;
> + __u16 total_data_size, data_in_this_rsp;
>
> if (pSMB->Command != SMB_COM_TRANSACTION2)
> return 0;
> @@ -248,8 +247,8 @@ static int check2ndT2(struct smb_hdr *pSMB, unsigned int maxBufSize)
>
> pSMBt = (struct smb_t2_rsp *)pSMB;
>
> - total_data_size = le16_to_cpu(pSMBt->t2_rsp.TotalDataCount);
> - data_in_this_rsp = le16_to_cpu(pSMBt->t2_rsp.DataCount);
> + total_data_size = get_unaligned_le16(&pSMBt->t2_rsp.TotalDataCount);
> + data_in_this_rsp = get_unaligned_le16(&pSMBt->t2_rsp.DataCount);
>
> remaining = total_data_size - data_in_this_rsp;
>
> @@ -275,21 +274,18 @@ static int coalesce_t2(struct smb_hdr *psecond, struct smb_hdr *pTargetSMB)
> {
> struct smb_t2_rsp *pSMB2 = (struct smb_t2_rsp *)psecond;
> struct smb_t2_rsp *pSMBt = (struct smb_t2_rsp *)pTargetSMB;
> - int total_data_size;
> - int total_in_buf;
> - int remaining;
> - int total_in_buf2;
> char *data_area_of_target;
> char *data_area_of_buf2;
> - __u16 byte_count;
> + int remaining;
> + __u16 byte_count, total_data_size, total_in_buf, total_in_buf2;
>
> - total_data_size = le16_to_cpu(pSMBt->t2_rsp.TotalDataCount);
> + total_data_size = get_unaligned_le16(&pSMBt->t2_rsp.TotalDataCount);
>
> - if (total_data_size != le16_to_cpu(pSMB2->t2_rsp.TotalDataCount)) {
> + if (total_data_size !=
> + get_unaligned_le16(&pSMB2->t2_rsp.TotalDataCount))
> cFYI(1, "total data size of primary and secondary t2 differ");
> - }
>
> - total_in_buf = le16_to_cpu(pSMBt->t2_rsp.DataCount);
> + total_in_buf = get_unaligned_le16(&pSMBt->t2_rsp.DataCount);
>
> remaining = total_data_size - total_in_buf;
>
> @@ -299,25 +295,25 @@ static int coalesce_t2(struct smb_hdr *psecond, struct smb_hdr *pTargetSMB)
> if (remaining == 0) /* nothing to do, ignore */
> return 0;
>
> - total_in_buf2 = le16_to_cpu(pSMB2->t2_rsp.DataCount);
> + total_in_buf2 = get_unaligned_le16(&pSMB2->t2_rsp.DataCount);
> if (remaining < total_in_buf2) {
> cFYI(1, "transact2 2nd response contains too much data");
> }
>
> /* find end of first SMB data area */
> data_area_of_target = (char *)&pSMBt->hdr.Protocol +
> - le16_to_cpu(pSMBt->t2_rsp.DataOffset);
> + get_unaligned_le16(&pSMBt->t2_rsp.DataOffset);
> /* validate target area */
>
> - data_area_of_buf2 = (char *) &pSMB2->hdr.Protocol +
> - le16_to_cpu(pSMB2->t2_rsp.DataOffset);
> + data_area_of_buf2 = (char *)&pSMB2->hdr.Protocol +
> + get_unaligned_le16(&pSMB2->t2_rsp.DataOffset);
>
> data_area_of_target += total_in_buf;
>
> /* copy second buffer into end of first buffer */
> memcpy(data_area_of_target, data_area_of_buf2, total_in_buf2);
> total_in_buf += total_in_buf2;
> - pSMBt->t2_rsp.DataCount = cpu_to_le16(total_in_buf);
> + put_unaligned_le16(total_in_buf, &pSMBt->t2_rsp.DataCount);
> byte_count = get_bcc_le(pTargetSMB);
> byte_count += total_in_buf2;
> put_bcc_le(byte_count, pTargetSMB);
> @@ -334,7 +330,6 @@ static int coalesce_t2(struct smb_hdr *psecond, struct smb_hdr *pTargetSMB)
> return 0; /* we are done */
> } else /* more responses to go */
> return 1;
> -
> }
>
> static void
> --
> 1.7.3.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Looks correct.
Reviewed-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] cifs: clean up unaligned accesses in cifs_unicode.c
[not found] ` <1295382799-7902-5-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2011-01-19 15:58 ` Shirish Pargaonkar
[not found] ` <AANLkTikZYP39UEVGt1Tn1PdYE9RnoBEhOjV+qWfd3oRm-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Shirish Pargaonkar @ 2011-01-19 15:58 UTC (permalink / raw)
To: Jeff Layton
Cc: smfrench-Re5JQEeQqe8AvxtiuMwx3w, linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Tue, Jan 18, 2011 at 2:33 PM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Make sure we use get/put_unaligned routines when accessing wide
> character strings.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> fs/cifs/cifs_unicode.c | 49 ++++++++++++++++++++++++++---------------------
> 1 files changed, 27 insertions(+), 22 deletions(-)
>
> diff --git a/fs/cifs/cifs_unicode.c b/fs/cifs/cifs_unicode.c
> index 430f510..8134443 100644
> --- a/fs/cifs/cifs_unicode.c
> +++ b/fs/cifs/cifs_unicode.c
> @@ -44,10 +44,14 @@ cifs_ucs2_bytes(const __le16 *from, int maxbytes,
> int charlen, outlen = 0;
> int maxwords = maxbytes / 2;
> char tmp[NLS_MAX_CHARSET_SIZE];
> + __u16 ftmp;
>
> - for (i = 0; i < maxwords && from[i]; i++) {
> - charlen = codepage->uni2char(le16_to_cpu(from[i]), tmp,
> - NLS_MAX_CHARSET_SIZE);
> + for (i = 0; i < maxwords; i++) {
> + ftmp = get_unaligned_le16(&from[i]);
> + if (ftmp == 0)
> + break;
> +
> + charlen = codepage->uni2char(ftmp, tmp, NLS_MAX_CHARSET_SIZE);
> if (charlen > 0)
> outlen += charlen;
> else
> @@ -60,7 +64,7 @@ cifs_ucs2_bytes(const __le16 *from, int maxbytes,
> /*
> * cifs_mapchar - convert a little-endian char to proper char in codepage
I think this wording is now incorrect since src_char is not le anymore.
> * @target - where converted character should be copied
> - * @src_char - 2 byte little-endian source character
> + * @src_char - 2 byte host-endian source character
> * @cp - codepage to which character should be converted
> * @mapchar - should character be mapped according to mapchars mount option?
> *
> @@ -69,7 +73,7 @@ cifs_ucs2_bytes(const __le16 *from, int maxbytes,
> * enough to hold the result of the conversion (at least NLS_MAX_CHARSET_SIZE).
> */
> static int
> -cifs_mapchar(char *target, const __le16 src_char, const struct nls_table *cp,
> +cifs_mapchar(char *target, const __u16 src_char, const struct nls_table *cp,
> bool mapchar)
> {
> int len = 1;
> @@ -82,7 +86,7 @@ cifs_mapchar(char *target, const __le16 src_char, const struct nls_table *cp,
> * build_path_from_dentry are modified, as they use slash as
> * separator.
> */
> - switch (le16_to_cpu(src_char)) {
> + switch (src_char) {
> case UNI_COLON:
> *target = ':';
> break;
> @@ -109,8 +113,7 @@ out:
> return len;
>
> cp_convert:
> - len = cp->uni2char(le16_to_cpu(src_char), target,
> - NLS_MAX_CHARSET_SIZE);
> + len = cp->uni2char(src_char, target, NLS_MAX_CHARSET_SIZE);
> if (len <= 0) {
> *target = '?';
> len = 1;
> @@ -149,6 +152,7 @@ cifs_from_ucs2(char *to, const __le16 *from, int tolen, int fromlen,
> int nullsize = nls_nullsize(codepage);
> int fromwords = fromlen / 2;
> char tmp[NLS_MAX_CHARSET_SIZE];
> + __u16 ftmp;
>
> /*
> * because the chars can be of varying widths, we need to take care
> @@ -158,19 +162,23 @@ cifs_from_ucs2(char *to, const __le16 *from, int tolen, int fromlen,
> */
> safelen = tolen - (NLS_MAX_CHARSET_SIZE + nullsize);
>
> - for (i = 0; i < fromwords && from[i]; i++) {
> + for (i = 0; i < fromwords; i++) {
> + ftmp = get_unaligned_le16(&from[i]);
> + if (ftmp == 0)
> + break;
> +
Can the contents of from[i] be 0 so ftmp is 0 but we did
not want to break out until fromwords?
> /*
> * check to see if converting this character might make the
> * conversion bleed into the null terminator
> */
> if (outlen >= safelen) {
> - charlen = cifs_mapchar(tmp, from[i], codepage, mapchar);
> + charlen = cifs_mapchar(tmp, ftmp, codepage, mapchar);
> if ((outlen + charlen) > (tolen - nullsize))
> break;
> }
>
> /* put converted char into 'to' buffer */
> - charlen = cifs_mapchar(&to[outlen], from[i], codepage, mapchar);
> + charlen = cifs_mapchar(&to[outlen], ftmp, codepage, mapchar);
> outlen += charlen;
> }
>
> @@ -193,24 +201,21 @@ cifs_strtoUCS(__le16 *to, const char *from, int len,
> {
> int charlen;
> int i;
> - wchar_t *wchar_to = (wchar_t *)to; /* needed to quiet sparse */
> + wchar_t wchar_to; /* needed to quiet sparse */
>
> for (i = 0; len && *from; i++, from += charlen, len -= charlen) {
> -
> - /* works for 2.4.0 kernel or later */
> - charlen = codepage->char2uni(from, len, &wchar_to[i]);
> + charlen = codepage->char2uni(from, len, &wchar_to);
> if (charlen < 1) {
> - cERROR(1, "strtoUCS: char2uni of %d returned %d",
> - (int)*from, charlen);
> + cERROR(1, "strtoUCS: char2uni of 0x%x returned %d",
> + *from, charlen);
> /* A question mark */
> - to[i] = cpu_to_le16(0x003f);
> + wchar_to = 0x003f;
> charlen = 1;
> - } else
> - to[i] = cpu_to_le16(wchar_to[i]);
> -
> + }
> + put_unaligned_le16(wchar_to, &to[i]);
> }
>
> - to[i] = 0;
> + put_unaligned_le16(0, &to[i]);
> return i;
> }
>
> --
> 1.7.3.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 5/5] cifs: fix unaligned accesses in cifsConvertToUCS
[not found] ` <1295382799-7902-6-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2011-01-19 16:02 ` Shirish Pargaonkar
0 siblings, 0 replies; 17+ messages in thread
From: Shirish Pargaonkar @ 2011-01-19 16:02 UTC (permalink / raw)
To: Jeff Layton
Cc: smfrench-Re5JQEeQqe8AvxtiuMwx3w, linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Tue, Jan 18, 2011 at 2:33 PM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> Move cifsConvertToUCS to cifs_unicode.c where all of the other unicode
> related functions live. Have it store mapped characters in 'temp' and
> then use put_unaligned_le16 to copy it to the target buffer. Also fix
> the comments to match kernel coding style.
>
> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> fs/cifs/cifs_unicode.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++
> fs/cifs/misc.c | 71 --------------------------------------------
> 2 files changed, 76 insertions(+), 71 deletions(-)
>
> diff --git a/fs/cifs/cifs_unicode.c b/fs/cifs/cifs_unicode.c
> index 8134443..6a5090d 100644
> --- a/fs/cifs/cifs_unicode.c
> +++ b/fs/cifs/cifs_unicode.c
> @@ -257,3 +257,79 @@ cifs_strndup_from_ucs(const char *src, const int maxlen, const bool is_unicode,
> return dst;
> }
>
> +/*
> + * Convert 16 bit Unicode pathname to wire format from string in current code
> + * page. Conversion may involve remapping up the six characters that are
> + * only legal in POSIX-like OS (if they are present in the string). Path
> + * names are little endian 16 bit Unicode on the wire
> + */
> +int
> +cifsConvertToUCS(__le16 *target, const char *source, int maxlen,
> + const struct nls_table *cp, int mapChars)
> +{
> + int i, j, charlen;
> + int len_remaining = maxlen;
> + char src_char;
> + __u16 temp;
> +
> + if (!mapChars)
> + return cifs_strtoUCS(target, source, PATH_MAX, cp);
> +
> + for (i = 0, j = 0; i < maxlen; j++) {
> + src_char = source[i];
> + switch (src_char) {
> + case 0:
> + put_unaligned_le16(0, &target[j]);
> + goto ctoUCS_out;
> + case ':':
> + temp = UNI_COLON;
> + break;
> + case '*':
> + temp = UNI_ASTERIK;
> + break;
> + case '?':
> + temp = UNI_QUESTION;
> + break;
> + case '<':
> + temp = UNI_LESSTHAN;
> + break;
> + case '>':
> + temp = UNI_GRTRTHAN;
> + break;
> + case '|':
> + temp = UNI_PIPE;
> + break;
> + /*
> + * FIXME: We can not handle remapping backslash (UNI_SLASH)
> + * until all the calls to build_path_from_dentry are modified,
> + * as they use backslash as separator.
> + */
> + default:
> + charlen = cp->char2uni(source+i, len_remaining,
> + &temp);
> + /*
> + * if no match, use question mark, which at least in
> + * some cases serves as wild card
> + */
> + if (charlen < 1) {
> + temp = 0x003f;
> + charlen = 1;
> + }
> + len_remaining -= charlen;
> + /*
> + * character may take more than one byte in the source
> + * string, but will take exactly two bytes in the
> + * target string
> + */
> + i += charlen;
> + continue;
> + }
> + put_unaligned_le16(temp, &target[j]);
> + i++; /* move to next char in source string */
> + len_remaining--;
> + }
> +
> +ctoUCS_out:
> + return i;
> +}
> +
> diff --git a/fs/cifs/misc.c b/fs/cifs/misc.c
> index c1f2a51..2036757 100644
> --- a/fs/cifs/misc.c
> +++ b/fs/cifs/misc.c
> @@ -626,77 +626,6 @@ dump_smb(struct smb_hdr *smb_buf, int smb_buf_length)
> return;
> }
>
> -/* Convert 16 bit Unicode pathname to wire format from string in current code
> - page. Conversion may involve remapping up the seven characters that are
> - only legal in POSIX-like OS (if they are present in the string). Path
> - names are little endian 16 bit Unicode on the wire */
> -int
> -cifsConvertToUCS(__le16 *target, const char *source, int maxlen,
> - const struct nls_table *cp, int mapChars)
> -{
> - int i, j, charlen;
> - int len_remaining = maxlen;
> - char src_char;
> - __u16 temp;
> -
> - if (!mapChars)
> - return cifs_strtoUCS(target, source, PATH_MAX, cp);
> -
> - for (i = 0, j = 0; i < maxlen; j++) {
> - src_char = source[i];
> - switch (src_char) {
> - case 0:
> - target[j] = 0;
> - goto ctoUCS_out;
> - case ':':
> - target[j] = cpu_to_le16(UNI_COLON);
> - break;
> - case '*':
> - target[j] = cpu_to_le16(UNI_ASTERIK);
> - break;
> - case '?':
> - target[j] = cpu_to_le16(UNI_QUESTION);
> - break;
> - case '<':
> - target[j] = cpu_to_le16(UNI_LESSTHAN);
> - break;
> - case '>':
> - target[j] = cpu_to_le16(UNI_GRTRTHAN);
> - break;
> - case '|':
> - target[j] = cpu_to_le16(UNI_PIPE);
> - break;
> - /* BB We can not handle remapping slash until
> - all the calls to build_path_from_dentry
> - are modified, as they use slash as separator BB */
> - /* case '\\':
> - target[j] = cpu_to_le16(UNI_SLASH);
> - break;*/
> - default:
> - charlen = cp->char2uni(source+i,
> - len_remaining, &temp);
> - /* if no match, use question mark, which
> - at least in some cases servers as wild card */
> - if (charlen < 1) {
> - target[j] = cpu_to_le16(0x003f);
> - charlen = 1;
> - } else
> - target[j] = cpu_to_le16(temp);
> - len_remaining -= charlen;
> - /* character may take more than one byte in the
> - the source string, but will take exactly two
> - bytes in the target string */
> - i += charlen;
> - continue;
> - }
> - i++; /* move to next char in source string */
> - len_remaining--;
> - }
> -
> -ctoUCS_out:
> - return i;
> -}
> -
> void
> cifs_autodisable_serverino(struct cifs_sb_info *cifs_sb)
> {
> --
> 1.7.3.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
Looks correct.
Reviewed-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] cifs: clean up unaligned accesses in cifs_unicode.c
[not found] ` <AANLkTikZYP39UEVGt1Tn1PdYE9RnoBEhOjV+qWfd3oRm-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2011-01-19 16:08 ` Shirish Pargaonkar
[not found] ` <AANLkTikxyAb2GSBxNeWYE6sAp7bSdakjWgp1_si1G7HU-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-01-19 16:11 ` Jeff Layton
1 sibling, 1 reply; 17+ messages in thread
From: Shirish Pargaonkar @ 2011-01-19 16:08 UTC (permalink / raw)
To: Jeff Layton
Cc: smfrench-Re5JQEeQqe8AvxtiuMwx3w, linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Wed, Jan 19, 2011 at 9:58 AM, Shirish Pargaonkar
<shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Tue, Jan 18, 2011 at 2:33 PM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> Make sure we use get/put_unaligned routines when accessing wide
>> character strings.
>>
>> Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>> ---
>> fs/cifs/cifs_unicode.c | 49 ++++++++++++++++++++++++++---------------------
>> 1 files changed, 27 insertions(+), 22 deletions(-)
>>
>> diff --git a/fs/cifs/cifs_unicode.c b/fs/cifs/cifs_unicode.c
>> index 430f510..8134443 100644
>> --- a/fs/cifs/cifs_unicode.c
>> +++ b/fs/cifs/cifs_unicode.c
>> @@ -44,10 +44,14 @@ cifs_ucs2_bytes(const __le16 *from, int maxbytes,
>> int charlen, outlen = 0;
>> int maxwords = maxbytes / 2;
>> char tmp[NLS_MAX_CHARSET_SIZE];
>> + __u16 ftmp;
>>
>> - for (i = 0; i < maxwords && from[i]; i++) {
>> - charlen = codepage->uni2char(le16_to_cpu(from[i]), tmp,
>> - NLS_MAX_CHARSET_SIZE);
>> + for (i = 0; i < maxwords; i++) {
>> + ftmp = get_unaligned_le16(&from[i]);
>> + if (ftmp == 0)
>> + break;
>> +
>> + charlen = codepage->uni2char(ftmp, tmp, NLS_MAX_CHARSET_SIZE);
>> if (charlen > 0)
>> outlen += charlen;
>> else
>> @@ -60,7 +64,7 @@ cifs_ucs2_bytes(const __le16 *from, int maxbytes,
>> /*
>> * cifs_mapchar - convert a little-endian char to proper char in codepage
>
> I think this wording is now incorrect since src_char is not le anymore.
>
>> * @target - where converted character should be copied
>> - * @src_char - 2 byte little-endian source character
>> + * @src_char - 2 byte host-endian source character
>> * @cp - codepage to which character should be converted
>> * @mapchar - should character be mapped according to mapchars mount option?
>> *
>> @@ -69,7 +73,7 @@ cifs_ucs2_bytes(const __le16 *from, int maxbytes,
>> * enough to hold the result of the conversion (at least NLS_MAX_CHARSET_SIZE).
>> */
>> static int
>> -cifs_mapchar(char *target, const __le16 src_char, const struct nls_table *cp,
>> +cifs_mapchar(char *target, const __u16 src_char, const struct nls_table *cp,
>> bool mapchar)
>> {
>> int len = 1;
>> @@ -82,7 +86,7 @@ cifs_mapchar(char *target, const __le16 src_char, const struct nls_table *cp,
>> * build_path_from_dentry are modified, as they use slash as
>> * separator.
>> */
>> - switch (le16_to_cpu(src_char)) {
>> + switch (src_char) {
>> case UNI_COLON:
>> *target = ':';
>> break;
>> @@ -109,8 +113,7 @@ out:
>> return len;
>>
>> cp_convert:
>> - len = cp->uni2char(le16_to_cpu(src_char), target,
>> - NLS_MAX_CHARSET_SIZE);
>> + len = cp->uni2char(src_char, target, NLS_MAX_CHARSET_SIZE);
>> if (len <= 0) {
>> *target = '?';
>> len = 1;
>> @@ -149,6 +152,7 @@ cifs_from_ucs2(char *to, const __le16 *from, int tolen, int fromlen,
>> int nullsize = nls_nullsize(codepage);
>> int fromwords = fromlen / 2;
>> char tmp[NLS_MAX_CHARSET_SIZE];
>> + __u16 ftmp;
>>
>> /*
>> * because the chars can be of varying widths, we need to take care
>> @@ -158,19 +162,23 @@ cifs_from_ucs2(char *to, const __le16 *from, int tolen, int fromlen,
>> */
>> safelen = tolen - (NLS_MAX_CHARSET_SIZE + nullsize);
>>
>> - for (i = 0; i < fromwords && from[i]; i++) {
>> + for (i = 0; i < fromwords; i++) {
>> + ftmp = get_unaligned_le16(&from[i]);
>> + if (ftmp == 0)
>> + break;
>> +
>
> Can the contents of from[i] be 0 so ftmp is 0 but we did
> not want to break out until fromwords?
>
Basically, why_does/what_it_means get_unaligned_le16()
returning 0 warrants breaking out of the loop!
>> /*
>> * check to see if converting this character might make the
>> * conversion bleed into the null terminator
>> */
>> if (outlen >= safelen) {
>> - charlen = cifs_mapchar(tmp, from[i], codepage, mapchar);
>> + charlen = cifs_mapchar(tmp, ftmp, codepage, mapchar);
>> if ((outlen + charlen) > (tolen - nullsize))
>> break;
>> }
>>
>> /* put converted char into 'to' buffer */
>> - charlen = cifs_mapchar(&to[outlen], from[i], codepage, mapchar);
>> + charlen = cifs_mapchar(&to[outlen], ftmp, codepage, mapchar);
>> outlen += charlen;
>> }
>>
>> @@ -193,24 +201,21 @@ cifs_strtoUCS(__le16 *to, const char *from, int len,
>> {
>> int charlen;
>> int i;
>> - wchar_t *wchar_to = (wchar_t *)to; /* needed to quiet sparse */
>> + wchar_t wchar_to; /* needed to quiet sparse */
>>
>> for (i = 0; len && *from; i++, from += charlen, len -= charlen) {
>> -
>> - /* works for 2.4.0 kernel or later */
>> - charlen = codepage->char2uni(from, len, &wchar_to[i]);
>> + charlen = codepage->char2uni(from, len, &wchar_to);
>> if (charlen < 1) {
>> - cERROR(1, "strtoUCS: char2uni of %d returned %d",
>> - (int)*from, charlen);
>> + cERROR(1, "strtoUCS: char2uni of 0x%x returned %d",
>> + *from, charlen);
>> /* A question mark */
>> - to[i] = cpu_to_le16(0x003f);
>> + wchar_to = 0x003f;
>> charlen = 1;
>> - } else
>> - to[i] = cpu_to_le16(wchar_to[i]);
>> -
>> + }
>> + put_unaligned_le16(wchar_to, &to[i]);
>> }
>>
>> - to[i] = 0;
>> + put_unaligned_le16(0, &to[i]);
>> return i;
>> }
>>
>> --
>> 1.7.3.4
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] cifs: clean up unaligned accesses in cifs_unicode.c
[not found] ` <AANLkTikZYP39UEVGt1Tn1PdYE9RnoBEhOjV+qWfd3oRm-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-01-19 16:08 ` Shirish Pargaonkar
@ 2011-01-19 16:11 ` Jeff Layton
[not found] ` <20110119111141.0567a9e9-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
1 sibling, 1 reply; 17+ messages in thread
From: Jeff Layton @ 2011-01-19 16:11 UTC (permalink / raw)
To: Shirish Pargaonkar
Cc: smfrench-Re5JQEeQqe8AvxtiuMwx3w, linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Wed, 19 Jan 2011 09:58:45 -0600
Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Tue, Jan 18, 2011 at 2:33 PM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > Make sure we use get/put_unaligned routines when accessing wide
> > character strings.
> >
> > Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
[...]
> > @@ -60,7 +64,7 @@ cifs_ucs2_bytes(const __le16 *from, int maxbytes,
> > /*
> > * cifs_mapchar - convert a little-endian char to proper char in codepage
>
> I think this wording is now incorrect since src_char is not le anymore.
>
Good catch. I'll fix it.
[...]
> > @@ -158,19 +162,23 @@ cifs_from_ucs2(char *to, const __le16 *from, int tolen, int fromlen,
> > */
> > safelen = tolen - (NLS_MAX_CHARSET_SIZE + nullsize);
> >
> > - for (i = 0; i < fromwords && from[i]; i++) {
> > + for (i = 0; i < fromwords; i++) {
> > + ftmp = get_unaligned_le16(&from[i]);
> > + if (ftmp == 0)
> > + break;
> > +
>
> Can the contents of from[i] be 0 so ftmp is 0 but we did
> not want to break out until fromwords?
>
I don't quite understand what you're asking. The code should be
equivalent to what was there before except that we use
get_unaligned_le16 to get the value and store it in a variable on the
stack. Note that the '&& from[i]' condition was removed from the "for"
statement.
--
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] cifs: clean up unaligned accesses in cifs_unicode.c
[not found] ` <20110119111141.0567a9e9-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
@ 2011-01-19 16:22 ` Shirish Pargaonkar
0 siblings, 0 replies; 17+ messages in thread
From: Shirish Pargaonkar @ 2011-01-19 16:22 UTC (permalink / raw)
To: Jeff Layton
Cc: smfrench-Re5JQEeQqe8AvxtiuMwx3w, linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Wed, Jan 19, 2011 at 10:11 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Wed, 19 Jan 2011 09:58:45 -0600
> Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
>> On Tue, Jan 18, 2011 at 2:33 PM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>> > Make sure we use get/put_unaligned routines when accessing wide
>> > character strings.
>> >
>> > Signed-off-by: Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>
> [...]
>
>> > @@ -60,7 +64,7 @@ cifs_ucs2_bytes(const __le16 *from, int maxbytes,
>> > /*
>> > * cifs_mapchar - convert a little-endian char to proper char in codepage
>>
>> I think this wording is now incorrect since src_char is not le anymore.
>>
>
> Good catch. I'll fix it.
>
>
> [...]
>
>> > @@ -158,19 +162,23 @@ cifs_from_ucs2(char *to, const __le16 *from, int tolen, int fromlen,
>> > */
>> > safelen = tolen - (NLS_MAX_CHARSET_SIZE + nullsize);
>> >
>> > - for (i = 0; i < fromwords && from[i]; i++) {
>> > + for (i = 0; i < fromwords; i++) {
>> > + ftmp = get_unaligned_le16(&from[i]);
>> > + if (ftmp == 0)
>> > + break;
>> > +
>>
>> Can the contents of from[i] be 0 so ftmp is 0 but we did
>> not want to break out until fromwords?
>>
>
> I don't quite understand what you're asking. The code should be
> equivalent to what was there before except that we use
> get_unaligned_le16 to get the value and store it in a variable on the
> stack. Note that the '&& from[i]' condition was removed from the "for"
> statement.
>
>
> --
> Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>
You are right, in that case I do not understand what it means to break
out of the loop if the contents of from[i] is 0.
So other than the comment part, rest looks correct.
Reviewed-by: Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 4/5] cifs: clean up unaligned accesses in cifs_unicode.c
[not found] ` <AANLkTikxyAb2GSBxNeWYE6sAp7bSdakjWgp1_si1G7HU-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2011-01-19 16:30 ` Jeff Layton
0 siblings, 0 replies; 17+ messages in thread
From: Jeff Layton @ 2011-01-19 16:30 UTC (permalink / raw)
To: Shirish Pargaonkar
Cc: smfrench-Re5JQEeQqe8AvxtiuMwx3w, linux-cifs-u79uwXL29TY76Z2rM5mHXA
On Wed, 19 Jan 2011 10:08:59 -0600
Shirish Pargaonkar <shirishpargaonkar-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> @@ -158,19 +162,23 @@ cifs_from_ucs2(char *to, const __le16 *from, int tolen, int fromlen,
> >> */
> >> safelen = tolen - (NLS_MAX_CHARSET_SIZE + nullsize);
> >>
> >> - for (i = 0; i < fromwords && from[i]; i++) {
> >> + for (i = 0; i < fromwords; i++) {
> >> + ftmp = get_unaligned_le16(&from[i]);
> >> + if (ftmp == 0)
> >> + break;
> >> +
> >
> > Can the contents of from[i] be 0 so ftmp is 0 but we did
> > not want to break out until fromwords?
> >
>
> Basically, why_does/what_it_means get_unaligned_le16()
> returning 0 warrants breaking out of the loop!
>
That means that we hit the NULL terminator in the "from" string (two
adjacent zero bytes). So, we absolutely want to break out there.
--
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2011-01-19 16:30 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-18 20:33 [PATCH 0/5] cifs: clean up some unaligned memory accesses Jeff Layton
[not found] ` <1295382799-7902-1-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-01-18 20:33 ` [PATCH 1/5] cifs: use get/put_unaligned functions to access ByteCount Jeff Layton
[not found] ` <1295382799-7902-2-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-01-19 15:33 ` Shirish Pargaonkar
[not found] ` <AANLkTi=XsEtjps=hq897Rt+akT9F-Toe1bpRuT=aMqPO-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-01-19 15:40 ` Jeff Layton
2011-01-18 20:33 ` [PATCH 2/5] cifs: clean up unaligned accesses in validate_t2 Jeff Layton
[not found] ` <1295382799-7902-3-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-01-19 15:39 ` Shirish Pargaonkar
2011-01-18 20:33 ` [PATCH 3/5] cifs: fix unaligned access in check2ndT2 and coalesce_t2 Jeff Layton
[not found] ` <1295382799-7902-4-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-01-19 15:42 ` Shirish Pargaonkar
2011-01-18 20:33 ` [PATCH 4/5] cifs: clean up unaligned accesses in cifs_unicode.c Jeff Layton
[not found] ` <1295382799-7902-5-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-01-19 15:58 ` Shirish Pargaonkar
[not found] ` <AANLkTikZYP39UEVGt1Tn1PdYE9RnoBEhOjV+qWfd3oRm-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-01-19 16:08 ` Shirish Pargaonkar
[not found] ` <AANLkTikxyAb2GSBxNeWYE6sAp7bSdakjWgp1_si1G7HU-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2011-01-19 16:30 ` Jeff Layton
2011-01-19 16:11 ` Jeff Layton
[not found] ` <20110119111141.0567a9e9-9yPaYZwiELC+kQycOl6kW4xkIHaj4LzF@public.gmane.org>
2011-01-19 16:22 ` Shirish Pargaonkar
2011-01-18 20:33 ` [PATCH 5/5] cifs: fix unaligned accesses in cifsConvertToUCS Jeff Layton
[not found] ` <1295382799-7902-6-git-send-email-jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2011-01-19 16:02 ` Shirish Pargaonkar
2011-01-19 14:20 ` [PATCH 0/5] cifs: clean up some unaligned memory accesses Pavel Shilovsky
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.