Ocfs2 devel - Jun 2018 - [PATCH] ocfs2: dlmglue: clean up timestamp handling

On Tue, Jun 19, 2018 at 7:14 PM, Goldwyn Rodrigues <rgoldwyn at suse.de>
wrote:
>
>
> On 06-19 17:58, Arnd Bergmann wrote:
>> The handling of timestamps outside of the 1970..2038 range in the dlm
>> glue is rather inconsistent: on 32-bit architectures, this has always
>> wrapped around to negative timestamps in the 1902..1969 range, while on
>> 64-bit kernels all timestamps are interpreted as positive 34 bit
numbers
>> in the 1970..2514 year range.
...
>
> Will all values written to LVB be the same with or without the patch?
> I am considering the situation where in a cluster some machines have this
> patch and some don't. Depending on that, this may require a version
> change.
There is one part that may change:
>> -static u64 ocfs2_pack_timespec(struct timespec *spec)
>> +static u64 ocfs2_pack_timespec(struct timespec64 *spec)
>>  {
>>       u64 res;
>> -     u64 sec = spec->tv_sec;
>> +     u64 sec = clamp_t(time64_t, spec->tv_sec, 0, 0x3ffffffffull);
>>       u32 nsec = spec->tv_nsec;
>>
>>       res = (sec << OCFS2_SEC_SHIFT) | (nsec &
OCFS2_NSEC_MASK);
Here, setting a timestamp before 1970 or after 2514 will get wrapped
around in unpatched kernels, but will be clamped to the minimum
and maximum times after the patch.

It is extremely rare for correct code to need timestamps outside of that
range, but it is also trivial to trigger that with a manual 'touch'
command
from user space.

If the change is a problem, I can resend the patch without that one
line change.

         Arnd

On 06-19 21:11, Arnd Bergmann wrote:
> On Tue, Jun 19, 2018 at 7:14 PM, Goldwyn Rodrigues <rgoldwyn at
suse.de> wrote:
> >
> >
> > On 06-19 17:58, Arnd Bergmann wrote:
> >> The handling of timestamps outside of the 1970..2038 range in the
dlm
> >> glue is rather inconsistent: on 32-bit architectures, this has
always
> >> wrapped around to negative timestamps in the 1902..1969 range,
while on
> >> 64-bit kernels all timestamps are interpreted as positive 34 bit
numbers
> >> in the 1970..2514 year range.
> ...
> >
> > Will all values written to LVB be the same with or without the patch?
> > I am considering the situation where in a cluster some machines have
this
> > patch and some don't. Depending on that, this may require a
version
> > change.
> 
> There is one part that may change:
> 
> >> -static u64 ocfs2_pack_timespec(struct timespec *spec)
> >> +static u64 ocfs2_pack_timespec(struct timespec64 *spec)
> >>  {
> >>       u64 res;
> >> -     u64 sec = spec->tv_sec;
> >> +     u64 sec = clamp_t(time64_t, spec->tv_sec, 0,
0x3ffffffffull);
> >>       u32 nsec = spec->tv_nsec;
> >>
> >>       res = (sec << OCFS2_SEC_SHIFT) | (nsec &
OCFS2_NSEC_MASK);
> 
> Here, setting a timestamp before 1970 or after 2514 will get wrapped
> around in unpatched kernels, but will be clamped to the minimum
> and maximum times after the patch.
> 
> It is extremely rare for correct code to need timestamps outside of that
> range, but it is also trivial to trigger that with a manual 'touch'
command
> from user space.
> 
> If the change is a problem, I can resend the patch without that one
> line change.
> 
I think you should keep the change, but incrment OCFS2_LVB_VERSION.


-- 
Goldwyn