Re: [PATCH] HID: uclogic: Add support for XP-PEN Artist 22R Pro

From: Dan Carpenter
Date: Thu Dec 29 2022 - 04:29:41 EST


Hi Joshua,

https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Joshua-Goins/HID-uclogic-Add-support-for-XP-PEN-Artist-22R-Pro/20221226-112302
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/2068502.VLH7GnMWUR%40adrastea
patch subject: [PATCH] HID: uclogic: Add support for XP-PEN Artist 22R Pro
config: i386-randconfig-m021-20221226
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@xxxxxxxxx>
| Reported-by: Dan Carpenter <error27@xxxxxxxxx>

New smatch warnings:
drivers/hid/hid-uclogic-params.c:1453 uclogic_params_init_ugee_xppen_pro() warn: variable dereferenced before check 'hdev' (see line 1447)
drivers/hid/hid-uclogic-params.c:1454 uclogic_params_init_ugee_xppen_pro() warn: possible memory leak of 'buf'
drivers/hid/hid-uclogic-params.c:1492 uclogic_params_init_ugee_xppen_pro() error: double free of 'buf'

Old smatch warnings:
drivers/hid/hid-uclogic-params.c:1502 uclogic_params_init_ugee_xppen_pro() error: double free of 'buf'

vim +/hdev +1453 drivers/hid/hid-uclogic-params.c

51d8c9b14fc55dc Aren Villanueva 2022-12-25 1436 static int uclogic_params_init_ugee_xppen_pro(struct hid_device *hdev,
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1437 struct uclogic_params *p,
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1438 const u8 probe_endpoint,
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1439 const u8 rdesc_init_packet[],
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1440 const size_t rdesc_init_size,
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1441 const u8 rdesc_tablet_arr[],
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1442 const size_t rdesc_tablet_size,
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1443 const u8 rdesc_frame_arr[],
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1444 const size_t rdesc_frame_size)
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1445 {
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1446 const size_t str_desc_len = 12;
51d8c9b14fc55dc Aren Villanueva 2022-12-25 @1447 struct usb_device *udev = hid_to_usb_dev(hdev);
^^^^
Dereference.

51d8c9b14fc55dc Aren Villanueva 2022-12-25 1448 u8 *buf = kmemdup(rdesc_init_packet, rdesc_init_size, GFP_KERNEL);

Never put functions which can fail in the declaration block. This
allocation has no check for NULL (common problem when done in
declaration block).

51d8c9b14fc55dc Aren Villanueva 2022-12-25 1449 s32 desc_params[UCLOGIC_RDESC_PH_ID_NUM];
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1450 int actual_len, rc;
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1451 u16 resolution;
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1452
51d8c9b14fc55dc Aren Villanueva 2022-12-25 @1453 if (hdev == NULL || p == NULL)
^^^^^^^^^^^^
Checked to late.

51d8c9b14fc55dc Aren Villanueva 2022-12-25 @1454 return -EINVAL;

Needs a kfree(buf);

51d8c9b14fc55dc Aren Villanueva 2022-12-25 1455
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1456 rc = usb_interrupt_msg(
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1457 udev,
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1458 usb_sndintpipe(udev, probe_endpoint),
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1459 buf,
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1460 rdesc_init_size,
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1461 &actual_len,
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1462 USB_CTRL_SET_TIMEOUT);
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1463 kfree(buf);
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1464 if (rc == -EPIPE) {
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1465 hid_err(hdev, "broken pipe sending init packet\n");
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1466 return rc;
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1467 } else if (rc < 0) {
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1468 hid_err(hdev, "failed sending init packet: %d\n", rc);
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1469 return rc;
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1470 } else if (actual_len != rdesc_init_size) {
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1471 hid_err(hdev,
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1472 "failed to transfer complete init packet, only %d bytes sent\n",
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1473 actual_len);
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1474 return -1;
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1475 }
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1476
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1477 rc = uclogic_params_get_str_desc(&buf, hdev, 100, str_desc_len);
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1478 if (rc != str_desc_len) {
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1479 if (rc == -EPIPE) {
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1480 hid_err(hdev,
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1481 "string descriptor with pen parameters not found\n");
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1482 } else if (rc < 0) {
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1483 hid_err(hdev,
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1484 "failed retrieving pen parameters: %d\n", rc);
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1485 } else {
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1486 hid_err(hdev,
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1487 "string descriptor with pen parameters has invalid length (got %d, expected %lu)\n",
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1488 rc,
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1489 str_desc_len);
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1490 rc = -1;
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1491 }
51d8c9b14fc55dc Aren Villanueva 2022-12-25 @1492 kfree(buf);

If uclogic_params_get_str_desc() fails then this is a double free.

51d8c9b14fc55dc Aren Villanueva 2022-12-25 1493 return rc;
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1494 }
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1495
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1496 desc_params[UCLOGIC_RDESC_PEN_PH_ID_X_LM] = get_unaligned_le16(buf + 2);
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1497 desc_params[UCLOGIC_RDESC_PEN_PH_ID_Y_LM] = get_unaligned_le16(buf + 4);
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1498 /* buf + 6 is the number of pad buttons? Its 0x0008 */
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1499 desc_params[UCLOGIC_RDESC_PEN_PH_ID_PRESSURE_LM] =
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1500 get_unaligned_le16(buf + 8);
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1501 resolution = get_unaligned_le16(buf + 10);
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1502 kfree(buf);
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1503 if (resolution == 0) {
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1504 hid_err(hdev, "resolution of 0 in descriptor string\n");
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1505 return -1;
51d8c9b14fc55dc Aren Villanueva 2022-12-25 1506 }

--
0-DAY CI Kernel Test Service
https://01.org/lkp