Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

drbd: Fix five use after free bugs in get_initial_state

In get_initial_state, it calls notify_initial_state_done(skb,..) if
cb->args[5]==1. If genlmsg_put() failed in notify_initial_state_done(),
the skb will be freed by nlmsg_free(skb).
Then get_initial_state will goto out and the freed skb will be used by
return value skb->len, which is a uaf bug.

What's worse, the same problem goes even further: skb can also be
freed in the notify_*_state_change -> notify_*_state calls below.
Thus 4 additional uaf bugs happened.

My patch lets the problem callee functions: notify_initial_state_done
and notify_*_state_change return an error code if errors happen.
So that the error codes could be propagated and the uaf bugs can be avoid.

v2 reports a compilation warning. This v3 fixed this warning and built
successfully in my local environment with no additional warnings.
v2: https://lore.kernel.org/patchwork/patch/1435218/

Fixes: a29728463b254 ("drbd: Backport the "events2" command")
Signed-off-by: Lv Yunlong <lyl2019@mail.ustc.edu.cn>
Reviewed-by: Christoph Böhmwalder <christoph.boehmwalder@linbit.com>
Signed-off-by: Jens Axboe <axboe@kernel.dk>

authored by

Lv Yunlong and committed by
Jens Axboe
aadb22ba 4ded53ea

