Re: SELinux/IP_PASSSEC regression in 4.13-rcX

On Mon, Jul 24, 2017 at 8:25 AM, Paolo Abeni [off-list ref] wrote:

Hi,

On Fri, 2017-07-21 at 18:19 -0400, Paul Moore wrote:

quoted

I've been seeing a SELinux regression with IP_PASSSEC on the v4.13-rcX
kernels and finally tracked the problem down to the
skb_release_head_state() call in __udp_queue_rcv_skb().  Looking at
the code and the git log it would appear that the likely culprit is
0a463c78d25b ("udp: avoid a cache miss on dequeue
"); it looks similar to IP option problem fixed in 0ddf3fb2c43d2.

Thank you for the report!
My bad, I completely missed that code path.

Hi Paolo,

No problem, things get missed from time to time, especially when we
deal with things that cross subsystems.

quoted

From a SELinux/IP_PASSSEC point of view we need access to the skb->sp
pointer to examine the SAs.  I'm posting this here without a patch
because it isn't clear to me how you would like to fix the problem; my
initial thought would be to simply make the skb_release_head_state()
conditional on the skb->sp pointer, much like the IP options fix, but
I'm not sure if you have a more clever idea.

Unfortunately explicitly checking skb->sp at skb free time will defeat
completely the intended optimization.

To preserve it, something like the following patch is required, could
you please test it in your environment?

Such patch is still prone to a kind of race, as only UDP packets
enqueued to the UDP socket after the setsockopt() will carry the
relevant cmsg info.

e.g. with the following event sequence:

<an UDP packet is enqueued to the revevant socket>
setsockopt(...,IP_CMSG_PASSSEC)
recvmsg(...);

the ancillary message data will not include the IP_CMSG_PASSSEC, while
kernel pre 0a463c78d25b will provide it. Do you think such behavior
will be acceptable?

If not, I fear a revert will be needed.

The change in behavior for userspace makes me a little nervous as
there is no way of knowing how any random application may be coded.
Even if we are confident that the majority of applications set
IP_PASSSEC before calling bind(), we are likely still stuck with a few
that will break, and that means a lot of hard to debug problem
reports.

I would feel much more comfortable if we could preserve the existing behavior.

-- 
paul moore
www.paul-moore.com