The warning reuses the uptime max of 30 years used by the
setitimeofday().
Note that the warning is only added for new filesystem mounts
through the mount syscall. Automounts do not have the same warning.
Signed-off-by: Deepa Dinamani <[email protected]>
---
fs/namespace.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/fs/namespace.c b/fs/namespace.c
index b26778bdc236..5314fac8035e 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2739,6 +2739,17 @@ static int do_new_mount_fc(struct fs_context *fc, struct path *mountpoint,
error = do_add_mount(real_mount(mnt), mountpoint, mnt_flags);
if (error < 0)
mntput(mnt);
+
+ if (!error && sb->s_time_max &&
+ (ktime_get_real_seconds() + TIME_UPTIME_SEC_MAX > sb->s_time_max)) {
+ char *buf = (char *)__get_free_page(GFP_KERNEL);
+ char *mntpath = buf ? d_path(mountpoint, buf, PAGE_SIZE) : ERR_PTR(-ENOMEM);
+
+ pr_warn("Mounted %s file system at %s supports timestamps until 0x%llx\n",
+ fc->fs_type->name, mntpath, (unsigned long long)sb->s_time_max);
+ free_page((unsigned long)buf);
+ }
+
return error;
}
--
2.17.1
On Mon, 2019-07-29 at 18:49 -0700, Deepa Dinamani wrote:
> The warning reuses the uptime max of 30 years used by the
> setitimeofday().
>
> Note that the warning is only added for new filesystem mounts
> through the mount syscall. Automounts do not have the same warning.
>
> Signed-off-by: Deepa Dinamani <[email protected]>
> ---
> fs/namespace.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index b26778bdc236..5314fac8035e 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -2739,6 +2739,17 @@ static int do_new_mount_fc(struct fs_context *fc, struct path *mountpoint,
> error = do_add_mount(real_mount(mnt), mountpoint, mnt_flags);
> if (error < 0)
> mntput(mnt);
> +
> + if (!error && sb->s_time_max &&
I don't know why you are testing sb->s_time_max here - it should always
be non-zero since alloc_super() sets it to TIME64_MAX.
> + (ktime_get_real_seconds() + TIME_UPTIME_SEC_MAX > sb->s_time_max)) {
> + char *buf = (char *)__get_free_page(GFP_KERNEL);
> + char *mntpath = buf ? d_path(mountpoint, buf, PAGE_SIZE) : ERR_PTR(-ENOMEM);
> +
> + pr_warn("Mounted %s file system at %s supports timestamps until 0x%llx\n",
> + fc->fs_type->name, mntpath, (unsigned long long)sb->s_time_max);
This doesn't seem like a helpful way to log the time. Maybe use
time64_to_tm() to convert to "broken down" time and then print it with
"%ptR"... but that wants struct rtc_time. If you apply the attached
patch, however, you should then be able to print struct tm with
"%ptT".
Ben.
> + free_page((unsigned long)buf);
> + }
> +
> return error;
> }
>
--
Ben Hutchings, Software Developer Codethink Ltd
https://www.codethink.co.uk/ Dale House, 35 Dale Street
Manchester, M1 2HF, United Kingdom
On Mon, 2019-07-29 at 18:49 -0700, Deepa Dinamani wrote:
> The warning reuses the uptime max of 30 years used by the
> setitimeofday().
>
> Note that the warning is only added for new filesystem mounts
> through the mount syscall. Automounts do not have the same warning.
[...]
Another thing - perhaps this warning should be suppressed for read-only
mounts?
Ben.
--
Ben Hutchings, Software Developer Codethink Ltd
https://www.codethink.co.uk/ Dale House, 35 Dale Street
Manchester, M1 2HF, United Kingdom
On Mon, Aug 5, 2019 at 4:12 PM Ben Hutchings
<[email protected]> wrote:
>
> On Mon, 2019-07-29 at 18:49 -0700, Deepa Dinamani wrote:
> > The warning reuses the uptime max of 30 years used by the
> > setitimeofday().
> >
> > Note that the warning is only added for new filesystem mounts
> > through the mount syscall. Automounts do not have the same warning.
> >
> > Signed-off-by: Deepa Dinamani <[email protected]>
> > ---
> > fs/namespace.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/fs/namespace.c b/fs/namespace.c
> > index b26778bdc236..5314fac8035e 100644
> > --- a/fs/namespace.c
> > +++ b/fs/namespace.c
> > @@ -2739,6 +2739,17 @@ static int do_new_mount_fc(struct fs_context *fc, struct path *mountpoint,
> > error = do_add_mount(real_mount(mnt), mountpoint, mnt_flags);
> > if (error < 0)
> > mntput(mnt);
> > +
> > + if (!error && sb->s_time_max &&
>
> I don't know why you are testing sb->s_time_max here - it should always
> be non-zero since alloc_super() sets it to TIME64_MAX.
I think we support some writable file systems that have no timestamps
at all, so both the minimum and maximum default to 0 (1970-01-01).
For these, there is no point in printing a warning, they just work
as designed, even though the maximum is expired.
Arnd
On Mon, Aug 5, 2019 at 7:14 AM Ben Hutchings
<[email protected]> wrote:
>
> On Mon, 2019-07-29 at 18:49 -0700, Deepa Dinamani wrote:
> > The warning reuses the uptime max of 30 years used by the
> > setitimeofday().
> >
> > Note that the warning is only added for new filesystem mounts
> > through the mount syscall. Automounts do not have the same warning.
> [...]
>
> Another thing - perhaps this warning should be suppressed for read-only
> mounts?
Many filesystems support read only mounts only. We do fill in right
granularities and limits for these filesystems as well. In keeping
with the trend, I have added the warning accordingly. I don't think I
have a preference either way. But, not warning for the red only mounts
adds another if case. If you have a strong preference, I could add it
in.
-Deepa
> This doesn't seem like a helpful way to log the time. Maybe use
> time64_to_tm() to convert to "broken down" time and then print it with
> "%ptR"... but that wants struct rtc_time. If you apply the attached
> patch, however, you should then be able to print struct tm with
> "%ptT".
OK. Will print a more user friendly date string here.
Thanks,
-Deepa
On Sat, 2019-08-10 at 13:44 -0700, Deepa Dinamani wrote:
> On Mon, Aug 5, 2019 at 7:14 AM Ben Hutchings
> <[email protected]> wrote:
> > On Mon, 2019-07-29 at 18:49 -0700, Deepa Dinamani wrote:
> > > The warning reuses the uptime max of 30 years used by the
> > > setitimeofday().
> > >
> > > Note that the warning is only added for new filesystem mounts
> > > through the mount syscall. Automounts do not have the same warning.
> > [...]
> >
> > Another thing - perhaps this warning should be suppressed for read-only
> > mounts?
>
> Many filesystems support read only mounts only. We do fill in right
> granularities and limits for these filesystems as well. In keeping
> with the trend, I have added the warning accordingly. I don't think I
> have a preference either way. But, not warning for the red only mounts
> adds another if case. If you have a strong preference, I could add it
> in.
It seems to me that the warning is needed if there is a possibility of
data loss (incorrect timestamps, potentially leading to incorrect
decisions about which files are newer). This can happen only when a
filesystem is mounted read-write, or when a filesystem image is
created.
I think that warning for read-only mounts would be an annoyance to
users retrieving files from old filesystems.
Ben.
--
Ben Hutchings, Software Developer Codethink Ltd
https://www.codethink.co.uk/ Dale House, 35 Dale Street
Manchester, M1 2HF, United Kingdom
On Mon, Aug 12, 2019 at 3:25 PM Ben Hutchings
<[email protected]> wrote:
> On Sat, 2019-08-10 at 13:44 -0700, Deepa Dinamani wrote:
> > On Mon, Aug 5, 2019 at 7:14 AM Ben Hutchings
> > <[email protected]> wrote:
> > > On Mon, 2019-07-29 at 18:49 -0700, Deepa Dinamani wrote:
> > > > The warning reuses the uptime max of 30 years used by the
> > > > setitimeofday().
> > > >
> > > > Note that the warning is only added for new filesystem mounts
> > > > through the mount syscall. Automounts do not have the same warning.
> > > [...]
> > >
> > > Another thing - perhaps this warning should be suppressed for read-only
> > > mounts?
> >
> > Many filesystems support read only mounts only. We do fill in right
> > granularities and limits for these filesystems as well. In keeping
> > with the trend, I have added the warning accordingly. I don't think I
> > have a preference either way. But, not warning for the red only mounts
> > adds another if case. If you have a strong preference, I could add it
> > in.
>
> It seems to me that the warning is needed if there is a possibility of
> data loss (incorrect timestamps, potentially leading to incorrect
> decisions about which files are newer). This can happen only when a
> filesystem is mounted read-write, or when a filesystem image is
> created.
>
> I think that warning for read-only mounts would be an annoyance to
> users retrieving files from old filesystems.
I agree, the warning is not helpful for read-only mounts. An earlier
plan was to completely disallow writable mounts that might risk an
overflow (in some configurations at least). The warning replaces that
now, and I think it should also just warn for the cases that would
otherwise have been dangerous.
Arnd
On Mon, Aug 12, 2019 at 7:11 AM Arnd Bergmann <[email protected]> wrote:
>
> On Mon, Aug 12, 2019 at 3:25 PM Ben Hutchings
> <[email protected]> wrote:
> > On Sat, 2019-08-10 at 13:44 -0700, Deepa Dinamani wrote:
> > > On Mon, Aug 5, 2019 at 7:14 AM Ben Hutchings
> > > <[email protected]> wrote:
> > > > On Mon, 2019-07-29 at 18:49 -0700, Deepa Dinamani wrote:
> > > > > The warning reuses the uptime max of 30 years used by the
> > > > > setitimeofday().
> > > > >
> > > > > Note that the warning is only added for new filesystem mounts
> > > > > through the mount syscall. Automounts do not have the same warning.
> > > > [...]
> > > >
> > > > Another thing - perhaps this warning should be suppressed for read-only
> > > > mounts?
> > >
> > > Many filesystems support read only mounts only. We do fill in right
> > > granularities and limits for these filesystems as well. In keeping
> > > with the trend, I have added the warning accordingly. I don't think I
> > > have a preference either way. But, not warning for the red only mounts
> > > adds another if case. If you have a strong preference, I could add it
> > > in.
> >
> > It seems to me that the warning is needed if there is a possibility of
> > data loss (incorrect timestamps, potentially leading to incorrect
> > decisions about which files are newer). This can happen only when a
> > filesystem is mounted read-write, or when a filesystem image is
> > created.
> >
> > I think that warning for read-only mounts would be an annoyance to
> > users retrieving files from old filesystems.
>
> I agree, the warning is not helpful for read-only mounts. An earlier
> plan was to completely disallow writable mounts that might risk an
> overflow (in some configurations at least). The warning replaces that
> now, and I think it should also just warn for the cases that would
> otherwise have been dangerous.
Ok, I will make the change to exclude new read only mounts. I will use
__mnt_is_readonly() so that it also exculdes filesystems that are
readonly also.
The diff looks like below:
- if (!error && sb->s_time_max &&
+ if (!error && !__mnt_is_readonly(mnt) &&
(ktime_get_real_seconds() + TIME_UPTIME_SEC_MAX > sb->s_time_max)) {
Note that we can get rid of checking for non zero sb->s_time_max now.
-Deepa
On Mon, Aug 12, 2019 at 9:09 AM Deepa Dinamani <[email protected]> wrote:
>
> On Mon, Aug 12, 2019 at 7:11 AM Arnd Bergmann <[email protected]> wrote:
> >
> > On Mon, Aug 12, 2019 at 3:25 PM Ben Hutchings
> > <[email protected]> wrote:
> > > On Sat, 2019-08-10 at 13:44 -0700, Deepa Dinamani wrote:
> > > > On Mon, Aug 5, 2019 at 7:14 AM Ben Hutchings
> > > > <[email protected]> wrote:
> > > > > On Mon, 2019-07-29 at 18:49 -0700, Deepa Dinamani wrote:
> > > > > > The warning reuses the uptime max of 30 years used by the
> > > > > > setitimeofday().
> > > > > >
> > > > > > Note that the warning is only added for new filesystem mounts
> > > > > > through the mount syscall. Automounts do not have the same warning.
> > > > > [...]
> > > > >
> > > > > Another thing - perhaps this warning should be suppressed for read-only
> > > > > mounts?
> > > >
> > > > Many filesystems support read only mounts only. We do fill in right
> > > > granularities and limits for these filesystems as well. In keeping
> > > > with the trend, I have added the warning accordingly. I don't think I
> > > > have a preference either way. But, not warning for the red only mounts
> > > > adds another if case. If you have a strong preference, I could add it
> > > > in.
> > >
> > > It seems to me that the warning is needed if there is a possibility of
> > > data loss (incorrect timestamps, potentially leading to incorrect
> > > decisions about which files are newer). This can happen only when a
> > > filesystem is mounted read-write, or when a filesystem image is
> > > created.
> > >
> > > I think that warning for read-only mounts would be an annoyance to
> > > users retrieving files from old filesystems.
> >
> > I agree, the warning is not helpful for read-only mounts. An earlier
> > plan was to completely disallow writable mounts that might risk an
> > overflow (in some configurations at least). The warning replaces that
> > now, and I think it should also just warn for the cases that would
> > otherwise have been dangerous.
>
> Ok, I will make the change to exclude new read only mounts. I will use
> __mnt_is_readonly() so that it also exculdes filesystems that are
> readonly also.
> The diff looks like below:
>
> - if (!error && sb->s_time_max &&
> + if (!error && !__mnt_is_readonly(mnt) &&
> (ktime_get_real_seconds() + TIME_UPTIME_SEC_MAX > sb->s_time_max)) {
>
> Note that we can get rid of checking for non zero sb->s_time_max now.
One more thing, we will probably have to add a second warning for when
the filesystem is re-mounted rw after the initial readonly mount.
-Deepa
On Mon, 2019-08-12 at 09:15 -0700, Deepa Dinamani wrote:
> On Mon, Aug 12, 2019 at 9:09 AM Deepa Dinamani <[email protected]> wrote:
> > On Mon, Aug 12, 2019 at 7:11 AM Arnd Bergmann <[email protected]> wrote:
> > > On Mon, Aug 12, 2019 at 3:25 PM Ben Hutchings
> > > <[email protected]> wrote:
> > > > On Sat, 2019-08-10 at 13:44 -0700, Deepa Dinamani wrote:
> > > > > On Mon, Aug 5, 2019 at 7:14 AM Ben Hutchings
> > > > > <[email protected]> wrote:
> > > > > > On Mon, 2019-07-29 at 18:49 -0700, Deepa Dinamani wrote:
> > > > > > > The warning reuses the uptime max of 30 years used by the
> > > > > > > setitimeofday().
> > > > > > >
> > > > > > > Note that the warning is only added for new filesystem mounts
> > > > > > > through the mount syscall. Automounts do not have the same warning.
> > > > > > [...]
> > > > > >
> > > > > > Another thing - perhaps this warning should be suppressed for read-only
> > > > > > mounts?
> > > > >
> > > > > Many filesystems support read only mounts only. We do fill in right
> > > > > granularities and limits for these filesystems as well. In keeping
> > > > > with the trend, I have added the warning accordingly. I don't think I
> > > > > have a preference either way. But, not warning for the red only mounts
> > > > > adds another if case. If you have a strong preference, I could add it
> > > > > in.
> > > >
> > > > It seems to me that the warning is needed if there is a possibility of
> > > > data loss (incorrect timestamps, potentially leading to incorrect
> > > > decisions about which files are newer). This can happen only when a
> > > > filesystem is mounted read-write, or when a filesystem image is
> > > > created.
> > > >
> > > > I think that warning for read-only mounts would be an annoyance to
> > > > users retrieving files from old filesystems.
> > >
> > > I agree, the warning is not helpful for read-only mounts. An earlier
> > > plan was to completely disallow writable mounts that might risk an
> > > overflow (in some configurations at least). The warning replaces that
> > > now, and I think it should also just warn for the cases that would
> > > otherwise have been dangerous.
> >
> > Ok, I will make the change to exclude new read only mounts. I will use
> > __mnt_is_readonly() so that it also exculdes filesystems that are
> > readonly also.
> > The diff looks like below:
> >
> > - if (!error && sb->s_time_max &&
> > + if (!error && !__mnt_is_readonly(mnt) &&
> > (ktime_get_real_seconds() + TIME_UPTIME_SEC_MAX > sb->s_time_max)) {
> >
> > Note that we can get rid of checking for non zero sb->s_time_max now.
>
> One more thing, we will probably have to add a second warning for when
> the filesystem is re-mounted rw after the initial readonly mount.
Indeed, there would need to be a check for remount-read-write. I
didn't check whether remount uses this same code path.
Ben.
--
Ben Hutchings, Software Developer Codethink Ltd
https://www.codethink.co.uk/ Dale House, 35 Dale Street
Manchester, M1 2HF, United Kingdom