[RFC] LSM hook for post recvmsg.

[RFC] LSM hook for post recvmsg.

[Tetsuo Handa]

Hello, David. Thank you for giving me suggestions at Japan Linux Symposium 2009.
As TOMOYO is getting functional and AppArmor is about to join mainline, I'd like
to resume discussions regarding LSM hooks for post accept()/recvmsg() operations.

Below is a patch for post recvmsg() operation. I modified the patch to call
skb_recv_datagram() again (for udp_recvmsg(), raw_recvmsg(), udpv6_recvmsg())
if LSM dicided to drop the message. (Regarding rawv6_recvmsg(), I didn't do so
in accordance with the comment at "csum_copy_err:".)
What do you think about this verion?

Regards.

diff --git a/include/linux/security.h b/include/linux/security.h
index 723a93d..409c44d 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -879,6 +879,12 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
  *	@size contains the size of message structure.
  *	@flags contains the operational flags.
  *	Return 0 if permission is granted.
+ * @socket_post_recvmsg:
+ *	Check permission after receiving a message from a socket.
+ *	The message is discarded if permission is not granted.
+ *	@sk contains the sock structure.
+ *	@skb contains the sk_buff structure.
+ *	Return 0 if permission is granted.
  * @socket_getsockname:
  *	Check permission before the local address (name) of the socket object
  *	@sock is retrieved.
@@ -1575,6 +1581,7 @@ struct security_operations {
 			       struct msghdr *msg, int size);
 	int (*socket_recvmsg) (struct socket *sock,
 			       struct msghdr *msg, int size, int flags);
+	int (*socket_post_recvmsg) (struct sock *sk, struct sk_buff *skb);
 	int (*socket_getsockname) (struct socket *sock);
 	int (*socket_getpeername) (struct socket *sock);
 	int (*socket_getsockopt) (struct socket *sock, int level, int optname);
@@ -2526,6 +2533,7 @@ int security_socket_accept(struct socket *sock, struct socket *newsock);
 int security_socket_sendmsg(struct socket *sock, struct msghdr *msg, int size);
 int security_socket_recvmsg(struct socket *sock, struct msghdr *msg,
 			    int size, int flags);
+int security_socket_post_recvmsg(struct sock *sk, struct sk_buff *skb);
 int security_socket_getsockname(struct socket *sock);
 int security_socket_getpeername(struct socket *sock);
 int security_socket_getsockopt(struct socket *sock, int level, int optname);
@@ -2617,6 +2625,12 @@ static inline int security_socket_recvmsg(struct socket *sock,
 	return 0;
 }
 
+static inline int security_socket_post_recvmsg(struct sock *sk,
+					       struct sk_buff *skb)
+{
+	return 0;
+}
+
 static inline int security_socket_getsockname(struct socket *sock)
 {
 	return 0;
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 2c7a163..69652d4 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -676,9 +676,15 @@ static int raw_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 		goto out;
 	}
 
