Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

gdev: graceful exit even after failed init #3

Merged
merged 1 commit into from

2 participants

@kris7t

Repeated calls to gdev_major_exit will not cause a kernel panic.

If gdev_major_init fails, it will call gdev_major_exit. At module unload, gdev_major_exit is called again.
This might result in a kernel panic, which makes writing and testing gdev code extremely time-consuming.

In this patch, this issue is fixed by introducing some null checks into gdev_major_exit and related code.
Thus it is safe the call gdev_major_exit several times in a row, effectively making the second and later calls
a no-op.

@kris7t kris7t gdev: graceful exit even after failed init
Repeated calls to gdev_major_exit will not cause a kernel panic.
76b43b0
@shinpei0208 shinpei0208 merged commit 0e4fa54 into shinpei0208:master
@shinpei0208
Owner

Hi Kristóf,

This is great! I have merged your commit to the master branch.

Thanks,

  • Shinpei
@shinpei0208 shinpei0208 referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jul 8, 2012
  1. @kris7t

    gdev: graceful exit even after failed init

    kris7t authored
    Repeated calls to gdev_major_exit will not cause a kernel panic.
This page is out of date. Refresh to see the latest.
Showing with 48 additions and 4 deletions.
  1. +27 −1 mod/gdev/gdev_drv.c
  2. +21 −3 mod/gdev/gdev_proc.c
View
28 mod/gdev/gdev_drv.c
@@ -53,6 +53,7 @@ MODULE_AUTHOR("Shinpei Kato");
* global variables.
*/
static dev_t dev;
+static int cdevs_registered = 0;
static struct cdev *cdevs; /* character devices for virtual devices */
/**
@@ -395,6 +396,12 @@ int gdev_minor_exit(int physid)
{
int i;
+ if (!gdevs) {
+ GDEV_PRINT("Failed to exit minor device %d, "
+ "major already exited\n", physid);
+ return -EINVAL;
+ }
+
if (gdevs[physid].users) {
GDEV_PRINT("Device %d has %d users\n", physid, gdevs[physid].users);
}
@@ -437,6 +444,7 @@ int gdev_major_init(void)
GDEV_PRINT("Failed to allocate module.\n");
goto fail_alloc_chrdev;
}
+ cdevs_registered = 1;
/* allocate Gdev physical device objects. */
if (!(gdevs = kzalloc(sizeof(*gdevs) * gdev_count, GFP_KERNEL))) {
@@ -482,12 +490,16 @@ int gdev_major_init(void)
cdev_del(&cdevs[i]);
}
kfree(cdevs);
+ cdevs = NULL;
fail_alloc_cdevs:
kfree(gdev_vds);
+ gdev_vds = NULL;
fail_alloc_gdev_vds:
kfree(gdevs);
+ gdevs = NULL;
fail_alloc_gdevs:
unregister_chrdev_region(dev, gdev_vcount);
+ cdevs_registered = 0;
fail_alloc_chrdev:
fail_getdevice:
return ret;
@@ -501,16 +513,30 @@ int gdev_major_exit(void)
gdev_proc_delete();
+ if (!cdevs)
+ goto end;
for (i = 0; i < gdev_vcount; i++) {
cdev_del(&cdevs[i]);
}
-
kfree(cdevs);
+ cdevs = NULL;
+
+ if (!gdev_vds)
+ goto end;
kfree(gdev_vds);
+ gdev_vds = NULL;
+
+ if (!gdevs)
+ goto end;
kfree(gdevs);
+ gdevs = NULL;
+ if (!cdevs_registered)
+ goto end;
unregister_chrdev_region(dev, gdev_vcount);
+ cdevs_registered = 0;
+end:
return 0;
}
View
24 mod/gdev/gdev_proc.c
@@ -32,7 +32,7 @@
#define GDEV_PROC_MAX_BUF 64
-static struct proc_dir_entry *gdev_proc;
+static struct proc_dir_entry *gdev_proc = NULL;
static struct proc_dir_entry *proc_dev_count;
static struct proc_dir_entry *proc_virt_dev_count;
static struct gdev_proc_vd {
@@ -44,7 +44,7 @@ static struct gdev_proc_vd {
struct proc_dir_entry *com_bw_used;
struct proc_dir_entry *mem_bw_used;
struct proc_dir_entry *phys;
-} *proc_vd;
+} *proc_vd = NULL;
static struct semaphore proc_sem;
static int gdev_proc_read(char *kbuf, char *page, int count, int *eof)
@@ -288,12 +288,14 @@ int gdev_proc_create(void)
remove_proc_entry("phys", proc_vd[i].dir);
}
kfree(proc_vd);
+ proc_vd = NULL;
fail_alloc_proc_vd:
remove_proc_entry("gdev/virtual_device_count", gdev_proc);
fail_proc_virt_dev_count:
remove_proc_entry("gdev/device_count", gdev_proc);
fail_proc_dev_count:
remove_proc_entry("gdev", NULL);
+ gdev_proc = NULL;
fail_proc:
return -EINVAL;
}
@@ -303,7 +305,15 @@ int gdev_proc_delete(void)
int i;
char name[256];
+ if (!gdev_proc)
+ goto end;
+
+ if (!proc_vd)
+ goto remove_gdev_proc_root;
+
for (i = 0; i < gdev_vcount; i++) {
+ if (!proc_vd[i].dir)
+ continue;
sprintf(name, "vd%d", i);
remove_proc_entry("compute_bandwidth", proc_vd[i].dir);
remove_proc_entry("memory_bandwidth", proc_vd[i].dir);
@@ -313,12 +323,20 @@ int gdev_proc_delete(void)
remove_proc_entry("memory_bandwidth_used", proc_vd[i].dir);
remove_proc_entry("phys", proc_vd[i].dir);
remove_proc_entry(name, gdev_proc);
+ proc_vd[i].dir = NULL;
}
- kfree(proc_vd);
+remove_gdev_proc_root:
remove_proc_entry("virtual_device_count", gdev_proc);
remove_proc_entry("device_count", gdev_proc);
remove_proc_entry("gdev", NULL);
+ gdev_proc = NULL;
+
+end:
+ if (proc_vd) {
+ kfree(proc_vd);
+ proc_vd = NULL;
+ }
return 0;
}
Something went wrong with that request. Please try again.