Subject: FILEASSOC_NHOOKS removal and some questions
To: None <elad@netbsd.org>
From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
List: tech-kern
Date: 12/08/2006 23:03:25
--NextPart-20061208225838-1213100
Content-Type: Text/Plain; charset=us-ascii
the attached patch does:
- remove FILEASSOC_NHOOKS.
- make fileassoc_t a pointer and remove FILEASSOC_INVAL.
- clean up kern_fileassoc.c. unify duplicated code.
- unexport fileassoc_init using RUN_ONCE(9).
- plug memory leaks in fileassoc_file_delete and fileassoc_table_delete.
now, i have questions:
- fileassoc_tabledata_clear doesn't call FILEASSOC_CLEANUP_TABLE callback
while fileassoc_clear calls FILEASSOC_CLEANUP_FILE. is it intended?
- when the associated data is NULL, whether cleanup callback is
invoked or not seems inconsistent among functions. is it intended?
YAMAMOTO Takashi
--NextPart-20061208225838-1213100
Content-Type: Text/Plain; charset=us-ascii
Content-Disposition: attachment; filename="a.diff"
Index: sys/fileassoc.h
===================================================================
--- sys/fileassoc.h (revision 1931)
+++ sys/fileassoc.h (working copy)
@@ -36,17 +36,14 @@
#include <sys/cdefs.h>
#include <sys/param.h>
-#define FILEASSOC_INVAL -1
-
-typedef int fileassoc_t;
+typedef struct fileassoc *fileassoc_t;
typedef void (*fileassoc_cleanup_cb_t)(void *, int);
typedef void (*fileassoc_cb_t)(void *);
#define FILEASSOC_CLEANUP_TABLE 0
#define FILEASSOC_CLEANUP_FILE 1
-void fileassoc_init(void);
-fileassoc_t fileassoc_register(const char *, fileassoc_cleanup_cb_t);
+int fileassoc_register(const char *, fileassoc_cleanup_cb_t, fileassoc_t *);
int fileassoc_deregister(fileassoc_t);
void *fileassoc_tabledata_lookup(struct mount *, fileassoc_t);
void *fileassoc_lookup(struct vnode *, fileassoc_t);
Index: kern/kern_fileassoc.c
===================================================================
--- kern/kern_fileassoc.c (revision 1931)
+++ kern/kern_fileassoc.c (working copy)
@@ -46,32 +46,36 @@ __KERNEL_RCSID(0, "$NetBSD: kern_fileass
#include <sys/inttypes.h>
#include <sys/errno.h>
#include <sys/fileassoc.h>
+#include <sys/specificdata.h>
#include <sys/hash.h>
#include <sys/fstypes.h>
-
-/* Max. number of hooks. */
-#ifndef FILEASSOC_NHOOKS
-#define FILEASSOC_NHOOKS 4
-#endif /* !FILEASSOC_NHOOKS */
+#include <sys/kmem.h>
+#include <sys/once.h>
static struct fileassoc_hash_entry *
fileassoc_file_lookup(struct vnode *, fhandle_t *);
static struct fileassoc_hash_entry *
fileassoc_file_add(struct vnode *, fhandle_t *);
+static specificdata_domain_t fileassoc_domain;
+
/*
* Hook entry.
* Includes the hook name for identification and private hook clear callback.
*/
-struct fileassoc_hook {
- const char *hook_name; /* Hook name. */
- fileassoc_cleanup_cb_t hook_cleanup_cb; /* Hook clear callback. */
+struct fileassoc {
+ LIST_ENTRY(fileassoc) list;
+ const char *name; /* name. */
+ fileassoc_cleanup_cb_t cleanup_cb; /* clear callback. */
+ specificdata_key_t key;
};
+static LIST_HEAD(, fileassoc) fileassoc_list;
+
/* An entry in the per-device hash table. */
struct fileassoc_hash_entry {
fhandle_t *handle; /* File handle */
- void *hooks[FILEASSOC_NHOOKS]; /* Hooks. */
+ specificdata_reference data; /* Hooks. */
LIST_ENTRY(fileassoc_hash_entry) entries; /* List pointer. */
};
@@ -82,13 +86,10 @@ struct fileassoc_table {
size_t hash_size; /* Number of slots. */
struct mount *tbl_mntpt;
u_long hash_mask;
- void *tables[FILEASSOC_NHOOKS];
+ specificdata_reference data;
LIST_ENTRY(fileassoc_table) hash_list; /* List pointer. */
};
-struct fileassoc_hook fileassoc_hooks[FILEASSOC_NHOOKS];
-int fileassoc_nhooks;
-
/* Global list of hash tables, one per device. */
LIST_HEAD(, fileassoc_table) fileassoc_tables;
@@ -100,54 +101,133 @@ LIST_HEAD(, fileassoc_table) fileassoc_t
(hash32_buf((handle), FHANDLE_SIZE(handle), HASH32_BUF_INIT) \
& ((tbl)->hash_mask))
+static void *
+table_getdata(struct fileassoc_table *tbl, const struct fileassoc *assoc)
+{
+
+ return specificdata_getspecific(fileassoc_domain, &tbl->data,
+ assoc->key);
+}
+
+static void
+table_setdata(struct fileassoc_table *tbl, const struct fileassoc *assoc,
+ void *data)
+{
+
+ specificdata_setspecific(fileassoc_domain, &tbl->data, assoc->key,
+ data);
+}
+
+static void
+table_cleanup(struct fileassoc_table *tbl, const struct fileassoc *assoc)
+{
+ fileassoc_cleanup_cb_t cb;
+ void *data;
+
+ cb = assoc->cleanup_cb;
+ if (cb == NULL) {
+ return;
+ }
+ data = table_getdata(tbl, assoc);
+ (*cb)(data, FILEASSOC_CLEANUP_TABLE);
+}
+
+static void *
+file_getdata(struct fileassoc_hash_entry *e, const struct fileassoc *assoc)
+{
+
+ return specificdata_getspecific(fileassoc_domain, &e->data,
+ assoc->key);
+}
+
+static void
+file_setdata(struct fileassoc_hash_entry *e, const struct fileassoc *assoc,
+ void *data)
+{
+
+ specificdata_setspecific(fileassoc_domain, &e->data, assoc->key,
+ data);
+}
+
+static void
+file_cleanup(struct fileassoc_hash_entry *e, const struct fileassoc *assoc)
+{
+ fileassoc_cleanup_cb_t cb;
+ void *data;
+
+ cb = assoc->cleanup_cb;
+ if (cb == NULL) {
+ return;
+ }
+ data = file_getdata(e, assoc);
+ (*cb)(data, FILEASSOC_CLEANUP_FILE);
+}
+
+static void
+file_free(struct fileassoc_hash_entry *e)
+{
+ struct fileassoc *assoc;
+
+ LIST_REMOVE(e, entries);
+
+ LIST_FOREACH(assoc, &fileassoc_list, list) {
+ file_cleanup(e, assoc);
+ }
+ vfs_composefh_free(e->handle);
+ specificdata_fini(fileassoc_domain, &e->data);
+ kmem_free(e, sizeof(*e));
+}
+
/*
* Initialize the fileassoc subsystem.
*/
-void
+static int
fileassoc_init(void)
{
- memset(fileassoc_hooks, 0, sizeof(fileassoc_hooks));
- fileassoc_nhooks = 0;
+
+ fileassoc_domain = specificdata_domain_create();
+
+ return 0;
}
/*
* Register a new hook.
*/
-fileassoc_t
-fileassoc_register(const char *name, fileassoc_cleanup_cb_t cleanup_cb)
+int
+fileassoc_register(const char *name, fileassoc_cleanup_cb_t cleanup_cb,
+ fileassoc_t *result)
{
- int i;
-
- if (fileassoc_nhooks >= FILEASSOC_NHOOKS)
- return (-1);
-
- for (i = 0; i < FILEASSOC_NHOOKS; i++)
- if (fileassoc_hooks[i].hook_name == NULL)
- break;
-
- fileassoc_hooks[i].hook_name = name;
- fileassoc_hooks[i].hook_cleanup_cb = cleanup_cb;
+ int error;
+ specificdata_key_t key;
+ struct fileassoc *assoc;
+ static ONCE_DECL(control);
- fileassoc_nhooks++;
+ RUN_ONCE(&control, fileassoc_init);
+ error = specificdata_key_create(fileassoc_domain, &key, NULL);
+ if (error) {
+ return error;
+ }
+ assoc = kmem_alloc(sizeof(*assoc), KM_SLEEP);
+ assoc->name = name;
+ assoc->cleanup_cb = cleanup_cb;
+ assoc->key = key;
+ LIST_INSERT_HEAD(&fileassoc_list, assoc, list);
+ *result = assoc;
- return (i);
+ return 0;
}
/*
* Deregister a hook.
*/
int
-fileassoc_deregister(fileassoc_t id)
+fileassoc_deregister(fileassoc_t assoc)
{
- if (id < 0 || id >= FILEASSOC_NHOOKS)
- return (EINVAL);
-
- fileassoc_hooks[id].hook_name = NULL;
- fileassoc_hooks[id].hook_cleanup_cb = NULL;
- fileassoc_nhooks--;
+ LIST_REMOVE(assoc, list);
+ kmem_free(assoc, sizeof(*assoc));
- return (0);
+ return 0;
}
/*
@@ -222,7 +302,7 @@ fileassoc_file_lookup(struct vnode *vp,
* Return hook data associated with a vnode.
*/
void *
-fileassoc_lookup(struct vnode *vp, fileassoc_t id)
+fileassoc_lookup(struct vnode *vp, fileassoc_t assoc)
{
struct fileassoc_hash_entry *mhe;
@@ -230,7 +310,7 @@ fileassoc_lookup(struct vnode *vp, filea
if (mhe == NULL)
return (NULL);
- return (mhe->hooks[id]);
+ return file_getdata(mhe, assoc);
}
/*
@@ -246,11 +326,12 @@ fileassoc_table_add(struct mount *mp, si
return (EEXIST);
/* Allocate and initialize a Veriexec hash table. */
- tbl = malloc(sizeof(*tbl), M_TEMP, M_WAITOK | M_ZERO);
+ tbl = kmem_zalloc(sizeof(*tbl), KM_SLEEP);
tbl->hash_size = size;
tbl->tbl_mntpt = mp;
tbl->hash_tbl = hashinit(size, HASH_LIST, M_TEMP,
M_WAITOK | M_ZERO, &tbl->hash_mask);
+ specificdata_init(fileassoc_domain, &tbl->data);
LIST_INSERT_HEAD(&fileassoc_tables, tbl, hash_list);
@@ -263,10 +344,10 @@ fileassoc_table_add(struct mount *mp, si
int
fileassoc_table_delete(struct mount *mp)
{
+ const struct fileassoc *assoc;
struct fileassoc_table *tbl;
struct fileassoc_hashhead *hh;
u_long i;
- int j;
tbl = fileassoc_table_lookup(mp);
if (tbl == NULL)
@@ -277,29 +358,20 @@ fileassoc_table_delete(struct mount *mp)
for (i = 0; i < tbl->hash_size; i++) {
struct fileassoc_hash_entry *mhe;
- while (LIST_FIRST(&hh[i]) != NULL) {
- mhe = LIST_FIRST(&hh[i]);
- LIST_REMOVE(mhe, entries);
-
- for (j = 0; j < fileassoc_nhooks; j++)
- if (fileassoc_hooks[j].hook_cleanup_cb != NULL)
- (fileassoc_hooks[j].hook_cleanup_cb)
- (mhe->hooks[j],
- FILEASSOC_CLEANUP_FILE);
-
- vfs_composefh_free(mhe->handle);
- free(mhe, M_TEMP);
+ while ((mhe = LIST_FIRST(&hh[i])) != NULL) {
+ file_free(mhe);
}
}
- for (j = 0; j < fileassoc_nhooks; j++)
- if (fileassoc_hooks[j].hook_cleanup_cb != NULL)
- (fileassoc_hooks[j].hook_cleanup_cb)(tbl->tables[j],
- FILEASSOC_CLEANUP_TABLE);
+ LIST_FOREACH(assoc, &fileassoc_list, list) {
+ table_cleanup(tbl, assoc);
+ }
/* Remove hash table and sysctl node */
hashdone(tbl->hash_tbl, M_TEMP);
LIST_REMOVE(tbl, hash_list);
+ specificdata_fini(fileassoc_domain, &tbl->data);
+ kmem_free(tbl, sizeof(*tbl));
return (0);
}
@@ -308,7 +380,7 @@ fileassoc_table_delete(struct mount *mp)
* Run a callback for each hook entry in a table.
*/
int
-fileassoc_table_run(struct mount *mp, fileassoc_t id, fileassoc_cb_t cb)
+fileassoc_table_run(struct mount *mp, fileassoc_t assoc, fileassoc_cb_t cb)
{
struct fileassoc_table *tbl;
struct fileassoc_hashhead *hh;
@@ -323,8 +395,11 @@ fileassoc_table_run(struct mount *mp, fi
struct fileassoc_hash_entry *mhe;
LIST_FOREACH(mhe, &hh[i], entries) {
- if (mhe->hooks[id] != NULL)
- cb(mhe->hooks[id]);
+ void *data;
+
+ data = file_getdata(mhe, assoc);
+ if (data != NULL)
+ cb(data);
}
}
@@ -335,36 +410,28 @@ fileassoc_table_run(struct mount *mp, fi
* Clear a table for a given hook.
*/
int
-fileassoc_table_clear(struct mount *mp, fileassoc_t id)
+fileassoc_table_clear(struct mount *mp, fileassoc_t assoc)
{
struct fileassoc_table *tbl;
struct fileassoc_hashhead *hh;
- fileassoc_cleanup_cb_t cleanup_cb;
u_long i;
tbl = fileassoc_table_lookup(mp);
if (tbl == NULL)
return (EEXIST);
- cleanup_cb = fileassoc_hooks[id].hook_cleanup_cb;
-
hh = tbl->hash_tbl;
for (i = 0; i < tbl->hash_size; i++) {
struct fileassoc_hash_entry *mhe;
LIST_FOREACH(mhe, &hh[i], entries) {
- if ((mhe->hooks[id] != NULL) && cleanup_cb != NULL)
- cleanup_cb(mhe->hooks[id],
- FILEASSOC_CLEANUP_FILE);
-
- mhe->hooks[id] = NULL;
+ file_cleanup(mhe, assoc);
+ file_setdata(mhe, assoc, NULL);
}
}
- if ((tbl->tables[id] != NULL) && cleanup_cb != NULL)
- cleanup_cb(tbl->tables[id], FILEASSOC_CLEANUP_TABLE);
-
- tbl->tables[id] = NULL;
+ table_cleanup(tbl, assoc);
+ table_setdata(tbl, assoc, NULL);
return (0);
}
@@ -373,7 +440,7 @@ fileassoc_table_clear(struct mount *mp,
* Add hook-specific data on a fileassoc table.
*/
int
-fileassoc_tabledata_add(struct mount *mp, fileassoc_t id, void *data)
+fileassoc_tabledata_add(struct mount *mp, fileassoc_t assoc, void *data)
{
struct fileassoc_table *tbl;
@@ -381,7 +448,7 @@ fileassoc_tabledata_add(struct mount *mp
if (tbl == NULL)
return (EFAULT);
- tbl->tables[id] = data;
+ table_setdata(tbl, assoc, data);
return (0);
}
@@ -390,24 +457,17 @@ fileassoc_tabledata_add(struct mount *mp
* Clear hook-specific data on a fileassoc table.
*/
int
-fileassoc_tabledata_clear(struct mount *mp, fileassoc_t id)
+fileassoc_tabledata_clear(struct mount *mp, fileassoc_t assoc)
{
- struct fileassoc_table *tbl;
-
- tbl = fileassoc_table_lookup(mp);
- if (tbl == NULL)
- return (EFAULT);
-
- tbl->tables[id] = NULL;
- return (0);
+ return fileassoc_tabledata_add(mp, assoc, NULL);
}
/*
* Retrieve hook-specific data from a fileassoc table.
*/
void *
-fileassoc_tabledata_lookup(struct mount *mp, fileassoc_t id)
+fileassoc_tabledata_lookup(struct mount *mp, fileassoc_t assoc)
{
struct fileassoc_table *tbl;
@@ -415,7 +475,7 @@ fileassoc_tabledata_lookup(struct mount
if (tbl == NULL)
return (NULL);
- return (tbl->tables[id]);
+ return table_getdata(tbl, assoc);
}
/*
@@ -457,8 +517,9 @@ fileassoc_file_add(struct vnode *vp, fha
indx = FILEASSOC_HASH(tbl, th);
vhh = &(tbl->hash_tbl[indx]);
- e = malloc(sizeof(*e), M_TEMP, M_WAITOK | M_ZERO);
+ e = kmem_zalloc(sizeof(*e), KM_SLEEP);
e->handle = th;
+ specificdata_init(fileassoc_domain, &e->data);
LIST_INSERT_HEAD(vhh, e, entries);
return (e);
@@ -471,20 +532,12 @@ int
fileassoc_file_delete(struct vnode *vp)
{
struct fileassoc_hash_entry *mhe;
- int i;
mhe = fileassoc_file_lookup(vp, NULL);
if (mhe == NULL)
return (ENOENT);
- LIST_REMOVE(mhe, entries);
-
- for (i = 0; i < fileassoc_nhooks; i++)
- if (fileassoc_hooks[i].hook_cleanup_cb != NULL)
- (fileassoc_hooks[i].hook_cleanup_cb)(mhe->hooks[i],
- FILEASSOC_CLEANUP_FILE);
-
- free(mhe, M_TEMP);
+ file_free(mhe);
return (0);
}
@@ -493,9 +546,10 @@ fileassoc_file_delete(struct vnode *vp)
* Add a hook to a vnode.
*/
int
-fileassoc_add(struct vnode *vp, fileassoc_t id, void *data)
+fileassoc_add(struct vnode *vp, fileassoc_t assoc, void *data)
{
struct fileassoc_hash_entry *e;
+ void *olddata;
e = fileassoc_file_lookup(vp, NULL);
if (e == NULL) {
@@ -504,10 +558,11 @@ fileassoc_add(struct vnode *vp, fileasso
return (ENOTDIR);
}
- if (e->hooks[id] != NULL)
+ olddata = file_getdata(e, assoc);
+ if (olddata != NULL)
return (EEXIST);
- e->hooks[id] = data;
+ file_setdata(e, assoc, data);
return (0);
}
@@ -516,20 +571,16 @@ fileassoc_add(struct vnode *vp, fileasso
* Clear a hook from a vnode.
*/
int
-fileassoc_clear(struct vnode *vp, fileassoc_t id)
+fileassoc_clear(struct vnode *vp, fileassoc_t assoc)
{
struct fileassoc_hash_entry *mhe;
- fileassoc_cleanup_cb_t cleanup_cb;
mhe = fileassoc_file_lookup(vp, NULL);
if (mhe == NULL)
return (ENOENT);
- cleanup_cb = fileassoc_hooks[id].hook_cleanup_cb;
- if ((mhe->hooks[id] != NULL) && cleanup_cb != NULL)
- cleanup_cb(mhe->hooks[id], FILEASSOC_CLEANUP_FILE);
-
- mhe->hooks[id] = NULL;
+ file_cleanup(mhe, assoc);
+ file_setdata(mhe, assoc, NULL);
return (0);
}
Index: kern/kern_verifiedexec.c
===================================================================
--- kern/kern_verifiedexec.c (revision 1926)
+++ kern/kern_verifiedexec.c (working copy)
@@ -102,7 +102,7 @@ size_t veriexec_name_max;
const struct sysctlnode *veriexec_count_node;
-int veriexec_hook;
+static fileassoc_t veriexec_hook;
LIST_HEAD(, veriexec_fpops) veriexec_fpops_list;
@@ -267,10 +267,12 @@ veriexec_fpops_add(const char *fp_type,
void
veriexec_init(void)
{
+ int error;
+
/* Register a fileassoc for Veriexec. */
- veriexec_hook = fileassoc_register("veriexec", veriexec_clear);
- if (veriexec_hook == FILEASSOC_INVAL)
- panic("Veriexec: Can't register fileassoc");
+ error = fileassoc_register("veriexec", veriexec_clear, &veriexec_hook);
+ if (error != 0)
+ panic("Veriexec: Can't register fileassoc: error=%d", error);
/* Register listener to handle raw disk access. */
if (kauth_listen_scope(KAUTH_SCOPE_DEVICE, veriexec_raw_cb, NULL) ==
Index: kern/kern_pax.c
===================================================================
--- kern/kern_pax.c (revision 1922)
+++ kern/kern_pax.c (working copy)
@@ -182,12 +182,20 @@ SYSCTL_SETUP(sysctl_security_pax_setup,
void
pax_init(void)
{
+#ifdef PAX_SEGVGUARD
+ int error;
+#endif /* PAX_SEGVGUARD */
+
#ifdef PAX_MPROTECT
proc_specific_key_create(&pax_mprotect_key, NULL);
#endif /* PAX_MPROTECT */
#ifdef PAX_SEGVGUARD
- segvguard_id = fileassoc_register("segvguard", pax_segvguard_cb);
+ error = fileassoc_register("segvguard", pax_segvguard_cb,
+ &segvguard_id);
+ if (error) {
+ panic("pax_init: segvguard_id: error=%d\n", error);
+ }
proc_specific_key_create(&pax_segvguard_key, NULL);
#endif /* PAX_SEGVGUARD */
}
@@ -286,7 +294,7 @@ pax_segvguard(struct lwp *l, struct vnod
(!pax_segvguard_global && t != PAX_SEGVGUARD_EXPLICIT_ENABLE))
return (0);
- if (segvguard_id == FILEASSOC_INVAL || vp == NULL)
+ if (vp == NULL)
return (EFAULT);
/* Check if we already monitor the file. */
--NextPart-20061208225838-1213100--