[EDT][Patch 1/1] socket family check in netlabel APIs

[EDT][Patch 1/1] socket family check in netlabel APIs

[Maninder Singh]

EP-E68D5E24548545C9BBB607A98ADD61E6

Hi Paul,

>On Monday, March 30, 2015 11:09:00 AM Maninder Singh wrote:
>> Dear All,
>> we found One Kernel Crash issue in cipso_v4_sock_delattr :-
>> As Cipso supports only inet sockets so cipso_v4_sock_delattr will crash when
>> try to access any other socket type.  cipso_v4_sock_delattr access
>> sk_inet->inet_opt which may contain not NULL but invalid address. we found
>> this issue with netlink socket.(reproducible by trinity using sendto system
>> call .) 

>Hello,

>First, please go read the Documentation/SubmittingPatches from the kernel 
>sources; your patch needs to be resubmitted and the instructions in that file 
>will show you how to do it correctly next time.

>Second, this appears to only affect Smack based systems, yes?  SELinux based 
>systems should have the proper checking in place to prevent this (the checks 
>are handled in the LSM).  That said, it probably wouldn't hurt to add the 
>extra checking to netlbl_sock_delattr().  If you properly resubmit your patch 
>I'll ACK it.

>-Paul

>-- 
>paul moore
>www.paul-moore.com

As suggested resubmitting the patch .

Subject : socket family check in netlabel APIs

Adding check for socket family in netlbl_sock_delattr and netlbl_req_delattr as check present
 in netlbl_sock_setattr and netlbl_req_setattr respectively.

as we faced crash in cipso_v4_sock_delattr due to other socket type.
Crash Logs :
[0-182.2400] [<c04c7fa4>] (cipso_v4_sock_delattr+0x0/0x74) from [<c0517b64>] (netlbl_sock_delattr+0x18/0x1c)
[0-182.2497]  r4:00000000 r3:c07872f8
[0-182.2531] [<c0517b4c>] (netlbl_sock_delattr+0x0/0x1c) from [<c027b2fc>] (smack_netlabel+0x88/0x9c)
[0-182.2622] [<c027b274>] (smack_netlabel+0x0/0x9c) from [<c027b43c>] (smack_netlabel_send+0x12c/0x144)
[0-182.2714]  r7 9ce9500 r6 7b67ef4 r5:c076f408 r4 8903dc0
[0-182.2770] [<c027b310>] (smack_netlabel_send+0x0/0x144) from [<c027b4a8>] (smack_socket_sendmsg+0x54/0x60)
[0-182.2866] [<c027b454>] (smack_socket_sendmsg+0x0/0x60) from [<c02789ec>] (security_socket_sendmsg+0x28/0x2c)
[0-182.2966] [<c02789c4>] (security_socket_sendmsg+0x0/0x2c) from [<c04343b0>] (sock_sendmsg+0x68/0xc0)
[0-182.3058] [<c0434348>] (sock_sendmsg+0x0/0xc0) from [<c04369e8>] (SyS_sendto+0xd8/0x110)

Signed-off-by: Vaneet Narang <v.narang@samsung.com>
Signed-off-by: Maninder Singh <maninder1.s@samsung.com>
Reviewed-by : Ajeet Yadav <ajeet.y@samsung.com>
---
 net/netlabel/netlabel_kapi.c |   16 ++++++++++++++--
 1 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/net/netlabel/netlabel_kapi.c b/net/netlabel/netlabel_kapi.c
index 28cddc8..606a5ce 100644
--- a/net/netlabel/netlabel_kapi.c
+++ b/net/netlabel/netlabel_kapi.c
@@ -824,7 +824,13 @@ socket_setattr_return:
  */
 void netlbl_sock_delattr(struct sock *sk)
 {
-	cipso_v4_sock_delattr(sk);
+	switch (sk->sk_family) {
+	case AF_INET:
+		cipso_v4_sock_delattr(sk);
+		break;
+	default:
+	}
+	return;
 }
 
 /**
@@ -987,7 +993,13 @@ req_setattr_return:
 */
 void netlbl_req_delattr(struct request_sock *req)
 {
-	cipso_v4_req_delattr(req);
+	switch (req->rsk_ops->family) {
+	case AF_INET:
+		cipso_v4_req_delattr(req);
+		break;
+	default:
+	}
+	return;
 }
 
 /**
-- 
1.7.1

Re: [EDT][Patch 1/1] socket family check in netlabel APIs

[Paul Moore]

On Thu, May 7, 2015 at 1:53 AM, Maninder Singh <maninder1.s@samsung.com> wrote:
> EP-E68D5E24548545C9BBB607A98ADD61E6
>
> Hi Paul,
>
>>On Monday, March 30, 2015 11:09:00 AM Maninder Singh wrote:
>>> Dear All,
>>> we found One Kernel Crash issue in cipso_v4_sock_delattr :-
>>> As Cipso supports only inet sockets so cipso_v4_sock_delattr will crash when
>>> try to access any other socket type.  cipso_v4_sock_delattr access
>>> sk_inet->inet_opt which may contain not NULL but invalid address. we found
>>> this issue with netlink socket.(reproducible by trinity using sendto system
>>> call .)
>
>>Hello,
>
>>First, please go read the Documentation/SubmittingPatches from the kernel
>>sources; your patch needs to be resubmitted and the instructions in that file
>>will show you how to do it correctly next time.
>
>>Second, this appears to only affect Smack based systems, yes?  SELinux based
>>systems should have the proper checking in place to prevent this (the checks
>>are handled in the LSM).  That said, it probably wouldn't hurt to add the
>>extra checking to netlbl_sock_delattr().  If you properly resubmit your patch
>>I'll ACK it.
>
>>-Paul
>
>>--
>>paul moore
>>www.paul-moore.com
>
> As suggested resubmitting the patch .

I was delayed in responding because your patch is still not in a form
that makes it easy to merge/review upstream, it appears to be MIME
encoded and not in plain text.  You should be able to save your raw
email message and apply it directly to the kernel source tree using
the patch command.

When you send MIME encoded emails with patches, we have to apply the
patches manually, line by line, which is both time consuming and error
prone.

-- 
paul moore
www.paul-moore.com