-	skb = skb_recv_datagram(sk, flags, noblock, &err);
-	if (!skb)
-		goto out;
+	for (;;) {
+		skb = skb_recv_datagram(sk, flags, noblock, &err);
+		if (!skb)
+			goto out;
+		err = security_socket_post_recvmsg(sk, skb);
+		if (likely(!err))
+			break;
+		skb_kill_datagram(sk, skb, flags);
+	}
 
 	copied = skb->len;
 	if (len < copied) {
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 5858574..9145685 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1125,6 +1125,7 @@ int udp_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 	int err;
 	int is_udplite = IS_UDPLITE(sk);
 	bool slow;
+	bool update_stat;
 
 	/*
 	 *	Check any passed addresses
@@ -1140,6 +1141,12 @@ try_again:
 				  &peeked, &err);
 	if (!skb)
 		goto out;
+	err = security_socket_post_recvmsg(sk, skb);
+	if (err) {
+		update_stat = false;
+		goto csum_copy_err;
+	}
+	update_stat = true;
 
 	ulen = skb->len - sizeof(struct udphdr);
 	if (len > ulen)
@@ -1200,7 +1207,7 @@ out:
 
 csum_copy_err:
 	slow = lock_sock_fast(sk);
-	if (!skb_kill_datagram(sk, skb, flags))
+	if (!skb_kill_datagram(sk, skb, flags) && update_stat)
 		UDP_INC_STATS_USER(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
 	unlock_sock_fast(sk, slow);
 
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index 4a4dcbe..135d4ed 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -467,6 +467,9 @@ static int rawv6_recvmsg(struct kiocb *iocb, struct sock *sk,
 	skb = skb_recv_datagram(sk, flags, noblock, &err);
 	if (!skb)
 		goto out;
+	err = security_socket_post_recvmsg(sk, skb);
+	if (unlikely(err))
+		goto csum_copy_err;
 
 	copied = skb->len;
 	if (copied > len) {
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 87be586..6cae276 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -329,6 +329,7 @@ int udpv6_recvmsg(struct kiocb *iocb, struct sock *sk,
 	int is_udplite = IS_UDPLITE(sk);
 	int is_udp4;
 	bool slow;
+	bool update_stat;
 
 	if (addr_len)
 		*addr_len=sizeof(struct sockaddr_in6);
@@ -344,6 +345,12 @@ try_again:
 				  &peeked, &err);
 	if (!skb)
 		goto out;
+	err = security_socket_post_recvmsg(sk, skb);
+	if (err) {
+		update_stat = false;
+		goto csum_copy_err;
+	}
+	update_stat = true;
 
 	ulen = skb->len - sizeof(struct udphdr);
 	if (len > ulen)
@@ -426,7 +433,7 @@ out:
 
 csum_copy_err:
 	slow = lock_sock_fast(sk);
-	if (!skb_kill_datagram(sk, skb, flags)) {
+	if (!skb_kill_datagram(sk, skb, flags) && update_stat) {
 		if (is_udp4)
 			UDP_INC_STATS_USER(sock_net(sk),
 					UDP_MIB_INERRORS, is_udplite);
diff --git a/security/capability.c b/security/capability.c
index 4aeb699..709aea3 100644
--- a/security/capability.c
+++ b/security/capability.c
@@ -597,6 +597,11 @@ static int cap_socket_recvmsg(struct socket *sock, struct msghdr *msg,
 	return 0;
 }
 
+static int cap_socket_post_recvmsg(struct sock *sk, struct sk_buff *skb)
+{
+	return 0;
+}
+
 static int cap_socket_getsockname(struct socket *sock)
 {
 	return 0;
@@ -1001,6 +1006,7 @@ void __init security_fixup_ops(struct security_operations *ops)
 	set_to_cap_if_null(ops, socket_accept);
 	set_to_cap_if_null(ops, socket_sendmsg);
 	set_to_cap_if_null(ops, socket_recvmsg);
+	set_to_cap_if_null(ops, socket_post_recvmsg);
 	set_to_cap_if_null(ops, socket_getsockname);
 	set_to_cap_if_null(ops, socket_getpeername);
 	set_to_cap_if_null(ops, socket_setsockopt);
diff --git a/security/security.c b/security/security.c
index e8c87b8..4291bd7 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1037,6 +1037,12 @@ int security_socket_recvmsg(struct socket *sock, struct msghdr *msg,
 	return security_ops->socket_recvmsg(sock, msg, size, flags);
 }
 
+int security_socket_post_recvmsg(struct sock *sk, struct sk_buff *skb)
+{
+	return security_ops->socket_post_recvmsg(sk, skb);
+}
+EXPORT_SYMBOL(security_socket_post_recvmsg);
+
 int security_socket_getsockname(struct socket *sock)
 {
 	return security_ops->socket_getsockname(sock);

Re: [RFC] LSM hook for post recvmsg.

[David Miller]

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Sat, 17 Jul 2010 01:14:38 +0900

> Below is a patch for post recvmsg() operation. I modified the patch to call
> skb_recv_datagram() again (for udp_recvmsg(), raw_recvmsg(), udpv6_recvmsg())
> if LSM dicided to drop the message. (Regarding rawv6_recvmsg(), I didn't do so
> in accordance with the comment at "csum_copy_err:".)
> What do you think about this verion?

This looks fine, but regardless of that comment I think the IPV6 raw recvmsg()
should loop just as the IPV4 one does in your patch.

[PATCH] LSM: Add post recvmsg() hook.

[Tetsuo Handa]

David Miller wrote:
> From: Tetsuo Handa
> Date: Sat, 17 Jul 2010 01:14:38 +0900
> 
> > Below is a patch for post recvmsg() operation. I modified the patch to call
> > skb_recv_datagram() again (for udp_recvmsg(), raw_recvmsg(), udpv6_recvmsg())
> > if LSM dicided to drop the message. (Regarding rawv6_recvmsg(), I didn't do so
> > in accordance with the comment at "csum_copy_err:".)
> > What do you think about this verion?
> 
> This looks fine, but regardless of that comment I think the IPV6 raw recvmsg()
> should loop just as the IPV4 one does in your patch.
> 
Thank you, David.
I updated to call skb_recv_datagram() for rawv6_recvmsg() case too.

NETWORKING [IPv4/IPv6] maintainers and Paul, is below patch fine for you?

Regards.
----------------------------------------
>From b43154a90bc7494ec1ee301e692d2bbf29c8f2f8 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Sat, 17 Jul 2010 09:52:38 +0900
Subject: [PATCH] LSM: Add post recvmsg() hook.

Current pre recvmsg hook (i.e. security_socket_recvmsg()) has two problems.

One is that it will cause eating 100% of CPU time if the caller does not
close() the socket when recvmsg() failed due to security_socket_recvmsg(), for
subsequent select() notifies the caller of readiness for recvmsg() since the
datagram which would have been already picked up if security_socket_recvmsg()
did not return error is remaining in the queue.

The other is that it is racy if LSM module wants to do filtering based on
"which process can pick up datagrams from which source" because the process
which picks up the datagram is not known until skb_recv_datagram() and lock
is not held between security_socket_recvmsg() and skb_recv_datagram().

This patch introduces post recvmsg hook (i.e. security_socket_post_recvmsg())
in order to solve above problems at the cost of ability to pick up the datagram
which would have been picked up if preceding security_socket_post_recvmsg() did
not return error.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 include/linux/security.h |   14 ++++++++++++++
 net/ipv4/raw.c           |   12 +++++++++---
 net/ipv4/udp.c           |    9 ++++++++-
 net/ipv6/raw.c           |   12 +++++++++---
 net/ipv6/udp.c           |    9 ++++++++-
 security/capability.c    |    6 ++++++
 security/security.c      |    6 ++++++
 7 files changed, 60 insertions(+), 8 deletions(-)

diff --git a/include/linux/security.h b/include/linux/security.h
index 723a93d..409c44d 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -879,6 +879,12 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
  *	@size contains the size of message structure.
  *	@flags contains the operational flags.
  *	Return 0 if permission is granted.
+ * @socket_post_recvmsg:
+ *	Check permission after receiving a message from a socket.
+ *	The message is discarded if permission is not granted.
+ *	@sk contains the sock structure.
+ *	@skb contains the sk_buff structure.
+ *	Return 0 if permission is granted.
  * @socket_getsockname:
  *	Check permission before the local address (name) of the socket object
  *	@sock is retrieved.
@@ -1575,6 +1581,7 @@ struct security_operations {
 			       struct msghdr *msg, int size);
 	int (*socket_recvmsg) (struct socket *sock,
 			       struct msghdr *msg, int size, int flags);
+	int (*socket_post_recvmsg) (struct sock *sk, struct sk_buff *skb);
 	int (*socket_getsockname) (struct socket *sock);
 	int (*socket_getpeername) (struct socket *sock);
 	int (*socket_getsockopt) (struct socket *sock, int level, int optname);
@@ -2526,6 +2533,7 @@ int security_socket_accept(struct socket *sock, struct socket *newsock);
 int security_socket_sendmsg(struct socket *sock, struct msghdr *msg, int size);
 int security_socket_recvmsg(struct socket *sock, struct msghdr *msg,
 			    int size, int flags);
+int security_socket_post_recvmsg(struct sock *sk, struct sk_buff *skb);
 int security_socket_getsockname(struct socket *sock);
 int security_socket_getpeername(struct socket *sock);
 int security_socket_getsockopt(struct socket *sock, int level, int optname);
@@ -2617,6 +2625,12 @@ static inline int security_socket_recvmsg(struct socket *sock,
 	return 0;
 }
 
+static inline int security_socket_post_recvmsg(struct sock *sk,
+					       struct sk_buff *skb)
+{
+	return 0;
+}
+
 static inline int security_socket_getsockname(struct socket *sock)
 {
 	return 0;
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 2c7a163..69652d4 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -676,9 +676,15 @@ static int raw_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 		goto out;
 	}
 
-	skb = skb_recv_datagram(sk, flags, noblock, &err);
-	if (!skb)
-		goto out;
+	for (;;) {
+		skb = skb_recv_datagram(sk, flags, noblock, &err);
+		if (!skb)
+			goto out;
+		err = security_socket_post_recvmsg(sk, skb);
+		if (likely(!err))
+			break;
+		skb_kill_datagram(sk, skb, flags);
+	}
 
 	copied = skb->len;
 	if (len < copied) {
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 5858574..9145685 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1125,6 +1125,7 @@ int udp_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 	int err;
 	int is_udplite = IS_UDPLITE(sk);
 	bool slow;
+	bool update_stat;
 
 	/*
 	 *	Check any passed addresses
@@ -1140,6 +1141,12 @@ try_again:
 				  &peeked, &err);
 	if (!skb)
 		goto out;
+	err = security_socket_post_recvmsg(sk, skb);
+	if (err) {
+		update_stat = false;
+		goto csum_copy_err;
+	}
+	update_stat = true;
 
 	ulen = skb->len - sizeof(struct udphdr);
 	if (len > ulen)
@@ -1200,7 +1207,7 @@ out:
 
 csum_copy_err:
 	slow = lock_sock_fast(sk);
-	if (!skb_kill_datagram(sk, skb, flags))
+	if (!skb_kill_datagram(sk, skb, flags) && update_stat)
 		UDP_INC_STATS_USER(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
 	unlock_sock_fast(sk, slow);
 
diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
index 4a4dcbe..6915b01 100644
--- a/net/ipv6/raw.c
+++ b/net/ipv6/raw.c
@@ -464,9 +464,15 @@ static int rawv6_recvmsg(struct kiocb *iocb, struct sock *sk,
 	if (np->rxpmtu && np->rxopt.bits.rxpmtu)
 		return ipv6_recv_rxpmtu(sk, msg, len);
 
-	skb = skb_recv_datagram(sk, flags, noblock, &err);
-	if (!skb)
-		goto out;
+	for (;;) {
+		skb = skb_recv_datagram(sk, flags, noblock, &err);
+		if (!skb)
+			goto out;
+		err = security_socket_post_recvmsg(sk, skb);
+		if (likely(!err))
+			break;
+		skb_kill_datagram(sk, skb, flags);
+	}
 
 	copied = skb->len;
 	if (copied > len) {
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 87be586..6cae276 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -329,6 +329,7 @@ int udpv6_recvmsg(struct kiocb *iocb, struct sock *sk,
 	int is_udplite = IS_UDPLITE(sk);
 	int is_udp4;
 	bool slow;
+	bool update_stat;
 
 	if (addr_len)
 		*addr_len=sizeof(struct sockaddr_in6);
@@ -344,6 +345,12 @@ try_again:
 				  &peeked, &err);
 	if (!skb)
 		goto out;
+	err = security_socket_post_recvmsg(sk, skb);
+	if (err) {
+		update_stat = false;
+		goto csum_copy_err;
+	}
+	update_stat = true;
 
 	ulen = skb->len - sizeof(struct udphdr);
 	if (len > ulen)
@@ -426,7 +433,7 @@ out:
 
 csum_copy_err:
 	slow = lock_sock_fast(sk);
-	if (!skb_kill_datagram(sk, skb, flags)) {
+	if (!skb_kill_datagram(sk, skb, flags) && update_stat) {
 		if (is_udp4)
 			UDP_INC_STATS_USER(sock_net(sk),
 					UDP_MIB_INERRORS, is_udplite);
diff --git a/security/capability.c b/security/capability.c
index 4aeb699..709aea3 100644
--- a/security/capability.c
+++ b/security/capability.c
@@ -597,6 +597,11 @@ static int cap_socket_recvmsg(struct socket *sock, struct msghdr *msg,
 	return 0;
 }
 
+static int cap_socket_post_recvmsg(struct sock *sk, struct sk_buff *skb)
+{
+	return 0;
+}
+
 static int cap_socket_getsockname(struct socket *sock)
 {
 	return 0;
@@ -1001,6 +1006,7 @@ void __init security_fixup_ops(struct security_operations *ops)
 	set_to_cap_if_null(ops, socket_accept);
 	set_to_cap_if_null(ops, socket_sendmsg);
 	set_to_cap_if_null(ops, socket_recvmsg);
+	set_to_cap_if_null(ops, socket_post_recvmsg);
 	set_to_cap_if_null(ops, socket_getsockname);
 	set_to_cap_if_null(ops, socket_getpeername);
 	set_to_cap_if_null(ops, socket_setsockopt);
diff --git a/security/security.c b/security/security.c
index e8c87b8..4291bd7 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1037,6 +1037,12 @@ int security_socket_recvmsg(struct socket *sock, struct msghdr *msg,
 	return security_ops->socket_recvmsg(sock, msg, size, flags);
 }
 
+int security_socket_post_recvmsg(struct sock *sk, struct sk_buff *skb)
+{
+	return security_ops->socket_post_recvmsg(sk, skb);
+}
+EXPORT_SYMBOL(security_socket_post_recvmsg);
+
 int security_socket_getsockname(struct socket *sock)
 {
 	return security_ops->socket_getsockname(sock);
-- 
1.6.1

Re: [PATCH] LSM: Add post recvmsg() hook.

[Paul Moore]

On Friday, July 16, 2010 09:17:10 pm Tetsuo Handa wrote:
> David Miller wrote:
> > From: Tetsuo Handa
> > Date: Sat, 17 Jul 2010 01:14:38 +0900
> > 
> > > Below is a patch for post recvmsg() operation. I modified the patch to
> > > call skb_recv_datagram() again (for udp_recvmsg(), raw_recvmsg(),
> > > udpv6_recvmsg()) if LSM dicided to drop the message. (Regarding
> > > rawv6_recvmsg(), I didn't do so in accordance with the comment at
> > > "csum_copy_err:".)
> > > What do you think about this verion?
> > 
> > This looks fine, but regardless of that comment I think the IPV6 raw
> > recvmsg() should loop just as the IPV4 one does in your patch.
> 
> Thank you, David.
> I updated to call skb_recv_datagram() for rawv6_recvmsg() case too.
> 
> NETWORKING [IPv4/IPv6] maintainers and Paul, is below patch fine for you?

Comments below ...

> >From b43154a90bc7494ec1ee301e692d2bbf29c8f2f8 Mon Sep 17 00:00:00 2001
> 
> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Date: Sat, 17 Jul 2010 09:52:38 +0900
> Subject: [PATCH] LSM: Add post recvmsg() hook.
> 
> Current pre recvmsg hook (i.e. security_socket_recvmsg()) has two problems.
> 
> One is that it will cause eating 100% of CPU time if the caller does not
> close() the socket when recvmsg() failed due to security_socket_recvmsg(),
> for subsequent select() notifies the caller of readiness for recvmsg()
> since the datagram which would have been already picked up if
> security_socket_recvmsg() did not return error is remaining in the queue.
> 
> The other is that it is racy if LSM module wants to do filtering based on
> "which process can pick up datagrams from which source" because the process
> which picks up the datagram is not known until skb_recv_datagram() and lock
> is not held between security_socket_recvmsg() and skb_recv_datagram().
> 
> This patch introduces post recvmsg hook (i.e.
> security_socket_post_recvmsg()) in order to solve above problems at the
> cost of ability to pick up the datagram which would have been picked up if
> preceding security_socket_post_recvmsg() did not return error.

We've had discussions before about the merits of queuing inbound packets to 
the socket buffer only to later reject them when the application reads from 
the socket.  I'd be much happier to see you drop the packets before queuing 
them to the socket, e.g. security_sock_rcv_skb(), but I understand that isn't 
possible with TOMOYO's approach to security.

At least we're not talking about TCP sockets :)

I'll go ahead and add my ACK to this patch, but I wonder if it makes more 
sense in the UDP path to add the LSM hook after the decision to calculate the 
checksum prior to the copy?  If we're going to reject the packet due to a bad 
checksum we might as well do that before we waste our time with the LSM 
processing - right?  Although, if we end up doing checksum verification with 
the copy in the majority of the cases it may not be worth it.

Acked-by: Paul Moore <paul.moore@hp.com>

> ---
>  include/linux/security.h |   14 ++++++++++++++
>  net/ipv4/raw.c           |   12 +++++++++---
>  net/ipv4/udp.c           |    9 ++++++++-
>  net/ipv6/raw.c           |   12 +++++++++---
>  net/ipv6/udp.c           |    9 ++++++++-
>  security/capability.c    |    6 ++++++
>  security/security.c      |    6 ++++++
>  7 files changed, 60 insertions(+), 8 deletions(-)
> 
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 723a93d..409c44d 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -879,6 +879,12 @@ static inline void security_free_mnt_opts(struct
> security_mnt_opts *opts) *	@size contains the size of message structure.
>   *	@flags contains the operational flags.
>   *	Return 0 if permission is granted.
> + * @socket_post_recvmsg:
> + *	Check permission after receiving a message from a socket.
> + *	The message is discarded if permission is not granted.
> + *	@sk contains the sock structure.
> + *	@skb contains the sk_buff structure.
> + *	Return 0 if permission is granted.
>   * @socket_getsockname:
>   *	Check permission before the local address (name) of the socket object
>   *	@sock is retrieved.
> @@ -1575,6 +1581,7 @@ struct security_operations {
>  			       struct msghdr *msg, int size);
>  	int (*socket_recvmsg) (struct socket *sock,
>  			       struct msghdr *msg, int size, int flags);
> +	int (*socket_post_recvmsg) (struct sock *sk, struct sk_buff *skb);
>  	int (*socket_getsockname) (struct socket *sock);
>  	int (*socket_getpeername) (struct socket *sock);
>  	int (*socket_getsockopt) (struct socket *sock, int level, int optname);
> @@ -2526,6 +2533,7 @@ int security_socket_accept(struct socket *sock,
> struct socket *newsock); int security_socket_sendmsg(struct socket *sock,
> struct msghdr *msg, int size); int security_socket_recvmsg(struct socket
> *sock, struct msghdr *msg, int size, int flags);
> +int security_socket_post_recvmsg(struct sock *sk, struct sk_buff *skb);
>  int security_socket_getsockname(struct socket *sock);
>  int security_socket_getpeername(struct socket *sock);
>  int security_socket_getsockopt(struct socket *sock, int level, int
> optname); @@ -2617,6 +2625,12 @@ static inline int
> security_socket_recvmsg(struct socket *sock, return 0;
>  }
> 
> +static inline int security_socket_post_recvmsg(struct sock *sk,
> +					       struct sk_buff *skb)
> +{
> +	return 0;
> +}
> +
>  static inline int security_socket_getsockname(struct socket *sock)
>  {
>  	return 0;
> diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
> index 2c7a163..69652d4 100644
> --- a/net/ipv4/raw.c
> +++ b/net/ipv4/raw.c
> @@ -676,9 +676,15 @@ static int raw_recvmsg(struct kiocb *iocb, struct sock
> *sk, struct msghdr *msg, goto out;
>  	}
> 
> -	skb = skb_recv_datagram(sk, flags, noblock, &err);
> -	if (!skb)
> -		goto out;
> +	for (;;) {
> +		skb = skb_recv_datagram(sk, flags, noblock, &err);
> +		if (!skb)
> +			goto out;
> +		err = security_socket_post_recvmsg(sk, skb);
> +		if (likely(!err))
> +			break;
> +		skb_kill_datagram(sk, skb, flags);
> +	}
> 
>  	copied = skb->len;
>  	if (len < copied) {
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 5858574..9145685 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -1125,6 +1125,7 @@ int udp_recvmsg(struct kiocb *iocb, struct sock *sk,
> struct msghdr *msg, int err;
>  	int is_udplite = IS_UDPLITE(sk);
>  	bool slow;
> +	bool update_stat;
> 
>  	/*
>  	 *	Check any passed addresses
> @@ -1140,6 +1141,12 @@ try_again:
>  				  &peeked, &err);
>  	if (!skb)
>  		goto out;
> +	err = security_socket_post_recvmsg(sk, skb);
> +	if (err) {
> +		update_stat = false;
> +		goto csum_copy_err;
> +	}
> +	update_stat = true;
> 
>  	ulen = skb->len - sizeof(struct udphdr);
>  	if (len > ulen)
> @@ -1200,7 +1207,7 @@ out:
> 
>  csum_copy_err:
>  	slow = lock_sock_fast(sk);
> -	if (!skb_kill_datagram(sk, skb, flags))
> +	if (!skb_kill_datagram(sk, skb, flags) && update_stat)
>  		UDP_INC_STATS_USER(sock_net(sk), UDP_MIB_INERRORS, is_udplite);
>  	unlock_sock_fast(sk, slow);
> 
> diff --git a/net/ipv6/raw.c b/net/ipv6/raw.c
> index 4a4dcbe..6915b01 100644
> --- a/net/ipv6/raw.c
> +++ b/net/ipv6/raw.c
> @@ -464,9 +464,15 @@ static int rawv6_recvmsg(struct kiocb *iocb, struct
> sock *sk, if (np->rxpmtu && np->rxopt.bits.rxpmtu)
>  		return ipv6_recv_rxpmtu(sk, msg, len);
> 
> -	skb = skb_recv_datagram(sk, flags, noblock, &err);
> -	if (!skb)
> -		goto out;
> +	for (;;) {
> +		skb = skb_recv_datagram(sk, flags, noblock, &err);
> +		if (!skb)
> +			goto out;
> +		err = security_socket_post_recvmsg(sk, skb);
> +		if (likely(!err))
> +			break;
> +		skb_kill_datagram(sk, skb, flags);
> +	}
> 
>  	copied = skb->len;
>  	if (copied > len) {
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index 87be586..6cae276 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -329,6 +329,7 @@ int udpv6_recvmsg(struct kiocb *iocb, struct sock *sk,
>  	int is_udplite = IS_UDPLITE(sk);
>  	int is_udp4;
>  	bool slow;
> +	bool update_stat;
> 
>  	if (addr_len)
>  		*addr_len=sizeof(struct sockaddr_in6);
> @@ -344,6 +345,12 @@ try_again:
>  				  &peeked, &err);
>  	if (!skb)
>  		goto out;
> +	err = security_socket_post_recvmsg(sk, skb);
> +	if (err) {
> +		update_stat = false;
> +		goto csum_copy_err;
> +	}
> +	update_stat = true;
> 
>  	ulen = skb->len - sizeof(struct udphdr);
>  	if (len > ulen)
> @@ -426,7 +433,7 @@ out:
> 
>  csum_copy_err:
>  	slow = lock_sock_fast(sk);
> -	if (!skb_kill_datagram(sk, skb, flags)) {
> +	if (!skb_kill_datagram(sk, skb, flags) && update_stat) {
>  		if (is_udp4)
>  			UDP_INC_STATS_USER(sock_net(sk),
>  					UDP_MIB_INERRORS, is_udplite);
> diff --git a/security/capability.c b/security/capability.c
> index 4aeb699..709aea3 100644
> --- a/security/capability.c
> +++ b/security/capability.c
> @@ -597,6 +597,11 @@ static int cap_socket_recvmsg(struct socket *sock,
> struct msghdr *msg, return 0;
>  }
> 
> +static int cap_socket_post_recvmsg(struct sock *sk, struct sk_buff *skb)
> +{
> +	return 0;
> +}
> +
>  static int cap_socket_getsockname(struct socket *sock)
>  {
>  	return 0;
> @@ -1001,6 +1006,7 @@ void __init security_fixup_ops(struct
> security_operations *ops) set_to_cap_if_null(ops, socket_accept);
>  	set_to_cap_if_null(ops, socket_sendmsg);
>  	set_to_cap_if_null(ops, socket_recvmsg);
> +	set_to_cap_if_null(ops, socket_post_recvmsg);
>  	set_to_cap_if_null(ops, socket_getsockname);
>  	set_to_cap_if_null(ops, socket_getpeername);
>  	set_to_cap_if_null(ops, socket_setsockopt);
> diff --git a/security/security.c b/security/security.c
> index e8c87b8..4291bd7 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1037,6 +1037,12 @@ int security_socket_recvmsg(struct socket *sock,
> struct msghdr *msg, return security_ops->socket_recvmsg(sock, msg, size,
> flags);
>  }
> 
> +int security_socket_post_recvmsg(struct sock *sk, struct sk_buff *skb)
> +{
> +	return security_ops->socket_post_recvmsg(sk, skb);
> +}
> +EXPORT_SYMBOL(security_socket_post_recvmsg);
> +
>  int security_socket_getsockname(struct socket *sock)
>  {
>  	return security_ops->socket_getsockname(sock);

-- 
paul moore
linux @ hp

Re: [PATCH] LSM: Add post recvmsg() hook.

[Eric Dumazet]

Le samedi 17 juillet 2010 à 10:17 +0900, Tetsuo Handa a écrit :
> David Miller wrote:
> > From: Tetsuo Handa
> > Date: Sat, 17 Jul 2010 01:14:38 +0900
> > 
> > > Below is a patch for post recvmsg() operation. I modified the patch to call
> > > skb_recv_datagram() again (for udp_recvmsg(), raw_recvmsg(), udpv6_recvmsg())
> > > if LSM dicided to drop the message. (Regarding rawv6_recvmsg(), I didn't do so
> > > in accordance with the comment at "csum_copy_err:".)
> > > What do you think about this verion?
> > 
> > This looks fine, but regardless of that comment I think the IPV6 raw recvmsg()
> > should loop just as the IPV4 one does in your patch.
> > 
> Thank you, David.
> I updated to call skb_recv_datagram() for rawv6_recvmsg() case too.
> 
> NETWORKING [IPv4/IPv6] maintainers and Paul, is below patch fine for you?
> 
> Regards.
> ----------------------------------------
> >From b43154a90bc7494ec1ee301e692d2bbf29c8f2f8 Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Date: Sat, 17 Jul 2010 09:52:38 +0900
> Subject: [PATCH] LSM: Add post recvmsg() hook.
> 
> Current pre recvmsg hook (i.e. security_socket_recvmsg()) has two problems.
> 
> One is that it will cause eating 100% of CPU time if the caller does not
> close() the socket when recvmsg() failed due to security_socket_recvmsg(), for
> subsequent select() notifies the caller of readiness for recvmsg() since the
> datagram which would have been already picked up if security_socket_recvmsg()
> did not return error is remaining in the queue.
> 
> The other is that it is racy if LSM module wants to do filtering based on
> "which process can pick up datagrams from which source" because the process
> which picks up the datagram is not known until skb_recv_datagram() and lock
> is not held between security_socket_recvmsg() and skb_recv_datagram().
> 
> This patch introduces post recvmsg hook (i.e. security_socket_post_recvmsg())
> in order to solve above problems at the cost of ability to pick up the datagram
> which would have been picked up if preceding security_socket_post_recvmsg() did
> not return error.
> 
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

I read this patch and could not find out if an SNMP counter was
increased in the case a frame was not delivered but dropped in kernel
land.



--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] LSM: Add post recvmsg() hook.

[Tetsuo Handa]

Eric Dumazet wrote:
> I read this patch and could not find out if an SNMP counter was
> increased in the case a frame was not delivered but dropped in kernel
> land.

UDP_MIB_INDATAGRAMS and UDP_MIB_INERRORS will not be increased
if dropped by security_socket_post_recvmsg()'s decision.
Should we increment UDP_MIB_INDATAGRAMS and/or UDP_MIB_INERRORS?

udpInDatagrams
    "The total number of UDP datagrams delivered to UDP users."

udpNoPorts
    "The total number of received UDP datagrams for which there was no
     application at the destination port."

udpInErrors
    "The number of received UDP datagrams that could not be delivered for
     reasons other than the lack of an application at the destination port."

Re: [PATCH] LSM: Add post recvmsg() hook.

[David Miller]

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Sun, 18 Jul 2010 19:49:11 +0900

> Eric Dumazet wrote:
>> I read this patch and could not find out if an SNMP counter was
>> increased in the case a frame was not delivered but dropped in kernel
>> land.
> 
> UDP_MIB_INDATAGRAMS and UDP_MIB_INERRORS will not be increased
> if dropped by security_socket_post_recvmsg()'s decision.
> Should we increment UDP_MIB_INDATAGRAMS and/or UDP_MIB_INERRORS?

This decision should be guided by what we do for in the case
of the other existing security hooks.

I don't think it makes any sense to make the post recvmsg() hook
behave any differently from the existing hooks in this regard.

[PATCH] LSM: Add post accept() hook.

[Tetsuo Handa]

David Miller wrote:
> > Eric Dumazet wrote:
> >> I read this patch and could not find out if an SNMP counter was
> >> increased in the case a frame was not delivered but dropped in kernel
> >> land.
> > 
> > UDP_MIB_INDATAGRAMS and UDP_MIB_INERRORS will not be increased
> > if dropped by security_socket_post_recvmsg()'s decision.
> > Should we increment UDP_MIB_INDATAGRAMS and/or UDP_MIB_INERRORS?
> 
> This decision should be guided by what we do for in the case
> of the other existing security hooks.
> 
> I don't think it makes any sense to make the post recvmsg() hook
> behave any differently from the existing hooks in this regard.

I see. Thank you.

I was misunderstanding assumption on select() -> recvmsg() sequence.
I was thinking that:

  If select() said "read operation will not block", the caller of recvmsg() can
  assume that recvmsg() which is preceded by select() will not be blocked.
  (The caller cannot assume that subsequent recvmsg() preceded by previous
  recvmsg() will not be blocked.) Therefore, the kernel must not wait for next
  message if current message was discarded by post recvmsg LSM hook. (And I
  thought that returning error code to the caller is the only way because the
  caller might be assuming that recvmsg() preceded by select() will not be
  blocked.)

But I understood that:

  Even if select() said "read operation will not block", the caller of recvmsg()
  can't assume that recvmsg() which is preceded by select() will not be blocked
  unless MSG_DONTWAIT or O_NONBLOCK was set.
  Therefore, the kernel is allowed to wait for next message if current message
  was discarded by post recvmsg LSM hook unless MSG_DONTWAIT or O_NONBLOCK was
  set.

Now, I'm thinking the same thing for select() -> accept() sequence:

  Even if select() said "read operation will not block", the caller of accept()
  can't assume that accept() which is preceded by select() will not be blocked
  unless MSG_DONTWAIT or O_NONBLOCK was set.
  Therefore, the kernel is allowed to wait for next connection if current
  connection was discarded by post accept LSM hook unless MSG_DONTWAIT or
  O_NONBLOCK was set.

Although "security_socket_post_accept() without retry loop" was proposed
in the past ( http://lkml.org/lkml/2010/3/2/297 ), I think I can propose
"security_socket_post_accept() with retry loop" (patch attached below)
if select() -> accept() case I wrote above is correct.

I can live with "security_socket_post_accept() without retry loop" by assigning
magic value to SOCK_INODE("struct socket *")->i_security field
( tomoyo_dead_sock() in http://lkml.org/lkml/2009/10/4/56 ) but below patch is
better for me because TOMOYO will not require the i_security field (which will
make it easier to realize LSM stacking/chaining) and will not need to implement
all LSM hooks for socket operations only for checking the i_security field.

May I have your opinion for below version?

Regards.
----------------------------------------
>From 54bc4ffee7998423e8c2d3a5cc9dfc647d5a892b Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Sat, 17 Jul 2010 12:04:18 +0900
Subject: [PATCH] LSM: Add post accept() hook.

Current pre accept hook (i.e. security_socket_accept()) has two problems.

One is that it will cause eating 100% of CPU time if the caller does not
close() the socket when accept() failed due to security_socket_accept(), for
subsequent select() notifies the caller of readiness for accept() since the
connection which would have been already picked up if security_socket_accept()
did not return error is remaining in the queue.

The other is that it is racy if LSM module wants to do filtering based on
"which process can pick up connections from which source" because the process
which picks up the connection is not known until sock->ops->accept() and lock
is not held between security_socket_accept() and sock->ops->accept.

This patch introduces post accept hook (i.e. security_socket_post_accept()) in
order to solve above problems at the cost of ability to pick up the connection
which would have been picked up if preceding security_socket_post_accept() did
not return error.

Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 include/linux/security.h |   21 +++++++++++++++++++++
 net/socket.c             |    7 +++++++
 security/capability.c    |    6 ++++++
 security/security.c      |    5 +++++
 4 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/include/linux/security.h b/include/linux/security.h
index 409c44d..2ed73c1 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -866,6 +866,19 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
  *	@sock contains the listening socket structure.
  *	@newsock contains the newly created server socket for connection.
  *	Return 0 if permission is granted.
+ * @socket_post_accept:
+ *	Check permission after accepting a new connection.
+ *	The connection is discarded if permission is not granted.
+ *	Return 0 after updating security information on the socket if you want
+ *	to restrict some of socket syscalls on the connection (e.g. forbid only
+ *	sending data). But you can't use this hook for updating security
+ *	information of the socket for preventing the connection from receiving
+ *	incoming data, for the kernel already started receiving incoming data
+ *	before accept() syscall. Return error if updating security information
+ *	failed or you want to forbid all of socket syscalls on the connection.
+ *	@sock contains the listening socket structure.
+ *	@newsock contains the accepted socket structure.
+ *	Return 0 if permission is granted.
  * @socket_sendmsg:
  *	Check permission before transmitting a message to another socket.
  *	@sock contains the socket structure.
@@ -1577,6 +1590,7 @@ struct security_operations {
 			       struct sockaddr *address, int addrlen);
 	int (*socket_listen) (struct socket *sock, int backlog);
 	int (*socket_accept) (struct socket *sock, struct socket *newsock);
+	int (*socket_post_accept) (struct socket *sock, struct socket *newsock);
 	int (*socket_sendmsg) (struct socket *sock,
 			       struct msghdr *msg, int size);
 	int (*socket_recvmsg) (struct socket *sock,
@@ -2530,6 +2544,7 @@ int security_socket_bind(struct socket *sock, struct sockaddr *address, int addr
 int security_socket_connect(struct socket *sock, struct sockaddr *address, int addrlen);
 int security_socket_listen(struct socket *sock, int backlog);
 int security_socket_accept(struct socket *sock, struct socket *newsock);
+int security_socket_post_accept(struct socket *sock, struct socket *newsock);
 int security_socket_sendmsg(struct socket *sock, struct msghdr *msg, int size);
 int security_socket_recvmsg(struct socket *sock, struct msghdr *msg,
 			    int size, int flags);
@@ -2612,6 +2627,12 @@ static inline int security_socket_accept(struct socket *sock,
 	return 0;
 }
 
+static inline int security_socket_post_accept(struct socket *sock,
+					      struct socket *newsock)
+{
+	return 0;
+}
+
 static inline int security_socket_sendmsg(struct socket *sock,
 					  struct msghdr *msg, int size)
 {
diff --git a/net/socket.c b/net/socket.c
index 367d547..97d644c 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1473,6 +1473,7 @@ SYSCALL_DEFINE4(accept4, int, fd, struct sockaddr __user *, upeer_sockaddr,
 	if (!sock)
 		goto out;
 
+ retry:
 	err = -ENFILE;
 	if (!(newsock = sock_alloc()))
 		goto out_put;
@@ -1500,6 +1501,12 @@ SYSCALL_DEFINE4(accept4, int, fd, struct sockaddr __user *, upeer_sockaddr,
 	err = sock->ops->accept(sock, newsock, sock->file->f_flags);
 	if (err < 0)
 		goto out_fd;
+	err = security_socket_post_accept(sock, newsock);
+	if (unlikely(err)) {
+		fput(newfile);
+		put_unused_fd(newfd);
+		goto retry;
+	}
 
 	if (upeer_sockaddr) {
 		if (newsock->ops->getname(newsock, (struct sockaddr *)&address,
diff --git a/security/capability.c b/security/capability.c
index 709aea3..1fb88f5 100644
--- a/security/capability.c
+++ b/security/capability.c
@@ -586,6 +586,11 @@ static int cap_socket_accept(struct socket *sock, struct socket *newsock)
 	return 0;
 }
 
+static int cap_socket_post_accept(struct socket *sock, struct socket *newsock)
+{
+	return 0;
+}
+
 static int cap_socket_sendmsg(struct socket *sock, struct msghdr *msg, int size)
 {
 	return 0;
@@ -1004,6 +1009,7 @@ void __init security_fixup_ops(struct security_operations *ops)
 	set_to_cap_if_null(ops, socket_connect);
 	set_to_cap_if_null(ops, socket_listen);
 	set_to_cap_if_null(ops, socket_accept);
+	set_to_cap_if_null(ops, socket_post_accept);
 	set_to_cap_if_null(ops, socket_sendmsg);
 	set_to_cap_if_null(ops, socket_recvmsg);
 	set_to_cap_if_null(ops, socket_post_recvmsg);
diff --git a/security/security.c b/security/security.c
index 4291bd7..5c9ab0a 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1026,6 +1026,11 @@ int security_socket_accept(struct socket *sock, struct socket *newsock)
 	return security_ops->socket_accept(sock, newsock);
 }
 
+int security_socket_post_accept(struct socket *sock, struct socket *newsock)
+{
+	return security_ops->socket_post_accept(sock, newsock);
+}
+
 int security_socket_sendmsg(struct socket *sock, struct msghdr *msg, int size)
 {
 	return security_ops->socket_sendmsg(sock, msg, size);
-- 
1.6.1

Re: [PATCH] LSM: Add post accept() hook.

[Paul Moore]

On Monday, July 19, 2010 12:25:25 am Tetsuo Handa wrote:
> Current pre accept hook (i.e. security_socket_accept()) has two problems.
> 
> One is that it will cause eating 100% of CPU time if the caller does not
> close() the socket when accept() failed due to security_socket_accept(),
> for subsequent select() notifies the caller of readiness for accept()
> since the connection which would have been already picked up if
> security_socket_accept() did not return error is remaining in the queue.
> 
> The other is that it is racy if LSM module wants to do filtering based on
> "which process can pick up connections from which source" because the
> process which picks up the connection is not known until
> sock->ops->accept() and lock is not held between security_socket_accept()
> and sock->ops->accept.
> 
> This patch introduces post accept hook (i.e. security_socket_post_accept())
> in order to solve above problems at the cost of ability to pick up the
> connection which would have been picked up if preceding
> security_socket_post_accept() did not return error.
> 
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

I think you need to show how you plan to use this hook in an LSM before we can 
consider merging it with mainline.  What you are proposing here is giving an 
LSM the ability to drop a connection _after_ allowing it to be established in 
the first place; this seems very wrong to me and I want to make sure everyone 
else is aware of that before accepting this code into the kernel.  I 
understand that TOMOYO's security model does not allow it to reject incoming 
connections at the beginning of the connection request like some of the LSMs 
currently in use, but I'm just not very happy with the idea of finishing a 
connection handshake only to later drop the connection on the floor.

> ---
>  include/linux/security.h |   21 +++++++++++++++++++++
>  net/socket.c             |    7 +++++++
>  security/capability.c    |    6 ++++++
>  security/security.c      |    5 +++++
>  4 files changed, 39 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 409c44d..2ed73c1 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -866,6 +866,19 @@ static inline void security_free_mnt_opts(struct
> security_mnt_opts *opts) *	@sock contains the listening socket structure.
>   *	@newsock contains the newly created server socket for connection.
>   *	Return 0 if permission is granted.
> + * @socket_post_accept:
> + *	Check permission after accepting a new connection.
> + *	The connection is discarded if permission is not granted.
> + *	Return 0 after updating security information on the socket if you 
want
> + *	to restrict some of socket syscalls on the connection (e.g. forbid 
only
> + *	sending data). But you can't use this hook for updating security
> + *	information of the socket for preventing the connection from 
receiving
> + *	incoming data, for the kernel already started receiving incoming data
> + *	before accept() syscall. Return error if updating security 
information
> + *	failed or you want to forbid all of socket syscalls on the 
connection.
> + *	@sock contains the listening socket structure.
> + *	@newsock contains the accepted socket structure.
> + *	Return 0 if permission is granted.
>   * @socket_sendmsg:
>   *	Check permission before transmitting a message to another socket.
>   *	@sock contains the socket structure.
> @@ -1577,6 +1590,7 @@ struct security_operations {
>  			       struct sockaddr *address, int addrlen);
>  	int (*socket_listen) (struct socket *sock, int backlog);
>  	int (*socket_accept) (struct socket *sock, struct socket *newsock);
> +	int (*socket_post_accept) (struct socket *sock, struct socket *newsock);
>  	int (*socket_sendmsg) (struct socket *sock,
>  			       struct msghdr *msg, int size);
>  	int (*socket_recvmsg) (struct socket *sock,
> @@ -2530,6 +2544,7 @@ int security_socket_bind(struct socket *sock, struct
> sockaddr *address, int addr int security_socket_connect(struct socket
> *sock, struct sockaddr *address, int addrlen); int
> security_socket_listen(struct socket *sock, int backlog);
>  int security_socket_accept(struct socket *sock, struct socket *newsock);
> +int security_socket_post_accept(struct socket *sock, struct socket
> *newsock); int security_socket_sendmsg(struct socket *sock, struct msghdr
> *msg, int size); int security_socket_recvmsg(struct socket *sock, struct
> msghdr *msg, int size, int flags);
> @@ -2612,6 +2627,12 @@ static inline int security_socket_accept(struct
> socket *sock, return 0;
>  }
> 
> +static inline int security_socket_post_accept(struct socket *sock,
> +					      struct socket *newsock)
> +{
> +	return 0;
> +}
> +
>  static inline int security_socket_sendmsg(struct socket *sock,
>  					  struct msghdr *msg, int size)
>  {
> diff --git a/net/socket.c b/net/socket.c
> index 367d547..97d644c 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -1473,6 +1473,7 @@ SYSCALL_DEFINE4(accept4, int, fd, struct sockaddr
> __user *, upeer_sockaddr, if (!sock)
>  		goto out;
> 
> + retry:
>  	err = -ENFILE;
>  	if (!(newsock = sock_alloc()))
>  		goto out_put;
> @@ -1500,6 +1501,12 @@ SYSCALL_DEFINE4(accept4, int, fd, struct sockaddr
> __user *, upeer_sockaddr, err = sock->ops->accept(sock, newsock,
> sock->file->f_flags);
>  	if (err < 0)
>  		goto out_fd;
> +	err = security_socket_post_accept(sock, newsock);
> +	if (unlikely(err)) {
> +		fput(newfile);
> +		put_unused_fd(newfd);
> +		goto retry;
> +	}
> 
>  	if (upeer_sockaddr) {
>  		if (newsock->ops->getname(newsock, (struct sockaddr *)&address,
> diff --git a/security/capability.c b/security/capability.c
> index 709aea3..1fb88f5 100644
> --- a/security/capability.c
> +++ b/security/capability.c
> @@ -586,6 +586,11 @@ static int cap_socket_accept(struct socket *sock,
> struct socket *newsock) return 0;
>  }
> 
> +static int cap_socket_post_accept(struct socket *sock, struct socket
> *newsock) +{
> +	return 0;
> +}
> +
>  static int cap_socket_sendmsg(struct socket *sock, struct msghdr *msg, int
> size) {
>  	return 0;
> @@ -1004,6 +1009,7 @@ void __init security_fixup_ops(struct
> security_operations *ops) set_to_cap_if_null(ops, socket_connect);
>  	set_to_cap_if_null(ops, socket_listen);
>  	set_to_cap_if_null(ops, socket_accept);
> +	set_to_cap_if_null(ops, socket_post_accept);
>  	set_to_cap_if_null(ops, socket_sendmsg);
>  	set_to_cap_if_null(ops, socket_recvmsg);
>  	set_to_cap_if_null(ops, socket_post_recvmsg);
> diff --git a/security/security.c b/security/security.c
> index 4291bd7..5c9ab0a 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -1026,6 +1026,11 @@ int security_socket_accept(struct socket *sock,
> struct socket *newsock) return security_ops->socket_accept(sock, newsock);
>  }
> 
> +int security_socket_post_accept(struct socket *sock, struct socket
> *newsock) +{
> +	return security_ops->socket_post_accept(sock, newsock);
> +}
> +
>  int security_socket_sendmsg(struct socket *sock, struct msghdr *msg, int
> size) {
>  	return security_ops->socket_sendmsg(sock, msg, size);

-- 
paul moore
linux @ hp

Re: [PATCH] LSM: Add post accept() hook.

[Tetsuo Handa]

Paul Moore wrote:
> I think you need to show how you plan to use this hook in an LSM before we can 
> consider merging it with mainline.  What you are proposing here is giving an 
> LSM the ability to drop a connection _after_ allowing it to be established in 
> the first place; this seems very wrong to me and I want to make sure everyone 
> else is aware of that before accepting this code into the kernel.  I 
> understand that TOMOYO's security model does not allow it to reject incoming 
> connections at the beginning of the connection request like some of the LSMs 
> currently in use, but I'm just not very happy with the idea of finishing a 
> connection handshake only to later drop the connection on the floor.

Yes. I'm planning to use security_socket_post_accept() for two purposes.



One is for dropping connections from unwanted hosts. Administrators define
policy before enabling enforcing mode (the mode which connections are dropped
if operation was not granted by policy). Administrators specify acceptable
hosts (i.e. hosts which this host needs to communicate with) and unacceptable
hosts (i.e. hosts which this host needn't to communicate with).
Dropping connections would happen if some process was hijacked and the process
attempted to communicate with other processes using TCP connections. But
dropping connections should not happen in normal circumstance.



The other is for updating process's state variable upon accept() operation.
LKM version of TOMOYO has per a task_struct variable that is used for
implementing stateful permissions. (As of now, not implemented for LSM version
of TOMOYO.) For example,

  allow_network TCP accept 10.0.0.0-10.255.255.255 1024-65535 ; set task.state[0]=1
  allow_network TCP accept 192.168.0.1-192.168.255.255 1024-65535 ; set task.state[0]=2

will change current thread's task state variable to 1 if current thread
accepted TCP connection from 10.0.0.0-10.255.255.255 and change it to 2 if from
192.168.0.1-192.168.255.255 . This variable is used for giving different
permissions for subsequent operations. For example,

  allow_execute /bin/bash if task.state[0]=1
  allow_execute /bin/tcsh if task.state[0]=2

will allow execution of /bin/bash if current thread is dealing connections from
10.0.0.0-10.255.255.255 and allow execution of /bin/tcsh if current thread is
dealing connections from 192.168.0.1-192.168.255.255 . Another example,

  allow_network TCP accept 0.0.0.0-255.255.255.255 1024-65535 ; set task.state[0]=3
  allow_network TCP accept 0.0.0.0-255.255.255.255 1-1023 ; set task.state[0]=4

will change it to 3 if from unprivileged port and change it to 4 if from
privileged port.

  allow_execute /bin/rbash if task.state[0]=3
  allow_execute /bin/bash if task.state[0]=4

will allow execution of /bin/rbash if dealing connections from unprivileged
ports and allow execution of /bin/bash if dealing connections from privileged
ports.

LSM hooks called before sock->ops->accept() cannot change current thread's task
state variable because it is racy, and LSM hook called after sock->ops->accept()
is missing.

Strictly speaking, it could be possible to update current thread's task state
variable in LSM hooks called by subsequent operations (e.g.
security_dentry_open(), security_bprm_set_creds()) by doing similar approach
done by tomoyo_dead_sock(), but updating it can fail (e.g. -ENOMEM) since
credentials are COW. If updating it failed, I want to drop the accept()ed
connection, but that is impossible from LSM hooks called by subsequent
operations. Killing current thread when updating it failed is possible, but
that looks worse for me than dropping connections upon accept() time (because
such action resembles OOM killer and likely gives larger damage to the caller).

Re: [PATCH] LSM: Add post accept() hook.

[Paul Moore]

On Monday, July 19, 2010 09:36:31 pm Tetsuo Handa wrote:
> Paul Moore wrote:
> > I think you need to show how you plan to use this hook in an LSM before
> > we can consider merging it with mainline.  What you are proposing here
> > is giving an LSM the ability to drop a connection _after_ allowing it to
> > be established in the first place; this seems very wrong to me and I
> > want to make sure everyone else is aware of that before accepting this
> > code into the kernel.  I understand that TOMOYO's security model does
> > not allow it to reject incoming connections at the beginning of the
> > connection request like some of the LSMs currently in use, but I'm just
> > not very happy with the idea of finishing a connection handshake only to
> > later drop the connection on the floor.
> 
> Yes. I'm planning to use security_socket_post_accept() for two purposes.
> 
> One is for dropping connections from unwanted hosts. Administrators define
> policy before enabling enforcing mode (the mode which connections are
> dropped if operation was not granted by policy). Administrators specify
> acceptable hosts (i.e. hosts which this host needs to communicate with)
> and unacceptable hosts (i.e. hosts which this host needn't to communicate
> with).

You can enforce per-host access controls without the need for a post-accept() 
hooks, e.g. security_sock_rcv_skb() and the netfilter hooks 
(NF_INET_POST_ROUTING, NF_INET_FORWARD, NF_INET_LOCAL_OUT).  Or are you 
interested in controlling which hosts an _application_ can communicate with?

> Dropping connections would happen if some process was hijacked and the
> process attempted to communicate with other processes using TCP
> connections. But dropping connections should not happen in normal
> circumstance.

It doesn't matter if dropping connections is normal or not, what matters is 
that it can happen.

> The other is for updating process's state variable upon accept() operation.
> LKM version of TOMOYO has per a task_struct variable that is used for
> implementing stateful permissions. (As of now, not implemented for LSM
> version of TOMOYO.)

I'm open to re-introducing a post-accept() hook that does not have a return 
value, in other words, a hook that can only be used to update LSM state and 
not affect the connection.  Although I do think you could probably achieve the 
same thing using some of the existing LSM hooks (look at how SELinux updates 
its state upon accept()) but that is something you would have to look it and 
see if it works for TOMOYO.

-- 
paul moore
linux @ hp

Re: [PATCH] LSM: Add post accept() hook.

[Tetsuo Handa]

Paul Moore wrote:
> On Monday, July 19, 2010 09:36:31 pm Tetsuo Handa wrote:
> > One is for dropping connections from unwanted hosts. Administrators define
> > policy before enabling enforcing mode (the mode which connections are
> > dropped if operation was not granted by policy). Administrators specify
> > acceptable hosts (i.e. hosts which this host needs to communicate with)
> > and unacceptable hosts (i.e. hosts which this host needn't to communicate
> > with).
> 
> You can enforce per-host access controls without the need for a post-accept() 
> hooks, e.g. security_sock_rcv_skb() and the netfilter hooks 
> (NF_INET_POST_ROUTING, NF_INET_FORWARD, NF_INET_LOCAL_OUT).  Or are you 
> interested in controlling which hosts an _application_ can communicate with?

I'm interested in controlling which ports on which hosts a _process_ can
communicate with. In TOMOYO's words, "processes that belong to which TOMOYO's
domain can communicate with which ports on which hosts".

TOMOYO's rules are

  Processes that belong to FOO domain can open /etc/fstab for reading.
     ( allow_read /etc/fstab )

  Processes that belong to FOO domain can create /tmp/file with mode 0600.
     ( allow_create /tmp/file 0600 )

  Processes that belong to FOO domain can connect to port 80 on host
  10.20.30.40 using TCP protocol.
     ( allow_network TCP connect 10.20.30.40 80 )

and so on. But currently,

  Processes that belong to FOO domain can accept TCP connections from port 1024
  on host 10.20.30.40.
     ( allow_network TCP accept 10.20.30.40 1024 )

  Processes that belong to FOO domain can receive UDP messages from port 65535
  on host 100.200.10.20.
     ( allow_network UDP connect 100.200.10.20 65535 )

are impossible.

Regarding outgoing connections/datagrams, we can specify address/port
parameters from the point of view of _process_ who actually sends requests.
But regarding incoming connections/datagrams, we cannot specify address/port
parameters from the point of view of _process_ who actually receives requests.

We can enforce per-host access controls using iptables.
But we can't use iptables for controlling address/port parameters for incoming
connections/datagrams because the process who actually receives requests
(ServewrApp2 in below example) is not always the same as the process who
created the socket (ServerApp1 in below example).

> > Dropping connections would happen if some process was hijacked and the
> > process attempted to communicate with other processes using TCP
> > connections. But dropping connections should not happen in normal
> > circumstance.
> 
> It doesn't matter if dropping connections is normal or not, what matters is 
> that it can happen.
> 
> > The other is for updating process's state variable upon accept() operation.
> > LKM version of TOMOYO has per a task_struct variable that is used for
> > implementing stateful permissions. (As of now, not implemented for LSM
> > version of TOMOYO.)
> 
> I'm open to re-introducing a post-accept() hook that does not have a return 
> value, in other words, a hook that can only be used to update LSM state and 
> not affect the connection.  Although I do think you could probably achieve the 
> same thing using some of the existing LSM hooks (look at how SELinux updates 
> its state upon accept()) but that is something you would have to look it and 
> see if it works for TOMOYO.

I can't figure out why the hook must not affect the connection.
Is it possible to clarify using below players?

Server1 and Client1 are hosts which are connected on TCP/IP network.
ServerApp1 and ServerApp2 are applications running on Server1 which might call
socket(), bind(), listen(), accept(), send(), recv(), shutdown(), close() and
execute().
ClientApp1 and ClientApp2 are applications running on Client1 which might call
socket(), connect(), send(), recv(), shutdown(), close().
Router1 and Router2 are routers which exist between Server1 and Client1.

  +-------+   +-------+   +-------+   +-------+
  |Server1|---|Router1|---|Router2|---|Client1|
  +-------+   +-------+   +-------+   +-------+

Event sequences:

Server1                       Client1

  ServerApp1 creates a socket using socket().

  ServerApp1 binds to an address using bind().

  ServerApp1 listens to the address using listen().

                                ClientApp1 creates a socket using socket().

                                ClientApp1 issues connect() request.

                                  Sends SYN.

    Receives SYN.

    Sends SYN/ACK.

                                  Receives SYN/ACK.

                                  Sends ACK.

    Receives ACK.

                                ClientApp1 issues send() request.

                                  Sends data.

    Receives data.

    Sends ACK.

                                  Receives ACK.

                                ClientApp1 issues send() request.

                                  Sends data.

    Receives data.

    Sends ACK.

                                  Receives ACK.

  ServerApp1 calls execve("ServerApp2").

  ServerApp2 issues accept() request.

    security_socket_accept() is called.

    sock->ops->accept() is called.

    security_socket_post_accept() is called. (*3)

    newsock->ops->getname() is called. (*1)

    move_addr_to_user() is called. (*2)

    fd_install() is called.

  ServerApp2 issues some requests.

    Some LSM hooks will be called.




*1: This may fail and the connection is discarded if failed.
    Thus, newsock->ops->getname() affects the connection.
    This is not fault of ServerApp2. Maybe this is fault of ClientApp1 or
    Router1 or Router2, but discarding already established connection is
    justified.

*2: This may fail and the connection is discarded if failed.
    Thus, move_addr_to_user() affects the connection.
    Is this the fault of ServerApp2?
    If the upeer_sockaddr supplied by ServerApp2 was bad, this is the fault of
    ServerApp2. Thus, discarding already established connection is justified.
    If the upeer_sockaddr supplied by ServerApp2 was good but physical RAM was
    not yet assigned for the upeer_sockaddr, and OOM killer was invoked when
    attempted to write to upeer_sockaddr and OOM killer chose ServerApp2, and
    the ServerApp2 is killed. This is not fault of ServerApp2. But discarding
    already established connection is justified.

*3: newsock->ops->getname() and move_addr_to_user() already affects the
    connection. They discard already established connections even if the cause
    is not ServerApp2's fault. Why security_socket_post_accept() affecting the
    connection cannot be justified?

Router1 and Router2 can inject RST into the already established connections
at any time (if they are IDS/IPS or broken or malicious).
How does security_socket_post_accept() returning an error differs from these
routers injecting RST?

Regards.

Re: [PATCH] LSM: Add post accept() hook.

[Paul Moore]

On 07/20/10 22:00, Tetsuo Handa wrote:
> Paul Moore wrote:
>> On Monday, July 19, 2010 09:36:31 pm Tetsuo Handa wrote:
>>> One is for dropping connections from unwanted hosts. Administrators define
>>> policy before enabling enforcing mode (the mode which connections are
>>> dropped if operation was not granted by policy). Administrators specify
>>> acceptable hosts (i.e. hosts which this host needs to communicate with)
>>> and unacceptable hosts (i.e. hosts which this host needn't to communicate
>>> with).
>>
>> You can enforce per-host access controls without the need for a post-accept() 
>> hooks, e.g. security_sock_rcv_skb() and the netfilter hooks 
>> (NF_INET_POST_ROUTING, NF_INET_FORWARD, NF_INET_LOCAL_OUT).  Or are you 
>> interested in controlling which hosts an _application_ can communicate with?
> 
> I'm interested in controlling which ports on which hosts a _process_ can
> communicate with. In TOMOYO's words, "processes that belong to which TOMOYO's
> domain can communicate with which ports on which hosts".

...

>>> Dropping connections would happen if some process was hijacked and the
>>> process attempted to communicate with other processes using TCP
>>> connections. But dropping connections should not happen in normal
>>> circumstance.
>>
>> It doesn't matter if dropping connections is normal or not, what matters is 
>> that it can happen.
>>
>>> The other is for updating process's state variable upon accept() operation.
>>> LKM version of TOMOYO has per a task_struct variable that is used for
>>> implementing stateful permissions. (As of now, not implemented for LSM
>>> version of TOMOYO.)
>>
>> I'm open to re-introducing a post-accept() hook that does not have a return 
>> value, in other words, a hook that can only be used to update LSM state and 
>> not affect the connection.  Although I do think you could probably achieve the 
>> same thing using some of the existing LSM hooks (look at how SELinux updates 
>> its state upon accept()) but that is something you would have to look it and 
>> see if it works for TOMOYO.
> 
> I can't figure out why the hook must not affect the connection.
> Is it possible to clarify using below players?

I understand what you are trying to accomplish and my concern is that
(using the example below) ClientApp1 has it's connection with Server1
dropped suddenly _after_ Server1 successfully completes the TCP
handshake.  I've stated this concern several times.  I would much prefer
you reject the incoming connection during the initial TCP handshake.

> Server1 and Client1 are hosts which are connected on TCP/IP network.
> ServerApp1 and ServerApp2 are applications running on Server1 which might call
> socket(), bind(), listen(), accept(), send(), recv(), shutdown(), close() and
> execute().
> ClientApp1 and ClientApp2 are applications running on Client1 which might call
> socket(), connect(), send(), recv(), shutdown(), close().
> Router1 and Router2 are routers which exist between Server1 and Client1.
> 
>   +-------+   +-------+   +-------+   +-------+
>   |Server1|---|Router1|---|Router2|---|Client1|
>   +-------+   +-------+   +-------+   +-------+

...

> Router1 and Router2 can inject RST into the already established connections
> at any time (if they are IDS/IPS or broken or malicious).
> How does security_socket_post_accept() returning an error differs from these
> routers injecting RST?

We can't control all of the different intermediate nodes on a given
network path and it is true that some of these intermediate nodes
sometimes do "Bad Things" to network traffic (due either to limitations
in the design, placement in the network or poor implementation).
However, just because other nodes do "Bad Things" does not mean we need
to allow "Bad Things" to happen in the Linux end nodes.

-- 
paul moore
linux @ hp

Re: [PATCH] LSM: Add post recvmsg() hook.

[David Miller]

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Sat, 17 Jul 2010 10:17:10 +0900

> NETWORKING [IPv4/IPv6] maintainers and Paul, is below patch fine for you?

Unfortunately, after further consideration, I must reject this patch
and also the post accept() LSM hook one.

Sorry.

I looked into history of the discussions on this issue, and I have found
that the core issue with these hooks has not been addressed.

We must ensure that if:

1) Application makes poll() on UDP socket in blocking mode, and UDP
   reports that receive data is available

and

2) Application, after such a poll() call, makes a blocking recvmsg() call
   and no other activity has occurred on the socket meanwhile

Then we MUST return immediately with that available data.

This LSM hook, when it triggers, can violate this rule, even if you do
this looping thing.

The post accept() hook has the same problems.

Here is where we originally discussed this, in detail:

http://www.spinics.net/lists/netdev/msg95660.html

Therefore, I think this shows that what Tomoyo is trying to do is
fatally flawed.  We brought this fundamental issue up to you about a
year ago, and the issue is still not addressed.

So consider very seriously, that what you are trying to do cannot be
performed without breaking applications and API behavioral
expectations.

Re: [PATCH] LSM: Add post recvmsg() hook.

[Tetsuo Handa]

David Miller wrote:
> From: Tetsuo Handa
> Date: Sat, 17 Jul 2010 10:17:10 +0900
> 
> > NETWORKING [IPv4/IPv6] maintainers and Paul, is below patch fine for you?
> 
> Unfortunately, after further consideration, I must reject this patch
> and also the post accept() LSM hook one.
> 
> Sorry.
> 
> I looked into history of the discussions on this issue, and I have found
> that the core issue with these hooks has not been addressed.
> 
> We must ensure that if:
> 
> 1) Application makes poll() on UDP socket in blocking mode, and UDP
>    reports that receive data is available
> 
> and
> 
> 2) Application, after such a poll() call, makes a blocking recvmsg() call
>    and no other activity has occurred on the socket meanwhile
> 
> Then we MUST return immediately with that available data.
> 
> This LSM hook, when it triggers, can violate this rule, even if you do
> this looping thing.
> 

Existing LSM hooks already violate this rule.
security_socket_accept() and security_socket_recvmsg() are allowed to return
immediately with error code instead of available data even if conditions (1)
and (2) are met.

> The post accept() hook has the same problems.
> 
> Here is where we originally discussed this, in detail:
> 
> http://www.spinics.net/lists/netdev/msg95660.html
> 
> Therefore, I think this shows that what Tomoyo is trying to do is
> fatally flawed.  We brought this fundamental issue up to you about a
> year ago, and the issue is still not addressed.
> 
> So consider very seriously, that what you are trying to do cannot be
> performed without breaking applications and API behavioral
> expectations.

LSM is something that breaks applications and API behavioral expectations.

For example, an application called access("/bin/sh", X_OK) and assumed that
execution of /bin/sh will be permitted unless somebody does chmod("/bin/sh", 0)
before the application calls execve("/bin/sh"). But however, if LSM's policy
changed from "allow execution of /bin/sh" to "deny execution of /bin/sh"
between access("/bin/sh", X_OK) and execve("/bin/sh"), the application was
broken by the LSM. Although such change unlikely happens in normal
circumstance, it can happen and we tolerate such breakage.

Another example. An application called select() on non-socket object (e.g.
regular file). The select() will say "read operation will not block" and
the application will call read() with expectations that read() returns
immediately with available data (or EOF) rather than error code unless
somebody does chmod("the file", 0). But however, if LSM's policy changed from
"allow reading the file" to "deny reading the file" between select() and
read(), the application was broken by the LSM. Although such change unlikely
happens in normal circumstance, it can happen and we tolerate such breakage.

Another example. An application called socket(), bind() and listen().
A connection request arrived and enqueued into the listening socket's backlog.
Now, select() starts saying "read operation will not block" and the application
calls accept() with expectations that accept() returns immediately with
established connection rather than error code. But however, if LSM's policy
changed from "allow picking up the connection" to "deny picking up the
connection" between select() and accept(), the application was broken by the
LSM. Although such change unlikely happens in normal circumstance, it can
happen and we tolerate such breakage.

The only difference between security_socket_accept()/security_socket_recvmsg()
and security_socket_post_accept()/security_socket_post_recvmsg() is that
the connection/datagram in the queue is removed or not when these LSM hooks
returned error code. Existing LSM hooks already made it impossible to return
available data even if conditions (1) and (2) are met.



Generally speaking, the connection/datagram being not removed from the queue
when these LSM hooks returned error code might be preferable. But for TOMOYO,
the connection/datagram being removed from the queue is preferable.
Reasons shown below.

TOMOYO is concerned with protecting applications with minimal side effects.
Being unable to boot the system by granting chmod("/sbin/init", 0) or being
unable to login to the system by granting rename("/etc/shadow", "/etc/shadow0")
or being unable to allocate CPU time for each application by making specific
applications to eat 100% of CPU time etc. are serious side effects for TOMOYO.

Say, there are 100 connections/datagrams in the socket's queue and 1 out of 100
is the connection/datagram which should not be delivered to the application.

(a) If the caller didn't close() the socket when security_socket_accept()/
    security_socket_recvmsg() returned error code, subsequent select() will say
    "read operation will not block" and the caller will immediately call
    accept()/recvmsg() again. This lets the application to spend 100% of CPU
    time for only 1 connection/datagram which can not be picked up. This is
    nearly DoS for server side and completely DoS for client side.

(b) If the caller close()d the socket when security_socket_accept()/
    security_socket_recvmsg() returned error code, all queued connections/
    datagrams are discarded. The connections/datagrams which should be
    delivered to the application are discarded (i.e. 99 connections/datagrams
    are disturbed by only 1 connection/datagram).
    Therefore, I will have to ask application developers to modify the
    application to call close(), socket(), bind(), listen(), accept()
    (regarding server side) and call close(), socket(), connect() (regarding
    client side) whenever security_socket_accept()/security_socket_recvmsg()
    returned an error. This is nearly DoS for client side.

Silently dropping the 1 connection/datagram with returning non-fatal error code
(e.g. -EAGAIN) (or wait for next connection/datagram unless MSG_DONTWAIT or
O_NONBLOCK is set) seems to give minimal side effects to both server side and
client side. But if you still cannot tolerate dropping the connection/datagram,
what about below idea?

 int udp_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 		size_t len, int noblock, int flags, int *addr_len)
 {
 	struct inet_sock *inet = inet_sk(sk);
 	struct sockaddr_in *sin = (struct sockaddr_in *)msg->msg_name;
 	struct sk_buff *skb;
 	unsigned int ulen;
 	int peeked;
 	int err;
 	int is_udplite = IS_UDPLITE(sk);
 	bool slow;
+	bool peek_forced;
 
 	/*
 	 *      Check any passed addresses
 	 */
 	if (addr_len)
 		*addr_len = sizeof(*sin);
 
 	if (flags & MSG_ERRQUEUE)
 		return ip_recv_error(sk, msg, len);
 
+	/* LSM wants to decide permission based on skb? */
+	peek_forced = security_socket_recvmsg_force_peek(sk);
 try_again:
-	skb = __skb_recv_datagram(sk, flags | (noblock ? MSG_DONTWAIT : 0),
-				  &peeked, &err);
+	skb = __skb_recv_datagram(sk, flags | (noblock ? MSG_DONTWAIT : 0) |
+				  (peek_forced ? MSG_PEEK : 0), &peeked, &err);
 	if (!skb)
 		goto out;
+	if (peek_forced) {
+		err = security_socket_post_recvmsg(sk, skb);
+		if (err < 0) {
+			/*
+			 * Do not remove this message from queue because LSM
+			 * decided not to deliver this message to the caller.
+			 */
+			peek_forced = false;
+			goto out_free;
+		}
+	}
 
 	ulen = skb->len - sizeof(struct udphdr);
 	if (len > ulen)
 		len = ulen;
 	else if (len < ulen)
 		msg->msg_flags |= MSG_TRUNC;
(...snipped...)
 out_free:
+	if (peek_forced && !(flags & MSG_PEEK)) {
+		/*
+		 * Remove this message from queue because this message was
+		 * peeked for LSM but the caller did not ask to peek.
+		 */
+		slow = lock_sock_fast(sk);
+		skb_kill_datagram(sk, skb, flags);
+		unlock_sock_fast(sk, slow);
+		goto out;
+	}
 	skb_free_datagram_locked(sk, skb);
 out:
 	return err;
(...snipped...)
 }

