| Message ID | 20220321160826.30814-1-pc@cjr.nz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] cifs: fix bad fids sent over wire | expand |
I agree this is the most practical way to address the issue, and any potential static checker warnings. Have you tested on both endian clients I assume? Might be tricky to catch all the ops/cases, especially that cancel. Reviewed-By: Tom Talpey <tom@talpey.com> On 3/21/2022 12:08 PM, Paulo Alcantara wrote: > The client used to partially convert the fids to le64, while storing > or sending them by using host endianness. This broke the client on > big-endian machines. Instead of converting them to le64, store them > as opaque integers and then avoid byteswapping when sending them over > wire. > > Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz> > --- > fs/cifs/smb2misc.c | 4 +-- > fs/cifs/smb2ops.c | 8 ++--- > fs/cifs/smb2pdu.c | 63 +++++++++++++++++---------------------- > fs/smbfs_common/smb2pdu.h | 24 +++++++-------- > 4 files changed, 46 insertions(+), 53 deletions(-) > > diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c > index b25623e3fe3d..3b7c636be377 100644 > --- a/fs/cifs/smb2misc.c > +++ b/fs/cifs/smb2misc.c > @@ -832,8 +832,8 @@ smb2_handle_cancelled_mid(struct mid_q_entry *mid, struct TCP_Server_Info *serve > rc = __smb2_handle_cancelled_cmd(tcon, > le16_to_cpu(hdr->Command), > le64_to_cpu(hdr->MessageId), > - le64_to_cpu(rsp->PersistentFileId), > - le64_to_cpu(rsp->VolatileFileId)); > + rsp->PersistentFileId, > + rsp->VolatileFileId); > if (rc) > cifs_put_tcon(tcon); > > diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c > index 0ecd6e1832a1..c122530e5043 100644 > --- a/fs/cifs/smb2ops.c > +++ b/fs/cifs/smb2ops.c > @@ -900,8 +900,8 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, > atomic_inc(&tcon->num_remote_opens); > > o_rsp = (struct smb2_create_rsp *)rsp_iov[0].iov_base; > - oparms.fid->persistent_fid = le64_to_cpu(o_rsp->PersistentFileId); > - oparms.fid->volatile_fid = le64_to_cpu(o_rsp->VolatileFileId); > + oparms.fid->persistent_fid = o_rsp->PersistentFileId; > + oparms.fid->volatile_fid = o_rsp->VolatileFileId; > #ifdef CONFIG_CIFS_DEBUG2 > oparms.fid->mid = le64_to_cpu(o_rsp->hdr.MessageId); > #endif /* CIFS_DEBUG2 */ > @@ -2410,8 +2410,8 @@ smb2_query_dir_first(const unsigned int xid, struct cifs_tcon *tcon, > cifs_dbg(FYI, "query_dir_first: open failed rc=%d\n", rc); > goto qdf_free; > } > - fid->persistent_fid = le64_to_cpu(op_rsp->PersistentFileId); > - fid->volatile_fid = le64_to_cpu(op_rsp->VolatileFileId); > + fid->persistent_fid = op_rsp->PersistentFileId; > + fid->volatile_fid = op_rsp->VolatileFileId; > > /* Anything else than ENODATA means a genuine error */ > if (rc && rc != -ENODATA) { > diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c > index 7e7909b1ae11..7e15b0092243 100644 > --- a/fs/cifs/smb2pdu.c > +++ b/fs/cifs/smb2pdu.c > @@ -2734,13 +2734,10 @@ int smb311_posix_mkdir(const unsigned int xid, struct inode *inode, > goto err_free_req; > } > > - trace_smb3_posix_mkdir_done(xid, le64_to_cpu(rsp->PersistentFileId), > - tcon->tid, > - ses->Suid, CREATE_NOT_FILE, > - FILE_WRITE_ATTRIBUTES); > + trace_smb3_posix_mkdir_done(xid, rsp->PersistentFileId, tcon->tid, ses->Suid, > + CREATE_NOT_FILE, FILE_WRITE_ATTRIBUTES); > > - SMB2_close(xid, tcon, le64_to_cpu(rsp->PersistentFileId), > - le64_to_cpu(rsp->VolatileFileId)); > + SMB2_close(xid, tcon, rsp->PersistentFileId, rsp->VolatileFileId); > > /* Eventually save off posix specific response info and timestaps */ > > @@ -3009,14 +3006,12 @@ SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path, > } else if (rsp == NULL) /* unlikely to happen, but safer to check */ > goto creat_exit; > else > - trace_smb3_open_done(xid, le64_to_cpu(rsp->PersistentFileId), > - tcon->tid, > - ses->Suid, oparms->create_options, > - oparms->desired_access); > + trace_smb3_open_done(xid, rsp->PersistentFileId, tcon->tid, ses->Suid, > + oparms->create_options, oparms->desired_access); > > atomic_inc(&tcon->num_remote_opens); > - oparms->fid->persistent_fid = le64_to_cpu(rsp->PersistentFileId); > - oparms->fid->volatile_fid = le64_to_cpu(rsp->VolatileFileId); > + oparms->fid->persistent_fid = rsp->PersistentFileId; > + oparms->fid->volatile_fid = rsp->VolatileFileId; > oparms->fid->access = oparms->desired_access; > #ifdef CONFIG_CIFS_DEBUG2 > oparms->fid->mid = le64_to_cpu(rsp->hdr.MessageId); > @@ -3313,8 +3308,8 @@ SMB2_close_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server, > if (rc) > return rc; > > - req->PersistentFileId = cpu_to_le64(persistent_fid); > - req->VolatileFileId = cpu_to_le64(volatile_fid); > + req->PersistentFileId = persistent_fid; > + req->VolatileFileId = volatile_fid; > if (query_attrs) > req->Flags = SMB2_CLOSE_FLAG_POSTQUERY_ATTRIB; > else > @@ -3677,8 +3672,8 @@ SMB2_notify_init(const unsigned int xid, struct smb_rqst *rqst, > if (rc) > return rc; > > - req->PersistentFileId = cpu_to_le64(persistent_fid); > - req->VolatileFileId = cpu_to_le64(volatile_fid); > + req->PersistentFileId = persistent_fid; > + req->VolatileFileId = volatile_fid; > /* See note 354 of MS-SMB2, 64K max */ > req->OutputBufferLength = > cpu_to_le32(SMB2_MAX_BUFFER_SIZE - MAX_SMB2_HDR_SIZE); > @@ -3951,8 +3946,8 @@ SMB2_flush_init(const unsigned int xid, struct smb_rqst *rqst, > if (rc) > return rc; > > - req->PersistentFileId = cpu_to_le64(persistent_fid); > - req->VolatileFileId = cpu_to_le64(volatile_fid); > + req->PersistentFileId = persistent_fid; > + req->VolatileFileId = volatile_fid; > > iov[0].iov_base = (char *)req; > iov[0].iov_len = total_len; > @@ -4033,8 +4028,8 @@ smb2_new_read_req(void **buf, unsigned int *total_len, > shdr = &req->hdr; > shdr->Id.SyncId.ProcessId = cpu_to_le32(io_parms->pid); > > - req->PersistentFileId = cpu_to_le64(io_parms->persistent_fid); > - req->VolatileFileId = cpu_to_le64(io_parms->volatile_fid); > + req->PersistentFileId = io_parms->persistent_fid; > + req->VolatileFileId = io_parms->volatile_fid; > req->ReadChannelInfoOffset = 0; /* reserved */ > req->ReadChannelInfoLength = 0; /* reserved */ > req->Channel = 0; /* reserved */ > @@ -4094,8 +4089,8 @@ smb2_new_read_req(void **buf, unsigned int *total_len, > */ > shdr->SessionId = cpu_to_le64(0xFFFFFFFFFFFFFFFF); > shdr->Id.SyncId.TreeId = cpu_to_le32(0xFFFFFFFF); > - req->PersistentFileId = cpu_to_le64(0xFFFFFFFFFFFFFFFF); > - req->VolatileFileId = cpu_to_le64(0xFFFFFFFFFFFFFFFF); > + req->PersistentFileId = (u64)-1; > + req->VolatileFileId = (u64)-1; > } > } > if (remaining_bytes > io_parms->length) > @@ -4307,21 +4302,19 @@ SMB2_read(const unsigned int xid, struct cifs_io_parms *io_parms, > cifs_stats_fail_inc(io_parms->tcon, SMB2_READ_HE); > cifs_dbg(VFS, "Send error in read = %d\n", rc); > trace_smb3_read_err(xid, > - le64_to_cpu(req->PersistentFileId), > + req->PersistentFileId, > io_parms->tcon->tid, ses->Suid, > io_parms->offset, io_parms->length, > rc); > } else > - trace_smb3_read_done(xid, > - le64_to_cpu(req->PersistentFileId), > - io_parms->tcon->tid, ses->Suid, > - io_parms->offset, 0); > + trace_smb3_read_done(xid, req->PersistentFileId, io_parms->tcon->tid, > + ses->Suid, io_parms->offset, 0); > free_rsp_buf(resp_buftype, rsp_iov.iov_base); > cifs_small_buf_release(req); > return rc == -ENODATA ? 0 : rc; > } else > trace_smb3_read_done(xid, > - le64_to_cpu(req->PersistentFileId), > + req->PersistentFileId, > io_parms->tcon->tid, ses->Suid, > io_parms->offset, io_parms->length); > > @@ -4463,8 +4456,8 @@ smb2_async_writev(struct cifs_writedata *wdata, > shdr = (struct smb2_hdr *)req; > shdr->Id.SyncId.ProcessId = cpu_to_le32(wdata->cfile->pid); > > - req->PersistentFileId = cpu_to_le64(wdata->cfile->fid.persistent_fid); > - req->VolatileFileId = cpu_to_le64(wdata->cfile->fid.volatile_fid); > + req->PersistentFileId = wdata->cfile->fid.persistent_fid; > + req->VolatileFileId = wdata->cfile->fid.volatile_fid; > req->WriteChannelInfoOffset = 0; > req->WriteChannelInfoLength = 0; > req->Channel = 0; > @@ -4562,7 +4555,7 @@ smb2_async_writev(struct cifs_writedata *wdata, > > if (rc) { > trace_smb3_write_err(0 /* no xid */, > - le64_to_cpu(req->PersistentFileId), > + req->PersistentFileId, > tcon->tid, tcon->ses->Suid, wdata->offset, > wdata->bytes, rc); > kref_put(&wdata->refcount, release); > @@ -4615,8 +4608,8 @@ SMB2_write(const unsigned int xid, struct cifs_io_parms *io_parms, > > req->hdr.Id.SyncId.ProcessId = cpu_to_le32(io_parms->pid); > > - req->PersistentFileId = cpu_to_le64(io_parms->persistent_fid); > - req->VolatileFileId = cpu_to_le64(io_parms->volatile_fid); > + req->PersistentFileId = io_parms->persistent_fid; > + req->VolatileFileId = io_parms->volatile_fid; > req->WriteChannelInfoOffset = 0; > req->WriteChannelInfoLength = 0; > req->Channel = 0; > @@ -4645,7 +4638,7 @@ SMB2_write(const unsigned int xid, struct cifs_io_parms *io_parms, > > if (rc) { > trace_smb3_write_err(xid, > - le64_to_cpu(req->PersistentFileId), > + req->PersistentFileId, > io_parms->tcon->tid, > io_parms->tcon->ses->Suid, > io_parms->offset, io_parms->length, rc); > @@ -4654,7 +4647,7 @@ SMB2_write(const unsigned int xid, struct cifs_io_parms *io_parms, > } else { > *nbytes = le32_to_cpu(rsp->DataLength); > trace_smb3_write_done(xid, > - le64_to_cpu(req->PersistentFileId), > + req->PersistentFileId, > io_parms->tcon->tid, > io_parms->tcon->ses->Suid, > io_parms->offset, *nbytes); > diff --git a/fs/smbfs_common/smb2pdu.h b/fs/smbfs_common/smb2pdu.h > index 38b8fc514860..6653b4be4556 100644 > --- a/fs/smbfs_common/smb2pdu.h > +++ b/fs/smbfs_common/smb2pdu.h > @@ -608,8 +608,8 @@ struct smb2_close_req { > __le16 StructureSize; /* Must be 24 */ > __le16 Flags; > __le32 Reserved; > - __le64 PersistentFileId; /* opaque endianness */ > - __le64 VolatileFileId; /* opaque endianness */ > + __u64 PersistentFileId; /* opaque endianness */ > + __u64 VolatileFileId; /* opaque endianness */ > } __packed; > > /* > @@ -653,8 +653,8 @@ struct smb2_read_req { > __u8 Flags; /* MBZ unless SMB3.02 or later */ > __le32 Length; > __le64 Offset; > - __le64 PersistentFileId; > - __le64 VolatileFileId; > + __u64 PersistentFileId; > + __u64 VolatileFileId; > __le32 MinimumCount; > __le32 Channel; /* MBZ except for SMB3 or later */ > __le32 RemainingBytes; > @@ -692,8 +692,8 @@ struct smb2_write_req { > __le16 DataOffset; /* offset from start of SMB2 header to write data */ > __le32 Length; > __le64 Offset; > - __le64 PersistentFileId; /* opaque endianness */ > - __le64 VolatileFileId; /* opaque endianness */ > + __u64 PersistentFileId; /* opaque endianness */ > + __u64 VolatileFileId; /* opaque endianness */ > __le32 Channel; /* MBZ unless SMB3.02 or later */ > __le32 RemainingBytes; > __le16 WriteChannelInfoOffset; > @@ -722,8 +722,8 @@ struct smb2_flush_req { > __le16 StructureSize; /* Must be 24 */ > __le16 Reserved1; > __le32 Reserved2; > - __le64 PersistentFileId; > - __le64 VolatileFileId; > + __u64 PersistentFileId; > + __u64 VolatileFileId; > } __packed; > > struct smb2_flush_rsp { > @@ -769,8 +769,8 @@ struct smb2_change_notify_req { > __le16 StructureSize; > __le16 Flags; > __le32 OutputBufferLength; > - __le64 PersistentFileId; /* opaque endianness */ > - __le64 VolatileFileId; /* opaque endianness */ > + __u64 PersistentFileId; /* opaque endianness */ > + __u64 VolatileFileId; /* opaque endianness */ > __le32 CompletionFilter; > __u32 Reserved; > } __packed; > @@ -978,8 +978,8 @@ struct smb2_create_rsp { > __le64 EndofFile; > __le32 FileAttributes; > __le32 Reserved2; > - __le64 PersistentFileId; > - __le64 VolatileFileId; > + __u64 PersistentFileId; > + __u64 VolatileFileId; > __le32 CreateContextsOffset; > __le32 CreateContextsLength; > __u8 Buffer[1];
Tom Talpey <tom@talpey.com> writes: > I agree this is the most practical way to address the issue, and > any potential static checker warnings. > > Have you tested on both endian clients I assume? Might be tricky > to catch all the ops/cases, especially that cancel. I did some quick tests on x86_64 and s390x under QEMU against Windows 2022 server and ksmbd and all seemed to work. Hopefully Steve & Namjae can also help with more tests. > Reviewed-By: Tom Talpey <tom@talpey.com> Thanks!
2022-03-22 6:48 GMT+09:00, Paulo Alcantara <pc@cjr.nz>: > Tom Talpey <tom@talpey.com> writes: > >> I agree this is the most practical way to address the issue, and >> any potential static checker warnings. >> >> Have you tested on both endian clients I assume? Might be tricky >> to catch all the ops/cases, especially that cancel. > > I did some quick tests on x86_64 and s390x under QEMU against Windows > 2022 server and ksmbd and all seemed to work. Hopefully Steve & Namjae > can also help with more tests. > >> Reviewed-By: Tom Talpey <tom@talpey.com> ^ Minor nit, B must be lowercase. This will cause a warning from checkpatch.pl. And looks good to me too, Reviewed-by: Namjae Jeon <linkinjeon@kernel.org> Thanks:) > > Thanks! >
diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c index b25623e3fe3d..3b7c636be377 100644 --- a/fs/cifs/smb2misc.c +++ b/fs/cifs/smb2misc.c @@ -832,8 +832,8 @@ smb2_handle_cancelled_mid(struct mid_q_entry *mid, struct TCP_Server_Info *serve rc = __smb2_handle_cancelled_cmd(tcon, le16_to_cpu(hdr->Command), le64_to_cpu(hdr->MessageId), - le64_to_cpu(rsp->PersistentFileId), - le64_to_cpu(rsp->VolatileFileId)); + rsp->PersistentFileId, + rsp->VolatileFileId); if (rc) cifs_put_tcon(tcon); diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c index 0ecd6e1832a1..c122530e5043 100644 --- a/fs/cifs/smb2ops.c +++ b/fs/cifs/smb2ops.c @@ -900,8 +900,8 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon, atomic_inc(&tcon->num_remote_opens); o_rsp = (struct smb2_create_rsp *)rsp_iov[0].iov_base; - oparms.fid->persistent_fid = le64_to_cpu(o_rsp->PersistentFileId); - oparms.fid->volatile_fid = le64_to_cpu(o_rsp->VolatileFileId); + oparms.fid->persistent_fid = o_rsp->PersistentFileId; + oparms.fid->volatile_fid = o_rsp->VolatileFileId; #ifdef CONFIG_CIFS_DEBUG2 oparms.fid->mid = le64_to_cpu(o_rsp->hdr.MessageId); #endif /* CIFS_DEBUG2 */ @@ -2410,8 +2410,8 @@ smb2_query_dir_first(const unsigned int xid, struct cifs_tcon *tcon, cifs_dbg(FYI, "query_dir_first: open failed rc=%d\n", rc); goto qdf_free; } - fid->persistent_fid = le64_to_cpu(op_rsp->PersistentFileId); - fid->volatile_fid = le64_to_cpu(op_rsp->VolatileFileId); + fid->persistent_fid = op_rsp->PersistentFileId; + fid->volatile_fid = op_rsp->VolatileFileId; /* Anything else than ENODATA means a genuine error */ if (rc && rc != -ENODATA) { diff --git a/fs/cifs/smb2pdu.c b/fs/cifs/smb2pdu.c index 7e7909b1ae11..7e15b0092243 100644 --- a/fs/cifs/smb2pdu.c +++ b/fs/cifs/smb2pdu.c @@ -2734,13 +2734,10 @@ int smb311_posix_mkdir(const unsigned int xid, struct inode *inode, goto err_free_req; } - trace_smb3_posix_mkdir_done(xid, le64_to_cpu(rsp->PersistentFileId), - tcon->tid, - ses->Suid, CREATE_NOT_FILE, - FILE_WRITE_ATTRIBUTES); + trace_smb3_posix_mkdir_done(xid, rsp->PersistentFileId, tcon->tid, ses->Suid, + CREATE_NOT_FILE, FILE_WRITE_ATTRIBUTES); - SMB2_close(xid, tcon, le64_to_cpu(rsp->PersistentFileId), - le64_to_cpu(rsp->VolatileFileId)); + SMB2_close(xid, tcon, rsp->PersistentFileId, rsp->VolatileFileId); /* Eventually save off posix specific response info and timestaps */ @@ -3009,14 +3006,12 @@ SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path, } else if (rsp == NULL) /* unlikely to happen, but safer to check */ goto creat_exit; else - trace_smb3_open_done(xid, le64_to_cpu(rsp->PersistentFileId), - tcon->tid, - ses->Suid, oparms->create_options, - oparms->desired_access); + trace_smb3_open_done(xid, rsp->PersistentFileId, tcon->tid, ses->Suid, + oparms->create_options, oparms->desired_access); atomic_inc(&tcon->num_remote_opens); - oparms->fid->persistent_fid = le64_to_cpu(rsp->PersistentFileId); - oparms->fid->volatile_fid = le64_to_cpu(rsp->VolatileFileId); + oparms->fid->persistent_fid = rsp->PersistentFileId; + oparms->fid->volatile_fid = rsp->VolatileFileId; oparms->fid->access = oparms->desired_access; #ifdef CONFIG_CIFS_DEBUG2 oparms->fid->mid = le64_to_cpu(rsp->hdr.MessageId); @@ -3313,8 +3308,8 @@ SMB2_close_init(struct cifs_tcon *tcon, struct TCP_Server_Info *server, if (rc) return rc; - req->PersistentFileId = cpu_to_le64(persistent_fid); - req->VolatileFileId = cpu_to_le64(volatile_fid); + req->PersistentFileId = persistent_fid; + req->VolatileFileId = volatile_fid; if (query_attrs) req->Flags = SMB2_CLOSE_FLAG_POSTQUERY_ATTRIB; else @@ -3677,8 +3672,8 @@ SMB2_notify_init(const unsigned int xid, struct smb_rqst *rqst, if (rc) return rc; - req->PersistentFileId = cpu_to_le64(persistent_fid); - req->VolatileFileId = cpu_to_le64(volatile_fid); + req->PersistentFileId = persistent_fid; + req->VolatileFileId = volatile_fid; /* See note 354 of MS-SMB2, 64K max */ req->OutputBufferLength = cpu_to_le32(SMB2_MAX_BUFFER_SIZE - MAX_SMB2_HDR_SIZE); @@ -3951,8 +3946,8 @@ SMB2_flush_init(const unsigned int xid, struct smb_rqst *rqst, if (rc) return rc; - req->PersistentFileId = cpu_to_le64(persistent_fid); - req->VolatileFileId = cpu_to_le64(volatile_fid); + req->PersistentFileId = persistent_fid; + req->VolatileFileId = volatile_fid; iov[0].iov_base = (char *)req; iov[0].iov_len = total_len; @@ -4033,8 +4028,8 @@ smb2_new_read_req(void **buf, unsigned int *total_len, shdr = &req->hdr; shdr->Id.SyncId.ProcessId = cpu_to_le32(io_parms->pid); - req->PersistentFileId = cpu_to_le64(io_parms->persistent_fid); - req->VolatileFileId = cpu_to_le64(io_parms->volatile_fid); + req->PersistentFileId = io_parms->persistent_fid; + req->VolatileFileId = io_parms->volatile_fid; req->ReadChannelInfoOffset = 0; /* reserved */ req->ReadChannelInfoLength = 0; /* reserved */ req->Channel = 0; /* reserved */ @@ -4094,8 +4089,8 @@ smb2_new_read_req(void **buf, unsigned int *total_len, */ shdr->SessionId = cpu_to_le64(0xFFFFFFFFFFFFFFFF); shdr->Id.SyncId.TreeId = cpu_to_le32(0xFFFFFFFF); - req->PersistentFileId = cpu_to_le64(0xFFFFFFFFFFFFFFFF); - req->VolatileFileId = cpu_to_le64(0xFFFFFFFFFFFFFFFF); + req->PersistentFileId = (u64)-1; + req->VolatileFileId = (u64)-1; } } if (remaining_bytes > io_parms->length) @@ -4307,21 +4302,19 @@ SMB2_read(const unsigned int xid, struct cifs_io_parms *io_parms, cifs_stats_fail_inc(io_parms->tcon, SMB2_READ_HE); cifs_dbg(VFS, "Send error in read = %d\n", rc); trace_smb3_read_err(xid, - le64_to_cpu(req->PersistentFileId), + req->PersistentFileId, io_parms->tcon->tid, ses->Suid, io_parms->offset, io_parms->length, rc); } else - trace_smb3_read_done(xid, - le64_to_cpu(req->PersistentFileId), - io_parms->tcon->tid, ses->Suid, - io_parms->offset, 0); + trace_smb3_read_done(xid, req->PersistentFileId, io_parms->tcon->tid, + ses->Suid, io_parms->offset, 0); free_rsp_buf(resp_buftype, rsp_iov.iov_base); cifs_small_buf_release(req); return rc == -ENODATA ? 0 : rc; } else trace_smb3_read_done(xid, - le64_to_cpu(req->PersistentFileId), + req->PersistentFileId, io_parms->tcon->tid, ses->Suid, io_parms->offset, io_parms->length); @@ -4463,8 +4456,8 @@ smb2_async_writev(struct cifs_writedata *wdata, shdr = (struct smb2_hdr *)req; shdr->Id.SyncId.ProcessId = cpu_to_le32(wdata->cfile->pid); - req->PersistentFileId = cpu_to_le64(wdata->cfile->fid.persistent_fid); - req->VolatileFileId = cpu_to_le64(wdata->cfile->fid.volatile_fid); + req->PersistentFileId = wdata->cfile->fid.persistent_fid; + req->VolatileFileId = wdata->cfile->fid.volatile_fid; req->WriteChannelInfoOffset = 0; req->WriteChannelInfoLength = 0; req->Channel = 0; @@ -4562,7 +4555,7 @@ smb2_async_writev(struct cifs_writedata *wdata, if (rc) { trace_smb3_write_err(0 /* no xid */, - le64_to_cpu(req->PersistentFileId), + req->PersistentFileId, tcon->tid, tcon->ses->Suid, wdata->offset, wdata->bytes, rc); kref_put(&wdata->refcount, release); @@ -4615,8 +4608,8 @@ SMB2_write(const unsigned int xid, struct cifs_io_parms *io_parms, req->hdr.Id.SyncId.ProcessId = cpu_to_le32(io_parms->pid); - req->PersistentFileId = cpu_to_le64(io_parms->persistent_fid); - req->VolatileFileId = cpu_to_le64(io_parms->volatile_fid); + req->PersistentFileId = io_parms->persistent_fid; + req->VolatileFileId = io_parms->volatile_fid; req->WriteChannelInfoOffset = 0; req->WriteChannelInfoLength = 0; req->Channel = 0; @@ -4645,7 +4638,7 @@ SMB2_write(const unsigned int xid, struct cifs_io_parms *io_parms, if (rc) { trace_smb3_write_err(xid, - le64_to_cpu(req->PersistentFileId), + req->PersistentFileId, io_parms->tcon->tid, io_parms->tcon->ses->Suid, io_parms->offset, io_parms->length, rc); @@ -4654,7 +4647,7 @@ SMB2_write(const unsigned int xid, struct cifs_io_parms *io_parms, } else { *nbytes = le32_to_cpu(rsp->DataLength); trace_smb3_write_done(xid, - le64_to_cpu(req->PersistentFileId), + req->PersistentFileId, io_parms->tcon->tid, io_parms->tcon->ses->Suid, io_parms->offset, *nbytes); diff --git a/fs/smbfs_common/smb2pdu.h b/fs/smbfs_common/smb2pdu.h index 38b8fc514860..6653b4be4556 100644 --- a/fs/smbfs_common/smb2pdu.h +++ b/fs/smbfs_common/smb2pdu.h @@ -608,8 +608,8 @@ struct smb2_close_req { __le16 StructureSize; /* Must be 24 */ __le16 Flags; __le32 Reserved; - __le64 PersistentFileId; /* opaque endianness */ - __le64 VolatileFileId; /* opaque endianness */ + __u64 PersistentFileId; /* opaque endianness */ + __u64 VolatileFileId; /* opaque endianness */ } __packed; /* @@ -653,8 +653,8 @@ struct smb2_read_req { __u8 Flags; /* MBZ unless SMB3.02 or later */ __le32 Length; __le64 Offset; - __le64 PersistentFileId; - __le64 VolatileFileId; + __u64 PersistentFileId; + __u64 VolatileFileId; __le32 MinimumCount; __le32 Channel; /* MBZ except for SMB3 or later */ __le32 RemainingBytes; @@ -692,8 +692,8 @@ struct smb2_write_req { __le16 DataOffset; /* offset from start of SMB2 header to write data */ __le32 Length; __le64 Offset; - __le64 PersistentFileId; /* opaque endianness */ - __le64 VolatileFileId; /* opaque endianness */ + __u64 PersistentFileId; /* opaque endianness */ + __u64 VolatileFileId; /* opaque endianness */ __le32 Channel; /* MBZ unless SMB3.02 or later */ __le32 RemainingBytes; __le16 WriteChannelInfoOffset; @@ -722,8 +722,8 @@ struct smb2_flush_req { __le16 StructureSize; /* Must be 24 */ __le16 Reserved1; __le32 Reserved2; - __le64 PersistentFileId; - __le64 VolatileFileId; + __u64 PersistentFileId; + __u64 VolatileFileId; } __packed; struct smb2_flush_rsp { @@ -769,8 +769,8 @@ struct smb2_change_notify_req { __le16 StructureSize; __le16 Flags; __le32 OutputBufferLength; - __le64 PersistentFileId; /* opaque endianness */ - __le64 VolatileFileId; /* opaque endianness */ + __u64 PersistentFileId; /* opaque endianness */ + __u64 VolatileFileId; /* opaque endianness */ __le32 CompletionFilter; __u32 Reserved; } __packed; @@ -978,8 +978,8 @@ struct smb2_create_rsp { __le64 EndofFile; __le32 FileAttributes; __le32 Reserved2; - __le64 PersistentFileId; - __le64 VolatileFileId; + __u64 PersistentFileId; + __u64 VolatileFileId; __le32 CreateContextsOffset; __le32 CreateContextsLength; __u8 Buffer[1];
The client used to partially convert the fids to le64, while storing or sending them by using host endianness. This broke the client on big-endian machines. Instead of converting them to le64, store them as opaque integers and then avoid byteswapping when sending them over wire. Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz> --- fs/cifs/smb2misc.c | 4 +-- fs/cifs/smb2ops.c | 8 ++--- fs/cifs/smb2pdu.c | 63 +++++++++++++++++---------------------- fs/smbfs_common/smb2pdu.h | 24 +++++++-------- 4 files changed, 46 insertions(+), 53 deletions(-)