On Tue, Jul 27, 2021 at 10:15 PM Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote: > > On Tue, Jul 27, 2021 at 06:01:51PM +0800, Hillf Danton wrote: > > Along the probe path, > > > > em28xx_usb_probe > > dev = kzalloc(sizeof(*dev), GFP_KERNEL); > > retval = em28xx_init_dev(dev, udev, intf, nr); > > em28xx_init_extension(dev); > > em28xx_ir_init(struct em28xx *dev) > > kref_get(&dev->ref); > > > > kref_init(&dev->ref); > Hi Dan, I have developed a patch [1] to fix this crash. Would you like to help me double-check if it correctly fixes the underlying bug? [1] [PATCH v2] [media] em28xx-input: fix refcount bug in em28xx_usb_disconnect https://lkml.org/lkml/2021/7/19/263 > Good detective work. > > I've created a Smatch check to try find these. It uses the fact that > Smatch creates a bunch of fake assignments to set all the struct members > of "dev" to zero. Then it uses the modification hook to find the > kref_init(). Those are sort of new uses for those hooks so that's quite > fun. > > I'll test it out overnight and see how it works. > > drivers/media/usb/em28xx/em28xx-cards.c:4086 em28xx_usb_probe() warn: kref has already been modifed (see line 3979) > > regards, > dan carpenter > > #include "smatch.h" > #include "smatch_slist.h" > #include "smatch_extra.h" > > static int my_id; > > STATE(fresh); > > static int get_line(struct sm_state *sm) > { > struct sm_state *tmp; > int line = 0; > > FOR_EACH_PTR(sm->possible, tmp) { > if (tmp->state == &undefined && > tmp->line > line) > line = tmp->line; > } END_FOR_EACH_PTR(tmp); > > if (!line) > return sm->line; > return line; > } > > static void match_modify(struct sm_state *sm, struct expression *mod_expr) > { > if (sm->state != &fresh && > mod_expr && > mod_expr->type == EXPR_CALL && > sym_name_is("kref_init", mod_expr->fn)) > sm_warning("kref has already been modifed (see line %d)", get_line(sm)); > > set_state(my_id, sm->name, sm->sym, &undefined); > } > > static bool is_alloc(struct expression *expr) > { > static struct expression *ignore, *alloc_expr; > struct expression *right; > > if (!expr || expr->type != EXPR_ASSIGNMENT || expr->op != '=') > return false; > if (expr == ignore) > return false; > if (expr == alloc_expr) > return true; > right = strip_expr(expr->right); > if (right->type == EXPR_CALL && > (sym_name_is("kzalloc", right->fn) || > sym_name_is("kmalloc", right->fn))) { > alloc_expr = expr; > return true; > } > ignore = expr; > return false; > } > > static void match_assign(struct expression *expr) > { > char *name; > > if (!is_alloc(get_faked_expression())) > return; > name = expr_to_str(expr->left); > if (name && strstr(name, "refcount.refs.counter")) > set_state_expr(my_id, expr->left, &fresh); > free_string(name); > } > > void check_kref_init_too_late(int id) > { > my_id = id; > > add_hook(&match_assign, ASSIGNMENT_HOOK_AFTER); > add_modification_hook(my_id, &match_modify); > } > > -- > You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group. > To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@xxxxxxxxxxxxxxxx. > To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20210727141455.GM1931%40kadam.
