[PATCH 0/1] netlabel: KMSAN warning

[PATCH 0/1] netlabel: KMSAN warning

[Andrew Kanner]

Recently I started to use KMSAN and found the warning in
security/selinux/hooks.c which triggers each time I use the official
guide to run syzkaller reproducers

Link: https://github.com/google/syzkaller/blob/master/docs/syzbot_assets.md#run-a-c-reproducer

I'm not quiet confident what are the rules in security subsystem. It's
not a bug, but a warning which is triggered by KMSAN for the argument
of the security_net_peersid_resolve() which was not initialized. It
will not affect anything inside this function, at least with the
current order of checking the variables, which might eventually
change.

Please reply if you're ok with such not-a-bug fixes. Otherwise I'll
just ignore this warning in my experiments with KMSAN.

PS: most likely if fixes commit 220deb966ea5 ("SELinux: Better
integration between peer labeling subsystems"). Not sure if this tag
is needed for this patch.

Andrew Kanner (1):
  selinux: netlabel: Prevent KMSAN warning in
    selinux_inet_conn_request()

 security/selinux/netlabel.c | 1 +
 1 file changed, 1 insertion(+)

-- 
2.39.3

[PATCH 1/1] selinux: netlabel: Prevent KMSAN warning in selinux_inet_conn_request()

[Andrew Kanner]

KMSAN reports the following issue:
[   81.822503] =====================================================
[   81.823222] BUG: KMSAN: uninit-value in selinux_inet_conn_request+0x2c8/0x4b0
[   81.823891]  selinux_inet_conn_request+0x2c8/0x4b0
[   81.824385]  security_inet_conn_request+0xc0/0x160
[   81.824886]  tcp_v4_route_req+0x30e/0x490
[   81.825343]  tcp_conn_request+0xdc8/0x3400
[   81.825813]  tcp_v4_conn_request+0x134/0x190
[   81.826292]  tcp_rcv_state_process+0x1f4/0x3b40
[   81.826797]  tcp_v4_do_rcv+0x9ca/0xc30
[   81.827236]  tcp_v4_rcv+0x3bf5/0x4180
[   81.827670]  ip_protocol_deliver_rcu+0x822/0x1230
[   81.828174]  ip_local_deliver_finish+0x259/0x370
[   81.828667]  ip_local_deliver+0x1c0/0x450
[   81.829105]  ip_sublist_rcv+0xdc1/0xf50
[   81.829534]  ip_list_rcv+0x72e/0x790
[   81.829941]  __netif_receive_skb_list_core+0x10d5/0x1180
[   81.830499]  netif_receive_skb_list_internal+0xc41/0x1190
[   81.831064]  napi_complete_done+0x2c4/0x8b0
[   81.831532]  e1000_clean+0x12bf/0x4d90
[   81.831983]  __napi_poll+0xa6/0x760
[   81.832391]  net_rx_action+0x84c/0x1550
[   81.832831]  __do_softirq+0x272/0xa6c
[   81.833239]  __irq_exit_rcu+0xb7/0x1a0
[   81.833654]  irq_exit_rcu+0x17/0x40
[   81.834044]  common_interrupt+0x8d/0xa0
[   81.834494]  asm_common_interrupt+0x2b/0x40
[   81.834949]  default_idle+0x17/0x20
[   81.835356]  arch_cpu_idle+0xd/0x20
[   81.835766]  default_idle_call+0x43/0x70
[   81.836210]  do_idle+0x258/0x800
[   81.836581]  cpu_startup_entry+0x26/0x30
[   81.837002]  __pfx_ap_starting+0x0/0x10
[   81.837444]  secondary_startup_64_no_verify+0x17a/0x17b
[   81.837979]
[   81.838166] Local variable nlbl_type.i created at:
[   81.838596]  selinux_inet_conn_request+0xe3/0x4b0
[   81.839078]  security_inet_conn_request+0xc0/0x160

KMSAN warning is reproducible with:
* netlabel_mgmt_protocount is 0 (e.g. netlbl_enabled() returns 0)
* CONFIG_SECURITY_NETWORK_XFRM may be set or not
* CONFIG_KMSAN=y
* `ssh USER@HOSTNAME /bin/date`

selinux_skb_peerlbl_sid() will call selinux_xfrm_skb_sid(), then fall
to selinux_netlbl_skbuff_getsid() which will not initialize nlbl_type,
but it will be passed to:

    err = security_net_peersid_resolve(nlbl_sid,
                                       nlbl_type, xfrm_sid, sid);

and checked by KMSAN, although it will not be used inside
security_net_peersid_resolve() (at least now), since this function
will check either (xfrm_sid == SECSID_NULL) or (nlbl_sid ==
SECSID_NULL) first and return before using uninitialized nlbl_type.

Signed-off-by: Andrew Kanner <andrew.kanner@gmail.com>
---
 security/selinux/netlabel.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/security/selinux/netlabel.c b/security/selinux/netlabel.c
index 528f5186e912..8f182800e412 100644
--- a/security/selinux/netlabel.c
+++ b/security/selinux/netlabel.c
@@ -198,6 +198,7 @@ int selinux_netlbl_skbuff_getsid(struct sk_buff *skb,
 	struct netlbl_lsm_secattr secattr;
 
 	if (!netlbl_enabled()) {
+		*type = NETLBL_NLTYPE_NONE;
 		*sid = SECSID_NULL;
 		return 0;
 	}
-- 
2.39.3

Re: [PATCH 1/1] selinux: netlabel: Prevent KMSAN warning in selinux_inet_conn_request()

[Greg KH]

On Tue, Aug 15, 2023 at 10:59:17PM +0200, Andrew Kanner wrote:
> KMSAN reports the following issue:
> [   81.822503] =====================================================
> [   81.823222] BUG: KMSAN: uninit-value in selinux_inet_conn_request+0x2c8/0x4b0
> [   81.823891]  selinux_inet_conn_request+0x2c8/0x4b0
> [   81.824385]  security_inet_conn_request+0xc0/0x160
> [   81.824886]  tcp_v4_route_req+0x30e/0x490
> [   81.825343]  tcp_conn_request+0xdc8/0x3400
> [   81.825813]  tcp_v4_conn_request+0x134/0x190
> [   81.826292]  tcp_rcv_state_process+0x1f4/0x3b40
> [   81.826797]  tcp_v4_do_rcv+0x9ca/0xc30
> [   81.827236]  tcp_v4_rcv+0x3bf5/0x4180
> [   81.827670]  ip_protocol_deliver_rcu+0x822/0x1230
> [   81.828174]  ip_local_deliver_finish+0x259/0x370
> [   81.828667]  ip_local_deliver+0x1c0/0x450
> [   81.829105]  ip_sublist_rcv+0xdc1/0xf50
> [   81.829534]  ip_list_rcv+0x72e/0x790
> [   81.829941]  __netif_receive_skb_list_core+0x10d5/0x1180
> [   81.830499]  netif_receive_skb_list_internal+0xc41/0x1190
> [   81.831064]  napi_complete_done+0x2c4/0x8b0
> [   81.831532]  e1000_clean+0x12bf/0x4d90
> [   81.831983]  __napi_poll+0xa6/0x760
> [   81.832391]  net_rx_action+0x84c/0x1550
> [   81.832831]  __do_softirq+0x272/0xa6c
> [   81.833239]  __irq_exit_rcu+0xb7/0x1a0
> [   81.833654]  irq_exit_rcu+0x17/0x40
> [   81.834044]  common_interrupt+0x8d/0xa0
> [   81.834494]  asm_common_interrupt+0x2b/0x40
> [   81.834949]  default_idle+0x17/0x20
> [   81.835356]  arch_cpu_idle+0xd/0x20
> [   81.835766]  default_idle_call+0x43/0x70
> [   81.836210]  do_idle+0x258/0x800
> [   81.836581]  cpu_startup_entry+0x26/0x30
> [   81.837002]  __pfx_ap_starting+0x0/0x10
> [   81.837444]  secondary_startup_64_no_verify+0x17a/0x17b
> [   81.837979]
> [   81.838166] Local variable nlbl_type.i created at:
> [   81.838596]  selinux_inet_conn_request+0xe3/0x4b0
> [   81.839078]  security_inet_conn_request+0xc0/0x160
> 
> KMSAN warning is reproducible with:
> * netlabel_mgmt_protocount is 0 (e.g. netlbl_enabled() returns 0)
> * CONFIG_SECURITY_NETWORK_XFRM may be set or not
> * CONFIG_KMSAN=y
> * `ssh USER@HOSTNAME /bin/date`
> 
> selinux_skb_peerlbl_sid() will call selinux_xfrm_skb_sid(), then fall
> to selinux_netlbl_skbuff_getsid() which will not initialize nlbl_type,
> but it will be passed to:
> 
>     err = security_net_peersid_resolve(nlbl_sid,
>                                        nlbl_type, xfrm_sid, sid);
> 
> and checked by KMSAN, although it will not be used inside
> security_net_peersid_resolve() (at least now), since this function
> will check either (xfrm_sid == SECSID_NULL) or (nlbl_sid ==
> SECSID_NULL) first and return before using uninitialized nlbl_type.
> 
> Signed-off-by: Andrew Kanner <andrew.kanner@gmail.com>
> ---
>  security/selinux/netlabel.c | 1 +
>  1 file changed, 1 insertion(+)

What git commit id does this fix?  Does it also need to go to older
kernels?  If so, how far back?

thanks,

greg k-h

Re: [PATCH 1/1] selinux: netlabel: Prevent KMSAN warning in selinux_inet_conn_request()

[Andrew Kanner]

On Tue, Aug 15, 2023 at 11:11:57PM +0200, Greg KH wrote:
> On Tue, Aug 15, 2023 at 10:59:17PM +0200, Andrew Kanner wrote:
> > KMSAN reports the following issue:
> > [   81.822503] =====================================================
> > [   81.823222] BUG: KMSAN: uninit-value in selinux_inet_conn_request+0x2c8/0x4b0
> > [   81.823891]  selinux_inet_conn_request+0x2c8/0x4b0
> > [   81.824385]  security_inet_conn_request+0xc0/0x160
> > [   81.824886]  tcp_v4_route_req+0x30e/0x490
> > [   81.825343]  tcp_conn_request+0xdc8/0x3400
> > [   81.825813]  tcp_v4_conn_request+0x134/0x190
> > [   81.826292]  tcp_rcv_state_process+0x1f4/0x3b40
> > [   81.826797]  tcp_v4_do_rcv+0x9ca/0xc30
> > [   81.827236]  tcp_v4_rcv+0x3bf5/0x4180
> > [   81.827670]  ip_protocol_deliver_rcu+0x822/0x1230
> > [   81.828174]  ip_local_deliver_finish+0x259/0x370
> > [   81.828667]  ip_local_deliver+0x1c0/0x450
> > [   81.829105]  ip_sublist_rcv+0xdc1/0xf50
> > [   81.829534]  ip_list_rcv+0x72e/0x790
> > [   81.829941]  __netif_receive_skb_list_core+0x10d5/0x1180
> > [   81.830499]  netif_receive_skb_list_internal+0xc41/0x1190
> > [   81.831064]  napi_complete_done+0x2c4/0x8b0
> > [   81.831532]  e1000_clean+0x12bf/0x4d90
> > [   81.831983]  __napi_poll+0xa6/0x760
> > [   81.832391]  net_rx_action+0x84c/0x1550
> > [   81.832831]  __do_softirq+0x272/0xa6c
> > [   81.833239]  __irq_exit_rcu+0xb7/0x1a0
> > [   81.833654]  irq_exit_rcu+0x17/0x40
> > [   81.834044]  common_interrupt+0x8d/0xa0
> > [   81.834494]  asm_common_interrupt+0x2b/0x40
> > [   81.834949]  default_idle+0x17/0x20
> > [   81.835356]  arch_cpu_idle+0xd/0x20
> > [   81.835766]  default_idle_call+0x43/0x70
> > [   81.836210]  do_idle+0x258/0x800
> > [   81.836581]  cpu_startup_entry+0x26/0x30
> > [   81.837002]  __pfx_ap_starting+0x0/0x10
> > [   81.837444]  secondary_startup_64_no_verify+0x17a/0x17b
> > [   81.837979]
> > [   81.838166] Local variable nlbl_type.i created at:
> > [   81.838596]  selinux_inet_conn_request+0xe3/0x4b0
> > [   81.839078]  security_inet_conn_request+0xc0/0x160
> > 
> > KMSAN warning is reproducible with:
> > * netlabel_mgmt_protocount is 0 (e.g. netlbl_enabled() returns 0)
> > * CONFIG_SECURITY_NETWORK_XFRM may be set or not
> > * CONFIG_KMSAN=y
> > * `ssh USER@HOSTNAME /bin/date`
> > 
> > selinux_skb_peerlbl_sid() will call selinux_xfrm_skb_sid(), then fall
> > to selinux_netlbl_skbuff_getsid() which will not initialize nlbl_type,
> > but it will be passed to:
> > 
> >     err = security_net_peersid_resolve(nlbl_sid,
> >                                        nlbl_type, xfrm_sid, sid);
> > 
> > and checked by KMSAN, although it will not be used inside
> > security_net_peersid_resolve() (at least now), since this function
> > will check either (xfrm_sid == SECSID_NULL) or (nlbl_sid ==
> > SECSID_NULL) first and return before using uninitialized nlbl_type.
> > 
> > Signed-off-by: Andrew Kanner <andrew.kanner@gmail.com>
> > ---
> >  security/selinux/netlabel.c | 1 +
> >  1 file changed, 1 insertion(+)
> 
> What git commit id does this fix?  Does it also need to go to older
> kernels?  If so, how far back?
> 
> thanks,
> 
> greg k-h

Thanks, Greg. I mentioned it in the cover letter only because I'm not
sure, it seems to be:

Fixes: 220deb966ea5 ("SELinux: Better integration between peer labeling subsystems")

which is ~v2.6.24, I believe. I wish I could use KMSAN there -)

But I checked - this uninit var will not be used on 220deb966ea5 and
the function will return earlier as well. As far as I see this is just
a KMSAN warning for the uninitialized function argument.

PS: maybe we should check if KMSAN is be able to check the possible
usage of such uninit arguments.

-- 
Andrew Kanner

Re: [PATCH 1/1] selinux: netlabel: Prevent KMSAN warning in selinux_inet_conn_request()

[Paul Moore]

On Aug 15, 2023 Andrew Kanner <andrew.kanner@gmail.com> wrote:
> 
> KMSAN reports the following issue:
> [   81.822503] =====================================================
> [   81.823222] BUG: KMSAN: uninit-value in selinux_inet_conn_request+0x2c8/0x4b0
> [   81.823891]  selinux_inet_conn_request+0x2c8/0x4b0
> [   81.824385]  security_inet_conn_request+0xc0/0x160
> [   81.824886]  tcp_v4_route_req+0x30e/0x490
> [   81.825343]  tcp_conn_request+0xdc8/0x3400
> [   81.825813]  tcp_v4_conn_request+0x134/0x190
> [   81.826292]  tcp_rcv_state_process+0x1f4/0x3b40
> [   81.826797]  tcp_v4_do_rcv+0x9ca/0xc30
> [   81.827236]  tcp_v4_rcv+0x3bf5/0x4180
> [   81.827670]  ip_protocol_deliver_rcu+0x822/0x1230
> [   81.828174]  ip_local_deliver_finish+0x259/0x370
> [   81.828667]  ip_local_deliver+0x1c0/0x450
> [   81.829105]  ip_sublist_rcv+0xdc1/0xf50
> [   81.829534]  ip_list_rcv+0x72e/0x790
> [   81.829941]  __netif_receive_skb_list_core+0x10d5/0x1180
> [   81.830499]  netif_receive_skb_list_internal+0xc41/0x1190
> [   81.831064]  napi_complete_done+0x2c4/0x8b0
> [   81.831532]  e1000_clean+0x12bf/0x4d90
> [   81.831983]  __napi_poll+0xa6/0x760
> [   81.832391]  net_rx_action+0x84c/0x1550
> [   81.832831]  __do_softirq+0x272/0xa6c
> [   81.833239]  __irq_exit_rcu+0xb7/0x1a0
> [   81.833654]  irq_exit_rcu+0x17/0x40
> [   81.834044]  common_interrupt+0x8d/0xa0
> [   81.834494]  asm_common_interrupt+0x2b/0x40
> [   81.834949]  default_idle+0x17/0x20
> [   81.835356]  arch_cpu_idle+0xd/0x20
> [   81.835766]  default_idle_call+0x43/0x70
> [   81.836210]  do_idle+0x258/0x800
> [   81.836581]  cpu_startup_entry+0x26/0x30
> [   81.837002]  __pfx_ap_starting+0x0/0x10
> [   81.837444]  secondary_startup_64_no_verify+0x17a/0x17b
> [   81.837979]
> [   81.838166] Local variable nlbl_type.i created at:
> [   81.838596]  selinux_inet_conn_request+0xe3/0x4b0
> [   81.839078]  security_inet_conn_request+0xc0/0x160
> 
> KMSAN warning is reproducible with:
> * netlabel_mgmt_protocount is 0 (e.g. netlbl_enabled() returns 0)
> * CONFIG_SECURITY_NETWORK_XFRM may be set or not
> * CONFIG_KMSAN=y
> * `ssh USER@HOSTNAME /bin/date`
> 
> selinux_skb_peerlbl_sid() will call selinux_xfrm_skb_sid(), then fall
> to selinux_netlbl_skbuff_getsid() which will not initialize nlbl_type,
> but it will be passed to:
> 
>     err = security_net_peersid_resolve(nlbl_sid,
>                                        nlbl_type, xfrm_sid, sid);
> 
> and checked by KMSAN, although it will not be used inside
> security_net_peersid_resolve() (at least now), since this function
> will check either (xfrm_sid == SECSID_NULL) or (nlbl_sid ==
> SECSID_NULL) first and return before using uninitialized nlbl_type.
> 
> Signed-off-by: Andrew Kanner <andrew.kanner@gmail.com>
> Fixes: 220deb966ea5 ("SELinux: Better integration between peer labeling subsystems")
> ---
>  security/selinux/netlabel.c | 1 +
>  1 file changed, 1 insertion(+)

Thanks Andrew.  I'm going to drop the "Fixes" tag as the current code
isn't broken, as you mention in the commit description above, however,
I think this change is still worthwhile so I'm going to go ahead and
merge it into selinux/next.

--
paul-moore.com

Re: [PATCH 0/1] netlabel: KMSAN warning

[Paul Moore]

On Tue, Aug 15, 2023 at 4:59 PM Andrew Kanner <andrew.kanner@gmail.com> wrote:
>
> Recently I started to use KMSAN and found the warning in
> security/selinux/hooks.c which triggers each time I use the official
> guide to run syzkaller reproducers
>
> Link: https://github.com/google/syzkaller/blob/master/docs/syzbot_assets.md#run-a-c-reproducer
>
> I'm not quiet confident what are the rules in security subsystem. It's
> not a bug, but a warning which is triggered by KMSAN for the argument
> of the security_net_peersid_resolve() which was not initialized. It
> will not affect anything inside this function, at least with the
> current order of checking the variables, which might eventually
> change.
>
> Please reply if you're ok with such not-a-bug fixes. Otherwise I'll
> just ignore this warning in my experiments with KMSAN.

I think the answer is going to depend on the particular "bug" and the
patch required to resolve it.  In this particular case I think the
patch is okay so I went ahead and merged it, although I did remove the
"Fixes" tag as the current code isn't broken.

In general, if a test tool dumps an error or warning for something
under security/ and you aren't sure if it's valid or if we need to
resolve it upstream, you can always send us an email and ask what to
do :)

-- 
paul-moore.com

Re: [PATCH 0/1] netlabel: KMSAN warning

[Andrew Kanner]

On Tue, Aug 15, 2023 at 06:27:43PM -0400, Paul Moore wrote:
> 
> I think the answer is going to depend on the particular "bug" and the
> patch required to resolve it.  In this particular case I think the
> patch is okay so I went ahead and merged it, although I did remove the
> "Fixes" tag as the current code isn't broken.
> 
> In general, if a test tool dumps an error or warning for something
> under security/ and you aren't sure if it's valid or if we need to
> resolve it upstream, you can always send us an email and ask what to
> do :)
> 
> -- 
> paul-moore.com

Thanks, Paul.
I got the idea.

-- 
Andrew Kanner