Re: PATCH] cifs: fix broken mounts when a SSH tunnel is used (try #4)

Steve French <[email protected]>
Newsgroups gmane.linux.file-systems.cifs
Message-ID <[email protected]>
Merged.

Note I fixed some checkpatch warnings thrown when I checked in this patch in
a followon patch immediately after merging this.

On Thu, Aug 20, 2009 at 2:33 AM, Suresh Jayaraman <[email protected]>wrote:

> One more try..
>
> It seems there is a regression that got introduced while Jeff fixed
> all the mount/umount races. While attempting to find whether a tcp
> session is already existing, we were not checking whether the "port"
> used are the same. When a second mount is attempted with a different
> "port=" option, it is being ignored. Because of this the cifs mounts
> that uses a SSH tunnel appears to be broken.
>
> Steps to reproduce:
>
> 1. create 2 shares
> # SSH Tunnel a SMB session
> 2. ssh -f -L 6111:127.0.0.1:445 root@localhost "sleep 86400"
> 3. ssh -f -L 6222:127.0.0.1:445 root@localhost "sleep 86400"
> 4. tcpdump -i lo 6111 &
> 5. mkdir -p /mnt/mnt1
> 6. mkdir -p /mnt/mnt2
> 7. mount.cifs //localhost/a /mnt/mnt1 -o
> username=guest,ip=127.0.0.1,port=6111
> #(shows tcpdump activity on port 6111)
> 8. mount.cifs //localhost/b /mnt/mnt2 -o
> username=guest,ip=127.0.0.1,port=6222
> #(shows tcpdump activity only on port 6111 and not on 6222
>
> Fix by adding a check to compare the port _only_ if the user tries to
> override the tcp port with "port=" option, before deciding that an
> existing tcp session is found. Also, clean up a bit by replacing
> if-else if by a switch statment while at it as suggested by Jeff.
>
> Signed-off-by: Suresh Jayaraman <[email protected]>
> ---
>
>  fs/cifs/connect.c |   45 +++++++++++++++++++++++++++++++++------------
>  1 files changed, 33 insertions(+), 12 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index 1f3345d..4e47a9b 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -1377,7 +1377,7 @@ cifs_parse_mount_options(char *options, const char
> *devname,
>  }
>
>  static struct TCP_Server_Info *
> -cifs_find_tcp_session(struct sockaddr_storage *addr)
> +cifs_find_tcp_session(struct sockaddr_storage *addr, unsigned short int
> port)
>  {
>        struct list_head *tmp;
>        struct TCP_Server_Info *server;
> @@ -1397,16 +1397,37 @@ cifs_find_tcp_session(struct sockaddr_storage
> *addr)
>                if (server->tcpStatus == CifsNew)
>                        continue;
>
> -               if (addr->ss_family == AF_INET &&
> -                   (addr4->sin_addr.s_addr !=
> -                    server->addr.sockAddr.sin_addr.s_addr))
> -                       continue;
> -               else if (addr->ss_family == AF_INET6 &&
> -
>  (!ipv6_addr_equal(&server->addr.sockAddr6.sin6_addr,
> -                                          &addr6->sin6_addr) ||
> -                         server->addr.sockAddr6.sin6_scope_id !=
> -                                          addr6->sin6_scope_id))
> -                       continue;
> +               switch (addr->ss_family) {
> +               case AF_INET:
> +                       if (addr4->sin_addr.s_addr ==
> +                           server->addr.sockAddr.sin_addr.s_addr) {
> +                               addr4->sin_port = htons(port);
> +                               /* user overrode default port? */
> +                               if (addr4->sin_port) {
> +                                       if (addr4->sin_port !=
> +                                           server->addr.sockAddr.sin_port)
> +                                               continue;
> +                               }
> +                               break;
> +                       } else
> +                               continue;
> +
> +               case AF_INET6:
> +                       if (ipv6_addr_equal(&addr6->sin6_addr,
> +                           &server->addr.sockAddr6.sin6_addr) &&
> +                           (addr6->sin6_scope_id ==
> +                           server->addr.sockAddr6.sin6_scope_id)) {
> +                               addr6->sin6_port = htons(port);
> +                               /* user overrode default port? */
> +                               if (addr6->sin6_port) {
> +                                      if (addr6->sin6_port !=
> +
>  server->addr.sockAddr6.sin6_port)
> +                                              continue;
> +                               }
> +                               break;
> +                       } else
> +                               continue;
> +               }
>
>                ++server->srv_count;
>                write_unlock(&cifs_tcp_ses_lock);
> @@ -1475,7 +1496,7 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
>        }
>
>        /* see if we already have a matching tcp_ses */
> -       tcp_ses = cifs_find_tcp_session(&addr);
> +       tcp_ses = cifs_find_tcp_session(&addr, volume_info->port);
>        if (tcp_ses)
>                return tcp_ses;
>
>


-- 
Thanks,

Steve

_______________________________________________
linux-cifs-client mailing list
[email protected]
https://lists.samba.org/mailman/listinfo/linux-cifs-client
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.