On Mon, Oct 23, 2023 at 01:45:28PM +0800, Dongliang Mu wrote:
> Hi Dan,
>
> I repeatedly read the mentionship program from you - Smatch: New ideas for
> Static Analysis[1]. There are some interesting ideas I would like to list as
> public projects in our club, e.g, Smatch on different C projects or Debian
> project, or any application of cross function database in Smatch.
Fantastic!
One thing that I do (with a former co-worker Harshit Mogalapalli) is
that we occasionally go through recent CVEs and look how these bugs
could be detected automatically.
Take commit b7c81f80246f ("firewire: fix potential uaf in
outbound_phy_packet_callback()") as an example. The queue_event() event
function has the following information in the database.
drivers/firewire/core-cdev.c | queue_event | 198 | | MAYBE_FREED | 1 | $ | |
There are two reasons why Smatch did not detect this bug. First of
all it's a MAYBE_FREED instead of a freed. So that's simple to change:
--- a/check_free_strict.c
+++ b/check_free_strict.c
@@ -153,6 +153,8 @@ static int is_freed(struct expression *expr)
sm = get_sm_state_expr(my_id, expr);
if (sm && slist_has_state(sm->possible, &freed))
return 1;
+ if (sm && slist_has_state(sm->possible, &maybe_freed))
+ return 1;
return 0;
}
The second problem is that we are passing "&e->event" as the parameter
instead of "e". It's the same address, but it looks different.
queue_event(e->client, &e->event, &e->phy_packet, ...);
^^^^^^^^^
So I wrote a function to change "&e->event" into "e".
+static struct expression *remove_addr(struct expression *expr)
+{
+ struct expression *deref;
+ int offset;
+
+ if (expr->type != EXPR_PREOP ||
+ expr->op != '&')
+ return expr;
+
+ deref = strip_expr(expr->unop);
+ offset = get_member_offset_from_deref(deref);
+ if (offset != 0)
+ return expr;
+
+ deref = strip_expr(deref->deref);
+ if (deref->type != EXPR_PREOP ||
+ deref->op != '*')
+ return expr;
+
+ return strip_expr(deref->unop);
+}
+
Then it's easy to say: arg = remove_addr(strip_expr(arg));
After we make these two changes the bug is detected. It finds quite a
few bugs that way. The MAYBE_FREED changes generate quite a few false
positives so I haven't decided out how to deal with that yet...
But there is always something in the CVE list that gives an idea like
that.
>
> Since you are maintainers of Smatch, I think students can ask questions
> about Smatch in the smatch mailing list or our mailing to acquire help and
> mentionship.
Of course. Feel free.
>
> BTW, Smatch is written based on an old sparse version. Is it required to
> port sparse to newest version?
I'm not sure I understand this... The Sparse tree hasn't been updated
since June last year. I have tried to email them patches but no one
responded.
regards,
dan carpenter
