On Mon, Jul 24, 2017 at 8:25 AM, Paolo Abeni <pabeni@xxxxxxxxxx> wrote:
> Hi,
>
> On Fri, 2017-07-21 at 18:19 -0400, Paul Moore wrote:
>> 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.
>> 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
