On Sat, Feb 10, 2024 at 11:52 AM Steve French <smfrench@xxxxxxxxx> wrote:
>
> It needed a small rebase to work around one of Paulo's patches which
> it depended on (which is for 6.9-rc) but was wondering if you want
> this patch in before 6.9 because its change to smb2_compound_op below
> has a dependency on another patch - can that change be safely removed?
>
It will be good to have this in 6.8-rc4, if it will be accepted. It's
the same small change made in several places.
It avoids a possible handle leak on the server.
> replay_again:
> /* reinitialize for possible replay */
> flags = 0;
> oplock = SMB2_OPLOCK_LEVEL_NONE;
> num_rqst = 0;
> server = cifs_pick_channel(ses);
> oparms->replay = !!(retries);
>
> On Fri, Feb 9, 2024 at 5:25 AM <nspmangalore@xxxxxxxxx> wrote:
> >
> > From: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
> >
> > File open requests made to the server contain a
> > CreateGuid, which is used by the server to identify
> > the open request. If the same request needs to be
> > replayed, it needs to be sent with the same CreateGuid
> > in the durable handle v2 context.
> >
> > Without doing so, we could end up leaking handles on
> > the server when:
> > 1. multichannel is used AND
> > 2. connection goes down, but not for all channels
> >
> > This is because the replayed open request would have a
> > new CreateGuid and the server will treat this as a new
> > request and open a new handle.
> >
> > This change fixes this by reusing the existing create_guid
> > stored in the cached fid struct.
> >
> > REF: MS-SMB2 4.9 Replay Create Request on an Alternate Channel
> >
> > Fixes: 4f1fffa23769 ("cifs: commands that are retried should have replay flag set")
> > Signed-off-by: Shyam Prasad N <sprasad@xxxxxxxxxxxxx>
> > ---
> > fs/smb/client/cached_dir.c | 1 +
> > fs/smb/client/cifsglob.h | 1 +
> > fs/smb/client/smb2inode.c | 1 +
> > fs/smb/client/smb2ops.c | 4 ++++
> > fs/smb/client/smb2pdu.c | 10 ++++++++--
> > 5 files changed, 15 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/smb/client/cached_dir.c b/fs/smb/client/cached_dir.c
> > index 1daeb5714faa..3de5047a7ff9 100644
> > --- a/fs/smb/client/cached_dir.c
> > +++ b/fs/smb/client/cached_dir.c
> > @@ -242,6 +242,7 @@ int open_cached_dir(unsigned int xid, struct cifs_tcon *tcon,
> > .desired_access = FILE_READ_DATA | FILE_READ_ATTRIBUTES,
> > .disposition = FILE_OPEN,
> > .fid = pfid,
> > + .replay = !!(retries),
> > };
> >
> > rc = SMB2_open_init(tcon, server,
> > diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
> > index cac10f8e17e4..efab4769de4e 100644
> > --- a/fs/smb/client/cifsglob.h
> > +++ b/fs/smb/client/cifsglob.h
> > @@ -1373,6 +1373,7 @@ struct cifs_open_parms {
> > struct cifs_fid *fid;
> > umode_t mode;
> > bool reconnect:1;
> > + bool replay:1; /* indicates that this open is for a replay */
> > struct kvec *ea_cctx;
> > };
> >
> > diff --git a/fs/smb/client/smb2inode.c b/fs/smb/client/smb2inode.c
> > index 63485078a6df..22bd01e7bf6e 100644
> > --- a/fs/smb/client/smb2inode.c
> > +++ b/fs/smb/client/smb2inode.c
> > @@ -203,6 +203,7 @@ static int smb2_compound_op(const unsigned int xid, struct cifs_tcon *tcon,
> > oplock = SMB2_OPLOCK_LEVEL_NONE;
> > num_rqst = 0;
> > server = cifs_pick_channel(ses);
> > + oparms->replay = !!(retries);
> >
> > vars = kzalloc(sizeof(*vars), GFP_ATOMIC);
> > if (vars == NULL)
> > diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
> > index 8d674aef8dd9..c0da1935b0bd 100644
> > --- a/fs/smb/client/smb2ops.c
> > +++ b/fs/smb/client/smb2ops.c
> > @@ -1205,6 +1205,7 @@ smb2_set_ea(const unsigned int xid, struct cifs_tcon *tcon,
> > .disposition = FILE_OPEN,
> > .create_options = cifs_create_options(cifs_sb, 0),
> > .fid = &fid,
> > + .replay = !!(retries),
> > };
> >
> > rc = SMB2_open_init(tcon, server,
> > @@ -1570,6 +1571,7 @@ smb2_ioctl_query_info(const unsigned int xid,
> > .disposition = FILE_OPEN,
> > .create_options = cifs_create_options(cifs_sb, create_options),
> > .fid = &fid,
> > + .replay = !!(retries),
> > };
> >
> > if (qi.flags & PASSTHRU_FSCTL) {
> > @@ -2296,6 +2298,7 @@ smb2_query_dir_first(const unsigned int xid, struct cifs_tcon *tcon,
> > .disposition = FILE_OPEN,
> > .create_options = cifs_create_options(cifs_sb, 0),
> > .fid = fid,
> > + .replay = !!(retries),
> > };
> >
> > rc = SMB2_open_init(tcon, server,
> > @@ -2682,6 +2685,7 @@ smb2_query_info_compound(const unsigned int xid, struct cifs_tcon *tcon,
> > .disposition = FILE_OPEN,
> > .create_options = cifs_create_options(cifs_sb, 0),
> > .fid = &fid,
> > + .replay = !!(retries),
> > };
> >
> > rc = SMB2_open_init(tcon, server,
> > diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c
> > index 2ecc5f210329..1ce9be3a7ca7 100644
> > --- a/fs/smb/client/smb2pdu.c
> > +++ b/fs/smb/client/smb2pdu.c
> > @@ -2406,8 +2406,13 @@ create_durable_v2_buf(struct cifs_open_parms *oparms)
> > */
> > buf->dcontext.Timeout = cpu_to_le32(oparms->tcon->handle_timeout);
> > buf->dcontext.Flags = cpu_to_le32(SMB2_DHANDLE_FLAG_PERSISTENT);
> > - generate_random_uuid(buf->dcontext.CreateGuid);
> > - memcpy(pfid->create_guid, buf->dcontext.CreateGuid, 16);
> > +
> > + /* for replay, we should not overwrite the existing create guid */
> > + if (!oparms->replay) {
> > + generate_random_uuid(buf->dcontext.CreateGuid);
> > + memcpy(pfid->create_guid, buf->dcontext.CreateGuid, 16);
> > + } else
> > + memcpy(buf->dcontext.CreateGuid, pfid->create_guid, 16);
> >
> > /* SMB2_CREATE_DURABLE_HANDLE_REQUEST is "DH2Q" */
> > buf->Name[0] = 'D';
> > @@ -3156,6 +3161,7 @@ SMB2_open(const unsigned int xid, struct cifs_open_parms *oparms, __le16 *path,
> > /* reinitialize for possible replay */
> > flags = 0;
> > server = cifs_pick_channel(ses);
> > + oparms->replay = !!(retries);
> >
> > cifs_dbg(FYI, "create/open\n");
> > if (!ses || !server)
> > --
> > 2.34.1
> >
>
>
> --
> Thanks,
>
> Steve
--
Regards,
Shyam
