| Message ID | 2bef6aabf201d1fc16cca139a744700cff9dcb04.1647527635.git.petrm@nvidia.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 0caf6d9922192dd1afa8dc2131abfb4df1443b9f |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] af_netlink: Fix shift out of bounds in group mask calculation | expand |
On Thu, Mar 17, 2022 at 03:53:06PM +0100, Petr Machata wrote: > When a netlink message is received, netlink_recvmsg() fills in the address > of the sender. One of the fields is the 32-bit bitfield nl_groups, which > carries the multicast group on which the message was received. The least > significant bit corresponds to group 1, and therefore the highest group > that the field can represent is 32. Above that, the UB sanitizer flags the > out-of-bounds shift attempts. > > Which bits end up being set in such case is implementation defined, but > it's either going to be a wrong non-zero value, or zero, which is at least > not misleading. Make the latter choice deterministic by always setting to 0 > for higher-numbered multicast groups. > > To get information about membership in groups >= 32, userspace is expected > to use nl_pktinfo control messages[0], which are enabled by NETLINK_PKTINFO > socket option. > [0] https://lwn.net/Articles/147608/ > > The way to trigger this issue is e.g. through monitoring the BRVLAN group: > > # bridge monitor vlan & > # ip link add name br type bridge > > Which produces the following citation: > > UBSAN: shift-out-of-bounds in net/netlink/af_netlink.c:162:19 > shift exponent 32 is too large for 32-bit type 'int' > > Fixes: f7fa9b10edbb ("[NETLINK]: Support dynamic number of multicast groups per netlink family") > Signed-off-by: Petr Machata <petrm@nvidia.com> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Hello: This patch was applied to netdev/net.git (master) by Jakub Kicinski <kuba@kernel.org>: On Thu, 17 Mar 2022 15:53:06 +0100 you wrote: > When a netlink message is received, netlink_recvmsg() fills in the address > of the sender. One of the fields is the 32-bit bitfield nl_groups, which > carries the multicast group on which the message was received. The least > significant bit corresponds to group 1, and therefore the highest group > that the field can represent is 32. Above that, the UB sanitizer flags the > out-of-bounds shift attempts. > > [...] Here is the summary with links: - [net] af_netlink: Fix shift out of bounds in group mask calculation https://git.kernel.org/netdev/net/c/0caf6d992219 You are awesome, thank you!
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c index 7b344035bfe3..47a876ccd288 100644 --- a/net/netlink/af_netlink.c +++ b/net/netlink/af_netlink.c @@ -159,6 +159,8 @@ EXPORT_SYMBOL(do_trace_netlink_extack); static inline u32 netlink_group_mask(u32 group) { + if (group > 32) + return 0; return group ? 1 << (group - 1) : 0; }
When a netlink message is received, netlink_recvmsg() fills in the address of the sender. One of the fields is the 32-bit bitfield nl_groups, which carries the multicast group on which the message was received. The least significant bit corresponds to group 1, and therefore the highest group that the field can represent is 32. Above that, the UB sanitizer flags the out-of-bounds shift attempts. Which bits end up being set in such case is implementation defined, but it's either going to be a wrong non-zero value, or zero, which is at least not misleading. Make the latter choice deterministic by always setting to 0 for higher-numbered multicast groups. To get information about membership in groups >= 32, userspace is expected to use nl_pktinfo control messages[0], which are enabled by NETLINK_PKTINFO socket option. [0] https://lwn.net/Articles/147608/ The way to trigger this issue is e.g. through monitoring the BRVLAN group: # bridge monitor vlan & # ip link add name br type bridge Which produces the following citation: UBSAN: shift-out-of-bounds in net/netlink/af_netlink.c:162:19 shift exponent 32 is too large for 32-bit type 'int' Fixes: f7fa9b10edbb ("[NETLINK]: Support dynamic number of multicast groups per netlink family") Signed-off-by: Petr Machata <petrm@nvidia.com> --- net/netlink/af_netlink.c | 2 ++ 1 file changed, 2 insertions(+)