where security_socket_recvmsg_force_peek() returns true (if LSM module wants to
do permission check based on skb) or false (if LSM module does not want to do
permission check based on skb) and security_socket_post_recvmsg() returns error
code (if LSM module decided not to deliver) or 0 (if LSM module decided to
deliver). security_socket_post_recvmsg() must not call skb_kill_datagram().
In this way, security_socket_post_recvmsg() can keep socket's queue state
intact like security_socket_recvmsg() (but side effects (a) and (b) remains as
well as security_socket_recvmsg() hook).

Do permission checks upon enqueue time and do not perform permission check upon
dequeue time cannot be an answer. Side effects with security_socket_accept()/
security_socket_recvmsg() are what SELinux is experiencing as well as TOMOYO
will experience. (Though, it seems to me that SELinux is not interested in such
side effects.)

Regards.

Re: [PATCH] LSM: Add post recvmsg() hook.

[David Miller]

Your analysis is wrong, and what Tomoyo is doing is so fundamentally
different than what the existing SELINUX hooks do.

The existing LSM hooks do not break BSD socket behavior.  Do you know
why?  Because someone who understood all of this spent a great deal
of time carefully designing them.

The existing hooks do not drop packets on recvmsg() when a security
check does not pass, they signal the error long before the socket
receive queue is even looked at.  It is just like seeing a -EFAULT,
-ENFILE, or similar error.

