On 7/20/2016 1:13 PM, Paul Moore wrote: > On Tue, Jul 19, 2016 at 7:37 PM, Casey Schaufler <ca...@schaufler-ca.com> > wrote: >> Digging into this further I have determined that the >> circumstances leading to this issue are somewhat complex. >> The good news is that there seems to be a very limited >> circumstances under which the problem manifests. >> >> I have a socket, and change the Smack attributes on the >> socket (security_inode_setsecurity) before connecting to >> a server. > This is a minty fresh, disconnected socket, yes? > >> The connect succeeds. The client sends a packet, >> also successfully. The response is received. Now here's >> where it gets interesting. I instrumented the code to print >> the Smack attributes on the socket both before and after >> the Smack access check. > I'm assuming that when you say "access check" you are talking about > the smk_access() call in smack_socket_sock_rcv_skb(), yes?
I have been able to track this down to my careless use of netlbl_skbuff_err(). Because the socket started life as unlabeled, and changed to labeled, calling netlbl_skbuff_err() resulted in multiple frees of some netlabel data under some circumstances. I don't know why it worked before, but the code certainly shouldn't have been making that call. I have a patch in final test. > > (as a totally unrelated side note, you really went nuts on the cpp > conditionals in there, was there a sale on #ifdefs that I missed? <g>) I can't say that I'm happy about how that code ended up. I hope to do a clean up in association with switching away from CIPSO to secmark for local access controls. That's something I have to do for Extreme Security Module Stacking. > >> Before the check is made the Smack >> data reflects the initial values from when the socket was >> created. After the check, they reflect the explicit change >> made earlier. > It has been too long since I looked at how Smack handled network > packets, I assume this is not the intended behavior? > >> The check reports failure based on the initial >> values. As a result, an attempt to notify the caller that >> the action failed is made (netlbl_skbuff_err) which results >> in a call to icmp_send that frees already freed memory. > What memory is being double freed? The original skb? I don't believe > netlbl_skbuff_err(), cipso_v4_error(), or icmp_send() frees the > original skb ... or rather it shouldn't, perhaps I'm missing > something. > > I'm not arguing, you saw what you saw, I'm just trying to understand > and make sense of it. Can you elaborate on what you saw, using very > small words, and concrete descriptions (I'm much more stupider than > everyone here so you have to make it easy for me to understand)? > >> If the Smack attributes in the sk_security blob are not >> explicitly set the problem does not occur. I have the same >> result if I change the Smack attributes within the socket >> security blob as I do if I replace the security blob.
