Project

General

Profile

Actions

Bug #65043

open

Unable to set timestamp to value > UINT32_MAX

Added by Sachin Prabhu about 1 month ago. Updated about 1 month ago.

Status:
Triaged
Priority:
Low
Assignee:
-
Category:
Correctness/Safety
Target version:
% Done:

0%

Source:
Development
Tags:
Backport:
Regression:
No
Severity:
3 - minor
Reviewed:
Affected Versions:
ceph-qa-suite:
fs
Component(FS):
Client, MDS
Labels (FS):
Pull request ID:
Crash signature (v1):
Crash signature (v2):

Description

I have been testing the samba integration with cephfs at the backend using smbtorture - a tool provided by Samba to test for protocol correctness. The test in question is
smb2.timestamps

This sets mtime, btime and atime on a file to various times and then checks the return values for consistency. I noticed that the problem arises when the timestamp sets the number of seconds either in a negative range or to values > UINT32_MAX.

It was bought to my notice that CephFS uses u32 for the tv_sec element for time_t and this explains the issue we see
https://github.com/ceph/ceph/blob/main/src/include/utime.h#L51

To demonstrate the problem, I have attached a reproducer which sets the atime/ctime to UINT32_MAX, UINT32_MAX+1 and UINT32_MAX+2 for a file opened on a cephfs filesystem and for a file opened in the cwd of the directory from which the test is run. The timestamps on the local filesystem are set correctly while those on the cephfs system fail for values > UINT32_MAX.

  1. gcc -o test -D_FILE_OFFSET_BITS=64 -lcephfs test.c
  2. ./test
    Ceph: Create a new file
    cephfs: Get time: 1711023453

local: Create a new file
local: Get time: 1711023453

Set time to UINT32_MAX
cephfs: Set time: 4294967295
cephfs: Get time: 4294967295
local: Set time: 4294967295
local: Get time: 4294967295

Set time to UINT32_MAX+1
cephfs: Set time: 4294967296
cephfs: Get time: 0
local: Set time: 4294967296
local: Get time: 4294967296

Set time to UINT32_MAX+2
cephfs: Set time: 4294967297
cephfs: Get time: 1
local: Set time: 4294967297
local: Get time: 4294967297

Is there any chance that we will see 64 bit values used in the future?


Files

test.c (3.18 KB) test.c Sachin Prabhu, 03/21/2024 03:08 PM
Actions #1

Updated by Greg Farnum about 1 month ago

We use unsigned int, which takes us to year 2106 — we'll have to fix it eventually, but why is anybody trying to set those kinds of future timestamps? Does something make it important today, or can we just set a flag to ignore it?

Actions #2

Updated by Venky Shankar about 1 month ago

  • Status changed from New to Triaged
Actions #3

Updated by Sachin Prabhu about 1 month ago

Greg Farnum wrote:

We use unsigned int, which takes us to year 2106 — we'll have to fix it eventually, but why is anybody trying to set those kinds of future timestamps? Does something make it important today, or can we just set a flag to ignore it?

The testing is part of the smbtorture tests in Samba which test if the protocol implementation is correct. The failing tests are essentially ones where the tv_sec values are set to > UINT32_MAX. This bug report is simply part of the process to document the failures we see with the smbtorture tests. This problem hasn't been reported by any user yet. For now, I intend to add this test to the list of tests where we have a known failure so that we are aware of the limitations when using cephfs as a backend for Samba.

We can close the issue if there is no intention to change the implementation in the near future.

Actions #4

Updated by Sachin Prabhu about 1 month ago

Adding additional information of the specifications for time representation in the SMB protocol.

https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-fscc/a69cc039-d288-4673-9598-772b6083f8bf
"
Unless otherwise noted, Time fields are 64-bit signed integers representing the number of 100-nanosecond intervals that have elapsed since January 1, 1601, Coordinated Universal Time (UTC).
"

https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-dtyp/2c57429b-fdd4-488f-b5fc-9e4cf020fcdf
"
The FILETIME structure is a 64-bit value that represents the number of 100-nanosecond intervals that have elapsed since January 1, 1601, Coordinated Universal Time (UTC).
"

Actions #5

Updated by Venky Shankar about 1 month ago

  • Category set to Correctness/Safety
  • Priority changed from Normal to Low
  • Target version set to v20.0.0
  • Source set to Development
  • Component(FS) Client added

Sachin Prabhu wrote:

Greg Farnum wrote:

We use unsigned int, which takes us to year 2106 — we'll have to fix it eventually, but why is anybody trying to set those kinds of future timestamps? Does something make it important today, or can we just set a flag to ignore it?

The testing is part of the smbtorture tests in Samba which test if the protocol implementation is correct. The failing tests are essentially ones where the tv_sec values are set to > UINT32_MAX. This bug report is simply part of the process to document the failures we see with the smbtorture tests. This problem hasn't been reported by any user yet. For now, I intend to add this test to the list of tests where we have a known failure so that we are aware of the limitations when using cephfs as a backend for Samba.

We can close the issue if there is no intention to change the implementation in the near future.

Instead of closing, we should lower the priority. We need to fix this eventually. BTW, we discussed this a bit in cephfs standup last week and one of things that came up is to switch to using realtime clock implementation (https://github.com/ceph/ceph/blob/main/src/common/ceph_time.h) in ceph rather than utime_t. I haven't looked at its implementation closely, but much of ceph (or at least cephfs) is moving towards that (and its monotonic implementation wherever necessary).

Actions

Also available in: Atom PDF