Received-SPF: pass (google.com: domain of linux-kernel+bounces-148608-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley
	Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United
	Kingdom.
	Registered in England and Wales under Company Registration No. 3798903
From: David Howells <dhowells@redhat.com>
In-Reply-To: <14e66691a65e3d05d3d8d50e74dfb366@manguebit.com>
References: <14e66691a65e3d05d3d8d50e74dfb366@manguebit.com> <3756406.1712244064@warthog.procyon.org.uk>
To: Paulo Alcantara <pc@manguebit.com>
Cc: dhowells@redhat.com, Steve French <sfrench@samba.org>,
    Shyam Prasad N <sprasad@microsoft.com>, linux-cifs@vger.kernel.org,
    linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] cifs: Fix reacquisition of volume cookie on still-live connection
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <188148.1713361310.1@warthog.procyon.org.uk>
Content-Transfer-Encoding: quoted-printable
Date: Wed, 17 Apr 2024 14:41:50 +0100
Message-ID: <188149.1713361310@warthog.procyon.org.uk>

Paulo Alcantara <pc@manguebit.com> wrote:

> > [!] Note: Looking at cifs_mount_get_tcon(), a more general solution ma=
y
> > actually be required.  Reacquiring the volume cookie isn't the only th=
ing
> > that function does: it also partially reinitialises the tcon record wi=
thout
> > any locking - which may cause live filesystem ops already using the tc=
on
> > through a previous mount to malfunction.
> =

> Agreed.

Looking over the code again, I'm not sure whether is actually necessary - =
or
whether it is necessary and will be a bit nasty to implement as it will
require read locking also.

Firstly, reset_cifs_unix_caps() seems to re-set tcon->fsUnixInfo.Capabilit=
y
and tcon->unix_ext, which it would presumably set to the same things - whi=
ch
is probably fine.

However, cifs_qfs_tcon() makes RPC operations that reloads tcon->fsDevInfo=
 and
tcon->fsAttrInfo - both of which may be being accessed without locks.

smb2_qfs_tcon() and smb3_qfs_tcon() alters everything cifs_qfs_tcon() does=
,
plus a bunch of extra tcon members.  Can this locally cached information
change over time on the server whilst we have a connection to it?

David