Checks are always made _BEFORE_ major state changes are made to the
socket.

That is critically important, and it's what you seem to fail to see.

The hooks you propose _LOSE_ information.  So even if another process
has the 'fd' for a socket, and they would be allowed to receive the
packet by LSM checks, the post hook does not allow that to happen
because the failing 'fd' just frees up the packet and loses it
forever.

The existing hooks signal before we pull the new connection out of the
accept queue during accept(), therefore avoiding the illegal situation
your post ->accept() hook would create since there is absolutely no
way (and there should not be a way) to push a connection back into the
sock accept queue after we've taken it from the protocol layer.

And again here, the proposed hooks _LOSE_ information.  The accepted
connection is lost forever, another process with valid security
credentials cannot accept the connection.  It is completely gone.

And I'm not even going to entertain adding facilities to allow pushing
things back into the socket state after they've been removed for
inspection.

I think we've been through this issue enough times that we have covered
the issues in their entirety, and nothing you have written convinces
me that my position is wrong and that it is valid to put the Tomoyo
post-recvmsg and post-accept hooks into the tree.

Sorry, but I'm not applying your patches, they are fundamentally flawed
unlike the existing hooks.

Re: [PATCH] LSM: Add post recvmsg() hook.

[Tetsuo Handa]

David Miller wrote:
> Your analysis is wrong, and what Tomoyo is doing is so fundamentally
> different than what the existing SELINUX hooks do.
> 
> The existing LSM hooks do not break BSD socket behavior.  Do you know
> why?  Because someone who understood all of this spent a great deal
> of time carefully designing them.
> 
> The existing hooks do not drop packets on recvmsg() when a security
> check does not pass, they signal the error long before the socket
> receive queue is even looked at.  It is just like seeing a -EFAULT,
> -ENFILE, or similar error.
> 
> Checks are always made _BEFORE_ major state changes are made to the
> socket.

