Skip to contentSkip to conversation

netfilter: ipset: Fix an error code in ip_set_sockfn_get()

PATCH Superseded Dan Carpenter 2 participants 5 messages Aug 21, 2019 – Aug 27, 2019

Subsystems (3)
Networking Kernel janitors netfilter
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 netfilter: ipset: Fix an error code in ip_set_sockfn_get()

The copy_to_user() function returns the number of bytes remaining to be copied. In this code, that positive return is checked at the end of the function and we return zero/success. What we should do instead is return -EFAULT.

Fixes a7b4f989a629 ("netfilter: ipset: IP set core support")
Signed-off-by Dan Carpenter <[email protected]>
---
 net/netfilter/ipset/ip_set_core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
net/netfilter/ipset/ip_set_core.c +2 -1
index e64d5f9a89dd..15b8d4318207 100644
@@ -2129,7 +2129,8 @@ ip_set_sockfn_get(struct sock *sk, int optval, void __user *user, int *len)
 	}	/* end of switch(op) */
 
 copy:
-	ret = copy_to_user(user, data, copylen);
+	if (copy_to_user(user, data, copylen))
+		ret = -EFAULT;
 
 done:
 	vfree(data);
Signature · 1 line
2.20.1
#1 lore raw

Hi Dan,

Quoted 4 lines · Dan Carpenter

The copy_to_user() function returns the number of bytes remaining to be copied. In this code, that positive return is checked at the end of the function and we return zero/success. What we should do instead is return -EFAULT.

Yes, you are right. There's another usage of copy_to_user() in this function, could you fix it as well?

Best regards,
Jozsef

Quoted 24 lines
Fixes a7b4f989a629 ("netfilter: ipset: IP set core support")
Signed-off-by Dan Carpenter <[email protected]>
---
 net/netfilter/ipset/ip_set_core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
net/netfilter/ipset/ip_set_core.c +2 -1
index e64d5f9a89dd..15b8d4318207 100644
@@ -2129,7 +2129,8 @@ ip_set_sockfn_get(struct sock *sk, int optval, void __user *user, int *len)
 	}	/* end of switch(op) */
 
 copy:
-	ret = copy_to_user(user, data, copylen);
+	if (copy_to_user(user, data, copylen))
+		ret = -EFAULT;
 
 done:
 	vfree(data);
Signature · 1 line
2.20.1

-
E-mail : [email protected], [email protected] PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences

          H-1525 Budapest 114, POB. 49, Hungary
author
#2 lore raw
Quoted 4 lines · Kadlecsik József

Hi Dan,

On Wed, 21 Aug 2019, Dan Carpenter wrote:

Quoted 8 lines
Quoted 4 lines

The copy_to_user() function returns the number of bytes remaining to be copied. In this code, that positive return is checked at the end of the function and we return zero/success. What we should do instead is return -EFAULT.

Yes, you are right. There's another usage of copy_to_user() in this function, could you fix it as well?

Yes, of course. Thanks for the review.

regards,
dan carpenter

author
#3 lore raw

PATCH v2 1/1 netfilter: ipset: Fix an error code in ip_set_sockfn_get()

In reply to Kadlecsik József

The copy_to_user() function returns the number of bytes remaining to be copied. In this code, that positive return is checked at the end of the function and we return zero/success. What we should do instead is return -EFAULT.

Fixes a7b4f989a629 ("netfilter: ipset: IP set core support")
Signed-off-by Dan Carpenter <[email protected]>

---
v2: I missed the other instance of this issue

 net/netfilter/ipset/ip_set_core.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
net/netfilter/ipset/ip_set_core.c +5 -3
index e64d5f9a89dd..e7288eab7512 100644
@@ -2069,8 +2069,9 @@ ip_set_sockfn_get(struct sock *sk, int optval, void __user *user, int *len)
 		}
 
 		req_version->version = IPSET_PROTOCOL;
-		ret = copy_to_user(user, req_version,
-				   sizeof(struct ip_set_req_version));
+		if (copy_to_user(user, req_version,
+				 sizeof(struct ip_set_req_version)))
+			ret = -EFAULT;
 		goto done;
 	}
 	case IP_SET_OP_GET_BYNAME: {

@@ -2129,7 +2130,8 @@ ip_set_sockfn_get(struct sock *sk, int optval, void __user *user, int *len)
 	}	/* end of switch(op) */
 
 copy:
-	ret = copy_to_user(user, data, copylen);
+	if (copy_to_user(user, data, copylen))
+		ret = -EFAULT;
 
 done:
 	vfree(data);
Signature · 1 line
2.11.0
#4 lore raw

Hi Dan,

Quoted 5 lines · Dan Carpenter

The copy_to_user() function returns the number of bytes remaining to be copied. In this code, that positive return is checked at the end of the function and we return zero/success. What we should do instead is return -EFAULT.

Quoted 7 lines
Fixes a7b4f989a629 ("netfilter: ipset: IP set core support")
Signed-off-by Dan Carpenter <[email protected]>

---
v2: I missed the other instance of this issue

 net/netfilter/ipset/ip_set_core.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Patch is applied in the ipset git tree, thanks!

Best regards,
Jozsef

Quoted 30 lines
net/netfilter/ipset/ip_set_core.c +5 -3
index e64d5f9a89dd..e7288eab7512 100644
@@ -2069,8 +2069,9 @@ ip_set_sockfn_get(struct sock *sk, int optval, void __user *user, int *len)
 		}
 
 		req_version->version = IPSET_PROTOCOL;
-		ret = copy_to_user(user, req_version,
-				   sizeof(struct ip_set_req_version));
+		if (copy_to_user(user, req_version,
+				 sizeof(struct ip_set_req_version)))
+			ret = -EFAULT;
 		goto done;
 	}
 	case IP_SET_OP_GET_BYNAME: {

@@ -2129,7 +2130,8 @@ ip_set_sockfn_get(struct sock *sk, int optval, void __user *user, int *len)
 	}	/* end of switch(op) */
 
 copy:
-	ret = copy_to_user(user, data, copylen);
+	if (copy_to_user(user, data, copylen))
+		ret = -EFAULT;
 
 done:
 	vfree(data);
Signature · 1 line
2.11.0

-
E-mail : [email protected], [email protected] PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences

          H-1525 Budapest 114, POB. 49, Hungary