| Message ID | 20211121114009.6039-1-linkinjeon@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | ksmbd-tools: fix file transfer stuck at 99% | expand |
On Sun, Nov 21, 2021 at 12:40 PM Namjae Jeon <linkinjeon@kernel.org> wrote: > > When user set share name included upper character in smb.conf, > Windows File transfer will stuck at 99%. When copying file, windows send > SRVSVC GET_SHARE_INFO command to ksmbd server. but ksmbd store after > converting share name from smb.conf to lower cases. So ksmbd.mountd > can't not find share and return error to windows client. > This patch find share using name converted share name from client to > lower cases. > > Reported-by: Olha Cherevyk <olha.cherevyk@gmail.com> > Tested-by: Oleksandr Natalenko <oleksandr@natalenko.name> > Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> > --- > mountd/rpc_srvsvc.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/mountd/rpc_srvsvc.c b/mountd/rpc_srvsvc.c > index 8608b2e..f3b4d06 100644 > --- a/mountd/rpc_srvsvc.c > +++ b/mountd/rpc_srvsvc.c > @@ -169,8 +169,11 @@ static int srvsvc_share_get_info_invoke(struct ksmbd_rpc_pipe *pipe, > { > struct ksmbd_share *share; > int ret; > + gchar *share_name; > > - share = shm_lookup_share(STR_VAL(hdr->share_name)); > + share_name = g_ascii_strdown(STR_VAL(hdr->share_name), > + strlen(STR_VAL(hdr->share_name))); > + share = shm_lookup_share(share_name); > if (!share) > return 0; > > @@ -188,9 +191,12 @@ static int srvsvc_share_get_info_invoke(struct ksmbd_rpc_pipe *pipe, > } > > if (ret != 0) { > + gchar *server_name = g_ascii_strdown(STR_VAL(hdr->server_name), > + strlen(STR_VAL(hdr->server_name))); > + > ret = shm_lookup_hosts_map(share, > KSMBD_SHARE_HOSTS_DENY_MAP, > - STR_VAL(hdr->server_name)); > + server_name); > if (ret == 0) { > put_ksmbd_share(share); > return 0; > -- > 2.25.1 > Awesome work tracking this down. This raises a question though: why is ksmbd.mountd converting share names to lowercase to begin with ? I checked samba, and the share name sent back to the client has the same case as defined in smb.conf.
2021-11-22 18:28 GMT+09:00, Marios Makassikis <mmakassikis@freebox.fr>: > On Sun, Nov 21, 2021 at 12:40 PM Namjae Jeon <linkinjeon@kernel.org> wrote: >> >> When user set share name included upper character in smb.conf, >> Windows File transfer will stuck at 99%. When copying file, windows send >> SRVSVC GET_SHARE_INFO command to ksmbd server. but ksmbd store after >> converting share name from smb.conf to lower cases. So ksmbd.mountd >> can't not find share and return error to windows client. >> This patch find share using name converted share name from client to >> lower cases. >> >> Reported-by: Olha Cherevyk <olha.cherevyk@gmail.com> >> Tested-by: Oleksandr Natalenko <oleksandr@natalenko.name> >> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> >> --- >> mountd/rpc_srvsvc.c | 10 ++++++++-- >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/mountd/rpc_srvsvc.c b/mountd/rpc_srvsvc.c >> index 8608b2e..f3b4d06 100644 >> --- a/mountd/rpc_srvsvc.c >> +++ b/mountd/rpc_srvsvc.c >> @@ -169,8 +169,11 @@ static int srvsvc_share_get_info_invoke(struct >> ksmbd_rpc_pipe *pipe, >> { >> struct ksmbd_share *share; >> int ret; >> + gchar *share_name; >> >> - share = shm_lookup_share(STR_VAL(hdr->share_name)); >> + share_name = g_ascii_strdown(STR_VAL(hdr->share_name), >> + strlen(STR_VAL(hdr->share_name))); >> + share = shm_lookup_share(share_name); >> if (!share) >> return 0; >> >> @@ -188,9 +191,12 @@ static int srvsvc_share_get_info_invoke(struct >> ksmbd_rpc_pipe *pipe, >> } >> >> if (ret != 0) { >> + gchar *server_name = >> g_ascii_strdown(STR_VAL(hdr->server_name), >> + strlen(STR_VAL(hdr->server_name))); >> + >> ret = shm_lookup_hosts_map(share, >> KSMBD_SHARE_HOSTS_DENY_MAP, >> - STR_VAL(hdr->server_name)); >> + server_name); >> if (ret == 0) { >> put_ksmbd_share(share); >> return 0; >> -- >> 2.25.1 >> > > Awesome work tracking this down. This raises a question though: why is > ksmbd.mountd > converting share names to lowercase to begin with ? Windows is case insensitive. So we need it to do a lookup using the share name from windows. > > I checked samba, and the share name sent back to the client has the same > case as > defined in smb.conf. There should be no problems with the current way. And if you test adding multiple case sensitive share names to smb.conf, You will find that only one of them is available. >
On Mon, Nov 22, 2021 at 11:46 AM Namjae Jeon <linkinjeon@kernel.org> wrote: > > 2021-11-22 18:28 GMT+09:00, Marios Makassikis <mmakassikis@freebox.fr>: > > On Sun, Nov 21, 2021 at 12:40 PM Namjae Jeon <linkinjeon@kernel.org> wrote: > >> > >> When user set share name included upper character in smb.conf, > >> Windows File transfer will stuck at 99%. When copying file, windows send > >> SRVSVC GET_SHARE_INFO command to ksmbd server. but ksmbd store after > >> converting share name from smb.conf to lower cases. So ksmbd.mountd > >> can't not find share and return error to windows client. > >> This patch find share using name converted share name from client to > >> lower cases. > >> > >> Reported-by: Olha Cherevyk <olha.cherevyk@gmail.com> > >> Tested-by: Oleksandr Natalenko <oleksandr@natalenko.name> > >> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> > >> --- > >> mountd/rpc_srvsvc.c | 10 ++++++++-- > >> 1 file changed, 8 insertions(+), 2 deletions(-) > >> > >> diff --git a/mountd/rpc_srvsvc.c b/mountd/rpc_srvsvc.c > >> index 8608b2e..f3b4d06 100644 > >> --- a/mountd/rpc_srvsvc.c > >> +++ b/mountd/rpc_srvsvc.c > >> @@ -169,8 +169,11 @@ static int srvsvc_share_get_info_invoke(struct > >> ksmbd_rpc_pipe *pipe, > >> { > >> struct ksmbd_share *share; > >> int ret; > >> + gchar *share_name; > >> > >> - share = shm_lookup_share(STR_VAL(hdr->share_name)); > >> + share_name = g_ascii_strdown(STR_VAL(hdr->share_name), > >> + strlen(STR_VAL(hdr->share_name))); > >> + share = shm_lookup_share(share_name); > >> if (!share) > >> return 0; > >> > >> @@ -188,9 +191,12 @@ static int srvsvc_share_get_info_invoke(struct > >> ksmbd_rpc_pipe *pipe, > >> } > >> > >> if (ret != 0) { > >> + gchar *server_name = > >> g_ascii_strdown(STR_VAL(hdr->server_name), > >> + strlen(STR_VAL(hdr->server_name))); > >> + > >> ret = shm_lookup_hosts_map(share, > >> KSMBD_SHARE_HOSTS_DENY_MAP, > >> - STR_VAL(hdr->server_name)); > >> + server_name); > >> if (ret == 0) { > >> put_ksmbd_share(share); > >> return 0; > >> -- > >> 2.25.1 > >> > > > > Awesome work tracking this down. This raises a question though: why is > > ksmbd.mountd > > converting share names to lowercase to begin with ? > Windows is case insensitive. So we need it to do a lookup using the > share name from windows. Right. I was thinking about Linux users using autofs (which can automount SMB shares). If the latter is configured to mount all shares on a server, switching from samba to ksmbd can cause /mnt/Share to become /mnt/share (since ksmbd will return lower case names when listing shares). This in turn can break other software that looks for things in /mnt/Share. > > I checked samba, and the share name sent back to the client has the same > > case as > > defined in smb.conf. > There should be no problems with the current way. And if you test > adding multiple case sensitive share names to smb.conf, You will find > that only one of them is available. > >
2021-11-22 21:14 GMT+09:00, Marios Makassikis <mmakassikis@freebox.fr>: > On Mon, Nov 22, 2021 at 11:46 AM Namjae Jeon <linkinjeon@kernel.org> wrote: >> >> 2021-11-22 18:28 GMT+09:00, Marios Makassikis <mmakassikis@freebox.fr>: >> > On Sun, Nov 21, 2021 at 12:40 PM Namjae Jeon <linkinjeon@kernel.org> >> > wrote: >> >> >> >> When user set share name included upper character in smb.conf, >> >> Windows File transfer will stuck at 99%. When copying file, windows >> >> send >> >> SRVSVC GET_SHARE_INFO command to ksmbd server. but ksmbd store after >> >> converting share name from smb.conf to lower cases. So ksmbd.mountd >> >> can't not find share and return error to windows client. >> >> This patch find share using name converted share name from client to >> >> lower cases. >> >> >> >> Reported-by: Olha Cherevyk <olha.cherevyk@gmail.com> >> >> Tested-by: Oleksandr Natalenko <oleksandr@natalenko.name> >> >> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> >> >> --- >> >> mountd/rpc_srvsvc.c | 10 ++++++++-- >> >> 1 file changed, 8 insertions(+), 2 deletions(-) >> >> >> >> diff --git a/mountd/rpc_srvsvc.c b/mountd/rpc_srvsvc.c >> >> index 8608b2e..f3b4d06 100644 >> >> --- a/mountd/rpc_srvsvc.c >> >> +++ b/mountd/rpc_srvsvc.c >> >> @@ -169,8 +169,11 @@ static int srvsvc_share_get_info_invoke(struct >> >> ksmbd_rpc_pipe *pipe, >> >> { >> >> struct ksmbd_share *share; >> >> int ret; >> >> + gchar *share_name; >> >> >> >> - share = shm_lookup_share(STR_VAL(hdr->share_name)); >> >> + share_name = g_ascii_strdown(STR_VAL(hdr->share_name), >> >> + strlen(STR_VAL(hdr->share_name))); >> >> + share = shm_lookup_share(share_name); >> >> if (!share) >> >> return 0; >> >> >> >> @@ -188,9 +191,12 @@ static int srvsvc_share_get_info_invoke(struct >> >> ksmbd_rpc_pipe *pipe, >> >> } >> >> >> >> if (ret != 0) { >> >> + gchar *server_name = >> >> g_ascii_strdown(STR_VAL(hdr->server_name), >> >> + strlen(STR_VAL(hdr->server_name))); >> >> + >> >> ret = shm_lookup_hosts_map(share, >> >> KSMBD_SHARE_HOSTS_DENY_MAP, >> >> - STR_VAL(hdr->server_name)); >> >> + server_name); >> >> if (ret == 0) { >> >> put_ksmbd_share(share); >> >> return 0; >> >> -- >> >> 2.25.1 >> >> >> > >> > Awesome work tracking this down. This raises a question though: why is >> > ksmbd.mountd >> > converting share names to lowercase to begin with ? >> Windows is case insensitive. So we need it to do a lookup using the >> share name from windows. > > Right. I was thinking about Linux users using autofs (which can automount > SMB shares). If the latter is configured to mount all shares on a server, > switching from samba to ksmbd can cause /mnt/Share to become /mnt/share > (since ksmbd will return lower case names when listing shares). This in > turn > can break other software that looks for things in /mnt/Share. I don't understand exactly how the problem is. Could you please check if this is actually a problem ? Even better if you can improve it ? :) > > >> > I checked samba, and the share name sent back to the client has the >> > same >> > case as >> > defined in smb.conf. >> There should be no problems with the current way. And if you test >> adding multiple case sensitive share names to smb.conf, You will find >> that only one of them is available. >> > >
diff --git a/mountd/rpc_srvsvc.c b/mountd/rpc_srvsvc.c index 8608b2e..f3b4d06 100644 --- a/mountd/rpc_srvsvc.c +++ b/mountd/rpc_srvsvc.c @@ -169,8 +169,11 @@ static int srvsvc_share_get_info_invoke(struct ksmbd_rpc_pipe *pipe, { struct ksmbd_share *share; int ret; + gchar *share_name; - share = shm_lookup_share(STR_VAL(hdr->share_name)); + share_name = g_ascii_strdown(STR_VAL(hdr->share_name), + strlen(STR_VAL(hdr->share_name))); + share = shm_lookup_share(share_name); if (!share) return 0; @@ -188,9 +191,12 @@ static int srvsvc_share_get_info_invoke(struct ksmbd_rpc_pipe *pipe, } if (ret != 0) { + gchar *server_name = g_ascii_strdown(STR_VAL(hdr->server_name), + strlen(STR_VAL(hdr->server_name))); + ret = shm_lookup_hosts_map(share, KSMBD_SHARE_HOSTS_DENY_MAP, - STR_VAL(hdr->server_name)); + server_name); if (ret == 0) { put_ksmbd_share(share); return 0;