Excuse me, below check is made inside recvmsg() and may return error if
SELinux's policy has changed after the select() said "ready" and before
security_socket_recvmsg() is called. No?

int avc_has_perm_noaudit(u32 ssid, u32 tsid,
                         u16 tclass, u32 requested,
                         unsigned flags,
                         struct av_decision *in_avd)
{
        struct avc_node *node;
        struct av_decision avd_entry, *avd;
        int rc = 0;
        u32 denied;

        BUG_ON(!requested);

        rcu_read_lock();

        node = avc_lookup(ssid, tsid, tclass);
        if (!node) {
                rcu_read_unlock();

                if (in_avd)
                        avd = in_avd;
                else
                        avd = &avd_entry;

                security_compute_av(ssid, tsid, tclass, avd);
                rcu_read_lock();
                node = avc_insert(ssid, tsid, tclass, avd);
        } else {
                if (in_avd)
                        memcpy(in_avd, &node->ae.avd, sizeof(*in_avd));
                avd = &node->ae.avd;
        }

        denied = requested & ~(avd->allowed);

        if (denied) {
                if (flags & AVC_STRICT)
                        rc = -EACCES;
                else if (!selinux_enforcing || (avd->flags & AVD_FLAGS_PERMISSIVE))
                        avc_update_node(AVC_CALLBACK_GRANT, requested, ssid,
                                        tsid, tclass, avd->seqno);
                else
                        rc = -EACCES;
        }