+42 -33
+4 -4
drivers/block/drbd/drbd_int.h
··· 1638 1638 }; 1639 1639 void drbd_bcast_event(struct drbd_device *device, const struct sib_info *sib); 1640 1640 1641 - extern void notify_resource_state(struct sk_buff *, 1641 + extern int notify_resource_state(struct sk_buff *, 1642 1642 unsigned int, 1643 1643 struct drbd_resource *, 1644 1644 struct resource_info *, 1645 1645 enum drbd_notification_type); 1646 - extern void notify_device_state(struct sk_buff *, 1646 + extern int notify_device_state(struct sk_buff *, 1647 1647 unsigned int, 1648 1648 struct drbd_device *, 1649 1649 struct device_info *, 1650 1650 enum drbd_notification_type); 1651 - extern void notify_connection_state(struct sk_buff *, 1651 + extern int notify_connection_state(struct sk_buff *, 1652 1652 unsigned int, 1653 1653 struct drbd_connection *, 1654 1654 struct connection_info *, 1655 1655 enum drbd_notification_type); 1656 - extern void notify_peer_device_state(struct sk_buff *, 1656 + extern int notify_peer_device_state(struct sk_buff *, 1657 1657 unsigned int, 1658 1658 struct drbd_peer_device *, 1659 1659 struct peer_device_info *,
+25 -16
drivers/block/drbd/drbd_nl.c
··· 4549 4549 return drbd_notification_header_to_skb(msg, &nh, true); 4550 4550 } 4551 4551 4552 - void notify_resource_state(struct sk_buff *skb, 4552 + int notify_resource_state(struct sk_buff *skb, 4553 4553 unsigned int seq, 4554 4554 struct drbd_resource *resource, 4555 4555 struct resource_info *resource_info, ··· 4591 4591 if (err && err != -ESRCH) 4592 4592 goto failed; 4593 4593 } 4594 - return; 4594 + return 0; 4595 4595 4596 4596 nla_put_failure: 4597 4597 nlmsg_free(skb); 4598 4598 failed: 4599 4599 drbd_err(resource, "Error %d while broadcasting event. Event seq:%u\n", 4600 4600 err, seq); 4601 + return err; 4601 4602 } 4602 4603 4603 - void notify_device_state(struct sk_buff *skb, 4604 + int notify_device_state(struct sk_buff *skb, 4604 4605 unsigned int seq, 4605 4606 struct drbd_device *device, 4606 4607 struct device_info *device_info, ··· 4641 4640 if (err && err != -ESRCH) 4642 4641 goto failed; 4643 4642 } 4644 - return; 4643 + return 0; 4645 4644 4646 4645 nla_put_failure: 4647 4646 nlmsg_free(skb); 4648 4647 failed: 4649 4648 drbd_err(device, "Error %d while broadcasting event. Event seq:%u\n", 4650 4649 err, seq); 4650 + return err; 4651 4651 } 4652 4652 4653 - void notify_connection_state(struct sk_buff *skb, 4653 + int notify_connection_state(struct sk_buff *skb, 4654 4654 unsigned int seq, 4655 4655 struct drbd_connection *connection, 4656 4656 struct connection_info *connection_info, ··· 4691 4689 if (err && err != -ESRCH) 4692 4690 goto failed; 4693 4691 } 4694 - return; 4692 + return 0; 4695 4693 4696 4694 nla_put_failure: 4697 4695 nlmsg_free(skb); 4698 4696 failed: 4699 4697 drbd_err(connection, "Error %d while broadcasting event. Event seq:%u\n", 4700 4698 err, seq); 4699 + return err; 4701 4700 } 4702 4701 4703 - void notify_peer_device_state(struct sk_buff *skb, 4702 + int notify_peer_device_state(struct sk_buff *skb, 4704 4703 unsigned int seq, 4705 4704 struct drbd_peer_device *peer_device, 4706 4705 struct peer_device_info *peer_device_info, ··· 4742 4739 if (err && err != -ESRCH) 4743 4740 goto failed; 4744 4741 } 4745 - return; 4742 + return 0; 4746 4743 4747 4744 nla_put_failure: 4748 4745 nlmsg_free(skb); 4749 4746 failed: 4750 4747 drbd_err(peer_device, "Error %d while broadcasting event. Event seq:%u\n", 4751 4748 err, seq); 4749 + return err; 4752 4750 } 4753 4751 4754 4752 void notify_helper(enum drbd_notification_type type, ··· 4800 4796 err, seq); 4801 4797 } 4802 4798 4803 - static void notify_initial_state_done(struct sk_buff *skb, unsigned int seq) 4799 + static int notify_initial_state_done(struct sk_buff *skb, unsigned int seq) 4804 4800 { 4805 4801 struct drbd_genlmsghdr *dh; 4806 4802 int err; ··· 4814 4810 if (nla_put_notification_header(skb, NOTIFY_EXISTS)) 4815 4811 goto nla_put_failure; 4816 4812 genlmsg_end(skb, dh); 4817 - return; 4813 + return 0; 4818 4814 4819 4815 nla_put_failure: 4820 4816 nlmsg_free(skb); 4821 4817 pr_err("Error %d sending event. Event seq:%u\n", err, seq); 4818 + return err; 4822 4819 } 4823 4820 4824 4821 static void free_state_changes(struct list_head *list) ··· 4846 4841 unsigned int seq = cb->args[2]; 4847 4842 unsigned int n; 4848 4843 enum drbd_notification_type flags = 0; 4844 + int err = 0; 4849 4845 4850 4846 /* There is no need for taking notification_mutex here: it doesn't 4851 4847 matter if the initial state events mix with later state chage ··· 4855 4849 4856 4850 cb->args[5]--; 4857 4851 if (cb->args[5] == 1) { 4858 - notify_initial_state_done(skb, seq); 4852 + err = notify_initial_state_done(skb, seq); 4859 4853 goto out; 4860 4854 } 4861 4855 n = cb->args[4]++; 4862 4856 if (cb->args[4] < cb->args[3]) 4863 4857 flags |= NOTIFY_CONTINUES; 4864 4858 if (n < 1) { 4865 - notify_resource_state_change(skb, seq, state_change->resource, 4859 + err = notify_resource_state_change(skb, seq, state_change->resource, 4866 4860 NOTIFY_EXISTS | flags); 4867 4861 goto next; 4868 4862 } 4869 4863 n--; 4870 4864 if (n < state_change->n_connections) { 4871 - notify_connection_state_change(skb, seq, &state_change->connections[n], 4865 + err = notify_connection_state_change(skb, seq, &state_change->connections[n], 4872 4866 NOTIFY_EXISTS | flags); 4873 4867 goto next; 4874 4868 } 4875 4869 n -= state_change->n_connections; 4876 4870 if (n < state_change->n_devices) { 4877 - notify_device_state_change(skb, seq, &state_change->devices[n], 4871 + err = notify_device_state_change(skb, seq, &state_change->devices[n], 4878 4872 NOTIFY_EXISTS | flags); 4879 4873 goto next; 4880 4874 } 4881 4875 n -= state_change->n_devices; 4882 4876 if (n < state_change->n_devices * state_change->n_connections) { 4883 - notify_peer_device_state_change(skb, seq, &state_change->peer_devices[n], 4877 + err = notify_peer_device_state_change(skb, seq, &state_change->peer_devices[n], 4884 4878 NOTIFY_EXISTS | flags); 4885 4879 goto next; 4886 4880 } ··· 4895 4889 cb->args[4] = 0; 4896 4890 } 4897 4891 out: 4898 - return skb->len; 4892 + if (err) 4893 + return err; 4894 + else 4895 + return skb->len; 4899 4896 } 4900 4897 4901 4898 int drbd_adm_get_initial_state(struct sk_buff *skb, struct netlink_callback *cb)
+9 -9
drivers/block/drbd/drbd_state.c
··· 1537 1537 return rv; 1538 1538 } 1539 1539 1540 - void notify_resource_state_change(struct sk_buff *skb, 1540 + int notify_resource_state_change(struct sk_buff *skb, 1541 1541 unsigned int seq, 1542 1542 struct drbd_resource_state_change *resource_state_change, 1543 1543 enum drbd_notification_type type) ··· 1550 1550 .res_susp_fen = resource_state_change->susp_fen[NEW], 1551 1551 }; 1552 1552 1553 - notify_resource_state(skb, seq, resource, &resource_info, type); 1553 + return notify_resource_state(skb, seq, resource, &resource_info, type); 1554 1554 } 1555 1555 1556 - void notify_connection_state_change(struct sk_buff *skb, 1556 + int notify_connection_state_change(struct sk_buff *skb, 1557 1557 unsigned int seq, 1558 1558 struct drbd_connection_state_change *connection_state_change, 1559 1559 enum drbd_notification_type type) ··· 1564 1564 .conn_role = connection_state_change->peer_role[NEW], 1565 1565 }; 1566 1566 1567 - notify_connection_state(skb, seq, connection, &connection_info, type); 1567 + return notify_connection_state(skb, seq, connection, &connection_info, type); 1568 1568 } 1569 1569 1570 - void notify_device_state_change(struct sk_buff *skb, 1570 + int notify_device_state_change(struct sk_buff *skb, 1571 1571 unsigned int seq, 1572 1572 struct drbd_device_state_change *device_state_change, 1573 1573 enum drbd_notification_type type) ··· 1577 1577 .dev_disk_state = device_state_change->disk_state[NEW], 1578 1578 }; 1579 1579 1580 - notify_device_state(skb, seq, device, &device_info, type); 1580 + return notify_device_state(skb, seq, device, &device_info, type); 1581 1581 } 1582 1582 1583 - void notify_peer_device_state_change(struct sk_buff *skb, 1583 + int notify_peer_device_state_change(struct sk_buff *skb, 1584 1584 unsigned int seq, 1585 1585 struct drbd_peer_device_state_change *p, 1586 1586 enum drbd_notification_type type) ··· 1594 1594 .peer_resync_susp_dependency = p->resync_susp_dependency[NEW], 1595 1595 }; 1596 1596 1597 - notify_peer_device_state(skb, seq, peer_device, &peer_device_info, type); 1597 + return notify_peer_device_state(skb, seq, peer_device, &peer_device_info, type); 1598 1598 } 1599 1599 1600 1600 static void broadcast_state_change(struct drbd_state_change *state_change) ··· 1602 1602 struct drbd_resource_state_change *resource_state_change = &state_change->resource[0]; 1603 1603 bool resource_state_has_changed; 1604 1604 unsigned int n_device, n_connection, n_peer_device, n_peer_devices; 1605 - void (*last_func)(struct sk_buff *, unsigned int, void *, 1605 + int (*last_func)(struct sk_buff *, unsigned int, void *, 1606 1606 enum drbd_notification_type) = NULL; 1607 1607 void *last_arg = NULL; 1608 1608
+4 -4
drivers/block/drbd/drbd_state_change.h
··· 44 44 extern void copy_old_to_new_state_change(struct drbd_state_change *); 45 45 extern void forget_state_change(struct drbd_state_change *); 46 46 47 - extern void notify_resource_state_change(struct sk_buff *, 47 + extern int notify_resource_state_change(struct sk_buff *, 48 48 unsigned int, 49 49 struct drbd_resource_state_change *, 50 50 enum drbd_notification_type type); 51 - extern void notify_connection_state_change(struct sk_buff *, 51 + extern int notify_connection_state_change(struct sk_buff *, 52 52 unsigned int, 53 53 struct drbd_connection_state_change *, 54 54 enum drbd_notification_type type); 55 - extern void notify_device_state_change(struct sk_buff *, 55 + extern int notify_device_state_change(struct sk_buff *, 56 56 unsigned int, 57 57 struct drbd_device_state_change *, 58 58 enum drbd_notification_type type); 59 - extern void notify_peer_device_state_change(struct sk_buff *, 59 + extern int notify_peer_device_state_change(struct sk_buff *, 60 60 unsigned int, 61 61 struct drbd_peer_device_state_change *, 62 62 enum drbd_notification_type type);