On 30.08.2018 02:33, Dan Carpenter wrote: > Hello Stefan Agner, > > The patch b2dd9f2e5a8a: "HID: core: fix memory leak on probe" from > Aug 28, 2018, leads to the following static checker warning: > > drivers/hid/hid-core.c:1051 hid_open_report() > error: we previously assumed 'parser' could be null (see line 1001) There is already a fix pending on the mailing list: https://lore.kernel.org/lkml/20180829152209.GA29831@xxxxxxxxxxxxxx/T/#u -- Stefan > > drivers/hid/hid-core.c > 958 int hid_open_report(struct hid_device *device) > 959 { > 960 struct hid_parser *parser; > 961 struct hid_item item; > 962 unsigned int size; > 963 __u8 *start; > 964 __u8 *buf; > 965 __u8 *end; > 966 int ret; > 967 static int (*dispatch_type[])(struct hid_parser *parser, > 968 struct hid_item *item) = { > 969 hid_parser_main, > 970 hid_parser_global, > 971 hid_parser_local, > 972 hid_parser_reserved > 973 }; > 974 > 975 if (WARN_ON(device->status & HID_STAT_PARSED)) > 976 return -EBUSY; > 977 > 978 start = device->dev_rdesc; > 979 if (WARN_ON(!start)) > 980 return -ENODEV; > 981 size = device->dev_rsize; > 982 > 983 buf = kmemdup(start, size, GFP_KERNEL); > 984 if (buf == NULL) > 985 return -ENOMEM; > 986 > 987 if (device->driver->report_fixup) > 988 start = device->driver->report_fixup(device, > buf, &size); > 989 else > 990 start = buf; > 991 > 992 start = kmemdup(start, size, GFP_KERNEL); > 993 kfree(buf); > 994 if (start == NULL) > 995 return -ENOMEM; > 996 > 997 device->rdesc = start; > 998 device->rsize = size; > 999 > 1000 parser = vzalloc(sizeof(struct hid_parser)); > 1001 if (!parser) { > 1002 ret = -ENOMEM; > 1003 goto err; > > "parser" is NULL. > > 1004 } > 1005 > 1006 parser->device = device; > 1007 > 1008 end = start + size; > 1009 > 1010 device->collection = kcalloc(HID_DEFAULT_NUM_COLLECTIONS, > 1011 sizeof(struct > hid_collection), GFP_KERNEL); > 1012 if (!device->collection) { > 1013 ret = -ENOMEM; > 1014 goto err; > 1015 } > 1016 device->collection_size = HID_DEFAULT_NUM_COLLECTIONS; > 1017 > 1018 ret = -EINVAL; > 1019 while ((start = fetch_item(start, end, &item)) != NULL) { > 1020 > 1021 if (item.format != HID_ITEM_FORMAT_SHORT) { > 1022 hid_err(device, "unexpected long > global item\n"); > 1023 goto err; > 1024 } > 1025 > 1026 if (dispatch_type[item.type](parser, &item)) { > 1027 hid_err(device, "item %u %u %u %u > parsing failed\n", > 1028 item.format, (unsigned)item.size, > 1029 (unsigned)item.type, > (unsigned)item.tag); > 1030 goto err; > 1031 } > 1032 > 1033 if (start == end) { > 1034 if (parser->collection_stack_ptr) { > 1035 hid_err(device, "unbalanced > collection at end of report description\n"); > 1036 goto err; > 1037 } > 1038 if (parser->local.delimiter_depth) { > 1039 hid_err(device, "unbalanced > delimiter at end of report description\n"); > 1040 goto err; > 1041 } > 1042 kfree(parser->collection_stack); > 1043 vfree(parser); > 1044 device->status |= HID_STAT_PARSED; > 1045 return 0; > 1046 } > 1047 } > 1048 > 1049 hid_err(device, "item fetching failed at offset %d\n", > (int)(end - start)); > 1050 err: > 1051 kfree(parser->collection_stack); > ^^^^^^^^^^^^^^^^^^^^^^^^ > NULL dereference. > > 1052 vfree(parser); > 1053 hid_close_report(device); > 1054 return ret; > 1055 } > > > regards, > dan carpenter
