lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <90946257-e667-dff1-b941-8d596636d1bf@tycho.nsa.gov> Date: Thu, 14 Feb 2019 13:59:27 -0500 From: Stephen Smalley <sds@...ho.nsa.gov> To: Nazarov Sergey <s-nazarov@...dex.ru>, Paul Moore <paul@...l-moore.com> Cc: "netdev@...r.kernel.org" <netdev@...r.kernel.org>, "linux-security-module@...r.kernel.org" <linux-security-module@...r.kernel.org>, "davem@...emloft.net" <davem@...emloft.net>, "kuznet@....inr.ac.ru" <kuznet@....inr.ac.ru>, "yoshfuji@...ux-ipv6.org" <yoshfuji@...ux-ipv6.org> Subject: Re: [PATCH] NETWORKING: avoid use IPCB in cipso_v4_error On 2/14/19 1:00 PM, Nazarov Sergey wrote: > Hi, Paul! > I've found the problem and testing it with some very specific custom lsm module. The test case was simple: > standard TCP/IP client-server application, where server opens CIPSO labeled TCP socket, and client connecting > to this socket with forbidden labels. After several connections kernel crashing with general memory protection or > kernel cache inconsistent error. > I think, the similar behaviour should be with selinux or smack in the same conditions. But I don't know them > so good to reproduce situation. For SELinux, you can use https://github.com/SELinuxProject/selinux-testsuite That includes testing of CIPSO, both connecting from a client with an authorized level and from a client with an unauthorized level. Not sure about Smack; there were some tests in LTP but I don't know if they would exercise it. > After applying patch, I haven't kernel crashes. > But now I've made additional checks and found no response icmp packets. The ip_options_compile requires > CAP_NET_RAW capability when CIPSO option compiling, if skb is NULL. I have no other ideas than returning to > the early patch version with ip_options_compile modified. What do you think about that? > > 14.02.2019, 00:42, "Paul Moore" <paul@...l-moore.com>: >> On Tue, Feb 12, 2019 at 10:10 AM Nazarov Sergey <s-nazarov@...dex.ru> wrote: >>> Since cipso_v4_error might be called from different network stack layers, we can't safely use icmp_send there. >>> icmp_send copies IP options with ip_option_echo, which uses IPCB to take access to IP header compiled data. >>> But after commit 971f10ec ("tcp: better TCP_SKB_CB layout to reduce cache line misses"), IPCB can't be used >>> above IP layer. >>> This patch fixes the problem by creating in cipso_v4_error a local copy of compiled IP options and using it with >>> introduced __icmp_send function. This looks some overloaded, but in quite rare error conditions only. >>> >>> The original discussion is here: >>> https://lore.kernel.org/linux-security-module/16659801547571984@sas1-890ba5c2334a.qloud-c.yandex.net/ >>> >>> Signed-off-by: Sergey Nazarov <s-nazarov@...dex.ru> >>> --- >>> include/net/icmp.h | 9 ++++++++- >>> net/ipv4/cipso_ipv4.c | 18 ++++++++++++++++-- >>> net/ipv4/icmp.c | 7 ++++--- >>> 3 files changed, 28 insertions(+), 6 deletions(-) >> >> Hi Sergey, >> >> Thanks for your work on finding this and putting a fix together. As >> we discussed previously, I think this looks good, but can you describe >> the testing you did to verify that this works correctly? >> >>> diff --git a/include/net/icmp.h b/include/net/icmp.h >>> index 6ac3a5b..e0f709d 100644 >>> --- a/include/net/icmp.h >>> +++ b/include/net/icmp.h >>> @@ -22,6 +22,7 @@ >>> >>> #include <net/inet_sock.h> >>> #include <net/snmp.h> >>> +#include <net/ip.h> >>> >>> struct icmp_err { >>> int errno; >>> @@ -39,7 +40,13 @@ struct icmp_err { >>> struct sk_buff; >>> struct net; >>> >>> -void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info); >>> +void __icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info, >>> + const struct ip_options *opt); >>> +static inline void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info) >>> +{ >>> + __icmp_send(skb_in, type, code, info, &IPCB(skb_in)->opt); >>> +} >>> + >>> int icmp_rcv(struct sk_buff *skb); >>> int icmp_err(struct sk_buff *skb, u32 info); >>> int icmp_init(void); >>> diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c >>> index 777fa3b..234d12e 100644 >>> --- a/net/ipv4/cipso_ipv4.c >>> +++ b/net/ipv4/cipso_ipv4.c >>> @@ -1735,13 +1735,27 @@ int cipso_v4_validate(const struct sk_buff *skb, unsigned char **option) >>> */ >>> void cipso_v4_error(struct sk_buff *skb, int error, u32 gateway) >>> { >>> + unsigned char optbuf[sizeof(struct ip_options) + 40]; >>> + struct ip_options *opt = (struct ip_options *)optbuf; >>> + >>> 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. >>> + */ >>> + >>> + memset(opt, 0, sizeof(struct ip_options)); >>> + opt->optlen = ip_hdr(skb)->ihl*4 - sizeof(struct iphdr); >>> + memcpy(opt->__data, (unsigned char *)&(ip_hdr(skb)[1]), opt->optlen); >>> + if (ip_options_compile(dev_net(skb->dev), opt, NULL)) >>> + return; >>> + >>> 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); >>> } >>> >>> /** >>> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c >>> index 065997f..3f24414 100644 >>> --- a/net/ipv4/icmp.c >>> +++ b/net/ipv4/icmp.c >>> @@ -570,7 +570,8 @@ static void icmp_reply(struct icmp_bxm *icmp_param, struct sk_buff *skb) >>> * MUST reply to only the first fragment. >>> */ >>> >>> -void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info) >>> +void __icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info, >>> + const struct ip_options *opt) >>> { >>> struct iphdr *iph; >>> int room; >>> @@ -691,7 +692,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info) >>> iph->tos; >>> mark = IP4_REPLY_MARK(net, skb_in->mark); >>> >>> - if (ip_options_echo(net, &icmp_param.replyopts.opt.opt, skb_in)) >>> + if (__ip_options_echo(net, &icmp_param.replyopts.opt.opt, skb_in, opt)) >>> goto out_unlock; >>> >>> @@ -742,7 +743,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info) >>> local_bh_enable(); >>> out:; >>> } >>> -EXPORT_SYMBOL(icmp_send); >>> +EXPORT_SYMBOL(__icmp_send); >>> >>> static void icmp_socket_deliver(struct sk_buff *skb, u32 info) >>> -- >> >> -- >> paul moore >> www.paul-moore.com
Powered by blists - more mailing lists