[PATCH v2 0/6] smb/server: fix return values of smb2_0_server_cmds proc

[PATCH v2 0/6] smb/server: fix return values of smb2_0_server_cmds proc

[chenxiaosong.chenxiaosong]

From: ChenXiaoSong <chenxiaosong@kylinos.cn>

These functions should return error code when an error occurs,
then __process_request() will print the error messages.

v1->v2: Update patch #01 #02 due to typos.

v1: https://lore.kernel.org/all/20251017084610.3085644-1-chenxiaosong.chenxiaosong@linux.dev/

ChenXiaoSong (6):
  smb/server: fix return value of smb2_read()
  smb/server: fix return value of smb2_notify()
  smb/server: fix return value of smb2_query_dir()
  smb/server: fix return value of smb2_ioctl()
  smb/server: fix return value of smb2_oplock_break()
  smb/server: update some misguided comment of smb2_0_server_cmds proc

 fs/smb/server/smb2pdu.c | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

-- 
2.43.0

[PATCH v2 1/6] smb/server: fix return value of smb2_read()

[chenxiaosong.chenxiaosong]

From: ChenXiaoSong <chenxiaosong@kylinos.cn>

STATUS_END_OF_FILE maps to the linux error -ENODATA. Perhaps in the future
we can move client/smb2maperror.c into common/ and then call
map_smb2_to_linux_error() to get the linux error.

Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
---
 fs/smb/server/smb2pdu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index f901ae18e68a..f80a3dbb2d4e 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -6842,7 +6842,7 @@ int smb2_read(struct ksmbd_work *work)
 		rsp->hdr.Status = STATUS_END_OF_FILE;
 		smb2_set_err_rsp(work);
 		ksmbd_fd_put(work, fp);
