Check that the format of the multicast link address is correct before taking it from dev->mc_list to priv->multicast_list. This way we never try to send a bogus address to the SA, and prevents badness from erronous 'ip maddr addr add', broken bonding drivers, or whatever.
Signed-off-by: Jason Gunthorpe <[email protected]> --- drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 19 +++++++++++++++++++ 1 files changed, 19 insertions(+), 0 deletions(-) > The idea seems sound but checkpatch.pl gives 6 errors for this small > patch! Also: Indeed, sorry, I don't do this very often, forgot that step. I added the 6 missing spaces. > name of the function could make it clearer what the expected return > value is ... eg mcast_addr_is_valid() or something like that. Yes, done > > + if (addrlen != 20) > We have INFINIBAND_ALEN defined, seems better than a magic # here. Great, I looked for that for a few mins.. > > + if (memcmp(addr,broadcast,6) != 0) > Personal taste here, but "if (foo != 0)" always seems silly to me when > we could just do "if (foo)" -- haven't looked at what usage of memcmp() > is more idiomatic in the kernel tho. OK, much of the rest of the kernel is without the operator. I do it this way because the discordance of: if (cmp(a, b)) means a is not b, if (!cmp(a, b)) means a is b if (is_something(a)) means a is something hurts my brain, even though I know that is how it all works.. diff --git a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c index 425e311..a6485c4 100644 --- a/drivers/infiniband/ulp/ipoib/ipoib_multicast.c +++ b/drivers/infiniband/ulp/ipoib/ipoib_multicast.c @@ -758,6 +758,20 @@ void ipoib_mcast_dev_flush(struct net_device *dev) } } +static int ipoib_mcast_addr_is_valid(const u8 *addr, unsigned int addrlen, + const u8 *broadcast) +{ + if (addrlen != INFINIBAND_ALEN) + return 0; + /* reserved QPN, prefix, scope */ + if (memcmp(addr, broadcast, 6)) + return 0; + /* signature lower, pkey */ + if (memcmp(addr + 7, broadcast + 7, 3)) + return 0; + return 1; +} + void ipoib_mcast_restart_task(struct work_struct *work) { struct ipoib_dev_priv *priv = @@ -791,6 +805,11 @@ void ipoib_mcast_restart_task(struct work_struct *work) for (mclist = dev->mc_list; mclist; mclist = mclist->next) { union ib_gid mgid; + if (!ipoib_mcast_addr_is_valid(mclist->dmi_addr, + mclist->dmi_addrlen, + dev->broadcast)) + continue; + memcpy(mgid.raw, mclist->dmi_addr + 4, sizeof mgid); mcast = __ipoib_mcast_find(dev, &mgid); -- 1.5.4.2 _______________________________________________ general mailing list [email protected] http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
