Re: [PATCH 5.12 083/161] libceph: dont set global_id until we get an auth ticket

[Sasha Levin]

On Tue, Jun 08, 2021 at 09:07:18PM +0200, Ilya Dryomov wrote:
> On Tue, Jun 8, 2021 at 8:48 PM Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > From: Ilya Dryomov <idryomov@xxxxxxxxx>
> >
> > [ Upstream commit 61ca49a9105faefa003b37542cebad8722f8ae22 ]
> >
> > With the introduction of enforcing mode, setting global_id as soon
> > as we get it in the first MAuth reply will result in EACCES if the
> > connection is reset before we get the second MAuth reply containing
> > an auth ticket -- because on retry we would attempt to reclaim that
> > global_id with no auth ticket at hand.
> >
> > Neither ceph_auth_client nor ceph_mon_client depend on global_id
> > being set ealy, so just delay the setting until we get and process
> > the second MAuth reply.  While at it, complain if the monitor sends
> > a zero global_id or changes our global_id as the session is likely
> > to fail after that.
> >
> > Cc: stable@xxxxxxxxxxxxxxx # needs backporting for < 5.11
> > Signed-off-by: Ilya Dryomov <idryomov@xxxxxxxxx>
> > Reviewed-by: Sage Weil <sage@xxxxxxxxxx>
> > Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
> > ---
> >  net/ceph/auth.c | 36 +++++++++++++++++++++++-------------
> >  1 file changed, 23 insertions(+), 13 deletions(-)
> >
> > diff --git a/net/ceph/auth.c b/net/ceph/auth.c
> > index eb261aa5fe18..de407e8feb97 100644
> > --- a/net/ceph/auth.c
> > +++ b/net/ceph/auth.c
> > @@ -36,6 +36,20 @@ static int init_protocol(struct ceph_auth_client *ac, int proto)
> >         }
> >  }
> >
> > +static void set_global_id(struct ceph_auth_client *ac, u64 global_id)
> > +{
> > +       dout("%s global_id %llu\n", __func__, global_id);
> > +
> > +       if (!global_id)
> > +               pr_err("got zero global_id\n");
> > +
> > +       if (ac->global_id && global_id != ac->global_id)
> > +               pr_err("global_id changed from %llu to %llu\n", ac->global_id,
> > +                      global_id);
> > +
> > +       ac->global_id = global_id;
> > +}
> > +
> >  /*
> >   * setup, teardown.
> >   */
> > @@ -222,11 +236,6 @@ int ceph_handle_auth_reply(struct ceph_auth_client *ac,
> >
> >         payload_end = payload + payload_len;
> >
> > -       if (global_id && ac->global_id != global_id) {
> > -               dout(" set global_id %lld -> %lld\n", ac->global_id, global_id);
> > -               ac->global_id = global_id;
> > -       }
> > -
> >         if (ac->negotiating) {
> >                 /* server does not support our protocols? */
> >                 if (!protocol && result < 0) {
> > @@ -253,11 +262,16 @@ int ceph_handle_auth_reply(struct ceph_auth_client *ac,
> >
> >         ret = ac->ops->handle_reply(ac, result, payload, payload_end,
> >                                     NULL, NULL, NULL, NULL);
> > -       if (ret == -EAGAIN)
> > +       if (ret == -EAGAIN) {
> >                 ret = build_request(ac, true, reply_buf, reply_len);
> > -       else if (ret)
> > +               goto out;
> > +       } else if (ret) {
> >                 pr_err("auth protocol '%s' mauth authentication failed: %d\n",
> >                        ceph_auth_proto_name(ac->protocol), result);
> > +               goto out;
> > +       }
> > +
> > +       set_global_id(ac, global_id);
> >
> >  out:
> >         mutex_unlock(&ac->mutex);
> > @@ -484,15 +498,11 @@ int ceph_auth_handle_reply_done(struct ceph_auth_client *ac,
> >         int ret;
> >
> >         mutex_lock(&ac->mutex);
> > -       if (global_id && ac->global_id != global_id) {
> > -               dout("%s global_id %llu -> %llu\n", __func__, ac->global_id,
> > -                    global_id);
> > -               ac->global_id = global_id;
> > -       }
> > -
> >         ret = ac->ops->handle_reply(ac, 0, reply, reply + reply_len,
> >                                     session_key, session_key_len,
> >                                     con_secret, con_secret_len);
> > +       if (!ret)
> > +               set_global_id(ac, global_id);
> >         mutex_unlock(&ac->mutex);
> >         return ret;
> >  }
>
> Hi Greg,
>
> I asked Sasha to drop this patch earlier today.

I've dropped it now, but I think I'm missing your previous request. Was
it as a reply to the added-to mail? I just want to make sure I'm not
missing your mails.

--
Thanks,
Sasha