On Thu, 7 Oct 2010, Kees Cook wrote: > Hi! > > I've started trying to learn coccinelle, but I keep running into stuff I'm > not sure how to accomplish. I'm hoping someone might be able to direct me > towards a sane way to do what I'm trying currently. > > I'd like to find all the occurrences of copy_from_user() where the target > is a pointer, but the length is not strictly a sizeof() for the pointer's > type. > > For example, I'd want to find these: > copy_from_user(&thing, buf, 1024); > copy_from_user(thingptr, buf, 1024); > but not these: > copy_from_user(&thing, buf, sizeof(thing)); > copy_from_user(thingptr, buf, sizeof(*thingptr)); > > My first glitch was being unsure how to handle both "sizeof(N)" and "sizeof > N". What I did feels sloppy. :)
sizeof(N) matches sizeof N via an isomorphism. You can see what will really be matched by typing spatch -parse_cocci foo.cocci > Secondly, I'm not sure how to express types in a more dynamic fashion. > Presently this will skip "&e, ..., sizeof(e)" but not "ptr, ..., > sizeof(*ptr)". What is the best way to approach this without just repeating > another chunk and using "p, ..., \(sizeof(*p)\|sizeof *p\)" ? This one it doesn't help with. I think you will just have to repeats the two cases. Perhaps without the sizeof problem this will be less bothersome :) > Thanks! > > -Kees > > > @cfu exists@ > position p1; > @@ > > copy_from_u...@p1(...) There is no need for exists. exists and forall have to do with ...s that match paths in the control-flow graph, to indicate eg whether you want the pattern to match through all permutations of if branches or just through one path. But here there are no paths, so there is no need for exists. It doesn't do anything bad, though. > @cfuso depends on cfu@ > identifier e; > position cfu.p1; > @@ > > copy_from_u...@p1(&e, ..., \(sizeof(e)\|sizeof e\)) > > @script:python depends on (cfu && !cfuso) @ You could leave out the cfu in the depends on. The rule will only apply if cfu matched because of the use of a metavariable inherited from cfu. Again, it doesn't hurt to put it either. > p1 << cfu.p1; > @@ > > print "* file: %s copy_from_user to pointer without sizeof %s" % > (p1[0].file,p1[0].line) You could have done the whole thing in just one rule: @@ @@ ( copy_from_user(with_good_arguments) | * copy_from_user(...) ) (copy_from_user(with_good_arguments) should be replaced with whatever cases you want, separated by | ). This give diff output with a - on the lines of interest. Obviously you don't want to actually use the diff, but it lets you easily see the problem in context. You could use the python version by dropping the * and adding a position variable. julia _______________________________________________ Cocci mailing list [email protected] http://lists.diku.dk/mailman/listinfo/cocci (Web access from inside DIKUs LAN only)
