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

genetlink: remove genl_bind

A potential deadlock can occur during registering or unregistering a
new generic netlink family between the main nl_table_lock and the
cb_lock where each thread wants the lock held by the other, as
demonstrated below.

1) Thread 1 is performing a netlink_bind() operation on a socket. As part
of this call, it will call netlink_lock_table(), incrementing the
nl_table_users count to 1.
2) Thread 2 is registering (or unregistering) a genl_family via the
genl_(un)register_family() API. The cb_lock semaphore will be taken for
writing.
3) Thread 1 will call genl_bind() as part of the bind operation to handle
subscribing to GENL multicast groups at the request of the user. It will
attempt to take the cb_lock semaphore for reading, but it will fail and
be scheduled away, waiting for Thread 2 to finish the write.
4) Thread 2 will call netlink_table_grab() during the (un)registration
call. However, as Thread 1 has incremented nl_table_users, it will not
be able to proceed, and both threads will be stuck waiting for the
other.

genl_bind() is a noop, unless a genl_family implements the mcast_bind()
function to handle setting up family-specific multicast operations. Since
no one in-tree uses this functionality as Cong pointed out, simply removing
the genl_bind() function will remove the possibility for deadlock, as there
is no attempt by Thread 1 above to take the cb_lock semaphore.

Fixes: c380d9a7afff ("genetlink: pass multicast bind/unbind to families")
Suggested-by: Cong Wang <xiyou.wangcong@gmail.com>
Acked-by: Johannes Berg <johannes.berg@intel.com>
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Sean Tranchetti <stranche@codeaurora.org>
Signed-off-by: David S. Miller <davem@davemloft.net>

authored by

Sean Tranchetti and committed by
David S. Miller
1e82a62f d3c54f7f

-57
-8
include/net/genetlink.h
··· 35 35 * do additional, common, filtering and return an error 36 36 * @post_doit: called after an operation's doit callback, it may 37 37 * undo operations done by pre_doit, for example release locks 38 - * @mcast_bind: a socket bound to the given multicast group (which 39 - * is given as the offset into the groups array) 40 - * @mcast_unbind: a socket was unbound from the given multicast group. 41 - * Note that unbind() will not be called symmetrically if the 42 - * generic netlink family is removed while there are still open 43 - * sockets. 44 38 * @mcgrps: multicast groups used by this family 45 39 * @n_mcgrps: number of multicast groups 46 40 * @mcgrp_offset: starting number of multicast group IDs in this family ··· 57 63 void (*post_doit)(const struct genl_ops *ops, 58 64 struct sk_buff *skb, 59 65 struct genl_info *info); 60 - int (*mcast_bind)(struct net *net, int group); 61 - void (*mcast_unbind)(struct net *net, int group); 62 66 const struct genl_ops * ops; 63 67 const struct genl_multicast_group *mcgrps; 64 68 unsigned int n_ops;
-49
net/netlink/genetlink.c
··· 1144 1144 .netnsok = true, 1145 1145 }; 1146 1146 1147 - static int genl_bind(struct net *net, int group) 1148 - { 1149 - struct genl_family *f; 1150 - int err = -ENOENT; 1151 - unsigned int id; 1152 - 1153 - down_read(&cb_lock); 1154 - 1155 - idr_for_each_entry(&genl_fam_idr, f, id) { 1156 - if (group >= f->mcgrp_offset && 1157 - group < f->mcgrp_offset + f->n_mcgrps) { 1158 - int fam_grp = group - f->mcgrp_offset; 1159 - 1160 - if (!f->netnsok && net != &init_net) 1161 - err = -ENOENT; 1162 - else if (f->mcast_bind) 1163 - err = f->mcast_bind(net, fam_grp); 1164 - else 1165 - err = 0; 1166 - break; 1167 - } 1168 - } 1169 - up_read(&cb_lock); 1170 - 1171 - return err; 1172 - } 1173 - 1174 - static void genl_unbind(struct net *net, int group) 1175 - { 1176 - struct genl_family *f; 1177 - unsigned int id; 1178 - 1179 - down_read(&cb_lock); 1180 - 1181 - idr_for_each_entry(&genl_fam_idr, f, id) { 1182 - if (group >= f->mcgrp_offset && 1183 - group < f->mcgrp_offset + f->n_mcgrps) { 1184 - int fam_grp = group - f->mcgrp_offset; 1185 - 1186 - if (f->mcast_unbind) 1187 - f->mcast_unbind(net, fam_grp); 1188 - break; 1189 - } 1190 - } 1191 - up_read(&cb_lock); 1192 - } 1193 - 1194 1147 static int __net_init genl_pernet_init(struct net *net) 1195 1148 { 1196 1149 struct netlink_kernel_cfg cfg = { 1197 1150 .input = genl_rcv, 1198 1151 .flags = NL_CFG_F_NONROOT_RECV, 1199 - .bind = genl_bind, 1200 - .unbind = genl_unbind, 1201 1152 }; 1202 1153 1203 1154 /* we'll bump the group number right afterwards */