        rcu_read_unlock();
        return rc;
}

int avc_has_perm(u32 ssid, u32 tsid, u16 tclass,
                 u32 requested, struct common_audit_data *auditdata)
{
        struct av_decision avd;
        int rc;

        rc = avc_has_perm_noaudit(ssid, tsid, tclass, requested, 0, &avd);
        avc_audit(ssid, tsid, tclass, requested, &avd, rc, auditdata);
        return rc;
}

static int socket_has_perm(struct task_struct *task, struct socket *sock,
                           u32 perms)
{
        struct inode_security_struct *isec;
        struct common_audit_data ad;
        u32 sid;
        int err = 0;

        isec = SOCK_INODE(sock)->i_security;

        if (isec->sid == SECINITSID_KERNEL)
                goto out;
        sid = task_sid(task);

        COMMON_AUDIT_DATA_INIT(&ad, NET);
        ad.u.net.sk = sock->sk;
        err = avc_has_perm(sid, isec->sid, isec->sclass, perms, &ad);

out:
        return err;
}

static int selinux_socket_recvmsg(struct socket *sock, struct msghdr *msg,
                                  int size, int flags)
{
        return socket_has_perm(current, sock, SOCKET__READ);
}

static struct security_operations selinux_ops = {
(...snipped...)
	.socket_recvmsg =               selinux_socket_recvmsg,
(...snipped...)
};

