Subject: user kfilters
To: None <tech-net@NetBSD.org>
From: Sean Boudreau <seanb@qnx.com>
List: tech-net
Date: 09/28/2006 12:58:20
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);