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 <danms@xxxxxxxxxx> > > 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 Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/containers