Messages in this thread Patch in this message | | | Date | Wed, 28 Apr 2004 14:03:16 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH] per-user signal pending and message queue limits |
| |
Marcelo Tosatti <marcelo.tosatti@cyclades.com> wrote: > > static void mqueue_delete_inode(struct inode *inode) > { > struct mqueue_inode_info *info; > + struct user_struct *user; > int i; > > if (S_ISDIR(inode->i_mode)) { > clear_inode(inode); > return; > } > + > info = MQUEUE_I(inode); > + > + user = find_user(info->creator_id); > + if (!user) > + BUG(); > spin_lock(&info->lock);
hmm, look at that. find_user() forgot to take any locks. Maybe it's relying on tasklist_lock? I think we need the below patch. Ingo, can you please confirm?
Also, you'll need to do a free_uid() in here - find_user() takes a ref.
Also, I'm not sure that it's legit to go BUG if the user wasn't found. Is it not possible that the user has gone away and it is root who is cleaning up the inode?
Finally, my head is gently rotating wrt this patch. Could you please maintain a description of what it does? We've made several significant design decisions in here and that info really should be captured. The relationship between the global and per-user limits, the sizing choices for the per-user limits, etc. If it can be captured in brief code comments, that's best. Otherwise for the changelog.
Thanks.
---
25-akpm/kernel/sys.c | 4 ++++ 25-akpm/kernel/user.c | 13 ++++++++++++- 2 files changed, 16 insertions(+), 1 deletion(-)
diff -puN kernel/user.c~find_user-locking kernel/user.c --- 25/kernel/user.c~find_user-locking Wed Apr 28 13:56:52 2004 +++ 25-akpm/kernel/user.c Wed Apr 28 13:58:28 2004 @@ -64,9 +64,20 @@ static inline struct user_struct *uid_ha return NULL; } +/* + * Locate the user_struct for the passed UID. If found, take a ref on it. The + * caller must undo that ref with free_uid(). + * + * If the user_struct could not be found, return NULL. + */ struct user_struct *find_user(uid_t uid) { - return uid_hash_find(uid, uidhashentry(uid)); + struct user_struct *ret; + + spin_lock(&uidhash_lock); + ret = uid_hash_find(uid, uidhashentry(uid)); + spin_unlock(&uidhash_lock); + return ret; } void free_uid(struct user_struct *up) diff -puN kernel/sys.c~find_user-locking kernel/sys.c --- 25/kernel/sys.c~find_user-locking Wed Apr 28 13:58:42 2004 +++ 25-akpm/kernel/sys.c Wed Apr 28 13:59:54 2004 @@ -348,6 +348,8 @@ asmlinkage long sys_setpriority(int whic if (p->uid == who) error = set_one_prio(p, niceval, error); while_each_thread(g, p); + if (who) + free_uid(user); /* For find_user() */ break; } out_unlock: @@ -410,6 +412,8 @@ asmlinkage long sys_getpriority(int whic retval = niceval; } while_each_thread(g, p); + if (who) + free_uid(user); /* for find_user() */ break; } out_unlock: _
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |