linux-kernel - Re: [PATCH 148/196] Infiniband: make ipath driver use default driver groups.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ada7ihxlfnh.fsf@cisco.com>
Date:	Fri, 25 Jan 2008 14:11:14 -0800
From:	Roland Dreier <rdreier@...co.com>
To:	Greg Kroah-Hartman <gregkh@...e.de>
Cc:	linux-kernel@...r.kernel.org,
	"Bryan O'Sullivan" <bryan.osullivan@...gic.com>,
	Arthur Jones <arthur.jones@...gic.com>,
	Cornelia Huck <cornelia.huck@...ibm.com>
Subject: Re: [PATCH 148/196] Infiniband: make ipath driver use default driver groups.

So I think it is coming from the following code in ipath_sysfs.c:

	int ipath_device_create_group(struct device *dev, struct ipath_devdata *dd)
	{
		int ret;
	
		ret = sysfs_create_group(&dev->kobj, &dev_attr_group);
		if (ret)
			goto bail;
	
		ret = sysfs_create_group(&dev->kobj, &dev_counter_attr_group);
		if (ret)
			goto bail_attrs;
	
		sysfs_remove_group(&dev->kobj, &dev_counter_attr_group);
	bail_attrs:
		sysfs_remove_group(&dev->kobj, &dev_attr_group);
	bail:
		return ret;
	}

note that the success path falls through into the error path and
removes the groups it just created, which means that on initialization
the groups are already removed, so we hit the BUG on the second try to
remove the groups on module exit.

And I think this chunk of your patch:

	-	snprintf(unit, sizeof(unit), "%02d", dd->ipath_unit);
	-	ret = sysfs_create_link(&dev->driver->kobj, &dev->kobj, unit);
	-	if (ret == 0)
	-		goto bail;
	-

was what broke it.  (Note the "if (ret == 0) then skip error
unwinding" code that got deleted by mistake -- not surprising given
how hard to read that construction is)

My bad for not testing -mm better.  I'll queue this up to fix it:

diff --git a/drivers/infiniband/hw/ipath/ipath_sysfs.c b/drivers/infiniband/hw/ipath/ipath_sysfs.c
index aa27ca9..e2a6534 100644
--- a/drivers/infiniband/hw/ipath/ipath_sysfs.c
+++ b/drivers/infiniband/hw/ipath/ipath_sysfs.c
@@ -770,7 +770,8 @@ int ipath_device_create_group(struct device *dev, struct ipath_devdata *dd)
 	if (ret)
 		goto bail_attrs;
 
-	sysfs_remove_group(&dev->kobj, &dev_counter_attr_group);
+	return 0;
+
 bail_attrs:
 	sysfs_remove_group(&dev->kobj, &dev_attr_group);
 bail:
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/