Are you saying that selinux_socket_recvmsg() always returns 0?

> That is critically important, and it's what you seem to fail to see.
> 
> The hooks you propose _LOSE_ information.  So even if another process
> has the 'fd' for a socket, and they would be allowed to receive the
> packet by LSM checks, the post hook does not allow that to happen
> because the failing 'fd' just frees up the packet and loses it
> forever.
> 
> The existing hooks signal before we pull the new connection out of the
> accept queue during accept(), therefore avoiding the illegal situation
> your post ->accept() hook would create since there is absolutely no
> way (and there should not be a way) to push a connection back into the
> sock accept queue after we've taken it from the protocol layer.
> 
> And again here, the proposed hooks _LOSE_ information.  The accepted
> connection is lost forever, another process with valid security
> credentials cannot accept the connection.  It is completely gone.
> 
> And I'm not even going to entertain adding facilities to allow pushing
> things back into the socket state after they've been removed for
> inspection.
> 
> I think we've been through this issue enough times that we have covered
> the issues in their entirety, and nothing you have written convinces
> me that my position is wrong and that it is valid to put the Tomoyo
> post-recvmsg and post-accept hooks into the tree.
> 
> Sorry, but I'm not applying your patches, they are fundamentally flawed
> unlike the existing hooks.

Did the idea described in previous mail _LOSE_ information?
I made the udp_recvmsg() to force MSG_PEEK so that the message will not be
removed from the queue if security_socket_post_recvmsg() returned error code
and remove the message from the queue only if security_socket_post_recvmsg()
returned 0 and the caller did not pass MSG_PEEK.

Re: [PATCH] LSM: Add post recvmsg() hook.

[David Miller]

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Thu, 22 Jul 2010 13:41:38 +0900

> Excuse me, below check is made inside recvmsg() and may return error if
> SELinux's policy has changed after the select() said "ready" and before
> security_socket_recvmsg() is called. No?

It does this before pulling the packet out of the receive queue of the
socket.  It's like signalling a parameter error to the process, no
socket state is changed.

Re: [PATCH] LSM: Add post recvmsg() hook.

[Tetsuo Handa]

David Miller wrote:
> From: Tetsuo Handa
> Date: Thu, 22 Jul 2010 13:41:38 +0900
> 
> > Excuse me, below check is made inside recvmsg() and may return error if
> > SELinux's policy has changed after the select() said "ready" and before
> > security_socket_recvmsg() is called. No?
> 
> It does this before pulling the packet out of the receive queue of the
> socket.  It's like signalling a parameter error to the process, no
> socket state is changed.

So, we agreed that security_socket_recvmsg() is allowed to return error code
rather than available data even if both conditions

1) Application makes poll() on UDP socket in blocking mode, and UDP
   reports that receive data is available

and

2) Application, after such a poll() call, makes a blocking recvmsg() call
   and no other activity has occurred on the socket meanwhile

are met.

Then, why does below proposal lose information?
The message is not removed if security_socket_post_recvmsg() returned error code.

 int udp_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 		size_t len, int noblock, int flags, int *addr_len)
 {
 	struct inet_sock *inet = inet_sk(sk);
 	struct sockaddr_in *sin = (struct sockaddr_in *)msg->msg_name;
 	struct sk_buff *skb;
 	unsigned int ulen;
 	int peeked;
 	int err;
 	int is_udplite = IS_UDPLITE(sk);
 	bool slow;
+	bool peek_forced;
 
 	/*
 	 *      Check any passed addresses
 	 */
 	if (addr_len)
 		*addr_len = sizeof(*sin);
 
 	if (flags & MSG_ERRQUEUE)
 		return ip_recv_error(sk, msg, len);
 
+	/* LSM wants to decide permission based on skb? */
+	peek_forced = security_socket_recvmsg_force_peek(sk);
 try_again:
-	skb = __skb_recv_datagram(sk, flags | (noblock ? MSG_DONTWAIT : 0),
-				  &peeked, &err);
+	skb = __skb_recv_datagram(sk, flags | (noblock ? MSG_DONTWAIT : 0) |
+				  (peek_forced ? MSG_PEEK : 0), &peeked, &err);
 	if (!skb)
 		goto out;
+	if (peek_forced) {
+		err = security_socket_post_recvmsg(sk, skb);
+		if (err < 0) {
+			/*
+			 * Do not remove this message from queue because LSM
+			 * decided not to deliver this message to the caller.
+			 */
+			peek_forced = false;
+			goto out_free;
+		}
+	}
 
 	ulen = skb->len - sizeof(struct udphdr);
 	if (len > ulen)
 		len = ulen;
 	else if (len < ulen)
 		msg->msg_flags |= MSG_TRUNC;
(...snipped...)
 out_free:
+	if (peek_forced && !(flags & MSG_PEEK)) {
+		/*
+		 * Remove this message from queue because this message was
+		 * peeked for LSM but the caller did not ask to peek.
+		 */
+		slow = lock_sock_fast(sk);
+		skb_kill_datagram(sk, skb, flags);
+		unlock_sock_fast(sk, slow);
+		goto out;
+	}
 	skb_free_datagram_locked(sk, skb);
 out:
 	return err;
(...snipped...)
 }

