Dan Smith wrote:
> Save this information when we checkpoint an skb and provide a mechanism
> to restore that information on restart. This will be used in the
> subsequent INET patch.
>
> Signed-off-by: Dan Smith <[email protected]>
>
> Changes in v2:
> - Add range checks to ensure that the header marks we restore
> are within the data length of the skb
[...]
> +int sock_restore_header_info(struct sk_buff *skb,
> + struct ckpt_hdr_socket_buffer *h)
> +{
> + if ((h->transport_header >= skb->len) ||
> + (h->network_header >= skb->len) ||
> + (h->mac_header >= skb->len) ||
> + (h->mac_len >= skb->len) ||
> + (h->hdr_len >= skb->len)) {
I wonder if the sanity test for mac_len and hdr_len are sufficient,
or whether a more constrained test is required.
For example, a quick grep shows that there are several places where:
skb->mac_len = skb->network_header - skb->mac_header;
I think there is some code that relies on it without validating
that the value makes sense.
> + ckpt_debug("skb header positions not within data length\n");
> + return -EINVAL;
> + }
> +
> + skb->mac_len = h->mac_len;
> + skb->hdr_len = h->hdr_len;
> +
> + skb_set_transport_header(skb, h->transport_header);
> + skb_set_network_header(skb, h->network_header);
> + skb_set_mac_header(skb, h->mac_header);
> +
> + memcpy(skb->cb, h->cb, sizeof(skb->cb));
The skb->cb holds can be used by any layer to put private variables.
Can the user mangle the data in there to create a disaster of some
sort ?
If the answer is "it's possible", and because this is per protocol
data, I suggest to add a per-protocol callback to sanitize the data
in this control buffer.
To not block this patchset infinitely, I guess you can put the
details of the sanity check in a separate patch(set). But I prefer
that the current set will at least mention and provision for such
a mechanism.
> +
> + return 0;
> +}
> +
> static int __sock_write_buffers(struct ckpt_ctx *ctx,
> struct sk_buff_head *queue,
> int dst_objref)
> @@ -123,6 +167,7 @@ static int __sock_write_buffers(struct ckpt_ctx *ctx,
> goto end;
> h->sk_objref = ret;
> h->pr_objref = dst_objref;
> + sock_record_header_info(skb, h);
>
> ret = ckpt_write_obj(ctx, (struct ckpt_hdr *) h);
> if (ret < 0)
Oren.
_______________________________________________
Containers mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/containers
_______________________________________________
Devel mailing list
[email protected]
https://openvz.org/mailman/listinfo/devel
