user kfilters

[Sean Boudreau]

Hi:


There's a couple of issues in kfilter_byname_user():
- There's no guarantee the slot at the end of the array
has name == NULL since all slots up to user_kfiltermaxc
are potentially used.
- The test against name != 0 should be *name == 0.

There's also an issue in kfilter_register() in that
a new slot is always added to the array: deregistered
slots are not reused.

The follwing uses a NULL name to indicate a deregistered
user filter and not the end of the array. It also reuses
slots.

Any comments before I commit?

Regards,

-seanb

Index: kern/kern_event.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_event.c,v
retrieving revision 1.30
diff -c -r1.30 kern_event.c
*** kern/kern_event.c 23 Jul 2006 22:06:11 -0000 1.30
--- kern/kern_event.c 28 Sep 2006 16:43:03 -0000
***************
*** 173,184 ****
{
int i;

! /* user_kfilters[] could be NULL if no filters were registered */
! if (!user_kfilters)
! return (NULL);
!
! for (i = 0; user_kfilters[i].name != NULL; i++) {
! if (user_kfilters[i].name != '\0' &&
strcmp(name, user_kfilters[i].name) == 0)
return (&user_kfilters[i]);
}
--- 173,181 ----
{
int i;

! /* user filter slots have a NULL name if previously deregistered */
! for (i = 0; i < user_kfilterc ; i++) {
! if (user_kfilters[i].name != NULL &&
strcmp(name, user_kfilters[i].name) == 0)
return (&user_kfilters[i]);
}
***************
*** 229,234 ****
--- 226,232 ----
struct kfilter *kfilter;
void *space;
int len;
+ int i;

if (name == NULL || name[0] == '\0' || filtops == NULL)
return (EINVAL); /* invalid args */
***************
*** 237,242 ****
--- 235,248 ----
if (user_kfilterc > 0xffffffff - EVFILT_SYSCOUNT)
return (EINVAL); /* too many */

+ for (i = 0; i < user_kfilterc; i++) {
+ kfilter = &user_kfilters[i];
+ if (kfilter->name == NULL) {
+ /* Previously deregistered slot. Reuse. */
+ goto reuse;
+ }
+ }
+
/* check if need to grow user_kfilters */
if (user_kfilterc + 1 > user_kfiltermaxc) {
/*
***************
*** 261,281 ****
free(user_kfilters, M_KEVENT);
user_kfilters = kfilter;
}
len = strlen(name) + 1; /* copy name */
space = malloc(len, M_KEVENT, M_WAITOK);
memcpy(space, name, len);
! user_kfilters[user_kfilterc].name = space;

! user_kfilters[user_kfilterc].filter = user_kfilterc + EVFILT_SYSCOUNT;

len = sizeof(struct filterops); /* copy filtops */
space = malloc(len, M_KEVENT, M_WAITOK);
memcpy(space, filtops, len);
! user_kfilters[user_kfilterc].filtops = space;

if (retfilter != NULL)
! *retfilter = user_kfilters[user_kfilterc].filter;
! user_kfilterc++; /* finally, increment count */
return (0);
}

--- 267,289 ----
free(user_kfilters, M_KEVENT);
user_kfilters = kfilter;
}
+ /* Adding new slot */
+ kfilter = &user_kfilters[user_kfilterc++];
+ reuse:
len = strlen(name) + 1; /* copy name */
space = malloc(len, M_KEVENT, M_WAITOK);
memcpy(space, name, len);
! kfilter->name = space;

! kfilter->filter = (kfilter - user_kfilters) + EVFILT_SYSCOUNT;

len = sizeof(struct filterops); /* copy filtops */
space = malloc(len, M_KEVENT, M_WAITOK);
memcpy(space, filtops, len);
! kfilter->filtops = space;

if (retfilter != NULL)
! *retfilter = kfilter->filter;
return (0);
}

***************
*** 300,310 ****
if (kfilter == NULL) /* not found */
return (ENOENT);

! if (kfilter->name[0] != '\0') {
! /* XXXUNCONST Cast away const (but we know it's safe. */
! free(__UNCONST(kfilter->name), M_KEVENT);
! kfilter->name = ""; /* mark as `not implemented' */
! }
if (kfilter->filtops != NULL) {
/* XXXUNCONST Cast away const (but we know it's safe. */
free(__UNCONST(kfilter->filtops), M_KEVENT);
--- 308,317 ----
if (kfilter == NULL) /* not found */
return (ENOENT);

! /* XXXUNCONST Cast away const (but we know it's safe. */
! free(__UNCONST(kfilter->name), M_KEVENT);
! kfilter->name = NULL; /* mark as `not implemented' */
!
if (kfilter->filtops != NULL) {
/* XXXUNCONST Cast away const (but we know it's safe. */
free(__UNCONST(kfilter->filtops), M_KEVENT);

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de

[Jason Thorpe]

Post by Sean Boudreau
- There's no guarantee the slot at the end of the array
has name == NULL since all slots up to user_kfiltermaxc
are potentially used.
- The test against name != 0 should be *name == 0.
There's also an issue in kfilter_register() in that
a new slot is always added to the array: deregistered
slots are not reused.
The follwing uses a NULL name to indicate a deregistered
user filter and not the end of the array. It also reuses
slots.
Any comments before I commit?

Looks cool to me!

-- thorpej


--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-***@muc.de