Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Sat, 2 Oct 2021 16:38:23 -0400
From:   "J. Bruce Fields" <bfields@fieldses.org>
To:     Volodymyr Khomenko <volodymyr@vastdata.com>
Cc:     linux-nfs@vger.kernel.org
Subject: Re: GSSAPI fix for pynfs nfs4.1 client code
Message-ID: <20211002203823.GC26608@fieldses.org>
References: <CANkgwetkTUjj-bMrM4XTvk0vhGiJt3wNKPpRvzgTk-u7ZfrdXg@mail.gmail.com>
 <20210930211123.GA16927@fieldses.org>
 <CANkgweuuo7VctNLNSGyVE2Unjv_RMdG7+zPYr6_QwSZAQTbPRQ@mail.gmail.com>
 <20211001141306.GD959@fieldses.org>
 <CANkgwevJURTVNcs8u3KS_jiZwxQZgGHX=YmU+kvbweQ0PLBHiA@mail.gmail.com>
 <20211001154821.GE959@fieldses.org>
 <CANkgwetH1jzxYcUp5+7AnhE_S8iQBnrG76hrKsUsXUAsUqYNJA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CANkgwetH1jzxYcUp5+7AnhE_S8iQBnrG76hrKsUsXUAsUqYNJA@mail.gmail.com>
User-Agent: Mutt/1.5.21 (2010-09-15)
Precedence: bulk

On Sat, Oct 02, 2021 at 09:12:25AM +0300, Volodymyr Khomenko wrote:
> P.S. Since the very 1st operation after NFS4 NULL is EXCHANGE_ID - it
> should be only single operation
> (client can't send few ECHANGE_ID because clientowner is only one per
> mount) and next CREATE_SESSION can't be sent
> until EXCHANGE_ID is replied from the server.
> So the use-case of 'any of the first 128 rpcs were out of order' is
> just a theoretical one and probably not possible in practice.

So our server uses a fixed-size gss sequence number window of 128.  We
keep track of sd_max, the largest sequence number we've seen so far.
Given an incoming rpc with sequence number seqno, we check:

	is seqno > sd_max?
		This is the normal case for in-order sequence numbers;
		update sd_max and our other sequence number data and
		continue normal processing.
	else is seqno < sd_max - 128?
		Oops, this is definitely too old; drop the request.
	else check our data about sequence numbers seen so far.

But our specific bug was we were doing the second check using unsigned
arithmetic, so if we hit the second check before sd_max hits 128, then
(sd_max - 128) is something very large, and we drop the request.

--b.