Commit ef17af2a authored Dec 05, 2014 by Rasmus Villemoes Committed by J. Bruce Fields Dec 09, 2014

fs: nfsd: Fix signedness bug in compare_blob



Bugs similar to the one in acbbe6fb (kcmp: fix standard comparison
bug) are in rich supply.

In this variant, the problem is that struct xdr_netobj::len has type
unsigned int, so the expression o1->len - o2->len _also_ has type
unsigned int; it has completely well-defined semantics, and the result
is some non-negative integer, which is always representable in a long
long. But this means that if the conditional triggers, we are
guaranteed to return a positive value from compare_blob.

In this case it could be fixed by

-       res = o1->len - o2->len;
+       res = (long long)o1->len - (long long)o2->len;

but I'd rather eliminate the usually broken 'return a - b;' idiom.

Reviewed-by: Jeff Layton <jlayton@primarydata.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Signed-off-by: J. Bruce Fields <bfields@redhat.com>

parent 83a712e0

fs/nfsd/nfs4state.c

+7 −8

Original line number	Diff line number	Diff line
		@@ -1714,15 +1714,14 @@ static int copy_cred(struct svc_cred target, struct svc_cred source)
		return 0;
		}

		static long long
		static int
		compare_blob(const struct xdr_netobj o1, const struct xdr_netobj o2)
		{
		long long res;

		res = o1->len - o2->len;
		if (res)
		return res;
		return (long long)memcmp(o1->data, o2->data, o1->len);
		if (o1->len < o2->len)
		return -1;
		if (o1->len > o2->len)
		return 1;
		return memcmp(o1->data, o2->data, o1->len);
		}

		static int same_name(const char n1, const char n2)
		@@ -1910,7 +1909,7 @@ add_clp_to_name_tree(struct nfs4_client new_clp, struct rb_root root)
		static struct nfs4_client *
		find_clp_in_name_tree(struct xdr_netobj name, struct rb_root root)
		{
		long long cmp;
		int cmp;
		struct rb_node *node = root->rb_node;
		struct nfs4_client *clp;

Admin message