On Fri, 24 Jan 2014 13:43:47 +0300 Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote: > So I tried the first most obvious check which is to match code like > this: > > foo = readl(); > > If you see that then mark "foo" as untrusted. Then if "foo" is used in > a condition print an error. > > if (foo & BAR) <-- Error. > > I wasn't sure how to tell which drivers are hot plugable (plus I'm > lazy) so I ran it on drivers/usb/. Something like the ixgbe or qla2xxx driver would be applicable here. > Also the test should ignore code which checks for "foo == 0xffffffff" > but I was too lazy to do that. It turns out that no one ever does > error handling so this wasn't a problem. > > I've looked at some of the warnings, and it's pretty hard to tell if > 0xffffffff is handled correctly or not. In the example you gave, then > it returned success and that would be treated like a warning. Another > potential problem would be infinite loops like: > > while (readl() & BAR) > ; > > There was a university group that did some work with infinite loops like > this: > http://pages.cs.wisc.edu/~kadav/new/carb/Site/splash.html > http://pages.cs.wisc.edu/~kadav/papers/carb-sosp09.pdf Good point regarding the loops. That's another construct that we inspect and add checks to avoid infinite or long loops. The links to the Carburizer project look interesting, I'll have take a look at their paper and see how they handled some of these issues. > I've attached my really dumb check that I wrote. You would need to add > it to check_list.h. Then run: > ~/progs/smatch/devel/smatch_scripts/kchecker drivers/usb/host/pci-quirks.c > > The check is not useful. We need to specifically check for cases where > 0xffffffff leads to returning success or when it causes an infinite > loop. What other similar bad results could we check for? I think that's it. The more I look at the readl instances in ixgbe, the more I think a manual approach is required. There are a lot of "don't care" conditions that we let through, even if the flow of subsequent code changes. (Well, maybe it would be more accurate to say the immediate flow changes, but bad data doesn't escape to upper layers.) Thanks again for taking a look at this. If I can think of any better way to narrow the checking down, I'll let you know. Regards, -- Joe -- To unsubscribe from this list: send the line "unsubscribe smatch" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
