On Wed, Jan 30, 2019 at 8:11 AM Nazarov Sergey <s-nazarov@xxxxxxxxx> wrote:
> 30.01.2019, 01:42, "Paul Moore" <paul@xxxxxxxxxxxxxx>:
> > 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