-		return 0;
+		return -ENODATA;
 	}
 
 	ksmbd_debug(SMB, "nbytes %zu, offset %lld mincount %zu\n",
-- 
2.43.0

Re: [PATCH v2 1/6] smb/server: fix return value of smb2_read()

[Markus Elfring]

> STATUS_END_OF_FILE maps to the linux error -ENODATA. Perhaps in the future
> we can move client/smb2maperror.c into common/ and then call
> map_smb2_to_linux_error() to get the linux error.

Will another imperative wording approach become more helpful for an improved
change description?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.18-rc1#n94


> Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>

Can a personal name deviate a bit more from an email identifier
according to the Developer's Certificate of Origin?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.18-rc1#n436

Regards,
Markus

Re: [v2 1/6] smb/server: fix return value of smb2_read()

[ChenXiaoSong]

Hi Steve and Namjae,

The "Fixes:" tags of the first five patches in this patchset are the same:

Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")

Thanks,
ChenXiaoSong.

On 10/19/25 7:13 PM, ChenXiaoSong wrote:
> Hi Steve and Markus,
> 
> Sorry for the late reply, I’ve been ill and sleeping a lot, so I only 
> just saw your emails.
> 
> Steve, thank you for your understanding of Chinese naming tradition.
> 
> Markus, thank you too for your other suggestions (such as the suggestion 
> of adding a "Fixes:" tag).
> 
> I am Chinese, and my Chinese name is "陈孝松", the pinyin is 
> "ChenXiaoSong". The first word is my family name, the second indicates 
> my generation(the second word of my siblings' names is the same), and 
> the third is my given name.
> 
> In China, no characters (such as spaces or punctuation) are used between 
> the word of a name.
> 
> Thanks again to Steve for your understanding of Chinese naming tradition.
> 
> Thanks!
> ChenXiaoSong.
> 
> On 10/19/25 5:47 PM, Steve French wrote:
>>  > My name pattern recognition would find a display like “… Chen Xiao 
>> Song”
>> clearer eventually.
>>
>> The convention of whether and where a space is used in approximating 
>> an Asian name in western ASCII characters apparently varies by 
>> country, but apparently natively in many Asian languages there would 
>> be no spaces so there does not appear to be a rule on this for 
>> Americanized (or westernized) Asian names. In addition the convention 
>> of whether given name precedes family name is reversed in some asian 
>> countries. In general a person has some freedom in how they wish to be 
>> "named" in git commits eg in western languages whether a nickname or 
>> strict legal name is used.
>>
>> Thanks,
>>
>> Steve
>>
>> On Sun, Oct 19, 2025, 3:50 AM Markus Elfring <Markus.Elfring@web.de 
>> <mailto:Markus.Elfring@web.de>> wrote:
>>
>>      >> Do you care for naming requirements which are indicated by
>>      >> the Developer's Certificate of Origin?
>>      >
>>      > Generally, these are good to follow for multiple reasons,
>>
>>     Thanks for another bit of positive indication.
>>
>>
>>      > but I still don't see your point.
>>
>>     I hope that remaining communication difficulties can be resolved 
>> better.
>>
>>     We are
>>
>>
>>      > If "Markus Elfring" chooses to use a (valid) email address
>>     "Markus.Elfring@web.de <mailto:Markus.Elfring@web.de> …" which is
>>     basically the same is his name how is that any different from
>>     "ChenXiaoSong" choosing to use a (valid) email address that is
>>     almost identical to the name ie "chenxiaosong@kylinos.cn
>>     <mailto:chenxiaosong@kylinos.cn> <mailto:chenxiaosong@kylinos.cn
>>     <mailto:chenxiaosong@kylinos.cn>>" obviously this is not using an
>>     Asian character set, but it appears accurate and I don't see
>>     anywhere that it violates. …
>>
>>     You stressed constraints for the email address selection.
>>
>>     I dared to point a concern out for the representation of a personal
>>     name.
>>     https://en.wikipedia.org/wiki/Personal_name <https://
>>     en.wikipedia.org/wiki/Personal_name>
>>
>>     We are occasionally struggling with the information
>>     “using a known identity (sorry, no anonymous contributions.)”,
>>     don't we?
>>     https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/
>>     tree/Documentation/process/submitting-patches.rst?h=v6.18-rc1#n440
>>     <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/
>>     tree/Documentation/process/submitting-patches.rst?h=v6.18-rc1#n440>
>>
>>     My name pattern recognition would find a display like “… Chen Xiao 
>> Song”
>>     clearer eventually.
>>     * https://en.wikipedia.org/wiki/Chen_(surname) <https://
>>     en.wikipedia.org/wiki/Chen_(surname)>
>>     * https://en.wikipedia.org/wiki/Xiao_(surname) <https://
>>     en.wikipedia.org/wiki/Xiao_(surname)>
>>     * https://en.wikipedia.org/wiki/
>>     Song_%28disambiguation%29#People_with_the_name <https://
>>     en.wikipedia.org/wiki/Song_%28disambiguation%29#People_with_the_name>
>>
>>
>>     Regards,
>>     Markus
>>
> 

Re: [v2 1/6] smb/server: fix return value of smb2_read()

[Markus Elfring]

> The "Fixes:" tags of the first five patches in this patchset are the same:
> 
> Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3")

Can such a technical detail indicate a need for the recombination
of proposed update steps?

Regards,
Markus

[PATCH v2 2/6] smb/server: fix return value of smb2_notify()

[chenxiaosong.chenxiaosong]

From: ChenXiaoSong <chenxiaosong@kylinos.cn>

smb2_notify() should return error code when an error occurs,
__process_request() will print the error messages.

I may implement the SMB2 CHANGE_NOTIFY response (see MS-SMB2 2.2.36)
in the future.

Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
---
 fs/smb/server/smb2pdu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index f80a3dbb2d4e..5b5f25a2eb8a 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -8787,7 +8787,7 @@ int smb2_oplock_break(struct ksmbd_work *work)
  * smb2_notify() - handler for smb2 notify request
  * @work:   smb work containing notify command buffer
  *
- * Return:      0
+ * Return:      0 on success, otherwise error
  */
 int smb2_notify(struct ksmbd_work *work)
 {
@@ -8801,12 +8801,12 @@ int smb2_notify(struct ksmbd_work *work)
 	if (work->next_smb2_rcv_hdr_off && req->hdr.NextCommand) {
 		rsp->hdr.Status = STATUS_INTERNAL_ERROR;
 		smb2_set_err_rsp(work);
-		return 0;
+		return -EIO;
 	}
 
 	smb2_set_err_rsp(work);
 	rsp->hdr.Status = STATUS_NOT_IMPLEMENTED;
-	return 0;
+	return -EOPNOTSUPP;
 }
 
 /**
-- 
2.43.0

Re: [PATCH v2 2/6] smb/server: fix return value of smb2_notify()

[Markus Elfring]

> smb2_notify() should return error code when an error occurs,
> __process_request() will print the error messages.
> 
> I may implement the SMB2 CHANGE_NOTIFY response (see MS-SMB2 2.2.36)
> in the future.

Will another imperative wording approach become more helpful for an improved
change description?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.18-rc1#n94

Regards,
Markus

Re: [PATCH v2 2/6] smb/server: fix return value of smb2_notify()

[ChenXiaoSong]

Hi Steve and Namjae,

https://chenxiaosong.com/en/smb2-change-notify.html

The link above is my analysis of the change notify feature, and my 
progress on developing this feature will be posted at this link. This 
link will remain permanently accessible.

I will complete the implementation of this feature as soon as possible.

Thanks,
ChenXiaoSong.

On 10/17/25 6:46 PM, chenxiaosong.chenxiaosong@linux.dev wrote:
> From: ChenXiaoSong <chenxiaosong@kylinos.cn>
> 
> smb2_notify() should return error code when an error occurs,
> __process_request() will print the error messages.
> 
> I may implement the SMB2 CHANGE_NOTIFY response (see MS-SMB2 2.2.36)
> in the future.
> 
> Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
> ---
>   fs/smb/server/smb2pdu.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
> index f80a3dbb2d4e..5b5f25a2eb8a 100644
> --- a/fs/smb/server/smb2pdu.c
> +++ b/fs/smb/server/smb2pdu.c
> @@ -8787,7 +8787,7 @@ int smb2_oplock_break(struct ksmbd_work *work)
>    * smb2_notify() - handler for smb2 notify request
>    * @work:   smb work containing notify command buffer
>    *
> - * Return:      0
> + * Return:      0 on success, otherwise error
>    */
>   int smb2_notify(struct ksmbd_work *work)
>   {
> @@ -8801,12 +8801,12 @@ int smb2_notify(struct ksmbd_work *work)
>   	if (work->next_smb2_rcv_hdr_off && req->hdr.NextCommand) {
>   		rsp->hdr.Status = STATUS_INTERNAL_ERROR;
>   		smb2_set_err_rsp(work);
> -		return 0;
> +		return -EIO;
>   	}
>   
>   	smb2_set_err_rsp(work);
>   	rsp->hdr.Status = STATUS_NOT_IMPLEMENTED;
> -	return 0;
> +	return -EOPNOTSUPP;
>   }
>   
>   /**

[PATCH v2 3/6] smb/server: fix return value of smb2_query_dir()

[chenxiaosong.chenxiaosong]

From: ChenXiaoSong <chenxiaosong@kylinos.cn>

__process_request() will not print error messages if smb2_query_dir()
always returns 0.

Fix this by returning the correct value at the end of function.

Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
---
 fs/smb/server/smb2pdu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 5b5f25a2eb8a..ff264249f405 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -4560,7 +4560,7 @@ int smb2_query_dir(struct ksmbd_work *work)
 	smb2_set_err_rsp(work);
 	ksmbd_fd_put(work, dir_fp);
 	ksmbd_revert_fsids(work);
-	return 0;
+	return rc;
 }
 
 /**
-- 
2.43.0

[PATCH v2 4/6] smb/server: fix return value of smb2_ioctl()

[chenxiaosong.chenxiaosong]

From: ChenXiaoSong <chenxiaosong@kylinos.cn>

__process_request() will not print error messages if smb2_ioctl()
always returns 0.

Fix this by returning the correct value at the end of function.

Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
---
 fs/smb/server/smb2pdu.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index ff264249f405..efd545882ba8 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -8164,7 +8164,7 @@ int smb2_ioctl(struct ksmbd_work *work)
 		id = req->VolatileFileId;
 
 	if (req->Flags != cpu_to_le32(SMB2_0_IOCTL_IS_FSCTL)) {
-		rsp->hdr.Status = STATUS_NOT_SUPPORTED;
+		ret = -EOPNOTSUPP;
 		goto out;
 	}
 
@@ -8184,8 +8184,9 @@ int smb2_ioctl(struct ksmbd_work *work)
 	case FSCTL_DFS_GET_REFERRALS:
 	case FSCTL_DFS_GET_REFERRALS_EX:
 		/* Not support DFS yet */
+		ret = -EOPNOTSUPP;
 		rsp->hdr.Status = STATUS_FS_DRIVER_REQUIRED;
-		goto out;
+		goto out2;
 	case FSCTL_CREATE_OR_GET_OBJECT_ID:
 	{
 		struct file_object_buf_type1_ioctl_rsp *obj_buf;
@@ -8475,8 +8476,10 @@ int smb2_ioctl(struct ksmbd_work *work)
 		rsp->hdr.Status = STATUS_BUFFER_TOO_SMALL;
 	else if (ret < 0 || rsp->hdr.Status == 0)
 		rsp->hdr.Status = STATUS_INVALID_PARAMETER;
+
+out2:
 	smb2_set_err_rsp(work);
-	return 0;
+	return ret;
 }
 
 /**
-- 
2.43.0

[PATCH v2 5/6] smb/server: fix return value of smb2_oplock_break()

[chenxiaosong.chenxiaosong]

From: ChenXiaoSong <chenxiaosong@kylinos.cn>

smb2_oplock_break() should return error code when an error occurs,
__process_request() will print the error messages.

Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
---
 fs/smb/server/smb2pdu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index efd545882ba8..06e4c21ad4a8 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -8758,7 +8758,7 @@ static void smb21_lease_break_ack(struct ksmbd_work *work)
  * smb2_oplock_break() - dispatcher for smb2.0 and 2.1 oplock/lease break
  * @work:	smb work containing oplock/lease break command buffer
  *
- * Return:	0
+ * Return:	0 on success, otherwise error
  */
 int smb2_oplock_break(struct ksmbd_work *work)
 {
@@ -8781,6 +8781,7 @@ int smb2_oplock_break(struct ksmbd_work *work)
 			    le16_to_cpu(req->StructureSize));
 		rsp->hdr.Status = STATUS_INVALID_PARAMETER;
 		smb2_set_err_rsp(work);
+		return -EINVAL;
 	}
 
 	return 0;
-- 
2.43.0

[PATCH v2 6/6] smb/server: update some misguided comment of smb2_0_server_cmds proc

[chenxiaosong.chenxiaosong]

From: ChenXiaoSong <chenxiaosong@kylinos.cn>

These functions return error code rather than always returning 0.

Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn>
---
 fs/smb/server/smb2pdu.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 06e4c21ad4a8..52cb0519b974 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -2168,7 +2168,7 @@ static int smb2_create_open_flags(bool file_present, __le32 access,
  * smb2_tree_disconnect() - handler for smb tree connect request
  * @work:	smb work containing request buffer
  *
- * Return:      0
+ * Return:      0 on success, otherwise error
  */
 int smb2_tree_disconnect(struct ksmbd_work *work)
 {
@@ -2232,7 +2232,7 @@ int smb2_tree_disconnect(struct ksmbd_work *work)
  * smb2_session_logoff() - handler for session log off request
  * @work:	smb work containing request buffer
  *
- * Return:      0
+ * Return:      0 on success, otherwise error
  */
 int smb2_session_logoff(struct ksmbd_work *work)
 {
@@ -5844,7 +5844,7 @@ static noinline int smb2_close_pipe(struct ksmbd_work *work)
  * smb2_close() - handler for smb2 close file command
  * @work:	smb work containing close request buffer
  *
- * Return:	0
+ * Return:	0 on success, otherwise error
  */
 int smb2_close(struct ksmbd_work *work)
 {
@@ -5969,7 +5969,7 @@ int smb2_close(struct ksmbd_work *work)
  * smb2_echo() - handler for smb2 echo(ping) command
  * @work:	smb work containing echo request buffer
  *
- * Return:	0
+ * Return:	0 on success, otherwise error
  */
 int smb2_echo(struct ksmbd_work *work)
 {
-- 
2.43.0

Re: [PATCH v2 0/6] smb/server: fix return values of smb2_0_server_cmds proc

[ChenXiaoSong]

Hi Namjae,

v1 has typos, and I've already sent this v2.

Thanks.

On 10/17/25 6:46 PM, chenxiaosong.chenxiaosong@linux.dev wrote:
> From: ChenXiaoSong <chenxiaosong@kylinos.cn>
> 
> These functions should return error code when an error occurs,
> then __process_request() will print the error messages.
> 
> v1->v2: Update patch #01 #02 due to typos.
> 
> v1: https://lore.kernel.org/all/20251017084610.3085644-1-chenxiaosong.chenxiaosong@linux.dev/
> 
> ChenXiaoSong (6):
>    smb/server: fix return value of smb2_read()
>    smb/server: fix return value of smb2_notify()
>    smb/server: fix return value of smb2_query_dir()
>    smb/server: fix return value of smb2_ioctl()
>    smb/server: fix return value of smb2_oplock_break()
>    smb/server: update some misguided comment of smb2_0_server_cmds proc
> 
>   fs/smb/server/smb2pdu.c | 30 +++++++++++++++++-------------
>   1 file changed, 17 insertions(+), 13 deletions(-)
> 

Re: [PATCH v2 0/6] smb/server: fix return values of smb2_0_server_cmds proc

[Namjae Jeon]

On Fri, Oct 17, 2025 at 11:56 PM ChenXiaoSong
<chenxiaosong.chenxiaosong@linux.dev> wrote:
>
> Hi Namjae,
>
> v1 has typos, and I've already sent this v2.
I have applied v2 patch-set.
Please check the ksmbd-for-next-next branch and let me know if you
find any issues.
Thanks.
>
> Thanks.
>
> On 10/17/25 6:46 PM, chenxiaosong.chenxiaosong@linux.dev wrote:
> > From: ChenXiaoSong <chenxiaosong@kylinos.cn>
> >
> > These functions should return error code when an error occurs,
> > then __process_request() will print the error messages.
> >
> > v1->v2: Update patch #01 #02 due to typos.
> >
> > v1: https://lore.kernel.org/all/20251017084610.3085644-1-chenxiaosong.chenxiaosong@linux.dev/
> >
> > ChenXiaoSong (6):
> >    smb/server: fix return value of smb2_read()
> >    smb/server: fix return value of smb2_notify()
> >    smb/server: fix return value of smb2_query_dir()
> >    smb/server: fix return value of smb2_ioctl()
> >    smb/server: fix return value of smb2_oplock_break()
> >    smb/server: update some misguided comment of smb2_0_server_cmds proc
> >
> >   fs/smb/server/smb2pdu.c | 30 +++++++++++++++++-------------
> >   1 file changed, 17 insertions(+), 13 deletions(-)
> >
>

Re: [PATCH v2 0/6] smb/server: fix return values of smb2_0_server_cmds proc

[Markus Elfring]

> These functions should return error code when an error occurs,
> then __process_request() will print the error messages.

You propose to correct some questionable implementation details.
How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.18-rc1#n605

Regards,
Markus

Re: [PATCH v2 0/6] smb/server: fix return values of smb2_0_server_cmds proc

[ChenXiaoSong]

Hi Steve and Namjae,

I have tested the following patches using xfstests and smbtorture, no 
additional failed test cases were observed in the test results.
   - [PATCH v2 0/6] smb/server: fix return values of smb2_0_server_cmds proc
   - Patches applied to the ksmbd-for-next-next branch
   - [PATCH v5 00/14] smb: move duplicate definitions to common header file

The detailed test results can be found in 
https://chenxiaosong.com/smb-test/20251109

Thanks,
ChenXiaoSong.

On 10/17/25 6:46 PM, chenxiaosong.chenxiaosong@linux.dev wrote:
> From: ChenXiaoSong <chenxiaosong@kylinos.cn>
> 
> These functions should return error code when an error occurs,
> then __process_request() will print the error messages.
> 
> v1->v2: Update patch #01 #02 due to typos.
> 
> v1: https://lore.kernel.org/all/20251017084610.3085644-1-chenxiaosong.chenxiaosong@linux.dev/
> 
> ChenXiaoSong (6):
>    smb/server: fix return value of smb2_read()
>    smb/server: fix return value of smb2_notify()
>    smb/server: fix return value of smb2_query_dir()
>    smb/server: fix return value of smb2_ioctl()
>    smb/server: fix return value of smb2_oplock_break()
>    smb/server: update some misguided comment of smb2_0_server_cmds proc
> 
>   fs/smb/server/smb2pdu.c | 30 +++++++++++++++++-------------
>   1 file changed, 17 insertions(+), 13 deletions(-)
>