Re: [PATCH] smb: client: fix atime clamp check in read completion

[Steve French]

merged into cifs-2.6.git for-next

On Tue, Jul 7, 2026 at 8:43 AM raoxu <raoxu@xxxxxxxxxxxxx> wrote:
>
> From: Xu Rao <raoxu@xxxxxxxxxxxxx>
>
> cifs_rreq_done() updates the inode atime to current_time(inode) after a
> netfs read.  It then preserves the CIFS rule that atime should not be
> older than mtime, because some applications break if atime is less than
> mtime.  That rule only requires clamping when atime < mtime.
>
> The current check uses the raw non-zero result of timespec64_compare().
> It therefore takes the clamp path for both atime < mtime and
> atime > mtime.  The latter is the normal case when reading an older file:
> the newly recorded atime is newer than the file mtime.  The completion
> handler then immediately moves atime back to mtime, losing the access
> time that was just recorded.  Userspace tools that rely on atime, such as
> stat, find -atime, backup tools or cold-data classifiers, can therefore
> see a recently read CIFS file as not recently accessed.
>
> This is easy to miss because the bug is silent: read I/O still succeeds,
> no error is reported, and many systems either do not check atime after
> reads or mount with policies such as relatime/noatime.  It becomes
> visible when a CIFS file has an mtime older than the current time, the
> file is read, and the local inode atime is inspected before a later
> revalidation replaces the cached timestamps.
>
> Clamp only when atime is actually older than mtime.  This matches the
> same atime/mtime rule used when applying CIFS inode attributes.
>
> Fixes: 69c3c023af25 ("cifs: Implement netfslib hooks")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Xu Rao <raoxu@xxxxxxxxxxxxx>
> ---
>  fs/smb/client/file.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c
> index 58430ba51b10..62605928d2b8 100644
> --- a/fs/smb/client/file.c
> +++ b/fs/smb/client/file.c
> @@ -301,7 +301,7 @@ static void cifs_rreq_done(struct netfs_io_request *rreq)
>         /* we do not want atime to be less than mtime, it broke some apps */
>         atime = inode_set_atime_to_ts(inode, current_time(inode));
>         mtime = inode_get_mtime(inode);
> -       if (timespec64_compare(&atime, &mtime))
> +       if (timespec64_compare(&atime, &mtime) < 0)
>                 inode_set_atime_to_ts(inode, inode_get_mtime(inode));
>  }
>
> --
> 2.50.1
>
>


-- 
Thanks,

Steve