Re: [PATCH] Driver Core update for 2.6.4

Greg KH
2004-03-16T01:21:53Z

ChangeSet 1.1608.84.2, 2004/03/10 16:04:33-08:00, chrisw@osdl.org

[PATCH] Patch to hook up PPP to simple class sysfs support

* Hanna Linder (hannal@us.ibm.com) wrote:
> + ppp_class = class_simple_create(THIS_MODULE, "ppp");
> + class_simple_device_add(ppp_class, MKDEV(PPP_MAJOR, 0), NULL, "ppp");

What happens if that class_simple_create() fails? Actually,
class_simple_device_add could fail too, but doesn't seem anybody is
checking for that.

> err = devfs_mk_cdev(MKDEV(PPP_MAJOR, 0),
> S_IFCHR|S_IRUSR|S_IWUSR, "ppp");
> - if (err)
> + if (err) {
> unregister_chrdev(PPP_MAJOR, "ppp");
> + class_simple_device_remove(MKDEV(PPP_MAJOR,0));
> + }

need to destroy the class on error path to avoid leak.

> @@ -2540,6 +2547,7 @@ static void __exit ppp_cleanup(void)
> if (unregister_chrdev(PPP_MAJOR, "ppp") != 0)
> printk(KERN_ERR "PPP: failed to unregister PPP device\n");
> devfs_remove("ppp");
> + class_simple_device_remove(MKDEV(PPP_MAJOR, 0));

ditto. this will leak and would cause oops on reload of module.

something like below.

drivers/net/ppp_generic.c | 21 ++++++++++++++++++++-
1 files changed, 20 insertions(+), 1 deletion(-)

diff -Nru a/drivers/net/ppp_generic.c b/drivers/net/ppp_generic.c
--- a/drivers/net/ppp_generic.c Mon Mar 15 15:30:17 2004
+++ b/drivers/net/ppp_generic.c Mon Mar 15 15:30:17 2004
@@ -45,6 +45,7 @@
#include <linux/smp_lock.h>
#include <linux/rwsem.h>
#include <linux/stddef.h>
+#include <linux/device.h>
#include <net/slhc_vj.h>
#include <asm/atomic.h>

@@ -271,6 +272,8 @@
static int ppp_disconnect_channel(struct channel *pch);
static void ppp_destroy_channel(struct channel *pch);

+static struct class_simple *ppp_class;
+
/* Translates a PPP protocol number to a NP index (NP == network protocol) */
static inline int proto_to_npindex(int proto)
{
@@ -804,15 +807,29 @@
printk(KERN_INFO "PPP generic driver version " PPP_VERSION "\n");
err = register_chrdev(PPP_MAJOR, "ppp", &ppp_device_fops);
if (!err) {
+ ppp_class = class_simple_create(THIS_MODULE, "ppp");
+ if (IS_ERR(ppp_class)) {
+ err = PTR_ERR(ppp_class);
+ goto out_chrdev;
+ }
+ class_simple_device_add(ppp_class, MKDEV(PPP_MAJOR, 0), NULL, "ppp");
err = devfs_mk_cdev(MKDEV(PPP_MAJOR, 0),
S_IFCHR|S_IRUSR|S_IWUSR, "ppp");
if (err)
- unregister_chrdev(PPP_MAJOR, "ppp");
+ goto out_class;
}

+out:
if (err)
printk(KERN_ERR "failed to register PPP device (%d)\n", err);
return err;
+
+out_class:
+ class_simple_device_remove(MKDEV(PPP_MAJOR,0));
+ class_simple_destroy(ppp_class);
+out_chrdev:
+ unregister_chrdev(PPP_MAJOR, "ppp");
+ goto out;
}

/*
@@ -2545,6 +2562,8 @@
if (unregister_chrdev(PPP_MAJOR, "ppp") != 0)
printk(KERN_ERR "PPP: failed to unregister PPP device\n");
devfs_remove("ppp");
+ class_simple_device_remove(MKDEV(PPP_MAJOR, 0));
+ class_simple_destroy(ppp_class);
}

/*