* [PATCH 0/3] NFSD copy offload fixes
@ 2019-12-04 20:13 Olga Kornievskaia
2019-12-04 20:13 ` [PATCH 1/3] NFSD fix mismatching type in nfsd4_set_netaddr Olga Kornievskaia
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Olga Kornievskaia @ 2019-12-04 20:13 UTC (permalink / raw)
To: bfields; +Cc: linux-nfs
A few fixes that were found with kbuild and static checked.
Mismatch of __be32 u32 assignments
Fix error path leading to null pointer dereferences
Olga Kornievskaia (3):
NFSD fix mismatching type in nfsd4_set_netaddr
NFSD fix nfserro errno mismatch
NFSD fixing possible null pointer derefering in copy offload
fs/nfsd/nfs4proc.c | 19 ++++++++++---------
fs/nfsd/nfsd.h | 2 +-
2 files changed, 11 insertions(+), 10 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/3] NFSD fix mismatching type in nfsd4_set_netaddr 2019-12-04 20:13 [PATCH 0/3] NFSD copy offload fixes Olga Kornievskaia @ 2019-12-04 20:13 ` Olga Kornievskaia 2019-12-04 20:13 ` [PATCH 2/3] NFSD fix nfserro errno mismatch Olga Kornievskaia 2019-12-04 20:13 ` [PATCH 3/3] NFSD fixing possible null pointer derefering in copy offload Olga Kornievskaia 2 siblings, 0 replies; 6+ messages in thread From: Olga Kornievskaia @ 2019-12-04 20:13 UTC (permalink / raw) To: bfields; +Cc: linux-nfs Fix __be32 and u32 mismatch in return and assignment. Reported-by: kbuild test robot <lkp@intel.com> Fixes: dbd4c2dd8f13 ("NFSD add COPY_NOTIFY operation") Signed-off-by: Olga Kornievskaia <kolga@netapp.com> --- fs/nfsd/nfsd.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/nfsd/nfsd.h b/fs/nfsd/nfsd.h index 0ff6ef9..c679afd 100644 --- a/fs/nfsd/nfsd.h +++ b/fs/nfsd/nfsd.h @@ -388,7 +388,7 @@ static inline bool nfsd4_spo_must_allow(struct svc_rqst *rqstp) extern const u32 nfsd_suppattrs[3][3]; -static inline u32 nfsd4_set_netaddr(struct sockaddr *addr, +static inline __be32 nfsd4_set_netaddr(struct sockaddr *addr, struct nfs42_netaddr *netaddr) { struct sockaddr_in *sin = (struct sockaddr_in *)addr; -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] NFSD fix nfserro errno mismatch 2019-12-04 20:13 [PATCH 0/3] NFSD copy offload fixes Olga Kornievskaia 2019-12-04 20:13 ` [PATCH 1/3] NFSD fix mismatching type in nfsd4_set_netaddr Olga Kornievskaia @ 2019-12-04 20:13 ` Olga Kornievskaia 2019-12-05 21:39 ` J. Bruce Fields 2019-12-04 20:13 ` [PATCH 3/3] NFSD fixing possible null pointer derefering in copy offload Olga Kornievskaia 2 siblings, 1 reply; 6+ messages in thread From: Olga Kornievskaia @ 2019-12-04 20:13 UTC (permalink / raw) To: bfields; +Cc: linux-nfs There is mismatch between __be32 and u32 in nfserr and errno. Reported-by: kbuild test robot <lkp@intel.com> Fixes: d5e54eeb0e3d ("NFSD add nfs4 inter ssc to nfsd4_copy") Signed-off-by: Olga Kornievskaia <kolga@netapp.com> --- fs/nfsd/nfs4proc.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index ec4f79c8..187cef6 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -1169,7 +1169,8 @@ extern struct file *nfs42_ssc_open(struct vfsmount *ss_mnt, size_t tmp_addrlen, match_netid_len = 3; char *startsep = "", *endsep = "", *match_netid = "tcp"; char *ipaddr, *dev_name, *raw_data; - int len, raw_len, status = -EINVAL; + int len, raw_len; + __be32 status = nfserr_inval; naddr = &nss->u.nl4_addr; tmp_addrlen = rpc_uaddr2sockaddr(SVC_NET(rqstp), naddr->addr, @@ -1207,7 +1208,7 @@ extern struct file *nfs42_ssc_open(struct vfsmount *ss_mnt, snprintf(raw_data, raw_len, NFSD42_INTERSSC_MOUNTOPS, ipaddr); - status = -ENODEV; + status = nfserr_nodev; type = get_fs_type("nfs"); if (!type) goto out_free_rawdata; @@ -1253,8 +1254,6 @@ extern struct file *nfs42_ssc_open(struct vfsmount *ss_mnt, * Called with COPY cstate: * SAVED_FH: source filehandle * CURRENT_FH: destination filehandle - * - * Returns errno (not nfserrxxx) */ static __be32 nfsd4_setup_inter_ssc(struct svc_rqst *rqstp, @@ -1263,7 +1262,7 @@ extern struct file *nfs42_ssc_open(struct vfsmount *ss_mnt, { struct svc_fh *s_fh = NULL; stateid_t *s_stid = ©->cp_src_stateid; - __be32 status = -EINVAL; + __be32 status = nfserr_inval; /* Verify the destination stateid and set dst struct file*/ status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->current_fh, @@ -1280,7 +1279,7 @@ extern struct file *nfs42_ssc_open(struct vfsmount *ss_mnt, copy->c_fh.size = s_fh->fh_handle.fh_size; memcpy(copy->c_fh.data, &s_fh->fh_handle.fh_base, copy->c_fh.size); - copy->stateid.seqid = s_stid->si_generation; + copy->stateid.seqid = cpu_to_be32(s_stid->si_generation); memcpy(copy->stateid.other, (void *)&s_stid->si_opaque, sizeof(stateid_opaque_t)); @@ -1308,7 +1307,7 @@ extern struct file *nfs42_ssc_open(struct vfsmount *ss_mnt, struct vfsmount **mount) { *mount = NULL; - return -EINVAL; + return nfserr_inval; } static void -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] NFSD fix nfserro errno mismatch 2019-12-04 20:13 ` [PATCH 2/3] NFSD fix nfserro errno mismatch Olga Kornievskaia @ 2019-12-05 21:39 ` J. Bruce Fields 2019-12-05 21:43 ` J. Bruce Fields 0 siblings, 1 reply; 6+ messages in thread From: J. Bruce Fields @ 2019-12-05 21:39 UTC (permalink / raw) To: Olga Kornievskaia; +Cc: bfields, linux-nfs On Wed, Dec 04, 2019 at 03:13:53PM -0500, Olga Kornievskaia wrote: > There is mismatch between __be32 and u32 in nfserr and errno. > ... > @@ -1280,7 +1279,7 @@ extern struct file *nfs42_ssc_open(struct vfsmount *ss_mnt, > > copy->c_fh.size = s_fh->fh_handle.fh_size; > memcpy(copy->c_fh.data, &s_fh->fh_handle.fh_base, copy->c_fh.size); > - copy->stateid.seqid = s_stid->si_generation; > + copy->stateid.seqid = cpu_to_be32(s_stid->si_generation); This one isn't an errno, and should really be its own patch. I've split it out as follows.--b. commit a1f3cb8bb088 Author: Olga Kornievskaia <olga.kornievskaia@gmail.com> Date: Wed Dec 4 15:13:53 2019 -0500 NFSD: fix seqid in copy stateid s_stid->si_generation is a u32, copy->stateid.seqid is a __be32, so we should be byte-swapping here if necessary. This effectively undoes the byte-swap performed when reading s_stid->s_generation in nfsd4_decode_copy(). Without this second swap, the stateid we sent to the source in READ could be different from the one the client provided us in the COPY. We didn't spot this in testing since our implementation always uses a 0 in the seqid field. But other implementations might not do that. You'd think we should just skip the byte-swapping entirely, but the s_stid field can be used for either our own stateids (in the intra-server case) or foreign stateids (in the inter-server case), and the former are interpreted by us and need byte-swapping. Reported-by: kbuild test robot <lkp@intel.com> Fixes: d5e54eeb0e3d ("NFSD add nfs4 inter ssc to nfsd4_copy") Signed-off-by: Olga Kornievskaia <kolga@netapp.com> Signed-off-by: J. Bruce Fields <bfields@redhat.com> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index ec4f79c8f71e..9a8debc0d725 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -1280,7 +1280,7 @@ nfsd4_setup_inter_ssc(struct svc_rqst *rqstp, copy->c_fh.size = s_fh->fh_handle.fh_size; memcpy(copy->c_fh.data, &s_fh->fh_handle.fh_base, copy->c_fh.size); - copy->stateid.seqid = s_stid->si_generation; + copy->stateid.seqid = cpu_to_be32(s_stid->si_generation); memcpy(copy->stateid.other, (void *)&s_stid->si_opaque, sizeof(stateid_opaque_t)); ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] NFSD fix nfserro errno mismatch 2019-12-05 21:39 ` J. Bruce Fields @ 2019-12-05 21:43 ` J. Bruce Fields 0 siblings, 0 replies; 6+ messages in thread From: J. Bruce Fields @ 2019-12-05 21:43 UTC (permalink / raw) To: Olga Kornievskaia; +Cc: bfields, linux-nfs On Thu, Dec 05, 2019 at 04:39:30PM -0500, bfields wrote: > On Wed, Dec 04, 2019 at 03:13:53PM -0500, Olga Kornievskaia wrote: > > There is mismatch between __be32 and u32 in nfserr and errno. > > > ... > > @@ -1280,7 +1279,7 @@ extern struct file *nfs42_ssc_open(struct vfsmount *ss_mnt, > > > > copy->c_fh.size = s_fh->fh_handle.fh_size; > > memcpy(copy->c_fh.data, &s_fh->fh_handle.fh_base, copy->c_fh.size); > > - copy->stateid.seqid = s_stid->si_generation; > > + copy->stateid.seqid = cpu_to_be32(s_stid->si_generation); > > This one isn't an errno, and should really be its own patch. I've split > it out as follows.--b. (And applied the others, thanks.) > > commit a1f3cb8bb088 > Author: Olga Kornievskaia <olga.kornievskaia@gmail.com> > Date: Wed Dec 4 15:13:53 2019 -0500 > > NFSD: fix seqid in copy stateid > > s_stid->si_generation is a u32, copy->stateid.seqid is a __be32, so we > should be byte-swapping here if necessary. > > This effectively undoes the byte-swap performed when reading > s_stid->s_generation in nfsd4_decode_copy(). Without this second swap, > the stateid we sent to the source in READ could be different from the > one the client provided us in the COPY. We didn't spot this in testing > since our implementation always uses a 0 in the seqid field. But other > implementations might not do that. > > You'd think we should just skip the byte-swapping entirely, but the > s_stid field can be used for either our own stateids (in the > intra-server case) or foreign stateids (in the inter-server case), and > the former are interpreted by us and need byte-swapping. > > Reported-by: kbuild test robot <lkp@intel.com> > Fixes: d5e54eeb0e3d ("NFSD add nfs4 inter ssc to nfsd4_copy") > Signed-off-by: Olga Kornievskaia <kolga@netapp.com> > Signed-off-by: J. Bruce Fields <bfields@redhat.com> > > diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c > index ec4f79c8f71e..9a8debc0d725 100644 > --- a/fs/nfsd/nfs4proc.c > +++ b/fs/nfsd/nfs4proc.c > @@ -1280,7 +1280,7 @@ nfsd4_setup_inter_ssc(struct svc_rqst *rqstp, > > copy->c_fh.size = s_fh->fh_handle.fh_size; > memcpy(copy->c_fh.data, &s_fh->fh_handle.fh_base, copy->c_fh.size); > - copy->stateid.seqid = s_stid->si_generation; > + copy->stateid.seqid = cpu_to_be32(s_stid->si_generation); > memcpy(copy->stateid.other, (void *)&s_stid->si_opaque, > sizeof(stateid_opaque_t)); > ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/3] NFSD fixing possible null pointer derefering in copy offload 2019-12-04 20:13 [PATCH 0/3] NFSD copy offload fixes Olga Kornievskaia 2019-12-04 20:13 ` [PATCH 1/3] NFSD fix mismatching type in nfsd4_set_netaddr Olga Kornievskaia 2019-12-04 20:13 ` [PATCH 2/3] NFSD fix nfserro errno mismatch Olga Kornievskaia @ 2019-12-04 20:13 ` Olga Kornievskaia 2 siblings, 0 replies; 6+ messages in thread From: Olga Kornievskaia @ 2019-12-04 20:13 UTC (permalink / raw) To: bfields; +Cc: linux-nfs Static checker revealed possible error path leading to possible NULL pointer dereferencing. Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Fixes: e0639dc5805a: ("NFSD introduce async copy feature") Signed-off-by: Olga Kornievskaia <kolga@netapp.com> --- fs/nfsd/nfs4proc.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c index 187cef6..d33c39c 100644 --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -1446,7 +1446,8 @@ static void cleanup_async_copy(struct nfsd4_copy *copy) { nfs4_free_copy_state(copy); nfsd_file_put(copy->nf_dst); - nfsd_file_put(copy->nf_src); + if (copy->cp_intra) + nfsd_file_put(copy->nf_src); spin_lock(©->cp_clp->async_lock); list_del(©->copies); spin_unlock(©->cp_clp->async_lock); @@ -1551,7 +1552,8 @@ static int nfsd4_do_async_copy(void *data) out: return status; out_err: - cleanup_async_copy(async_copy); + if (async_copy) + cleanup_async_copy(async_copy); status = nfserrno(-ENOMEM); if (!copy->cp_intra) nfsd4_interssc_disconnect(copy->ss_mnt); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-12-05 21:43 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-04 20:13 [PATCH 0/3] NFSD copy offload fixes Olga Kornievskaia 2019-12-04 20:13 ` [PATCH 1/3] NFSD fix mismatching type in nfsd4_set_netaddr Olga Kornievskaia 2019-12-04 20:13 ` [PATCH 2/3] NFSD fix nfserro errno mismatch Olga Kornievskaia 2019-12-05 21:39 ` J. Bruce Fields 2019-12-05 21:43 ` J. Bruce Fields 2019-12-04 20:13 ` [PATCH 3/3] NFSD fixing possible null pointer derefering in copy offload Olga Kornievskaia
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).