Kernel memory corruption in CIPSO labeled TCP packets processing.

[Paul Moore]

On Wed, Jan 30, 2019 at 8:11 AM Nazarov Sergey <s-nazarov at yandex.ru> wrote:
> 30.01.2019, 01:42, "Paul Moore" <paul at paul-moore.com>:
> > 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.com
>
> In 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.

> 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