Skip to contentSkip to conversation

nfsd: fix return of nfs4_acl_write_who

PATCH In review Jeff Layton 4 participants 4 messages Jun 17, 2014 – Jun 18, 2014

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 nfsd: fix return of nfs4_acl_write_who

AFAICT, the only way to hit this error is to pass this function a bogus "who" value. In that case, we probably don't want to return -1 as that could get sent back to the client. Turn this into nfserr_serverfault, which is a more appropriate error for a server bug like this.

---
 fs/nfsd/nfs4acl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/nfsd/nfs4acl.c +1 -1
index d714156a19fd..b0cf00d3ee7d 100644
@@ -935,5 +935,5 @@ __be32 nfs4_acl_write_who(struct xdr_stream *xdr, int who)
 		return 0;
 	}
 	WARN_ON_ONCE(1);
-	return -1;
+	return nfserr_serverfault;
 }
Signature · 1 line
1.9.3
Reviewed-by on v1 1/1
#1 lore raw
Quoted 6 lines · Jeff Layton

AFAICT, the only way to hit this error is to pass this function a bogus "who" value. In that case, we probably don't want to return -1 as that could get sent back to the client. Turn this into nfserr_serverfault, which is a more appropriate error for a server bug like this.

Looks good,

#2 lore raw
In reply to Jeff Layton
Quoted 4 lines · Jeff Layton

AFAICT, the only way to hit this error is to pass this function a bogus "who" value. In that case, we probably don't want to return -1 as that could get sent back to the client. Turn this into nfserr_serverfault, which is a more appropriate error for a server bug like this.

I don't really care since there shouldn't be any way to hit this, but OK, applying for 3.17.

--b.

Quoted 20 lines
---
 fs/nfsd/nfs4acl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/nfsd/nfs4acl.c +1 -1
index d714156a19fd..b0cf00d3ee7d 100644
@@ -935,5 +935,5 @@ __be32 nfs4_acl_write_who(struct xdr_stream *xdr, int who)
 		return 0;
 	}
 	WARN_ON_ONCE(1);
-	return -1;
+	return nfserr_serverfault;
 }
Signature · 1 line
1.9.3
#3 lore raw
Quoted 11 lines · On Wed, 18 Jun 2014 11:42:04 -0400 "J. Bruce Fields" <[email protected]>…
Quoted 4 lines · Jeff Layton

AFAICT, the only way to hit this error is to pass this function a bogus "who" value. In that case, we probably don't want to return -1 as that could get sent back to the client. Turn this into nfserr_serverfault, which is a more appropriate error for a server bug like this.

I don't really care since there shouldn't be any way to hit this, but OK, applying for 3.17.

--b.

Thanks. Yeah, it's pretty unlikely...

I saw a sparse warning from that and silencing it was the main impetus for the patch. I guess we can also call it "future proofing". In the event that some other bug ends causing this function to get passed a bogus "who" arg, then I think we'll stand a better chance of noticing it with this change.

Quoted 20 lines
Quoted 20 lines
---
 fs/nfsd/nfs4acl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/nfsd/nfs4acl.c +1 -1
index d714156a19fd..b0cf00d3ee7d 100644
@@ -935,5 +935,5 @@ __be32 nfs4_acl_write_who(struct xdr_stream *xdr, int who)
 		return 0;
 	}
 	WARN_ON_ONCE(1);
-	return -1;
+	return nfserr_serverfault;
 }
Signature · 1 line
1.9.3
Signature · 1 line
Jeff Layton <[email protected]>