lkml.org 
[lkml]   [2020]   [Apr]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: cifs - Race between IP address change and sget()?
From
Date
On Mon, 2020-04-20 at 23:14 +0100, David Howells wrote:
> Paulo Alcantara <pc@cjr.nz> wrote:
>
> > > > > What happens if the IP address the superblock is going to changes, then
> > > > > another mount is made back to the original IP address? Does the second
> > > > > mount just pick the original superblock?
> > > >
> > > > It is going to transparently reconnect to the new ip address, SMB share,
> > > > and cifs superblock is kept unchanged. We, however, update internal
> > > > TCP_Server_Info structure to reflect new destination ip address.
> > > >
> > > > For the second mount, since the hostname (extracted out of the UNC path
> > > > at mount time) resolves to a new ip address and that address was saved
> > > > earlier in TCP_Server_Info structure during reconnect, we will end up
> > > > reusing same cifs superblock as per fs/cifs/connect.c:cifs_match_super().
> > >
> > > Would that be a bug?
> >
> > Probably.
> >
> > I'm not sure how that code is supposed to work, TBH.
>
> Hmmm... I think there may be a race here then - but I'm not sure it can be
> avoided or if it matters.
>
> Since the address is part of the primary key to sget() for cifs, changing the
> IP address will change the primary key. Jeff tells me that this is governed
> by a spinlock taken by cifs_match_super(). However, sget() may be busy
> attaching a new mount to the old superblock under the sb_lock core vfs lock,
> having already found a match.
>

Not exactly. Both places that match TCP_Server_Info objects by address
hold the cifs_tcp_ses_lock. The address looks like it gets changed in
reconn_set_ipaddr, and the lock is not currently taken there, AFAICT. I
think it probably should be (at least around the cifs_convert_address
call).

> Should the change of parameters made by cifs be effected with sb_lock held to
> try and avoid ending up using the wrong superblock?
>
> However, because the TCP_Server_Info is apparently updated, it looks like my
> original concern is not actually a problem (the idea that if a mounted server
> changes its IP address and then a new server comes online at the old IP
> address, it might end up sharing superblocks because the IP address is part of
> the key).
>

I'm not sure we should concern ourselves with much more than just not
allowing addresses to change while matching/searching. If you're
standing up new servers at old addresses while you still have clients
are migrating, then you are probably Doing it Wrong.

--
Jeff Layton <jlayton@redhat.com>

\
 
 \ /
  Last update: 2020-04-21 00:31    [W:0.069 / U:0.112 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site