What is lock_sock() before skb_free_datagram() for?

David Miller wrote:
> 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.
You don't want LSM modules to perform protocol specific test inside
__skb_recv_datagram(). I see.

> You may want to think about how you can achieve your goals by putting
> these unpleasant hooks into some other location.
May I insert security_socket_post_recv_datagram() into the caller of
skb_recv_datagram() (as shown below)?

 include/linux/security.h |   39 +++++++++++++++++++++++++++++++++++++++
 net/ipv4/raw.c           |    5 +++++
 net/ipv4/udp.c           |    7 +++++++
 net/ipv6/raw.c           |    5 +++++
 net/ipv6/udp.c           |    7 +++++++
 net/socket.c             |    5 +++++
 security/capability.c    |   13 +++++++++++++
 security/security.c      |   11 +++++++++++
 8 files changed, 92 insertions(+)

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

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Sun, 19 Apr 2009 14:12:15 +0900

> David Miller wrote:
>> 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.
> You don't want LSM modules to perform protocol specific test inside
> __skb_recv_datagram(). I see.
> 
>> You may want to think about how you can achieve your goals by putting
>> these unpleasant hooks into some other location.
> May I insert security_socket_post_recv_datagram() into the caller of
> skb_recv_datagram() (as shown below)?

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

Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> writes:

> David Miller wrote:
>> 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.
> You don't want LSM modules to perform protocol specific test inside
> __skb_recv_datagram(). I see.
>
>> You may want to think about how you can achieve your goals by putting
>> these unpleasant hooks into some other location.
> May I insert security_socket_post_recv_datagram() into the caller of
> skb_recv_datagram() (as shown below)?

what is the purpose of having such hooks ?

>  include/linux/security.h |   39 +++++++++++++++++++++++++++++++++++++++
>  net/ipv4/raw.c           |    5 +++++
>  net/ipv4/udp.c           |    7 +++++++
>  net/ipv6/raw.c           |    5 +++++
>  net/ipv6/udp.c           |    7 +++++++
>  net/socket.c             |    5 +++++
>  security/capability.c    |   13 +++++++++++++
>  security/security.c      |   11 +++++++++++
>  8 files changed, 92 insertions(+)
>
> --- security-testing-2.6.git.orig/net/ipv4/raw.c
> +++ security-testing-2.6.git/net/ipv4/raw.c
> @@ -666,6 +666,11 @@ static int raw_recvmsg(struct kiocb *ioc
>  	skb = skb_recv_datagram(sk, flags, noblock, &err);
>  	if (!skb)
>  		goto out;
> +	err = security_socket_post_recv_datagram(sk, skb, flags);
> +	if (err) {
> +		skb_kill_datagram(sk, skb, flags);
> +		goto out;
> +	}
>  
>  	copied = skb->len;
>  	if (len < copied) {
> --- security-testing-2.6.git.orig/net/ipv4/udp.c
> +++ security-testing-2.6.git/net/ipv4/udp.c
> @@ -901,6 +901,13 @@ try_again:
>  				  &peeked, &err);
>  	if (!skb)
>  		goto out;
> +	err = security_socket_post_recv_datagram(sk, skb, flags);
> +	if (err) {
> +		lock_sock(sk);
> +		skb_kill_datagram(sk, skb, flags);
> +		release_sock(sk);
> +		goto out;
> +	}
>  
>  	ulen = skb->len - sizeof(struct udphdr);
>  	copied = len;
> --- security-testing-2.6.git.orig/net/ipv6/raw.c
> +++ security-testing-2.6.git/net/ipv6/raw.c
> @@ -465,6 +465,11 @@ static int rawv6_recvmsg(struct kiocb *i
>  	skb = skb_recv_datagram(sk, flags, noblock, &err);
>  	if (!skb)
>  		goto out;
> +	err = security_socket_post_recv_datagram(sk, skb, flags);
> +	if (err) {
> +		skb_kill_datagram(sk, skb, flags);
> +		goto out;
> +	}
>  
>  	copied = skb->len;
>  	if (copied > len) {
> --- security-testing-2.6.git.orig/net/ipv6/udp.c
> +++ security-testing-2.6.git/net/ipv6/udp.c
> @@ -208,6 +208,13 @@ try_again:
>  				  &peeked, &err);
>  	if (!skb)
>  		goto out;
> +	err = security_socket_post_recv_datagram(sk, skb, flags);
> +	if (err) {
> +		lock_sock(sk);
> +		skb_kill_datagram(sk, skb, flags);
> +		release_sock(sk);
> +		goto out;
> +	}
>  
>  	ulen = skb->len - sizeof(struct udphdr);
>  	copied = len;
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Samir Bellabes wrote:
> what is the purpose of having such hooks ?
Same as security_socket_post_accept() (i.e. to drop datagrams from unwanted
peers).

I need to understand the meaning of "poll()" returning "ready" to understand
why security_socket_accept() and security_socket_recvmsg() are permitted to
return an error (though these hooks don't remove from the queue).

My understanding is that "poll()" returning "ready" does not guarantee that
accept()/recvmsg() shall return a valid file descriptor/datagram;
"poll()" returning "ready" guarantees merely accept()/recvmsg() does not
need to wait for connection/datagram. (Otherwise, security_socket_accept()
and security_socket_recvmsg() have to be gone.)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html