Re: [Outreachy kernel] Re: [Y2038] [PATCH 1/5] fs: nfsd: Replace time_t with u32 in boot_time

[Arnd Bergmann <arnd@arndb.de>]

On Tuesday 03 November 2015 18:17:48 Ksenija Stanojević wrote:
> On Tue, Nov 3, 2015 at 4:04 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tuesday 27 October 2015 09:08:35 Ksenija Stanojevic wrote:
> >> Replace time_t type and get_seconds function which are not y2038 safe on
> >> 32-bit systems. Function ktime_get_seconds use monotonic instead of real
> >> time and therefore will not cause overflow.
> >>
> >> Signed-off-by: Ksenija Stanojevic <ksenija.stanojevic@gmail.com>
> >
> > I don't think using monotonic time is safe here:
> 
> I was under the impression that comment:
> 
> * We're assuming the clid was not given out from a boot
> * precisely 2^32 (about 136 years) before this one.  That seems
> * a safe assumption:
> 
> is implying that monotonic time is used (should be used).

You almost convinced me, and I just spent an hour trying to
understand what is actually going on. I think I've got it now,
and this is what happens:

The NFS client sends a SETCLIENTID request to the server, which
generates the clientid using boot_time and a unique (for this instance)
number, together these two make up a 'clientid_t'. This is sent
back to the client in the response, and it gets encoded as part
of nfsd4_encode_setclientid() in the line

        p = xdr_encode_opaque_fixed(p, &scd->se_clientid, 8);

The reason this does not show up when you grep for cl_boot is that
xdr_encode_opaque_fixed just takes the number as set of eight bytes
that are not meaningful to the client. The client then sends back
the number to the server on other requests, e.g. in nfsd4_sessionid,
and the server checks cl_boot to ensure that is the same number
as boot_time, and this way it can tell whether the server has been
rebooted while keeping the client session running.

If you use monotonic times for generating boot_time, that means
it could be the same after a reboot because the time to get
from poweron to starting the NFS server is relatively deterministic.
This however defeats the purpose of comparing the boot times.

So you have to use real time here, but it is safe to truncate the
time to the low 32 bits because the number is never used as the
actual time, just as a token that is required to be unique
for each time the server gets booted.

	Arnd