* [PATCH] ksmbd: add default data stream name in FILE_STREAM_INFORMATION @ 2021-09-18 12:02 Namjae Jeon 2021-09-18 12:39 ` Tom Talpey 0 siblings, 1 reply; 10+ messages in thread From: Namjae Jeon @ 2021-09-18 12:02 UTC (permalink / raw) To: linux-cifs; +Cc: Namjae Jeon 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; } -- 2.25.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] ksmbd: add default data stream name in FILE_STREAM_INFORMATION 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 2021-09-20 4:47 ` Steve French 0 siblings, 2 replies; 10+ messages in thread From: Tom Talpey @ 2021-09-18 12:39 UTC (permalink / raw) To: Namjae Jeon, linux-cifs 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. 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. I'd suggest asking Microsoft dochelp for clarification before implementing this. 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; > } > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ksmbd: add default data stream name in FILE_STREAM_INFORMATION 2021-09-18 12:39 ` Tom Talpey @ 2021-09-18 21:36 ` Namjae Jeon 2021-09-20 4:47 ` Steve French 1 sibling, 0 replies; 10+ messages in thread From: Namjae Jeon @ 2021-09-18 21:36 UTC (permalink / raw) To: Tom Talpey; +Cc: linux-cifs 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; >> } >> >> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ksmbd: add default data stream name in FILE_STREAM_INFORMATION 2021-09-18 12:39 ` Tom Talpey 2021-09-18 21:36 ` Namjae Jeon @ 2021-09-20 4:47 ` Steve French 2021-09-20 4:53 ` Steve French 2021-09-20 16:08 ` Tom Talpey 1 sibling, 2 replies; 10+ messages in thread From: Steve French @ 2021-09-20 4:47 UTC (permalink / raw) To: Tom Talpey; +Cc: Namjae Jeon, CIFS On Sat, Sep 18, 2021 at 9:06 PM Tom Talpey <tom@talpey.com> wrote: > > 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. I tried a quick test to Windows and it does appear to return $DATA stream by default: # ./smbinfo filestreaminfo /mnt/junk Name: ::$DATA Size: 179765 bytes Allocation size: 196608 bytes when I tried the same thing to a directory on a share mounted to Windows 10 (NTFS), I get no streams returned. So it does look like default stream ($DATA) is only returned for files > 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. > > I'd suggest asking Microsoft dochelp for clarification before > implementing this. > > 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. -- Thanks, Steve ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ksmbd: add default data stream name in FILE_STREAM_INFORMATION 2021-09-20 4:47 ` Steve French @ 2021-09-20 4:53 ` Steve French 2021-09-20 16:08 ` Tom Talpey 1 sibling, 0 replies; 10+ messages in thread From: Steve French @ 2021-09-20 4:53 UTC (permalink / raw) To: Tom Talpey; +Cc: Namjae Jeon, CIFS Patch looks correct - tentatively merged pending testing (and additional review) On Sun, Sep 19, 2021 at 11:47 PM Steve French <smfrench@gmail.com> wrote: > > On Sat, Sep 18, 2021 at 9:06 PM Tom Talpey <tom@talpey.com> wrote: > > > > 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. > > I tried a quick test to Windows and it does appear to return $DATA > stream by default: > > # ./smbinfo filestreaminfo /mnt/junk > Name: ::$DATA > Size: 179765 bytes > Allocation size: 196608 bytes > > when I tried the same thing to a directory on a share mounted to Windows 10 > (NTFS), I get no streams returned. > > So it does look like default stream ($DATA) is only returned for files > > > 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. > > > > I'd suggest asking Microsoft dochelp for clarification before > > implementing this. > > > > 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. > > > > > -- > Thanks, > > Steve -- Thanks, Steve ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ksmbd: add default data stream name in FILE_STREAM_INFORMATION 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-20 16:36 ` Steve French 1 sibling, 2 replies; 10+ messages in thread From: Tom Talpey @ 2021-09-20 16:08 UTC (permalink / raw) To: Steve French; +Cc: Namjae Jeon, CIFS On 9/20/2021 12:47 AM, Steve French wrote: > On Sat, Sep 18, 2021 at 9:06 PM Tom Talpey <tom@talpey.com> wrote: >> >> 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. > > I tried a quick test to Windows and it does appear to return $DATA > stream by default: > > # ./smbinfo filestreaminfo /mnt/junk > Name: ::$DATA > Size: 179765 bytes > Allocation size: 196608 bytes Ok, so the implication is that the default stream is indeed always present, if the filesystem supports streams. The language in MS-FSA would therefore be correct, but a bit vague. I agree that returning a ::$DATA for ordinary files is appropriate. > when I tried the same thing to a directory on a share mounted to Windows 10 > (NTFS), I get no streams returned. > > So it does look like default stream ($DATA) is only returned for files My concern here is, what's so special about directories? A special file or fifo, a symlink or reparse/junction, etc. Is it appropriate to cons up a ::$DATA for these? What should the size values be, if so? Tom. >> 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. >> >> I'd suggest asking Microsoft dochelp for clarification before >> implementing this. >> >> 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. > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ksmbd: add default data stream name in FILE_STREAM_INFORMATION 2021-09-20 16:08 ` Tom Talpey @ 2021-09-20 16:34 ` Namjae Jeon 2021-09-21 18:19 ` Tom Talpey 2021-09-20 16:36 ` Steve French 1 sibling, 1 reply; 10+ messages in thread From: Namjae Jeon @ 2021-09-20 16:34 UTC (permalink / raw) To: Tom Talpey; +Cc: Steve French, CIFS 2021-09-21 1:08 GMT+09:00, Tom Talpey <tom@talpey.com>: > On 9/20/2021 12:47 AM, Steve French wrote: >> On Sat, Sep 18, 2021 at 9:06 PM Tom Talpey <tom@talpey.com> wrote: >>> >>> 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. >> >> I tried a quick test to Windows and it does appear to return $DATA >> stream by default: >> >> # ./smbinfo filestreaminfo /mnt/junk >> Name: ::$DATA >> Size: 179765 bytes >> Allocation size: 196608 bytes > > Ok, so the implication is that the default stream is indeed always > present, if the filesystem supports streams. The language in MS-FSA > would therefore be correct, but a bit vague. I agree that returning > a ::$DATA for ordinary files is appropriate. > >> when I tried the same thing to a directory on a share mounted to Windows >> 10 >> (NTFS), I get no streams returned. >> >> So it does look like default stream ($DATA) is only returned for files > > > My concern here is, what's so special about directories? A special file > or fifo, a symlink or reparse/junction, etc. Is it appropriate to cons > up a ::$DATA for these? What should the size values be, if so? Special files in linux(ksmbd share) is showing as regular file on windows client. > > Tom. > > >>> 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. >>> >>> I'd suggest asking Microsoft dochelp for clarification before >>> implementing this. >>> >>> 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. >> >> >> >> > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ksmbd: add default data stream name in FILE_STREAM_INFORMATION 2021-09-20 16:34 ` Namjae Jeon @ 2021-09-21 18:19 ` Tom Talpey 2021-09-21 19:17 ` Steve French 0 siblings, 1 reply; 10+ messages in thread From: Tom Talpey @ 2021-09-21 18:19 UTC (permalink / raw) To: Namjae Jeon; +Cc: Steve French, CIFS On 9/20/2021 12:34 PM, Namjae Jeon wrote: > 2021-09-21 1:08 GMT+09:00, Tom Talpey <tom@talpey.com>: <snip> >> >> My concern here is, what's so special about directories? A special file >> or fifo, a symlink or reparse/junction, etc. Is it appropriate to cons >> up a ::$DATA for these? What should the size values be, if so? > Special files in linux(ksmbd share) is showing as regular file on > windows client. This brings up an interesting second question. Is that a good thing? It seems risky, and perhaps wrong, that one can open such a special file and read or write it over SMB3. I can see allowing read attributes, etc, but certainly not full access. To me, at a minimum the read or write should be failed by ksmbd, if not the CREATE itself. ksmbd should not be representing these as ordinary files. Tom. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ksmbd: add default data stream name in FILE_STREAM_INFORMATION 2021-09-21 18:19 ` Tom Talpey @ 2021-09-21 19:17 ` Steve French 0 siblings, 0 replies; 10+ messages in thread From: Steve French @ 2021-09-21 19:17 UTC (permalink / raw) To: Tom Talpey; +Cc: Namjae Jeon, CIFS On Tue, Sep 21, 2021 at 1:19 PM Tom Talpey <tom@talpey.com> wrote: > > On 9/20/2021 12:34 PM, Namjae Jeon wrote: > > 2021-09-21 1:08 GMT+09:00, Tom Talpey <tom@talpey.com>: > <snip> > >> > >> My concern here is, what's so special about directories? A special file > >> or fifo, a symlink or reparse/junction, etc. Is it appropriate to cons > >> up a ::$DATA for these? What should the size values be, if so? > > Special files in linux(ksmbd share) is showing as regular file on > > windows client. > > This brings up an interesting second question. Is that a good thing? > > It seems risky, and perhaps wrong, that one can open such a special file > and read or write it over SMB3. I can see allowing read attributes, etc, > but certainly not full access. To me, at a minimum the read or write > should be failed by ksmbd, if not the CREATE itself. ksmbd should not be > representing these as ordinary files. Symlinks, FIFO, and char and block devices have special reparse points defined by Windows for these, but they would not typically be read, in some sense they act more like an empty directory. The exception seems to be hard links. -- Thanks, Steve ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ksmbd: add default data stream name in FILE_STREAM_INFORMATION 2021-09-20 16:08 ` Tom Talpey 2021-09-20 16:34 ` Namjae Jeon @ 2021-09-20 16:36 ` Steve French 1 sibling, 0 replies; 10+ messages in thread From: Steve French @ 2021-09-20 16:36 UTC (permalink / raw) To: Tom Talpey; +Cc: Namjae Jeon, CIFS On Mon, Sep 20, 2021 at 11:08 AM Tom Talpey <tom@talpey.com> wrote: > > On 9/20/2021 12:47 AM, Steve French wrote: > > On Sat, Sep 18, 2021 at 9:06 PM Tom Talpey <tom@talpey.com> wrote: > >> > >> 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. > > > > I tried a quick test to Windows and it does appear to return $DATA > > stream by default: > > > > # ./smbinfo filestreaminfo /mnt/junk > > Name: ::$DATA > > Size: 179765 bytes > > Allocation size: 196608 bytes > > Ok, so the implication is that the default stream is indeed always > present, if the filesystem supports streams. The language in MS-FSA > would therefore be correct, but a bit vague. I agree that returning > a ::$DATA for ordinary files is appropriate. > > > when I tried the same thing to a directory on a share mounted to Windows 10 > > (NTFS), I get no streams returned. > > > > So it does look like default stream ($DATA) is only returned for files > > > My concern here is, what's so special about directories? A special file > or fifo, a symlink or reparse/junction, etc. Is it appropriate to cons > up a ::$DATA for these? What should the size values be, if so? Here are examples (share exported Windows 10 NTFS) of files vs. directories with or without streams, note that directories do not show the "default stream" $DATA but files do, but both support adding streams, and reparse points at least in the case of hardlinks to files show the target's streams. It is a bit confusing because the Windows "dir /R" command filters out the ($DATA) stream returned on files even though it is returned across the network. # smbinfo filestreaminfo /mnt/dir-without-stream/ # smbinfo filestreaminfo /mnt/dir-with-stream/ Name: test_stream Size: 19 bytes Allocation size: 24 bytes # smbinfo filestreaminfo /mnt/file-without-stream Name: ::$DATA Size: 24 bytes Allocation size: 24 bytes # smbinfo filestreaminfo /mnt/file-with-stream Name: ::$DATA Size: 17 bytes Allocation size: 24 bytes Name: test_stream Size: 19 bytes Allocation size: 24 bytes # smbinfo filestreaminfo /mnt/link-file-with-stream Name: ::$DATA Size: 17 bytes Allocation size: 24 bytes Name: test_stream Size: 19 bytes Allocation size: 24 bytes # smbinfo filestreaminfo /mnt/link-file-without-stream Name: ::$DATA Size: 24 bytes Allocation size: 24 bytes Note that locally on Windows "DIR /R" filters out the $DATA (default) stream C:\shares\scratch>dir /R \shares\scratch Volume in drive C is OSDisk Volume Serial Number is 0AF4-9CC1 Directory of C:\shares\scratch 09/20/2021 11:32 AM <DIR> . 09/20/2021 11:32 AM <DIR> .. 09/20/2021 11:16 AM <DIR> dir-with-stream 19 dir-with-stream:test_stream:$DATA 09/20/2021 11:15 AM <DIR> dir-without-stream 09/20/2021 11:14 AM 17 file-with-stream 19 file-with-stream:test_stream:$DATA 09/20/2021 11:14 AM 24 file-without-stream 09/20/2021 11:14 AM 17 link-file-with-stream 19 link-file-with-stream:test_stream:$DATA 09/20/2021 11:14 AM 24 link-file-without-stream -- Thanks, Steve ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-09-21 19:17 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 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
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.