I added the smatch@xxxxxxxxxxxxxxx mailing list to the CC.
On Mon, Nov 24, 2014 at 02:31:38PM -0800, Kees Cook wrote:
> Hi Dan,
>
> I've got a weird behavior out of smatch that I can't reproduce at
> will. It just happens sometimes under differing build environments. I
> haven't been able to isolate it yet, but I thought I'd check with you
> in case it was something obvious. The warning I've been getting is:
>
> net/ipv4/icmp.c:514 icmp_route_lookup() error: potential NULL dereference 'rt'.
>
> The code is:
>
> static struct rtable *icmp_route_lookup(struct net *net,
> ...
> {
> ...
> rt2 = rt;
>
> rt = (struct rtable *) xfrm_lookup(net, &rt->dst,
> flowi4_to_flowi(fl4), NULL, 0);
> if (!IS_ERR(rt)) {
> if (rt != rt2)
> return rt;
> } else if (PTR_ERR(rt) == -EPERM) {
> rt = NULL;
> } else
> return rt;
> ...
>
> if (!IS_ERR(rt2)) {
> dst_release(&rt->dst);
>
> With line 514 being the dst_release() above.
>
> There's a clear path "possible" between "rt = NULL" and "rt->", though
> I think in the real world it's not possible. (Though I haven't studied
> it closely -- it's a bit weird.)
>
> So, with my local copy of smatch, on the regular upstream tree, I do
> NOT get the warning, but it seems like I should. Any idea what's going
> on here?
I've figured it out partly, sort of:
smatch/check_deref.c
179 static void match_assign(struct expression *expr)
180 {
181 struct statement *stmt;
182
183 if (!is_zero(expr->right))
184 return;
185
186 FOR_EACH_PTR_REVERSE(big_statement_stack, stmt) {
187 if (stmt->type == STMT_DECLARATION)
188 return;
189 break;
190 } END_FOR_EACH_PTR_REVERSE(stmt);
191
192 set_state_expr(my_id, expr->left, &null);
193 }
The big_statement_stack is getting corrupted somehow so it
randomly/unpredictably returns instead of setting "rt" to &null.
I was trying to ignore initializers when I wrote that code because
everyone initializes variables to NULL to silence GCC warnings so
initializers cause a lot of false positives.
I guess I'll try figure out how big_statement_stack is getting
corrupted but memory corruption is a pain to figure out. Valgrind
hasn't helped me here.
regards,
dan carpenter
--
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
