Re: Revised patch to convert MessageID in smb2_hdr to LE

[Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

Hello Steve,

I am working on the patch and in the process of testing it. It is
similar to the patch you have with a few small modifications. I can send
it over as soon as I've tested it on a ppc64 machine.

Sachin Prabhu

On Mon, 2014-12-08 at 16:46 -0600, Steve French wrote:
> Resending (attaching patch)
> 
> Redhat had encountered a problem with the SMB2/SMB3 message id (mid)
> on PPC - the message id was being considered opaque and endian neutral.
> Change the SMB2/SMB3 message id to be le64 on the wire (as we already
> do with cifs).
> 
> Signed-off-by: Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> CC: Sachin Prabhu <sprabhu-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  fs/cifs/smb2misc.c      | 12 +++++++-----
>  fs/cifs/smb2ops.c       |  2 +-
>  fs/cifs/smb2pdu.h       |  2 +-
>  fs/cifs/smb2transport.c |  4 ++--
>  4 files changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c
> index 1a08a34..6e01933 100644
> --- a/fs/cifs/smb2misc.c
> +++ b/fs/cifs/smb2misc.c
> @@ -32,12 +32,14 @@
>  static int
>  check_smb2_hdr(struct smb2_hdr *hdr, __u64 mid)
>  {
> +    __u64 wire_mid = le64_to_cpu(hdr->MessageId);
> +
>      /*
>       * Make sure that this really is an SMB, that it is a response,
>       * and that the message ids match.
>       */
>      if ((*(__le32 *)hdr->ProtocolId == SMB2_PROTO_NUMBER) &&
> -        (mid == hdr->MessageId)) {
> +        (mid == wire_mid)) {
>          if (hdr->Flags & SMB2_FLAGS_SERVER_TO_REDIR)
>              return 0;
>          else {
> @@ -51,11 +53,11 @@ check_smb2_hdr(struct smb2_hdr *hdr, __u64 mid)
>          if (*(__le32 *)hdr->ProtocolId != SMB2_PROTO_NUMBER)
>              cifs_dbg(VFS, "Bad protocol string signature header %x\n",
>                   *(unsigned int *) hdr->ProtocolId);
> -        if (mid != hdr->MessageId)
> +        if (mid != wire_mid)
>              cifs_dbg(VFS, "Mids do not match: %llu and %llu\n",
> -                 mid, hdr->MessageId);
> +                 mid, wire_mid);
>      }
> -    cifs_dbg(VFS, "Bad SMB detected. The Mid=%llu\n", hdr->MessageId);
> +    cifs_dbg(VFS, "Bad SMB detected. The Mid=%llu\n", wire_mid);
>      return 1;
>  }
> 
> @@ -95,7 +97,7 @@ smb2_check_message(char *buf, unsigned int length)
>  {
>      struct smb2_hdr *hdr = (struct smb2_hdr *)buf;
>      struct smb2_pdu *pdu = (struct smb2_pdu *)hdr;
> -    __u64 mid = hdr->MessageId;
> +    __u64 mid = le64_to_cpu(hdr->MessageId);
>      __u32 len = get_rfc1002_length(buf);
>      __u32 clc_len;  /* calculated length */
>      int command;
> diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
> index 568f323..e5b33d0 100644
> --- a/fs/cifs/smb2ops.c
> +++ b/fs/cifs/smb2ops.c
> @@ -179,7 +179,7 @@ smb2_find_mid(struct TCP_Server_Info *server, char *buf)
> 
>      spin_lock(&GlobalMid_Lock);
>      list_for_each_entry(mid, &server->pending_mid_q, qhead) {
> -        if ((mid->mid == hdr->MessageId) &&
> +        if ((mid->mid == le64_to_cpu(hdr->MessageId)) &&
>              (mid->mid_state == MID_REQUEST_SUBMITTED) &&
>              (mid->command == hdr->Command)) {
>              spin_unlock(&GlobalMid_Lock);
> diff --git a/fs/cifs/smb2pdu.h b/fs/cifs/smb2pdu.h
> index d84f46c..2d6d388 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;    /* opaque - so can stay little endian */
>      __le32 ProcessId;
>      __u32  TreeId;        /* opaque - so do not make little endian */
>      __u64  SessionId;    /* opaque - so do not make little endian */
> diff --git a/fs/cifs/smb2transport.c b/fs/cifs/smb2transport.c
> index 5111e72..6bdee1b5 100644
> --- a/fs/cifs/smb2transport.c
> +++ b/fs/cifs/smb2transport.c
> @@ -468,7 +468,7 @@ smb2_seq_num_into_buf(struct TCP_Server_Info
> *server, struct smb2_hdr *hdr)
>  {
>      unsigned int i, num = le16_to_cpu(hdr->CreditCharge);
> 
> -    hdr->MessageId = get_next_mid64(server);
> +    hdr->MessageId = cpu_to_le64(get_next_mid64(server));
>      /* skip message numbers according to CreditCharge field */
>      for (i = 1; i < num; i++)
>          get_next_mid(server);
> @@ -490,7 +490,7 @@ smb2_mid_entry_alloc(const struct smb2_hdr *smb_buffer,
>          return temp;
>      else {
>          memset(temp, 0, sizeof(struct mid_q_entry));
> -        temp->mid = smb_buffer->MessageId;    /* always LE */
> +        temp->mid = le64_to_cpu(smb_buffer->MessageId);
>          temp->pid = current->pid;
>          temp->command = smb_buffer->Command;    /* Always LE */
>          temp->when_alloc = jiffies;
> -- 
> 
> 
> On Mon, Dec 8, 2014 at 4:30 PM, Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > Sachin,
> > Since your patch had various sparse warnings, I did a different
> > version, basically converting the SMB2/SMB3 MessageID field on the
> > wire  to le64 from u64 (we used to assume that the MessageId/MID was
> > opaque) and then modifying places that used it to match that
> > endianness.
> >
> > Would you try that and make sure it works in your big endian
> > reproduction scenario (or propose an alternate patch)?
> >
> >
> >
> > --
> > Thanks,
> >
> > Steve
> 
> 
>