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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <201007220502.o6M52GJU098071@www262.sakura.ne.jp>
Date:	Thu, 22 Jul 2010 14:02:16 +0900
From:	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To:	davem@...emloft.net
Cc:	kuznet@....inr.ac.ru, pekkas@...core.fi, jmorris@...ei.org,
	yoshfuji@...ux-ipv6.org, kaber@...sh.net, paul.moore@...com,
	netdev@...r.kernel.org, linux-security-module@...r.kernel.org
Subject: Re: [PATCH] LSM: Add post recvmsg() hook.

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...)
 }

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