| Message ID | 165294669215.3283481.13374322806917745974.stgit@warthog.procyon.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nfs: Fix fscache volume key rendering for endianness | expand |
On Thu, May 19, 2022 at 3:51 AM David Howells <dhowells@redhat.com> wrote: > > Fix fscache volume key rendering for endianness. Convert the BE numbers in > the address to host-endian before printing them so that they're consistent > if the cache is copied between architectures. > > Question: This change could lead to misidentification of a volume directory > in the cache on a LE machine (it's unlikely because the port number as well > as the address numbers all get flipped), but it was introduced in -rc1 in > this cycle so probably isn't in any distro kernels yet. Should I add a > version number to enforce non-matching? > > Signed-off-by: David Howells <dhowells@redhat.com> > cc: Dave Wysochanski <dwysocha@redhat.com> > cc: Trond Myklebust <trond.myklebust@hammerspace.com> > cc: Anna Schumaker <anna@kernel.org> > cc: Jeff Layton <jlayton@kernel.org> > cc: linux-nfs@vger.kernel.org > cc: linux-cachefs@redhat.com > --- > > fs/nfs/fscache.c | 14 +++++++------- > 1 file changed, 7 insertions(+), 7 deletions(-) > > diff --git a/fs/nfs/fscache.c b/fs/nfs/fscache.c > index f73c09a9cf0a..0e5572b192b2 100644 > --- a/fs/nfs/fscache.c > +++ b/fs/nfs/fscache.c > @@ -54,17 +54,17 @@ static bool nfs_fscache_get_client_key(struct nfs_client *clp, > > switch (clp->cl_addr.ss_family) { > case AF_INET: > - if (!nfs_append_int(key, _len, sin->sin_port) || > - !nfs_append_int(key, _len, sin->sin_addr.s_addr)) > + if (!nfs_append_int(key, _len, ntohs(sin->sin_port)) || > + !nfs_append_int(key, _len, ntohl(sin->sin_addr.s_addr))) > return false; > return true; > > case AF_INET6: > - if (!nfs_append_int(key, _len, sin6->sin6_port) || > - !nfs_append_int(key, _len, sin6->sin6_addr.s6_addr32[0]) || > - !nfs_append_int(key, _len, sin6->sin6_addr.s6_addr32[1]) || > - !nfs_append_int(key, _len, sin6->sin6_addr.s6_addr32[2]) || > - !nfs_append_int(key, _len, sin6->sin6_addr.s6_addr32[3])) > + if (!nfs_append_int(key, _len, ntohs(sin6->sin6_port)) || > + !nfs_append_int(key, _len, ntohl(sin6->sin6_addr.s6_addr32[0])) || > + !nfs_append_int(key, _len, ntohl(sin6->sin6_addr.s6_addr32[1])) || > + !nfs_append_int(key, _len, ntohl(sin6->sin6_addr.s6_addr32[2])) || > + !nfs_append_int(key, _len, ntohl(sin6->sin6_addr.s6_addr32[3]))) > return false; > return true; > > > IMO it's not worth versioning in this case but I agree with this change. Did someone report the "cache copied between architectures" issue, or is that mostly a theoretical problem you noticed? Acked-by: Dave Wysochanski <dwysocha@redhat.com>
David Wysochanski <dwysocha@redhat.com> wrote: > Did someone report the "cache copied between architectures" issue, or > is that mostly a theoretical problem you noticed? Ah, I didn't mention. Checker noticed it (make C=1). David
diff --git a/fs/nfs/fscache.c b/fs/nfs/fscache.c index f73c09a9cf0a..0e5572b192b2 100644 --- a/fs/nfs/fscache.c +++ b/fs/nfs/fscache.c @@ -54,17 +54,17 @@ static bool nfs_fscache_get_client_key(struct nfs_client *clp, switch (clp->cl_addr.ss_family) { case AF_INET: - if (!nfs_append_int(key, _len, sin->sin_port) || - !nfs_append_int(key, _len, sin->sin_addr.s_addr)) + if (!nfs_append_int(key, _len, ntohs(sin->sin_port)) || + !nfs_append_int(key, _len, ntohl(sin->sin_addr.s_addr))) return false; return true; case AF_INET6: - if (!nfs_append_int(key, _len, sin6->sin6_port) || - !nfs_append_int(key, _len, sin6->sin6_addr.s6_addr32[0]) || - !nfs_append_int(key, _len, sin6->sin6_addr.s6_addr32[1]) || - !nfs_append_int(key, _len, sin6->sin6_addr.s6_addr32[2]) || - !nfs_append_int(key, _len, sin6->sin6_addr.s6_addr32[3])) + if (!nfs_append_int(key, _len, ntohs(sin6->sin6_port)) || + !nfs_append_int(key, _len, ntohl(sin6->sin6_addr.s6_addr32[0])) || + !nfs_append_int(key, _len, ntohl(sin6->sin6_addr.s6_addr32[1])) || + !nfs_append_int(key, _len, ntohl(sin6->sin6_addr.s6_addr32[2])) || + !nfs_append_int(key, _len, ntohl(sin6->sin6_addr.s6_addr32[3]))) return false; return true;
Fix fscache volume key rendering for endianness. Convert the BE numbers in the address to host-endian before printing them so that they're consistent if the cache is copied between architectures. Question: This change could lead to misidentification of a volume directory in the cache on a LE machine (it's unlikely because the port number as well as the address numbers all get flipped), but it was introduced in -rc1 in this cycle so probably isn't in any distro kernels yet. Should I add a version number to enforce non-matching? Signed-off-by: David Howells <dhowells@redhat.com> cc: Dave Wysochanski <dwysocha@redhat.com> cc: Trond Myklebust <trond.myklebust@hammerspace.com> cc: Anna Schumaker <anna@kernel.org> cc: Jeff Layton <jlayton@kernel.org> cc: linux-nfs@vger.kernel.org cc: linux-cachefs@redhat.com --- fs/nfs/fscache.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)