Commit 28eb24ff75c5 ("cifs: Always resolve hostname before reconnecting").
In case a hostname resolves to a different IP address (e.g. long running mounts), make sure to resolve it every time prior to calling generic_ip_connect() in reconnect.
This patch needs user space changes of cifs.upcall that set a timeout value for the "dns_resolver" key.
Suggested-by: Steve French stfrench@microsoft.com Signed-off-by: Paulo Alcantara palcantara@suse.de Signed-off-by: Steve French stfrench@microsoft.com Signed-off-by: Pavel Shilovsky pshilov@microsoft.com --- fs/cifs/connect.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+)
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index 6f24f12..2229182 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -50,6 +50,7 @@ #include "cifs_unicode.h" #include "cifs_debug.h" #include "cifs_fs_sb.h" +#include "dns_resolve.h" #include "ntlmssp.h" #include "nterr.h" #include "rfc1002pdu.h" @@ -319,6 +320,53 @@ static int cifs_setup_volume_info(struct smb_vol *volume_info, char *mount_data, const char *devname, bool is_smb3);
/* + * Resolve hostname and set ip addr in tcp ses. Useful for hostnames that may + * get their ip addresses changed at some point. + * + * This should be called with server->srv_mutex held. + */ +#ifdef CONFIG_CIFS_DFS_UPCALL +static int reconn_set_ipaddr(struct TCP_Server_Info *server) +{ + int rc; + int len; + char *unc, *ipaddr = NULL; + + if (!server->hostname) + return -EINVAL; + + len = strlen(server->hostname) + 3; + + unc = kmalloc(len, GFP_KERNEL); + if (!unc) { + cifs_dbg(FYI, "%s: failed to create UNC path\n", __func__); + return -ENOMEM; + } + snprintf(unc, len, "\\%s", server->hostname); + + rc = dns_resolve_server_name_to_ip(unc, &ipaddr); + kfree(unc); + + if (rc < 0) { + cifs_dbg(FYI, "%s: failed to resolve server part of %s to IP: %d\n", + __func__, server->hostname, rc); + return rc; + } + + rc = cifs_convert_address((struct sockaddr *)&server->dstaddr, ipaddr, + strlen(ipaddr)); + kfree(ipaddr); + + return !rc ? -1 : 0; +} +#else +static inline int reconn_set_ipaddr(struct TCP_Server_Info *server) +{ + return 0; +} +#endif + +/* * cifs tcp session reconnection * * mark tcp session as reconnecting so temporarily locked @@ -418,6 +466,11 @@ cifs_reconnect(struct TCP_Server_Info *server) rc = generic_ip_connect(server); if (rc) { cifs_dbg(FYI, "reconnect error %d\n", rc); + rc = reconn_set_ipaddr(server); + if (rc) { + cifs_dbg(FYI, "%s: failed to resolve hostname: %d\n", + __func__, rc); + } mutex_unlock(&server->srv_mutex); msleep(3000); } else {
On Wed, Jan 30, 2019 at 04:07:30PM -0800, Pavel Shilovsky wrote:
Commit 28eb24ff75c5 ("cifs: Always resolve hostname before reconnecting").
In case a hostname resolves to a different IP address (e.g. long running mounts), make sure to resolve it every time prior to calling generic_ip_connect() in reconnect.
This patch needs user space changes of cifs.upcall that set a timeout value for the "dns_resolver" key.
Suggested-by: Steve French stfrench@microsoft.com Signed-off-by: Paulo Alcantara palcantara@suse.de Signed-off-by: Steve French stfrench@microsoft.com Signed-off-by: Pavel Shilovsky pshilov@microsoft.com
fs/cifs/connect.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+)
What stable kernel(s) is this to be applied to?
thanks,
greg k-h
ср, 30 янв. 2019 г. в 23:16, Greg KH gregkh@linuxfoundation.org:
On Wed, Jan 30, 2019 at 04:07:30PM -0800, Pavel Shilovsky wrote:
Commit 28eb24ff75c5 ("cifs: Always resolve hostname before reconnecting").
In case a hostname resolves to a different IP address (e.g. long running mounts), make sure to resolve it every time prior to calling generic_ip_connect() in reconnect.
This patch needs user space changes of cifs.upcall that set a timeout value for the "dns_resolver" key.
Suggested-by: Steve French stfrench@microsoft.com Signed-off-by: Paulo Alcantara palcantara@suse.de Signed-off-by: Steve French stfrench@microsoft.com Signed-off-by: Pavel Shilovsky pshilov@microsoft.com
fs/cifs/connect.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+)
What stable kernel(s) is this to be applied to? \
All stable kernels that are under active maintenance may have the fix if it applies clearly. Please let me know if it doesn't.
-- Best regards, Pavel Shilovsky
On Thu, Jan 31, 2019 at 10:37:54AM -0800, Pavel Shilovsky wrote:
ср, 30 янв. 2019 г. в 23:16, Greg KH gregkh@linuxfoundation.org:
On Wed, Jan 30, 2019 at 04:07:30PM -0800, Pavel Shilovsky wrote:
Commit 28eb24ff75c5 ("cifs: Always resolve hostname before reconnecting").
In case a hostname resolves to a different IP address (e.g. long running mounts), make sure to resolve it every time prior to calling generic_ip_connect() in reconnect.
This patch needs user space changes of cifs.upcall that set a timeout value for the "dns_resolver" key.
Suggested-by: Steve French stfrench@microsoft.com Signed-off-by: Paulo Alcantara palcantara@suse.de Signed-off-by: Steve French stfrench@microsoft.com Signed-off-by: Pavel Shilovsky pshilov@microsoft.com
fs/cifs/connect.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+)
What stable kernel(s) is this to be applied to? \
All stable kernels that are under active maintenance may have the fix if it applies clearly. Please let me know if it doesn't.
So this wasn't actually tested against any of the stable kernels? Why not? Microsoft has a solid testsuite for it.
-- Thanks, Sasha
пт, 1 февр. 2019 г. в 06:11, Sasha Levin sashal@kernel.org:
On Thu, Jan 31, 2019 at 10:37:54AM -0800, Pavel Shilovsky wrote:
ср, 30 янв. 2019 г. в 23:16, Greg KH gregkh@linuxfoundation.org:
On Wed, Jan 30, 2019 at 04:07:30PM -0800, Pavel Shilovsky wrote:
Commit 28eb24ff75c5 ("cifs: Always resolve hostname before reconnecting").
In case a hostname resolves to a different IP address (e.g. long running mounts), make sure to resolve it every time prior to calling generic_ip_connect() in reconnect.
This patch needs user space changes of cifs.upcall that set a timeout value for the "dns_resolver" key.
Suggested-by: Steve French stfrench@microsoft.com Signed-off-by: Paulo Alcantara palcantara@suse.de Signed-off-by: Steve French stfrench@microsoft.com Signed-off-by: Pavel Shilovsky pshilov@microsoft.com
fs/cifs/connect.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+)
What stable kernel(s) is this to be applied to? \
All stable kernels that are under active maintenance may have the fix if it applies clearly. Please let me know if it doesn't.
So this wasn't actually tested against any of the stable kernels? Why not? Microsoft has a solid testsuite for it.
This was successfully tested on v4.4.y kernel.
-- Best regards, Pavel Shilovsky
On Fri, Feb 01, 2019 at 12:17:21PM -0800, Pavel Shilovsky wrote:
пт, 1 февр. 2019 г. в 06:11, Sasha Levin sashal@kernel.org:
On Thu, Jan 31, 2019 at 10:37:54AM -0800, Pavel Shilovsky wrote:
ср, 30 янв. 2019 г. в 23:16, Greg KH gregkh@linuxfoundation.org:
On Wed, Jan 30, 2019 at 04:07:30PM -0800, Pavel Shilovsky wrote:
Commit 28eb24ff75c5 ("cifs: Always resolve hostname before reconnecting").
In case a hostname resolves to a different IP address (e.g. long running mounts), make sure to resolve it every time prior to calling generic_ip_connect() in reconnect.
This patch needs user space changes of cifs.upcall that set a timeout value for the "dns_resolver" key.
Suggested-by: Steve French stfrench@microsoft.com Signed-off-by: Paulo Alcantara palcantara@suse.de Signed-off-by: Steve French stfrench@microsoft.com Signed-off-by: Pavel Shilovsky pshilov@microsoft.com
fs/cifs/connect.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+)
What stable kernel(s) is this to be applied to? \
All stable kernels that are under active maintenance may have the fix if it applies clearly. Please let me know if it doesn't.
So this wasn't actually tested against any of the stable kernels? Why not? Microsoft has a solid testsuite for it.
This was successfully tested on v4.4.y kernel.
That's odd, as you generated this diff against the 4.20.y kernel.
Oh well, I'll go queue this up, to all of the kernel trees, but be careful...
greg k-h
On Wed, Jan 30, 2019 at 04:07:30PM -0800, Pavel Shilovsky wrote:
Commit 28eb24ff75c5 ("cifs: Always resolve hostname before reconnecting").
In case a hostname resolves to a different IP address (e.g. long running mounts), make sure to resolve it every time prior to calling generic_ip_connect() in reconnect.
This patch needs user space changes of cifs.upcall that set a timeout value for the "dns_resolver" key.
Wait, you are requiring userspace to upgrade tools to support this? And you think that will happen to all systems running older kernels?
This really feels like a new feature being added, what bug is this fixing that requires it to be backported to all of the stable kernel trees?
Suggested-by: Steve French stfrench@microsoft.com Signed-off-by: Paulo Alcantara palcantara@suse.de Signed-off-by: Steve French stfrench@microsoft.com Signed-off-by: Pavel Shilovsky pshilov@microsoft.com
At the very least, I would like the CIFS maintainer to ack this type of thing before accepting it, as it was not tagged for the stable kernels for some reason...
thanks,
greg k-h
сб, 2 февр. 2019 г. в 03:33, Greg KH gregkh@linuxfoundation.org:
On Wed, Jan 30, 2019 at 04:07:30PM -0800, Pavel Shilovsky wrote:
Commit 28eb24ff75c5 ("cifs: Always resolve hostname before reconnecting").
In case a hostname resolves to a different IP address (e.g. long running mounts), make sure to resolve it every time prior to calling generic_ip_connect() in reconnect.
This patch needs user space changes of cifs.upcall that set a timeout value for the "dns_resolver" key.
Wait, you are requiring userspace to upgrade tools to support this? And you think that will happen to all systems running older kernels?
This really feels like a new feature being added, what bug is this fixing that requires it to be backported to all of the stable kernel trees?
Let's me describe what is going to happen without upgrade of the userspace tools. The 1st reconnect will cause proper resolving of DNS name and storing this name into the cache. All subsequent reconnects will use the value stored in the cache and do not attempt to resolve this DNS name. That means the fix will help partially but doesn't break anything.
Once the userspace tools are upgraded, the cached DNS name will expire in 10 min, so every reconnect routine happening after that will use new IP. So, Linux distros will fully benefit from the patch once they upgrade the userspace tools but they do not need to synchronize updates of the kernel and the tools.
The diff has been generated against the newest kernel but I checked that it cherry-picks to at least 4.4.y (and probably older ones).
-- Best regards, Pavel Shilovsky
сб, 2 февр. 2019 г. в 10:13, Pavel Shilovsky piastryyy@gmail.com:
сб, 2 февр. 2019 г. в 03:33, Greg KH gregkh@linuxfoundation.org:
On Wed, Jan 30, 2019 at 04:07:30PM -0800, Pavel Shilovsky wrote:
Commit 28eb24ff75c5 ("cifs: Always resolve hostname before reconnecting").
In case a hostname resolves to a different IP address (e.g. long running mounts), make sure to resolve it every time prior to calling generic_ip_connect() in reconnect.
This patch needs user space changes of cifs.upcall that set a timeout value for the "dns_resolver" key.
Wait, you are requiring userspace to upgrade tools to support this? And you think that will happen to all systems running older kernels?
This really feels like a new feature being added, what bug is this fixing that requires it to be backported to all of the stable kernel trees?
Let's me describe what is going to happen without upgrade of the userspace tools. The 1st reconnect will cause proper resolving of DNS name and storing this name into the cache. All subsequent reconnects will use the value stored in the cache and do not attempt to resolve this DNS name. That means the fix will help partially but doesn't break anything.
Once the userspace tools are upgraded, the cached DNS name will expire in 10 min, so every reconnect routine happening after that will use new IP. So, Linux distros will fully benefit from the patch once they upgrade the userspace tools but they do not need to synchronize updates of the kernel and the tools.
The diff has been generated against the newest kernel but I checked that it cherry-picks to at least 4.4.y (and probably older ones).
Adding Steve to comment on that.
-- Best regards, Pavel Shilovsky
On Sat, Feb 2, 2019 at 12:17 PM Pavel Shilovsky piastryyy@gmail.com wrote:
сб, 2 февр. 2019 г. в 10:13, Pavel Shilovsky piastryyy@gmail.com:
сб, 2 февр. 2019 г. в 03:33, Greg KH gregkh@linuxfoundation.org:
On Wed, Jan 30, 2019 at 04:07:30PM -0800, Pavel Shilovsky wrote:
Commit 28eb24ff75c5 ("cifs: Always resolve hostname before reconnecting").
In case a hostname resolves to a different IP address (e.g. long running mounts), make sure to resolve it every time prior to calling generic_ip_connect() in reconnect.
This patch needs user space changes of cifs.upcall that set a timeout value for the "dns_resolver" key.
Wait, you are requiring userspace to upgrade tools to support this? And you think that will happen to all systems running older kernels?
This really feels like a new feature being added, what bug is this fixing that requires it to be backported to all of the stable kernel trees?
Let's me describe what is going to happen without upgrade of the userspace tools. The 1st reconnect will cause proper resolving of DNS name and storing this name into the cache. All subsequent reconnects will use the value stored in the cache and do not attempt to resolve this DNS name. That means the fix will help partially but doesn't break anything.
Once the userspace tools are upgraded, the cached DNS name will expire in 10 min, so every reconnect routine happening after that will use new IP. So, Linux distros will fully benefit from the patch once they upgrade the userspace tools but they do not need to synchronize updates of the kernel and the tools.
The diff has been generated against the newest kernel but I checked that it cherry-picks to at least 4.4.y (and probably older ones).
Adding Steve to comment on that.
-- Best regards, Pavel Shilovsky
Given the seriousness of the problem it fixes (hang on reconnect when server IP address changes) and that it could reduce possibility of data loss, this seems like it is reasonable to include in stable for kernels where it merges cleanly (which as Pavel noted may even include kernels older than 4.4).
On Sat, Feb 02, 2019 at 02:30:41PM -0600, Steve French wrote:
On Sat, Feb 2, 2019 at 12:17 PM Pavel Shilovsky piastryyy@gmail.com wrote:
сб, 2 февр. 2019 г. в 10:13, Pavel Shilovsky piastryyy@gmail.com:
сб, 2 февр. 2019 г. в 03:33, Greg KH gregkh@linuxfoundation.org:
On Wed, Jan 30, 2019 at 04:07:30PM -0800, Pavel Shilovsky wrote:
Commit 28eb24ff75c5 ("cifs: Always resolve hostname before reconnecting").
In case a hostname resolves to a different IP address (e.g. long running mounts), make sure to resolve it every time prior to calling generic_ip_connect() in reconnect.
This patch needs user space changes of cifs.upcall that set a timeout value for the "dns_resolver" key.
Wait, you are requiring userspace to upgrade tools to support this? And you think that will happen to all systems running older kernels?
This really feels like a new feature being added, what bug is this fixing that requires it to be backported to all of the stable kernel trees?
Let's me describe what is going to happen without upgrade of the userspace tools. The 1st reconnect will cause proper resolving of DNS name and storing this name into the cache. All subsequent reconnects will use the value stored in the cache and do not attempt to resolve this DNS name. That means the fix will help partially but doesn't break anything.
Once the userspace tools are upgraded, the cached DNS name will expire in 10 min, so every reconnect routine happening after that will use new IP. So, Linux distros will fully benefit from the patch once they upgrade the userspace tools but they do not need to synchronize updates of the kernel and the tools.
The diff has been generated against the newest kernel but I checked that it cherry-picks to at least 4.4.y (and probably older ones).
Adding Steve to comment on that.
-- Best regards, Pavel Shilovsky
Given the seriousness of the problem it fixes (hang on reconnect when server IP address changes) and that it could reduce possibility of data loss, this seems like it is reasonable to include in stable for kernels where it merges cleanly (which as Pavel noted may even include kernels older than 4.4).
Ok, thanks for the information, now queued up.
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Pavel Shilovsky -
Sasha Levin -
Steve French