From: Corey Minyard <minyard@acm.org>
* On a 32-bit architecture, the idr code will cease to work if you add
more than 2^20 entries. You will not be able to find many of the
entries. The problem is that the IDR code uses 5-bit chunks of the
number and the lower portion used by IDR is 24 bits, so you have one bit
that leaks over into the comparisons that should not be there. The
solution is to mask off that bit before doing IDR processing. This
actually causes the POSIX timer code to crash if you create that many
timers. I have included an idr_test.tar.gz file that demonstrates this
with and without the fix, in case you need more evidence :).
* When the IDR fills up, it returns -1. However, there was no way to
check for this condition. This patch adds the ability to check for the
idr being full and fixes all the users. It also fixes a problem in
fs/super.c where the idr code wasn't checking for -1.
* There was a race condition creating POSIX timers. The timer was added
to a task struct for another process then the data for the timer was
filled out. The other task could use/destroy time timer as soon as it is
in the task's queue and the lock is released. This moves settup up the
timer data to before the timer is enqueued or (for some data) into the
lock.
DESC
idr-overflow-fixes fix
EDESC
net/sctp/sm_make_chunk.c: In function `sctp_process_init':
net/sctp/sm_make_chunk.c:1850: warning: comparison between pointer and integer
*** Warning: "idr_full" [net/sctp/sctp.ko] undefined!
DESC
More fixes for idr code
EDESC
From: Corey Minyard <minyard@acm.org>
Change things so that the caller doesn't need to run idr_full() to find out
the reason for an idr_get_new() failure.
Just return -ENOSPC if the tree was full, or -EAGAIN if the caller needs to
re-run idr_pre_get() and try again.
DESC
Fixes for POSIX timers
EDESC
From: Corey Minyard <minyard@acm.org>
* POSIX timers have no limit on creation. This means that a normal user
can create an arbitrary number of POSIX timers and run the system out of
kernel memory. This patch adds a limit on the number of POSIX timers.
I'm not sure if this is the best way to handle the problem, but allowing
a user to use up all kernel memory is bad. Actually, in 2.6, the queued
signals allocated by the timer will limit it to max_queued_signals, but
that means the user can use up all the queued signals by allocating a
bunch of timers. Bad, too. Note that I would like to make the default
value of the maximum number of posix timers relative to the queued
signal's default, but it is a magic number and I couldn't find a good
place to put the value.
DESC
timers-signals-rlimits-setuid-fix
EDESC
DESC
timers-signals-rlimits-fix
EDESC
DESC
timers-signals-rlimits-rename-stuff
EDESC
DESC
idr-overflow-fixes fix
EDESC
net/sctp/sm_make_chunk.c: In function `sctp_process_init':
net/sctp/sm_make_chunk.c:1850: warning: comparison between pointer and integer
*** Warning: "idr_full" [net/sctp/sctp.ko] undefined!
DESC
More fixes for idr code
EDESC
From: Corey Minyard <minyard@acm.org>
Change things so that the caller doesn't need to run idr_full() to find out
the reason for an idr_get_new() failure.
Just return -ENOSPC if the tree was full, or -EAGAIN if the caller needs to
re-run idr_pre_get() and try again.
---
25-akpm/fs/proc/generic.c | 10 ++--
25-akpm/fs/super.c | 15 ++++--
25-akpm/include/linux/idr.h | 8 +++
25-akpm/kernel/posix-timers.c | 86 +++++++++++++++++++++++----------------
25-akpm/lib/idr.c | 45 ++++++++++++++++----
25-akpm/net/sctp/sm_make_chunk.c | 25 ++++++-----
6 files changed, 127 insertions(+), 62 deletions(-)
diff -puN fs/proc/generic.c~idr-overflow-fixes fs/proc/generic.c
--- 25/fs/proc/generic.c~idr-overflow-fixes 2004-05-12 21:02:25.970597776 -0700
+++ 25-akpm/fs/proc/generic.c 2004-05-12 21:02:25.981596104 -0700
@@ -288,18 +288,20 @@ static spinlock_t proc_inum_lock = SPIN_
*/
static unsigned int get_inode_number(void)
{
- unsigned int i, inum = 0;
+ int i, inum = 0;
+ int error;
retry:
if (idr_pre_get(&proc_inum_idr, GFP_KERNEL) == 0)
return 0;
spin_lock(&proc_inum_lock);
- i = idr_get_new(&proc_inum_idr, NULL);
+ error = idr_get_new(&proc_inum_idr, NULL, &i);
spin_unlock(&proc_inum_lock);
-
- if (i == -1)
+ if (error == -EAGAIN)
goto retry;
+ else if (error)
+ return 0;
inum = (i & MAX_ID_MASK) + PROC_DYNAMIC_FIRST;
diff -puN fs/super.c~idr-overflow-fixes fs/super.c
--- 25/fs/super.c~idr-overflow-fixes 2004-05-12 21:02:25.971597624 -0700
+++ 25-akpm/fs/super.c 2004-05-12 21:02:25.981596104 -0700
@@ -570,14 +570,19 @@ static spinlock_t unnamed_dev_lock = SPI
int set_anon_super(struct super_block *s, void *data)
{
int dev;
+ int error;
- spin_lock(&unnamed_dev_lock);
- if (idr_pre_get(&unnamed_dev_idr, GFP_ATOMIC) == 0) {
- spin_unlock(&unnamed_dev_lock);
+ retry:
+ if (idr_pre_get(&unnamed_dev_idr, GFP_ATOMIC) == 0)
return -ENOMEM;
- }
- dev = idr_get_new(&unnamed_dev_idr, NULL);
+ spin_lock(&unnamed_dev_lock);
+ error = idr_get_new(&unnamed_dev_idr, NULL, &dev);
spin_unlock(&unnamed_dev_lock);
+ if (error == -EAGAIN)
+ /* We raced and lost with another CPU. */
+ goto retry;
+ else if (error)
+ return -EAGAIN;
if ((dev & MAX_ID_MASK) == (1 << MINORBITS)) {
spin_lock(&unnamed_dev_lock);
diff -puN include/linux/idr.h~idr-overflow-fixes include/linux/idr.h
--- 25/include/linux/idr.h~idr-overflow-fixes 2004-05-12 21:02:25.972597472 -0700
+++ 25-akpm/include/linux/idr.h 2004-05-12 21:02:25.982595952 -0700
@@ -16,9 +16,15 @@
#if BITS_PER_LONG == 32
# define IDR_BITS 5
# define IDR_FULL 0xfffffffful
+/* We can only use half the bits in the top level because there are
+ only four possible bits in the top level (5 bits * 4 levels = 25
+ bits, but you only use 24 bits in the id). */
+# define TOP_LEVEL_FULL (IDR_FULL >> 16)
#elif BITS_PER_LONG == 64
# define IDR_BITS 6
# define IDR_FULL 0xfffffffffffffffful
+/* We can use all the bits in a 64-bit long at the top level. */
+# define TOP_LEVEL_FULL IDR_FULL
#else
# error "BITS_PER_LONG is not 32 or 64"
#endif
@@ -71,7 +77,7 @@ struct idr {
void *idr_find(struct idr *idp, int id);
int idr_pre_get(struct idr *idp, unsigned gfp_mask);
-int idr_get_new(struct idr *idp, void *ptr);
+int idr_get_new(struct idr *idp, void *ptr, int *id);
void idr_remove(struct idr *idp, int id);
void idr_init(struct idr *idp);
diff -puN kernel/posix-timers.c~idr-overflow-fixes kernel/posix-timers.c
--- 25/kernel/posix-timers.c~idr-overflow-fixes 2004-05-12 21:02:25.974597168 -0700
+++ 25-akpm/kernel/posix-timers.c 2004-05-12 21:02:25.984595648 -0700
@@ -397,7 +397,6 @@ static struct k_itimer * alloc_posix_tim
if (!tmr)
return tmr;
memset(tmr, 0, sizeof (struct k_itimer));
- tmr->it_id = (timer_t)-1;
if (unlikely(!(tmr->sigq = sigqueue_alloc()))) {
kmem_cache_free(posix_timers_cache, tmr);
tmr = 0;
@@ -405,9 +404,11 @@ static struct k_itimer * alloc_posix_tim
return tmr;
}
-static void release_posix_timer(struct k_itimer *tmr)
+#define IT_ID_SET 1
+#define IT_ID_NOT_SET 0
+static void release_posix_timer(struct k_itimer *tmr, int it_id_set)
{
- if (tmr->it_id != -1) {
+ if (it_id_set) {
unsigned long flags;
spin_lock_irqsave(&idr_lock, flags);
idr_remove(&posix_timers_id, tmr->it_id);
@@ -429,10 +430,11 @@ sys_timer_create(clockid_t which_clock,
{
int error = 0;
struct k_itimer *new_timer = NULL;
- timer_t new_timer_id;
+ int new_timer_id;
struct task_struct *process = 0;
unsigned long flags;
sigevent_t event;
+ int it_id_set = IT_ID_NOT_SET;
if ((unsigned) which_clock >= MAX_CLOCKS ||
!posix_clocks[which_clock].res)
@@ -443,19 +445,38 @@ sys_timer_create(clockid_t which_clock,
return -EAGAIN;
spin_lock_init(&new_timer->it_lock);
- do {
- if (unlikely(!idr_pre_get(&posix_timers_id, GFP_KERNEL))) {
- error = -EAGAIN;
- new_timer->it_id = (timer_t)-1;
- goto out;
- }
- spin_lock_irq(&idr_lock);
- new_timer_id = (timer_t) idr_get_new(&posix_timers_id,
- (void *) new_timer);
- spin_unlock_irq(&idr_lock);
- } while (unlikely(new_timer_id == -1));
+ retry:
+ if (unlikely(!idr_pre_get(&posix_timers_id, GFP_KERNEL))) {
+ error = -EAGAIN;
+ goto out;
+ }
+ spin_lock_irq(&idr_lock);
+ error = idr_get_new(&posix_timers_id,
+ (void *) new_timer,
+ &new_timer_id);
+ spin_unlock_irq(&idr_lock);
+ if (error == -EAGAIN)
+ goto retry;
+ else if (error) {
+ /*
+ * Wierd looking, but we return EAGAIN if the IDR is
+ * full (proper POSIX return value for this)
+ */
+ error = -EAGAIN;
+ goto out;
+ }
+
+ it_id_set = IT_ID_SET;
+ new_timer->it_id = (timer_t) new_timer_id;
+ new_timer->it_clock = which_clock;
+ new_timer->it_incr = 0;
+ new_timer->it_overrun = -1;
+ init_timer(&new_timer->it_timer);
+ new_timer->it_timer.expires = 0;
+ new_timer->it_timer.data = (unsigned long) new_timer;
+ new_timer->it_timer.function = posix_timer_fn;
+ set_timer_inactive(new_timer);
- new_timer->it_id = new_timer_id;
/*
* return the timer_id now. The next step is hard to
* back out if there is an error.
@@ -470,6 +491,10 @@ sys_timer_create(clockid_t which_clock,
error = -EFAULT;
goto out;
}
+ new_timer->it_sigev_notify = event.sigev_notify;
+ new_timer->it_sigev_signo = event.sigev_signo;
+ new_timer->it_sigev_value = event.sigev_value;
+
read_lock(&tasklist_lock);
if ((process = good_sigevent(&event))) {
/*
@@ -489,6 +514,7 @@ sys_timer_create(clockid_t which_clock,
*/
spin_lock_irqsave(&process->sighand->siglock, flags);
if (!(process->flags & PF_EXITING)) {
+ new_timer->it_process = process;
list_add(&new_timer->list,
&process->signal->posix_timers);
spin_unlock_irqrestore(&process->sighand->siglock, flags);
@@ -503,35 +529,27 @@ sys_timer_create(clockid_t which_clock,
error = -EINVAL;
goto out;
}
- new_timer->it_sigev_notify = event.sigev_notify;
- new_timer->it_sigev_signo = event.sigev_signo;
- new_timer->it_sigev_value = event.sigev_value;
} else {
new_timer->it_sigev_notify = SIGEV_SIGNAL;
new_timer->it_sigev_signo = SIGALRM;
new_timer->it_sigev_value.sival_int = new_timer->it_id;
process = current->group_leader;
spin_lock_irqsave(&process->sighand->siglock, flags);
+ new_timer->it_process = process;
list_add(&new_timer->list, &process->signal->posix_timers);
spin_unlock_irqrestore(&process->sighand->siglock, flags);
}
- new_timer->it_clock = which_clock;
- new_timer->it_incr = 0;
- new_timer->it_overrun = -1;
- init_timer(&new_timer->it_timer);
- new_timer->it_timer.expires = 0;
- new_timer->it_timer.data = (unsigned long) new_timer;
- new_timer->it_timer.function = posix_timer_fn;
- set_timer_inactive(new_timer);
-
- /*
- * Once we set the process, it can be found so do it last...
+ /*
+ * In the case of the timer belonging to another task, after
+ * the task is unlocked, the timer is owned by the other task
+ * and may cease to exist at any time. Don't use or modify
+ * new_timer after the unlock call.
*/
- new_timer->it_process = process;
+
out:
if (error)
- release_posix_timer(new_timer);
+ release_posix_timer(new_timer, it_id_set);
return error;
}
@@ -952,7 +970,7 @@ retry_delete:
timer->it_process = NULL;
}
unlock_timer(timer, flags);
- release_posix_timer(timer);
+ release_posix_timer(timer, IT_ID_SET);
return 0;
}
/*
@@ -989,7 +1007,7 @@ retry_delete:
timer->it_process = NULL;
}
unlock_timer(timer, flags);
- release_posix_timer(timer);
+ release_posix_timer(timer, IT_ID_SET);
}
/*
diff -puN lib/idr.c~idr-overflow-fixes lib/idr.c
--- 25/lib/idr.c~idr-overflow-fixes 2004-05-12 21:02:25.975597016 -0700
+++ 25-akpm/lib/idr.c 2004-05-12 21:02:25.985595496 -0700
@@ -70,14 +70,15 @@
* sleep, so must not be called with any spinlocks held. If the system is
* REALLY out of memory this function returns 0, other wise 1.
- * int idr_get_new(struct idr *idp, void *ptr);
+ * int idr_get_new(struct idr *idp, void *ptr, int *id);
* This is the allocate id function. It should be called with any
* required locks. In fact, in the SMP case, you MUST lock prior to
- * calling this function to avoid possible out of memory problems. If
- * memory is required, it will return a -1, in which case you should
- * unlock and go back to the idr_pre_get() call. ptr is the pointer
- * you want associated with the id. In other words:
+ * calling this function to avoid possible out of memory problems.
+ * If memory is required, it will return -EAGAIN, you should unlock
+ * and go back to the idr_pre_get() call. If the idr is full, it
+ * will return a -ENOSPC. ptr is the pointer you want associated
+ * with the id. The value is returned in the "id" field.
* void *idr_find(struct idr *idp, int id);
@@ -92,6 +93,10 @@
* removes the given id, freeing that slot and any memory that may
* now be unused. See idr_find() for locking restrictions.
+ * int idr_full(struct idr *idp);
+
+ * Returns true if the idr is full and false if not.
+
*/
@@ -276,12 +281,30 @@ build_up:
}
EXPORT_SYMBOL(idr_get_new_above);
-int idr_get_new(struct idr *idp, void *ptr)
+static int idr_full(struct idr *idp)
{
- return idr_get_new_above(idp, ptr, 0);
+ return ((idp->layers >= MAX_LEVEL)
+ && (idp->top->bitmap == TOP_LEVEL_FULL));
}
-EXPORT_SYMBOL(idr_get_new);
+int idr_get_new(struct idr *idp, void *ptr, int *id)
+{
+ int rv;
+ rv = idr_get_new_above(idp, ptr, 0);
+ /*
+ * This is a cheap hack until the IDR code can be fixed to
+ * return proper error values.
+ */
+ if (rv == -1) {
+ if (idr_full(idp))
+ return -ENOSPC;
+ else
+ return -EAGAIN;
+ }
+ *id = rv;
+ return 0;
+}
+EXPORT_SYMBOL(idr_get_new);
static void sub_remove(struct idr *idp, int shift, int id)
{
@@ -315,6 +338,9 @@ void idr_remove(struct idr *idp, int id)
{
struct idr_layer *p;
+ /* Mask off upper bits we don't use for the search. */
+ id &= MAX_ID_MASK;
+
sub_remove(idp, (idp->layers - 1) * IDR_BITS, id);
if ( idp->top && idp->top->count == 1 &&
(idp->layers > 1) &&
@@ -350,6 +376,9 @@ void *idr_find(struct idr *idp, int id)
if ( unlikely( (id & ~(~0 << MAX_ID_SHIFT)) >> (n + IDR_BITS)))
return NULL;
#endif
+ /* Mask off upper bits we don't use for the search. */
+ id &= MAX_ID_MASK;
+
while (n > 0 && p) {
n -= IDR_BITS;
p = p->ary[(id >> n) & IDR_MASK];
diff -puN net/sctp/sm_make_chunk.c~idr-overflow-fixes net/sctp/sm_make_chunk.c
--- 25/net/sctp/sm_make_chunk.c~idr-overflow-fixes 2004-05-12 21:02:25.977596712 -0700
+++ 25-akpm/net/sctp/sm_make_chunk.c 2004-05-12 21:02:25.987595192 -0700
@@ -1834,23 +1834,28 @@ int sctp_process_init(struct sctp_associ
/* Allocate storage for the negotiated streams if it is not a temporary * association.
*/
if (!asoc->temp) {
- sctp_assoc_t assoc_id;
+ int assoc_id;
+ int error;
asoc->ssnmap = sctp_ssnmap_new(asoc->c.sinit_max_instreams,
asoc->c.sinit_num_ostreams, gfp);
if (!asoc->ssnmap)
goto clean_up;
- do {
- if (unlikely(!idr_pre_get(&sctp_assocs_id, gfp)))
- goto clean_up;
- spin_lock_bh(&sctp_assocs_id_lock);
- assoc_id = (sctp_assoc_t)idr_get_new(&sctp_assocs_id,
- (void *)asoc);
- spin_unlock_bh(&sctp_assocs_id_lock);
- } while (unlikely((int)assoc_id == -1));
+ retry:
+ if (unlikely(!idr_pre_get(&sctp_assocs_id, gfp)))
+ goto clean_up;
+ spin_lock_bh(&sctp_assocs_id_lock);
+ error = idr_get_new(&sctp_assocs_id,
+ (void *)asoc,
+ &assoc_id);
+ spin_unlock_bh(&sctp_assocs_id_lock);
+ if (error == -EAGAIN)
+ goto retry;
+ else if (error)
+ goto clean_up;
- asoc->assoc_id = assoc_id;
+ asoc->assoc_id = (sctp_assoc_t) assoc_id;
}
/* ADDIP Section 4.1 ASCONF Chunk Procedures
_