All of lore.kernel.org
 help / color / mirror / Atom feed
From: Namjae Jeon <linkinjeon@kernel.org>
To: Tom Talpey <tom@talpey.com>
Cc: linux-cifs@vger.kernel.org
Subject: Re: [PATCH] ksmbd: add default data stream name in FILE_STREAM_INFORMATION
Date: Sun, 19 Sep 2021 06:36:50 +0900	[thread overview]
Message-ID: <CAKYAXd8kcZY_BbtOeKXR2ycH+tOSjYkNev=yhznH6Hbx-QEKMg@mail.gmail.com> (raw)
In-Reply-To: <ac18e062-e835-b575-66af-72631df7ef7d@talpey.com>

Hi Tom,
2021-09-18 21:39 GMT+09:00, Tom Talpey <tom@talpey.com>:
> This doesn't appear to be what's documented in MS-FSA section 2.1.5.11.29.
> There, it appears to call for returning an empty stream list,
> and STATUS_SUCCESS, when no streams are present.
As I know, Specification doesn't describe all windows behavior. So
smbtorture testcase test such corner cases.
>
> Also, why does the code special-case directories? The conditionals
> on StreamSize and StreamAllocation size are entirely redundant,
> after the top-level if (!S_ISDIR...), btw.
As I know, default data stream(::DATA) is for only file, not
directory. And streams tests in smbtorture pass, Please see:

$ ./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234 smb2.streams.dir
smbtorture 4.10.6
Using seed 1631969885
time: 2021-09-18 12:58:05.268099
test: dir
time: 2021-09-18 12:58:05.268848
teststreams\stream.txt:Stream One
(../../source4/torture/smb2/streams.c:248) opening non-existent directory stream
(../../source4/torture/smb2/streams.c:263) opening basedir  stream
(../../source4/torture/smb2/streams.c:279) opening basedir ::$DATA stream
(../../source4/torture/smb2/streams.c:295) list the streams on the basedir
time: 2021-09-18 12:58:05.313561
success: dir
The command "./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234
smb2.streams.dir" exited with 0.
0.11s$ ./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234 smb2.streams.io
smbtorture 4.10.6
Using seed 1631969885
time: 2021-09-18 12:58:05.357178
test: io
time: 2021-09-18 12:58:05.357884
(../../source4/torture/smb2/streams.c:335) creating a stream on a
non-existent file
(../../source4/torture/smb2/streams.c:355) check that open of base
file is allowed
(../../source4/torture/smb2/streams.c:362) writing to stream
(../../source4/torture/smb2/streams.c:377) modifying stream
(../../source4/torture/smb2/streams.c:387) creating a stream2 on a existing file
(../../source4/torture/smb2/streams.c:394) modifying stream
(../../source4/torture/smb2/streams.c:431) deleting stream
(../../source4/torture/smb2/streams.c:444) delete a stream via delete-on-close
(../../source4/torture/smb2/streams.c:476) deleting file
time: 2021-09-18 12:58:05.422830
success: io
The command "./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234
smb2.streams.io" exited with 0.
0.09s$ ./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234
smb2.streams.sharemodes
smbtorture 4.10.6
Using seed 1631969885
time: 2021-09-18 12:58:05.467364
test: sharemodes
time: 2021-09-18 12:58:05.468186
(../../source4/torture/smb2/streams.c:592) Testing stream share mode conflicts
time: 2021-09-18 12:58:05.521840
success: sharemodes
The command "./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234
smb2.streams.sharemodes" exited with 0.
0.10s$ ./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234
smb2.streams.names
smbtorture 4.10.6
Using seed 1631969885
time: 2021-09-18 12:58:05.565595
test: names
time: 2021-09-18 12:58:05.566433
(../../source4/torture/smb2/streams.c:869) testing stream names
time: 2021-09-18 12:58:05.626686
success: names
The command "./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234
smb2.streams.names" exited with 0.
0.12s$ ./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234
smb2.streams.names2
smbtorture 4.10.6
Using seed 1631969885
time: 2021-09-18 12:58:05.671829
test: names2
time: 2021-09-18 12:58:05.672704
(../../source4/torture/smb2/streams.c:1160) testing stream names
time: 2021-09-18 12:58:05.751656
success: names2
The command "./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234
smb2.streams.names2" exited with 0.
0.08s$ ./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234
smb2.streams.names3
smbtorture 4.10.6
Using seed 1631969885
time: 2021-09-18 12:58:05.796248
test: names3
time: 2021-09-18 12:58:05.797156
(../../source4/torture/smb2/streams.c:1288) testing case insensitive
stream names
time: 2021-09-18 12:58:05.835048
success: names3
The command "./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234
smb2.streams.names3" exited with 0.
0.10s$ ./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234
smb2.streams.rename
smbtorture 4.10.6
Using seed 1631969885
time: 2021-09-18 12:58:05.879226
test: rename
time: 2021-09-18 12:58:05.880108
(../../source4/torture/smb2/streams.c:1379) testing stream renames
time: 2021-09-18 12:58:05.938765
success: rename
The command "./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234
smb2.streams.rename" exited with 0.
0.11s$ ./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234
smb2.streams.rename2
smbtorture 4.10.6
Using seed 1631969885
time: 2021-09-18 12:58:05.983358
test: rename2
time: 2021-09-18 12:58:05.984182
(../../source4/torture/smb2/streams.c:1504) Checking SMB2 rename of a
stream using :<stream>
(../../source4/torture/smb2/streams.c:1518) Checking SMB2 rename of an
overwriting stream using :<stream>
(../../source4/torture/smb2/streams.c:1548) Checking SMB2 rename of a
stream using <base>:<stream>
(../../source4/torture/smb2/streams.c:1559) Checking SMB2 rename to
default stream using :<stream>
time: 2021-09-18 12:58:06.048102
success: rename2
The command "./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234
smb2.streams.rename2" exited with 0.
0.10s$ ./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234
smb2.streams.create-disposition
smbtorture 4.10.6
Using seed 1631969886
time: 2021-09-18 12:58:06.094196
test: create-disposition
time: 2021-09-18 12:58:06.095203
(../../source4/torture/smb2/streams.c:1666) Checking create disp: open
(../../source4/torture/smb2/streams.c:1680) Checking create disp: overwrite
(../../source4/torture/smb2/streams.c:1694) Checking create disp: overwrite_if
(../../source4/torture/smb2/streams.c:1712) Checking create disp: supersede
(../../source4/torture/smb2/streams.c:1732) Checking create disp:
overwrite_if on stream
time: 2021-09-18 12:58:06.148406
success: create-disposition
The command "./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234
smb2.streams.create-disposition" exited with 0.
0.10s$ ./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234
smb2.streams.attributes
smbtorture 4.10.6
Using seed 1631969886
time: 2021-09-18 12:58:06.192777
test: attributes
time: 2021-09-18 12:58:06.193624
(../../source4/torture/smb2/streams.c:1810) testing attribute setting on stream
time: 2021-09-18 12:58:06.256161
success: attributes
The command "./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234
smb2.streams.attributes" exited with 0.
0.09s$ ./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234
smb2.streams.zero-byte
smbtorture 4.10.6
Using seed 1631969886
time: 2021-09-18 12:58:06.302540
test: zero-byte
time: 2021-09-18 12:58:06.303431
(../../source4/torture/smb2/streams.c:512) Check 0 byte named stream
time: 2021-09-18 12:58:06.353826
success: zero-byte
The command "./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234
smb2.streams.zero-byte" exited with 0.
0.21s$ ./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234
smb2.streams.basefile-rename-with-open-stream
smbtorture 4.10.6
Using seed 1631969886
time: 2021-09-18 12:58:06.397700
test: basefile-rename-with-open-stream
time: 2021-09-18 12:58:06.398478
Creating file with stream
Writing to stream
Renaming base file
time: 2021-09-18 12:58:06.565832
success: basefile-rename-with-open-stream
The command "./bin/smbtorture //127.0.0.1/cifsd-test3/ -Utestuser%1234
smb2.streams.basefile-rename-with-open-stream" exited with 0.
>
> I'd suggest asking Microsoft dochelp for clarification before
> implementing this.
I'm not sure I'll get the answer I want from the dochelp quickly. I
vote we can apply this patch to fix existing issue, and parallel
contact the dochelp and update the code if there is some points.

