diff mbox

socket: Merge getsockname and getpeername into a single function

Message ID 20100304032612.GB9016@home.unix.ba
State Rejected, archived
Delegated to: David Miller
Headers show

Commit Message

Kenan Kalajdzic March 4, 2010, 3:26 a.m. UTC 
The code of getsockname is almost identical to getpeername. This patch removes
duplicate code and merges both functions into a single common function.

Signed-off-by: Kenan Kalajdzic <kenan@unix.ba>
---
 net/socket.c |   50 +++++++++++++++++---------------------------------
 1 files changed, 17 insertions(+), 33 deletions(-)

Comments

David Miller March 8, 2010, 8:24 p.m. UTC | #1 
From: Kenan Kalajdzic <kenan@unix.ba>
Date: Thu, 4 Mar 2010 04:26:13 +0100

> The code of getsockname is almost identical to getpeername. This patch removes
> duplicate code and merges both functions into a single common function.
> 
> Signed-off-by: Kenan Kalajdzic <kenan@unix.ba>

Since you still have to conditionalize things like the security
calls, this doesn't look any cleaner to me than what we have
there already.

I'm not applying this, sorry.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kenan Kalajdzic March 9, 2010, 3:30 p.m. UTC | #2 
On Mon, Mar 8, 2010 at 9:24 PM, David Miller <davem@davemloft.net> wrote:
>
> Since you still have to conditionalize things like the security
> calls, this doesn't look any cleaner to me than what we have
> there already.

You are absolutely right about this - the condition around security
calls ruins the whole refactoring attempt.  It could easily go away if
we were ready to make minor changes to the security code.  It is
because getting the local socket info and getting the peer socket info
are essentially the same type of operation.  Therefore, both
security_socket_getsockname() and security_socket_getpeername()
eventually evaluate to a call to socket_has_perm() with
SOCKET__GETATTR.  The question remains whether it is worth breaking
the cleanness of the security hooks code to get rid of the condition
in 'our' two functions?
diff mbox

Patch

diff --git a/net/socket.c b/net/socket.c
index 769c386..526e1f5 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1593,12 +1593,12 @@  out:
 }
 
 /*
- *	Get the local address ('name') of a socket object. Move the obtained
- *	name to user space.
+ *  Common code for getsockname and getpeername system calls
+ *  Get local or remote address ('name') of a socket object.
+ *  Move the obtained name to user space.
  */
-
-SYSCALL_DEFINE3(getsockname, int, fd, struct sockaddr __user *, usockaddr,
-		int __user *, usockaddr_len)
+static int common_getsockpeername(int fd, struct sockaddr __user *usockaddr,
+				int __user * usockaddr_len, int remote)
 {
 	struct socket *sock;
 	struct sockaddr_storage address;
@@ -1608,50 +1608,34 @@  SYSCALL_DEFINE3(getsockname, int, fd, struct sockaddr __user *, usockaddr,
 	if (!sock)
 		goto out;
 
-	err = security_socket_getsockname(sock);
+	if (remote)
+		err = security_socket_getpeername(sock);
+	else
+		err = security_socket_getsockname(sock);
 	if (err)
 		goto out_put;
 
-	err = sock->ops->getname(sock, (struct sockaddr *)&address, &len, 0);
+	err = sock->ops->getname(sock, (struct sockaddr *)&address, &len, remote);
 	if (err)
 		goto out_put;
-	err = move_addr_to_user((struct sockaddr *)&address, len, usockaddr, usockaddr_len);
 
+	err = move_addr_to_user((struct sockaddr *)&address, len, usockaddr, usockaddr_len);
 out_put:
 	fput_light(sock->file, fput_needed);
 out:
 	return err;
 }
 
-/*
- *	Get the remote address ('name') of a socket object. Move the obtained
- *	name to user space.
- */
+SYSCALL_DEFINE3(getsockname, int, fd, struct sockaddr __user *, usockaddr,
+		int __user *, usockaddr_len)
+{
+	return common_getsockpeername(fd, usockaddr, usockaddr_len, 0);
+}
 
 SYSCALL_DEFINE3(getpeername, int, fd, struct sockaddr __user *, usockaddr,
 		int __user *, usockaddr_len)
 {
-	struct socket *sock;
-	struct sockaddr_storage address;
-	int len, err, fput_needed;
-
-	sock = sockfd_lookup_light(fd, &err, &fput_needed);
-	if (sock != NULL) {
-		err = security_socket_getpeername(sock);
-		if (err) {
-			fput_light(sock->file, fput_needed);
-			return err;
-		}
-
-		err =
-		    sock->ops->getname(sock, (struct sockaddr *)&address, &len,
-				       1);
-		if (!err)
-			err = move_addr_to_user((struct sockaddr *)&address, len, usockaddr,
-						usockaddr_len);
-		fput_light(sock->file, fput_needed);
-	}
-	return err;
+	return common_getsockpeername(fd, usockaddr, usockaddr_len, 1);
 }
 
 /*