diff mbox

[v2] Convert MessageID in smb2_hdr to LE

Message ID 1416844438-22012-1-git-send-email-sprabhu@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sachin Prabhu Nov. 24, 2014, 3:53 p.m. UTC
We have encountered failures when When testing smb2 mounts on ppc64
machines when using both Samba as well as Windows 2012.

On poking around, the problem was determined to be caused by the
high endian MessageID passed in the header for smb2. On checking the
corresponding MID for smb1 is converted to LE before being sent on the
wire.

We have tested this using the RHEL 7 kernel where the patch fixes the
issue.

Signed-off-by: Sachin Prabhu <sprabhu@redhat.com>
---
 fs/cifs/cifsglob.h | 6 +++---
 fs/cifs/smb2pdu.h  | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

Comments

Shirish Pargaonkar Nov. 25, 2014, 2:36 a.m. UTC | #1
Acked-by: Shirish Pargaonkar <shirishpargaonkar@gmail.com>

On Mon, Nov 24, 2014 at 9:53 AM, Sachin Prabhu <sprabhu@redhat.com> wrote:
> We have encountered failures when When testing smb2 mounts on ppc64
> machines when using both Samba as well as Windows 2012.
>
> On poking around, the problem was determined to be caused by the
> high endian MessageID passed in the header for smb2. On checking the
> corresponding MID for smb1 is converted to LE before being sent on the
> wire.
>
> We have tested this using the RHEL 7 kernel where the patch fixes the
> issue.
>
> Signed-off-by: Sachin Prabhu <sprabhu@redhat.com>
> ---
>  fs/cifs/cifsglob.h | 6 +++---
>  fs/cifs/smb2pdu.h  | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index 02a33e5..41ec69d 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -661,16 +661,16 @@ set_credits(struct TCP_Server_Info *server, const int val)
>         server->ops->set_credits(server, val);
>  }
>
> -static inline __u64
> +static inline __le64
>  get_next_mid64(struct TCP_Server_Info *server)
>  {
> -       return server->ops->get_next_mid(server);
> +       return cpu_to_le64(server->ops->get_next_mid(server));
>  }
>
>  static inline __le16
>  get_next_mid(struct TCP_Server_Info *server)
>  {
> -       __u16 mid = get_next_mid64(server);
> +       __u16 mid = server->ops->get_next_mid(server);
>         /*
>          * The value in the SMB header should be little endian for easy
>          * on-the-wire decoding.
> diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
> index e3188ab..2d4914e 100644
> --- a/fs/cifs/smb2pdu.h
> +++ b/fs/cifs/smb2pdu.h
> @@ -110,7 +110,7 @@ struct smb2_hdr {
>         __le16 CreditRequest;  /* CreditResponse */
>         __le32 Flags;
>         __le32 NextCommand;
> -       __u64  MessageId;       /* opaque - so can stay little endian */
> +       __le64 MessageId;
>         __le32 ProcessId;
>         __u32  TreeId;          /* opaque - so do not make little endian */
>         __u64  SessionId;       /* opaque - so do not make little endian */
> --
> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steve French Dec. 8, 2014, 4:57 a.m. UTC | #2
Merged into cifs-2.6.git for-next

On Mon, Nov 24, 2014 at 8:36 PM, Shirish Pargaonkar
<shirishpargaonkar@gmail.com> wrote:
> Acked-by: Shirish Pargaonkar <shirishpargaonkar@gmail.com>
>
> On Mon, Nov 24, 2014 at 9:53 AM, Sachin Prabhu <sprabhu@redhat.com> wrote:
>> We have encountered failures when When testing smb2 mounts on ppc64
>> machines when using both Samba as well as Windows 2012.
>>
>> On poking around, the problem was determined to be caused by the
>> high endian MessageID passed in the header for smb2. On checking the
>> corresponding MID for smb1 is converted to LE before being sent on the
>> wire.
>>
>> We have tested this using the RHEL 7 kernel where the patch fixes the
>> issue.
>>
>> Signed-off-by: Sachin Prabhu <sprabhu@redhat.com>
>> ---
>>  fs/cifs/cifsglob.h | 6 +++---
>>  fs/cifs/smb2pdu.h  | 2 +-
>>  2 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> index 02a33e5..41ec69d 100644
>> --- a/fs/cifs/cifsglob.h
>> +++ b/fs/cifs/cifsglob.h
>> @@ -661,16 +661,16 @@ set_credits(struct TCP_Server_Info *server, const int val)
>>         server->ops->set_credits(server, val);
>>  }
>>
>> -static inline __u64
>> +static inline __le64
>>  get_next_mid64(struct TCP_Server_Info *server)
>>  {
>> -       return server->ops->get_next_mid(server);
>> +       return cpu_to_le64(server->ops->get_next_mid(server));
>>  }
>>
>>  static inline __le16
>>  get_next_mid(struct TCP_Server_Info *server)
>>  {
>> -       __u16 mid = get_next_mid64(server);
>> +       __u16 mid = server->ops->get_next_mid(server);
>>         /*
>>          * The value in the SMB header should be little endian for easy
>>          * on-the-wire decoding.
>> diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
>> index e3188ab..2d4914e 100644
>> --- a/fs/cifs/smb2pdu.h
>> +++ b/fs/cifs/smb2pdu.h
>> @@ -110,7 +110,7 @@ struct smb2_hdr {
>>         __le16 CreditRequest;  /* CreditResponse */
>>         __le32 Flags;
>>         __le32 NextCommand;
>> -       __u64  MessageId;       /* opaque - so can stay little endian */
>> +       __le64 MessageId;
>>         __le32 ProcessId;
>>         __u32  TreeId;          /* opaque - so do not make little endian */
>>         __u64  SessionId;       /* opaque - so do not make little endian */
>> --
>> 1.9.3
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Steve French Dec. 8, 2014, 5:45 a.m. UTC | #3
I backed this out temporarily - it generated various sparse endian warnings.

On Sun, Dec 7, 2014 at 10:57 PM, Steve French <smfrench@gmail.com> wrote:
> Merged into cifs-2.6.git for-next
>
> On Mon, Nov 24, 2014 at 8:36 PM, Shirish Pargaonkar
> <shirishpargaonkar@gmail.com> wrote:
>> Acked-by: Shirish Pargaonkar <shirishpargaonkar@gmail.com>
>>
>> On Mon, Nov 24, 2014 at 9:53 AM, Sachin Prabhu <sprabhu@redhat.com> wrote:
>>> We have encountered failures when When testing smb2 mounts on ppc64
>>> machines when using both Samba as well as Windows 2012.
>>>
>>> On poking around, the problem was determined to be caused by the
>>> high endian MessageID passed in the header for smb2. On checking the
>>> corresponding MID for smb1 is converted to LE before being sent on the
>>> wire.
>>>
>>> We have tested this using the RHEL 7 kernel where the patch fixes the
>>> issue.
>>>
>>> Signed-off-by: Sachin Prabhu <sprabhu@redhat.com>
>>> ---
>>>  fs/cifs/cifsglob.h | 6 +++---
>>>  fs/cifs/smb2pdu.h  | 2 +-
>>>  2 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>>> index 02a33e5..41ec69d 100644
>>> --- a/fs/cifs/cifsglob.h
>>> +++ b/fs/cifs/cifsglob.h
>>> @@ -661,16 +661,16 @@ set_credits(struct TCP_Server_Info *server, const int val)
>>>         server->ops->set_credits(server, val);
>>>  }
>>>
>>> -static inline __u64
>>> +static inline __le64
>>>  get_next_mid64(struct TCP_Server_Info *server)
>>>  {
>>> -       return server->ops->get_next_mid(server);
>>> +       return cpu_to_le64(server->ops->get_next_mid(server));
>>>  }
>>>
>>>  static inline __le16
>>>  get_next_mid(struct TCP_Server_Info *server)
>>>  {
>>> -       __u16 mid = get_next_mid64(server);
>>> +       __u16 mid = server->ops->get_next_mid(server);
>>>         /*
>>>          * The value in the SMB header should be little endian for easy
>>>          * on-the-wire decoding.
>>> diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
>>> index e3188ab..2d4914e 100644
>>> --- a/fs/cifs/smb2pdu.h
>>> +++ b/fs/cifs/smb2pdu.h
>>> @@ -110,7 +110,7 @@ struct smb2_hdr {
>>>         __le16 CreditRequest;  /* CreditResponse */
>>>         __le32 Flags;
>>>         __le32 NextCommand;
>>> -       __u64  MessageId;       /* opaque - so can stay little endian */
>>> +       __le64 MessageId;
>>>         __le32 ProcessId;
>>>         __u32  TreeId;          /* opaque - so do not make little endian */
>>>         __u64  SessionId;       /* opaque - so do not make little endian */
>>> --
>>> 1.9.3
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Thanks,
>
> Steve
diff mbox

Patch

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 02a33e5..41ec69d 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -661,16 +661,16 @@  set_credits(struct TCP_Server_Info *server, const int val)
 	server->ops->set_credits(server, val);
 }
 
-static inline __u64
+static inline __le64
 get_next_mid64(struct TCP_Server_Info *server)
 {
-	return server->ops->get_next_mid(server);
+	return cpu_to_le64(server->ops->get_next_mid(server));
 }
 
 static inline __le16
 get_next_mid(struct TCP_Server_Info *server)
 {
-	__u16 mid = get_next_mid64(server);
+	__u16 mid = server->ops->get_next_mid(server);
 	/*
 	 * The value in the SMB header should be little endian for easy
 	 * on-the-wire decoding.
diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
index e3188ab..2d4914e 100644
--- a/fs/cifs/smb2pdu.h
+++ b/fs/cifs/smb2pdu.h
@@ -110,7 +110,7 @@  struct smb2_hdr {
 	__le16 CreditRequest;  /* CreditResponse */
 	__le32 Flags;
 	__le32 NextCommand;
-	__u64  MessageId;	/* opaque - so can stay little endian */
+	__le64 MessageId;
 	__le32 ProcessId;
 	__u32  TreeId;		/* opaque - so do not make little endian */
 	__u64  SessionId;	/* opaque - so do not make little endian */