Re: Kernel memory corruption in CIPSO labeled TCP packets processing.
From: Paul Moore <paul@paul-moore.com>
Date: 2019-01-31 02:10:32
Also in:
netdev, selinux
On Wed, Jan 30, 2019 at 8:11 AM Nazarov Sergey [off-list ref] wrote:
30.01.2019, 01:42, "Paul Moore" [off-list ref]:quoted
There are several cases where the stack ends up calling icmp_send() after the skb has been through ip_options_compile(), that should be okay. -- paul moore www.paul-moore.comIn those cases precompiled ip_options struct used, without the need to reuse ip_options_compile. I think, for error ICMP packet, we can discard all other options except CIPSO. It will be better, than send packet, contains wrong option's data. Modified patch 2: --- net/ipv4/cipso_ipv4.c | 24 ++++++++++++++++++++++-- 1 files changed, 22 insertions(+), 2 deletions(-)
This isn't how the rest of the stack works, look at ip_local_deliver_finish() for one example. Perhaps the behavior you are proposing is correct, but please show me where in the various RFC specs it is defined so that I can better understand why it should work this way.
quoted hunk ↗ jump to hunk
diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c index 777fa3b..797826c 100644 --- a/net/ipv4/cipso_ipv4.c +++ b/net/ipv4/cipso_ipv4.c@@ -1735,13 +1735,33 @@ int cipso_v4_validate(const struct sk_buff *skb, unsigned char **option) */ void cipso_v4_error(struct sk_buff *skb, int error, u32 gateway) { + struct ip_options opt; + unsigned char *optptr; + if (ip_hdr(skb)->protocol == IPPROTO_ICMP || error != -EACCES) return; + /* + * We might be called above the IP layer, + * so we can not use icmp_send and IPCB here. + * + * For the generated ICMP packet, we create a + * temporary ip _options structure, contains + * the CIPSO option only, since the other options data + * could be modified when the original packet receiving. + */ + + memset(&opt, 0, sizeof(struct ip_options)); + optptr = cipso_v4_optptr(skb); + if (optptr) { + opt.optlen = optptr[1]; + opt.cipso = optptr - skb_network_header(skb); + } + if (gateway) - icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0); + __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0, &opt); else - icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0); + __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0, &opt); } /**
-- paul moore www.paul-moore.com