Re: [PATCH] cifs: fix bad fids sent over wire

[Paulo Alcantara <pc@cjr.nz>]

Tom Talpey <tom@talpey.com> writes:

> On 3/20/2022 8:20 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
>> verbatim 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  | 53 ++++++++++++++++++++--------------------------
>>   3 files changed, 29 insertions(+), 36 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);
>
> This conflicts with the statement "store them verbatim". Because the
> rsp->{Persistent,Volatile}FileId fields are u64 (integer) types,
> they are not being stored verbatim, they are being manipulated
> by the CPU load/store instructions. Storing them into a u8[8]
> array is more to the point.

Yes, makes sense.

> If course, if the rsp structure is purely private to the code, then
> the structure element type is similarly private. But a debugger, or
> a future structure reference, may once again get it wrong
>
> Are you rejecting the idea of using a byte array?

No.  That would work, too.  I was just trying to avoid changing a lot of
places and eventually making it harder to backport.

I'll go with the byte array then.