Thanks!
>
> Tom.
>
> On 9/18/2021 8:02 AM, Namjae Jeon wrote:
>> Windows client expect to get default stream name(::DATA) in
>> FILE_STREAM_INFORMATION response even if there is no stream data in file.
>> This patch fix update failure when writing ppt or doc files.
>>
>> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
>> ---
>>   fs/ksmbd/smb2pdu.c | 8 +++-----
>>   1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
>> index 49a1ca75f427..301605e0cbf7 100644
>> --- a/fs/ksmbd/smb2pdu.c
>> +++ b/fs/ksmbd/smb2pdu.c
>> @@ -4465,17 +4465,15 @@ static void get_file_stream_info(struct ksmbd_work
>> *work,
>>   		file_info->NextEntryOffset = cpu_to_le32(next);
>>   	}
>>
>> -	if (nbytes) {
>> +	if (!S_ISDIR(stat.mode)) {
>>   		file_info = (struct smb2_file_stream_info *)
>>   			&rsp->Buffer[nbytes];
>>   		streamlen = smbConvertToUTF16((__le16 *)file_info->StreamName,
>>   					      "::$DATA", 7, conn->local_nls, 0);
>>   		streamlen *= 2;
>>   		file_info->StreamNameLength = cpu_to_le32(streamlen);
>> -		file_info->StreamSize = S_ISDIR(stat.mode) ? 0 :
>> -			cpu_to_le64(stat.size);
>> -		file_info->StreamAllocationSize = S_ISDIR(stat.mode) ? 0 :
>> -			cpu_to_le64(stat.size);
>> +		file_info->StreamSize = 0;
>> +		file_info->StreamAllocationSize = 0;
>>   		nbytes += sizeof(struct smb2_file_stream_info) + streamlen;
>>   	}
>>
>>
>

  reply	other threads:[~2021-09-18 21:36 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-18 12:02 [PATCH] ksmbd: add default data stream name in FILE_STREAM_INFORMATION Namjae Jeon
2021-09-18 12:39 ` Tom Talpey
2021-09-18 21:36   ` Namjae Jeon [this message]
2021-09-20  4:47   ` Steve French
2021-09-20  4:53     ` Steve French
2021-09-20 16:08     ` Tom Talpey
2021-09-20 16:34       ` Namjae Jeon
2021-09-21 18:19         ` Tom Talpey
2021-09-21 19:17           ` Steve French
2021-09-20 16:36       ` Steve French

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKYAXd8kcZY_BbtOeKXR2ycH+tOSjYkNev=yhznH6Hbx-QEKMg@mail.gmail.com' \
    --to=linkinjeon@kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=tom@talpey.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.