On Mon, Jul 04, 2016 at 04:21:23PM +0200, Thomas Graf wrote: > 2016-07-04 13:02 GMT+02:00 Daniel Borkmann <[email protected]>: > > Yeah, checks cannot be carried over in two occasions: i) calling helpers > > that change skb->data (and therefore prior checks become invalid) and > > ii) tail calls. For tail calls the verifier doesn't know how such programs > > will be used or shared (e.g. they could be part of one or multiple tail > > call maps but at the same time attached somewhere directly) so they strictly > > need to be validated and assumed as stand-alone at that point in time. > > > > So, I think with carrying checks across tail calls you then mean that you > > store some non constant start offset/immediate e.g. into skb->cb[0] from > > the program that executes the tail call, and would like to do the 'skb->data > > + > > skb->cb[0] < skb->data_end' check once in the new program to give the > > verifier > > some context? Since non constant, this would currently fail with "cannot > > add integer value with 0 upper zero bits to ptr_to_packet" since the > > unknown value from cb[0] has no tracking (imm==0) of upper zero bits (that > > prevents/checks for overflows wrt skb->data since we do skb->data + > > skb->cb[0]). Meaning we cannot safely perform such test against > > skb->data_end > > before doing anything further. > > Exactly. This is what I tried and it is what I would believe be an > elegant solution to calculate variable headers once and carry it to > subsequent programs.
that would mean that we'd need to teach verifier to carry the register types into cb[], so that subsequent prog can inherit them? I'm not sure how that's possible yet. Is length check is so expensive? The next prog will do it once. Comparing to map lookup that's noise, no? > > So I think this might then require either some context field with similar > > semantics as skb->data that you could set safely (since can be verified) > > via a new helper function from the main program that does the tail call, or > > you could perhaps advance skb->data from ctx to a save offset, so the tail > > called program thinks it's as original skb->data then, but this would > > disallow > > access before skb->data obviously. But also then you need to redo checks > > when calling helpers that change skb's data since we potentially can call > > into pskb_expand_head() and thus pointers to skb->data/data_end can change. > > Do you need r+w case, right? > > I thought about the extend skb->data but it is of limited use in > practice as variable headers are usually at L4 and the final action is > usually a L2/L3 rewrite of some sort. I guess if this is the only > option then it is definitely better than what we have now. you mean something like tlv geneve like parsing? Why not to do an optimisic const lenght check before parsing them? Like assume there is always < 64 bytes of them and check for that once. _______________________________________________ iovisor-dev mailing list [email protected] https://lists.iovisor.org/mailman/listinfo/iovisor-dev