Re: [PATCH] LSM: Add post recvmsg() hook.

[David Miller]

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Thu, 22 Jul 2010 14:02:16 +0900

> Then, why does below proposal lose information?

Peek changes state, now it's possible that two processes end up
receiving the packet.

Please consider deeply how your desired semantics are unobtainable
without breaking thigngs fundamentally.

Re: [PATCH] LSM: Add post recvmsg() hook.

[Tetsuo Handa]

David Miller wrote:
> > Then, why does below proposal lose information?
> 
> Peek changes state, now it's possible that two processes end up
> receiving the packet.

Indeed. We will need to protect sock->ops->recvmsg() call using a lock like

 static inline int __sock_recvmsg_nosec(struct kiocb *iocb, struct socket *sock,
 				       struct msghdr *msg, size_t size, int flags)
 {
+	int err;
 	struct sock_iocb *si = kiocb_to_siocb(iocb);
 
 	sock_update_classid(sock->sk);
 
 	si->sock = sock;
 	si->scm = NULL;
 	si->msg = msg;
 	si->size = size;
 	si->flags = flags;
 
-	return sock->ops->recvmsg(iocb, sock, msg, size, flags);
+	err = security_socket_read_lock(sock);
+	if (err)
+		return err;
+	err = sock->ops->recvmsg(iocb, sock, msg, size, flags);
+	security_socket_read_unlock(sock);
+	return err;
 }

in addition to security_socket_recvmsg_force_peek() and
security_socket_post_recvmsg().

But locks like above break MSG_DONTWAIT since recv() without MSG_DONTWAIT
calls wait_for_packet() inside __skb_recv_datagram().
To make MSG_DONTWAIT work, I have to do like below.

 struct sk_buff *__skb_recv_datagram(struct sock *sk, unsigned flags,
 				    int *peeked, int *err)
(...snipped...)
 	do {
 		/* Again only user level code calls this function, so nothing
 		 * interrupt level will suddenly eat the receive_queue.
 		 *
 		 * Look at current nfs client by the way...
 		 * However, this function was corrent in any case. 8)
 		 */
 		unsigned long cpu_flags;
 
+		/* < 0 if lock failed, 0 if no need to lock, > 0 if locked */
+		int serialized = security_socket_read_lock(sk);
+		if (serialized < 0) {
+			error = serialized;
+			goto no_packet;
+		} else if (serialized > 0) {
+			int err;
+			spin_lock_irqsave(&sk->sk_receive_queue.lock, cpu_flags);
+			skb = skb_peek(&sk->sk_receive_queue);
+			spin_unlock_irqrestore(&sk->sk_receive_queue.lock,
+					       cpu_flags);
+			if (!skb)
+				goto no_skb;
+			err = security_socket_pre_recvmsg(sk, skb);
+			if (err < 0) {
+				error = err;
+				security_socket_read_unlock(sk);
+				goto no_packet;
+			}
+		}
+
 		spin_lock_irqsave(&sk->sk_receive_queue.lock, cpu_flags);
 		skb = skb_peek(&sk->sk_receive_queue);
 		if (skb) {
 			*peeked = skb->peeked;
 			if (flags & MSG_PEEK) {
 				skb->peeked = 1;
 				atomic_inc(&skb->users);
 			} else
 				__skb_unlink(skb, &sk->sk_receive_queue);
 		}
 		spin_unlock_irqrestore(&sk->sk_receive_queue.lock, cpu_flags);
 
+no_skb:
+		if (serialized > 0)
+			security_socket_read_unlock(sk);
 		if (skb)
 			return skb;
 
 		/* User doesn't want to wait */
 		error = -EAGAIN;
 		if (!timeo)
 			goto no_packet;
 
 	} while (!wait_for_packet(sk, err, &timeo));
(...snipped...)
 }

Inserting LSM hooks like above will be the only way to work properly (i.e.
handle MSG_DONTWAIT and avoid showing the same message to multiple readers
and keep the queue's state unchanged upon error).
But you said ( http://marc.info/?l=linux-netdev&m=124022463014713&w=2 )

> We worked so hard to split out this common code, it is simply
> a non-starter for anyone to start putting protocol specific test
> into here, or even worse to move this code back to being locally
> copied into every protocol implementation.

when I proposed inserting LSM hooks into __skb_recv_datagram()
( http://marc.info/?l=linux-netdev&m=124022463014672&w=2 ).
So, I have no way to allow performing permission checks based on combination of
"process who issued recv() request" and "source address/port of the message
which the process is about to pick up" without breaking things (unless you
accept inserting LSM hooks into __skb_recv_datagram())...

Re: [PATCH] LSM: Add post recvmsg() hook.

[David Miller]

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Thu, 22 Jul 2010 21:46:55 +0900

> David Miller wrote:
>> > Then, why does below proposal lose information?
>> 
>> Peek changes state, now it's possible that two processes end up
>> receiving the packet.
> 
> Indeed. We will need to protect sock->ops->recvmsg() call using a lock like

But this doesn't matter.

The fact is going to remain that you will be unable to return data
from recvmsg() to a blocking socket when ->poll() returns true even
though data is in fact there in the socket receive queue.

This is something that the existing LSM hooks do not do.

You can't create this silly situation where some packets in the socket
receive queue can be recvmsg()'d by some processes, but not by others.

At best, it is pure crazyness.

Re: [PATCH] LSM: Add post recvmsg() hook.

[David Miller]

From: David Miller <davem@davemloft.net>
Date: Thu, 22 Jul 2010 10:22:51 -0700 (PDT)

> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Date: Thu, 22 Jul 2010 21:46:55 +0900
> 
>> David Miller wrote:
>>> > Then, why does below proposal lose information?
>>> 
>>> Peek changes state, now it's possible that two processes end up
>>> receiving the packet.
>> 
>> Indeed. We will need to protect sock->ops->recvmsg() call using a lock like
> 
> But this doesn't matter.

Also, btw, we're not adding a lock to a code path which we've worked
so hard to make largely lockless.  This lock is going to kill
performance.

Re: [PATCH] LSM: Add post recvmsg() hook.

[Tetsuo Handa]

David Miller wrote:
> The fact is going to remain that you will be unable to return data
> from recvmsg() to a blocking socket when ->poll() returns true even
> though data is in fact there in the socket receive queue.
> 
> This is something that the existing LSM hooks do not do.

This is something that the existing security_socket_recvmsg() hook does do.
SELinux is unable to return data from recvmsg() to a blocking socket when
->poll() returns true even though data is in fact there in the socket receive
queue.
We agreed below situation, didn't we?

Tetsuo Handa wrote:
> > > Excuse me, below check is made inside recvmsg() and may return error if
> > > SELinux's policy has changed after the select() said "ready" and before
> > > security_socket_recvmsg() is called. No?
> > 
> > It does this before pulling the packet out of the receive queue of the
> > socket.  It's like signalling a parameter error to the process, no
> > socket state is changed.
> 
> So, we agreed that security_socket_recvmsg() is allowed to return error code
> rather than available data even if both conditions
> 
> 1) Application makes poll() on UDP socket in blocking mode, and UDP
>    reports that receive data is available
> 
> and
> 
> 2) Application, after such a poll() call, makes a blocking recvmsg() call
>    and no other activity has occurred on the socket meanwhile
> 
> are met.

Re: [PATCH] LSM: Add post recvmsg() hook.

[David Miller]

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Fri, 23 Jul 2010 09:22:20 +0900

> David Miller wrote:
>> The fact is going to remain that you will be unable to return data
>> from recvmsg() to a blocking socket when ->poll() returns true even
>> though data is in fact there in the socket receive queue.
>> 
>> This is something that the existing LSM hooks do not do.
> 
> This is something that the existing security_socket_recvmsg() hook does do.
> SELinux is unable to return data from recvmsg() to a blocking socket when
> ->poll() returns true even though data is in fact there in the socket receive
> queue.
> We agreed below situation, didn't we?

Existing LSM hook returns an error early, as if the user passed in
incorrect parameters or similar.

It's completely stateless and dependent upon purely the labels
associated with state visible on recvmsg() entry, and independent
of other things such as attributes in the packets contained in
the socket's receive queue.

Re: [PATCH] LSM: Add post recvmsg() hook.

[Tetsuo Handa]

David Miller wrote:
> From: Tetsuo Handa
> Date: Fri, 23 Jul 2010 09:22:20 +0900
> 
> > David Miller wrote:
> >> The fact is going to remain that you will be unable to return data
> >> from recvmsg() to a blocking socket when ->poll() returns true even
> >> though data is in fact there in the socket receive queue.
> >> 
> >> This is something that the existing LSM hooks do not do.
> > 
> > This is something that the existing security_socket_recvmsg() hook does do.
> > SELinux is unable to return data from recvmsg() to a blocking socket when
> > ->poll() returns true even though data is in fact there in the socket receive
> > queue.
> > We agreed below situation, didn't we?
> 
> Existing LSM hook returns an error early, as if the user passed in
> incorrect parameters or similar.
> 
> It's completely stateless and dependent upon purely the labels
> associated with state visible on recvmsg() entry, and independent
> of other things such as attributes in the packets contained in
> the socket's receive queue.

Users calling recvmsg() after select() said "read operation will return with
available data without blocking as long as you pass correct parameters" will
get error code even if they passed correct parameters.

If they passed incorrect parameters, they must accept the error code.
But they passed correct parameters, and they expected that recvmsg() returns
success with available data. From the point of view of select()->recvmsg()
users, they don't care whether internal implementation is stateful and/or
dependent of other things or not, they care what the specification says.

My understanding was that the specification requires

  If select() said "read operation will return with available data without
  blocking", recvmsg() must return available data. No access control mechanism
  in recvmsg() path is allowed to reject the request.

and security_socket_recvmsg() conflicts with this understanding.
I'd like to see the specification which states disclaimers like below.

  (i) Even if select() said "read operation will return with available data
      without blocking", recvmsg() is allowed to return error code rather than
      available data when an access control mechanism in recvmsg() path
      rejected the request.

  (ii) The access control mechanism in recvmsg() path may use attribute of
       the socket.

  (iii) The access control mechanism in recvmsg() path must not use the
        packets contained in the socket's receive queue.

We agreed on (i) and I don't mind (ii).
Would you please show me URLs to the specification which states (iii)?

Re: [PATCH] LSM: Add post recvmsg() hook.

[David Miller]

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Fri, 23 Jul 2010 16:21:38 +0900

> Users calling recvmsg() after select() said "read operation will return with
> available data without blocking as long as you pass correct parameters" will
> get error code even if they passed correct parameters.

This is nonsense, because not all "parameters" are not visible to poll().

These "parameters" I speak of are virtual, they are my way of saying
that the errors returned by the existing LSM hooks are more like -EFAULT
or -EACCESS than -EINTR/-EAGAIN or blocking, the latter of which are
what are illegal for blocking socket invoking recvmsg() after poll
indicates there is data to read.

Re: [PATCH] LSM: Add post recvmsg() hook.

[Tetsuo Handa]

David Miller wrote:
> The fact is going to remain that you will be unable to return data
> from recvmsg() to a blocking socket when ->poll() returns true even
> though data is in fact there in the socket receive queue.
> 
Maybe I misunderstood here.

Are you worrying that TOMOYO will no longer be able to deliver remaining
packets in a receive queue once a packet from address/port which is not
permitted to pick up reached the head of the receive queue?
(Please answer "yes" or "no" here.)

If your answer is "yes", I tolerate the side effect (i.e. applications have to
close and recreate the socket in order to resume receiving packets).
My preferred behavior is to drop the packet but you will not accept it.
I accept not to drop the packet if you can accept security_socket_pre_recvmsg().

If your answer is "no", let me restart from reconfirming requirements which
LSM hook for recvmsg() must satisfy, before continuing discussion on
security_socket_pre_recvmsg() hook.