From: Bryan Schumaker <[email protected]>
xfstests generic/258 attempts to set atime of a file to a value before the
unix epoch by running touch -t 196001010101. I found that both the client
and the server were zero-filling the first 32bits rather than sending the
correct 64bit time value during a setattr. These patches fix this on both
the client and the server.
Version 2:
- Cast time.tv_sec values to a s64 instead of a long for 32bit systems
Questions? Comments?
- Bryan
Bryan Schumaker (2):
nfs: Send atime and mtime as a 64bit value
nfsd: Decode and send 64bit time values
fs/nfs/nfs4xdr.c | 6 ++----
fs/nfsd/nfs4xdr.c | 19 +++++--------------
2 files changed, 7 insertions(+), 18 deletions(-)
--
1.8.2.1
From: Bryan Schumaker <[email protected]>
The seconds field of an nfstime4 structure is 64bit, but we are assuming
that the first 32bits are zero-filled. So if the client tries to set
atime to a value before the epoch (touch -t 196001010101), then the
server will save the wrong value on disk.
Signed-off-by: Bryan Schumaker <[email protected]>
---
fs/nfsd/nfs4xdr.c | 19 +++++--------------
1 file changed, 5 insertions(+), 14 deletions(-)
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 2a27456..372e765 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -344,10 +344,7 @@ nfsd4_decode_fattr(struct nfsd4_compoundargs *argp, u32 *bmval,
all 32 bits of 'nseconds'. */
READ_BUF(12);
len += 12;
- READ32(dummy32);
- if (dummy32)
- return nfserr_inval;
- READ32(iattr->ia_atime.tv_sec);
+ READ64(iattr->ia_atime.tv_sec);
READ32(iattr->ia_atime.tv_nsec);
if (iattr->ia_atime.tv_nsec >= (u32)1000000000)
return nfserr_inval;
@@ -370,10 +367,7 @@ nfsd4_decode_fattr(struct nfsd4_compoundargs *argp, u32 *bmval,
all 32 bits of 'nseconds'. */
READ_BUF(12);
len += 12;
- READ32(dummy32);
- if (dummy32)
- return nfserr_inval;
- READ32(iattr->ia_mtime.tv_sec);
+ READ64(iattr->ia_mtime.tv_sec);
READ32(iattr->ia_mtime.tv_nsec);
if (iattr->ia_mtime.tv_nsec >= (u32)1000000000)
return nfserr_inval;
@@ -2401,8 +2395,7 @@ out_acl:
if (bmval1 & FATTR4_WORD1_TIME_ACCESS) {
if ((buflen -= 12) < 0)
goto out_resource;
- WRITE32(0);
- WRITE32(stat.atime.tv_sec);
+ WRITE64((s64)stat.atime.tv_sec);
WRITE32(stat.atime.tv_nsec);
}
if (bmval1 & FATTR4_WORD1_TIME_DELTA) {
@@ -2415,15 +2408,13 @@ out_acl:
if (bmval1 & FATTR4_WORD1_TIME_METADATA) {
if ((buflen -= 12) < 0)
goto out_resource;
- WRITE32(0);
- WRITE32(stat.ctime.tv_sec);
+ WRITE64((s64)stat.ctime.tv_sec);
WRITE32(stat.ctime.tv_nsec);
}
if (bmval1 & FATTR4_WORD1_TIME_MODIFY) {
if ((buflen -= 12) < 0)
goto out_resource;
- WRITE32(0);
- WRITE32(stat.mtime.tv_sec);
+ WRITE64((s64)stat.mtime.tv_sec);
WRITE32(stat.mtime.tv_nsec);
}
if (bmval1 & FATTR4_WORD1_MOUNTED_ON_FILEID) {
--
1.8.2.1
On Fri, 2013-04-19 at 16:09 -0400, [email protected] wrote:
> From: Bryan Schumaker <[email protected]>
>
> RFC 3530 says that the seconds value of a nfstime4 structure is a 64bit
> value, but we are instead sending a 32-bit 0 and then a 32bit conversion
> of the 64bit Linux value. This means that if we try to set atime to a
> value before the epoch (touch -t 196001010101) the client will only send
> part of the new value due to lost precision.
>
> Signed-off-by: Bryan Schumaker <[email protected]>
Thanks Bryan! Applied.
I assume that Bruce will pick up the 2/2.
--
Trond Myklebust
Linux NFS client maintainer
NetApp
[email protected]
http://www.netapp.com
On Fri, Apr 19, 2013 at 09:22:29PM +0000, Myklebust, Trond wrote:
> On Fri, 2013-04-19 at 16:09 -0400, [email protected] wrote:
> > From: Bryan Schumaker <[email protected]>
> >
> > RFC 3530 says that the seconds value of a nfstime4 structure is a 64bit
> > value, but we are instead sending a 32-bit 0 and then a 32bit conversion
> > of the 64bit Linux value. This means that if we try to set atime to a
> > value before the epoch (touch -t 196001010101) the client will only send
> > part of the new value due to lost precision.
> >
> > Signed-off-by: Bryan Schumaker <[email protected]>
>
> Thanks Bryan! Applied.
>
> I assume that Bruce will pick up the 2/2.
Yep, thanks.
(These should go to stable too, right?)
--b.
From: Bryan Schumaker <[email protected]>
RFC 3530 says that the seconds value of a nfstime4 structure is a 64bit
value, but we are instead sending a 32-bit 0 and then a 32bit conversion
of the 64bit Linux value. This means that if we try to set atime to a
value before the epoch (touch -t 196001010101) the client will only send
part of the new value due to lost precision.
Signed-off-by: Bryan Schumaker <[email protected]>
---
fs/nfs/nfs4xdr.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 325eddc..3c79c58 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -1054,8 +1054,7 @@ static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap, const
if (iap->ia_valid & ATTR_ATIME_SET) {
bmval1 |= FATTR4_WORD1_TIME_ACCESS_SET;
*p++ = cpu_to_be32(NFS4_SET_TO_CLIENT_TIME);
- *p++ = cpu_to_be32(0);
- *p++ = cpu_to_be32(iap->ia_atime.tv_sec);
+ p = xdr_encode_hyper(p, (s64)iap->ia_atime.tv_sec);
*p++ = cpu_to_be32(iap->ia_atime.tv_nsec);
}
else if (iap->ia_valid & ATTR_ATIME) {
@@ -1065,8 +1064,7 @@ static void encode_attrs(struct xdr_stream *xdr, const struct iattr *iap, const
if (iap->ia_valid & ATTR_MTIME_SET) {
bmval1 |= FATTR4_WORD1_TIME_MODIFY_SET;
*p++ = cpu_to_be32(NFS4_SET_TO_CLIENT_TIME);
- *p++ = cpu_to_be32(0);
- *p++ = cpu_to_be32(iap->ia_mtime.tv_sec);
+ p = xdr_encode_hyper(p, (s64)iap->ia_mtime.tv_sec);
*p++ = cpu_to_be32(iap->ia_mtime.tv_nsec);
}
else if (iap->ia_valid & ATTR_MTIME) {
--
1.8.2.1