Re: [syzbot] KASAN: use-after-free Read in em28xx_close_extension

[Dongliang Mu <mudongliangabcd@gmail.com>]

On Tue, Jul 27, 2021 at 10:15 PM Dan Carpenter <dan.carpenter@oracle.com> 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@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/20210727141455.GM1931%40kadam.