Messages in this thread | | | Date | Thu, 6 May 2004 09:32:22 -0300 | From | Marcelo Tosatti <> | Subject | Re: [PATCH] per-user signal pending and message queue limits |
| |
> This BUG() is too easy to trigger, e.g. user creates mqueue, logs out, > root comes by later and cleans up...BUG(). Simply caching user directly > eliminates this altogether.
And with user_struct->__count you deal with that, yes?
Caching the user_struct directly is indeed much nicer.
> > Some inconsistent use of p and current below. > > > struct msg_msg **msgs = NULL; > > struct mq_attr attr; > > int ret; > > @@ -553,15 +578,26 @@ > > attr.mq_msgsize > msgsize_max) > > return ERR_PTR(-EINVAL); > > } > > + if(p->user->msg_queues+ ((attr.mq_maxmsg * sizeof(struct msg_msg *) > > + + (attr.mq_maxmsg * attr.mq_msgsize))) > > + >= p->rlim[RLIMIT_MSGQUEUE].rlim_cur) > > Hrm, this thing can overflow. Seems like the hard maxes should be > smaller. As it stands, looks like the hard max mq_msgsize that root > could setup is INT_MAX.
Eek. Just decreasing max mq_msgsize to something _much_ smaller is ok, isnt it?
Like, say, 64MB.
> > + return ERR_PTR(-ENOMEM); > > + > > msgs = kmalloc(attr.mq_maxmsg * sizeof(*msgs), GFP_KERNEL); > > if (!msgs) > > return ERR_PTR(-ENOMEM); > > + > > + spin_lock(&mq_lock); > > + current->user->msg_queues += (attr.mq_maxmsg * sizeof(*msgs) + > > + (attr.mq_maxmsg * attr.mq_msgsize)); > > + spin_unlock(&mq_lock); > > This path means the user is penalized for the mq_attr sized accounting, > plus the default sized accounting which happens later in mqueue_get_inode(). > It is removed below, but as mentioned above, this could incorrectly > cause mq_open() to fail.
OK!
> > } else { > > msgs = NULL; > > } > > > > ret = vfs_create(dir->d_inode, dentry, mode, NULL); > > if (ret) { > > + /* kfree(msgs): msgs can be NULL -mt */ > > kfree(msgs); > > return ERR_PTR(ret); > > } > > @@ -572,8 +608,17 @@ > > if (msgs) { > > info->attr.mq_maxmsg = attr.mq_maxmsg; > > info->attr.mq_msgsize = attr.mq_msgsize; > > + spin_lock(&mq_lock); > > + current->user->msg_queues -= (info->attr.mq_maxmsg > > + * sizeof (struct msg_msg *) + > > + (info->attr.mq_maxmsg * > > + info->attr.mq_msgsize)); > > + if (current->user->msg_queues < 0) > > + current->user->msg_queues = 0; > > Oops, I think the subtraction is slightly wrong here. Should be before > the info->attr is updated, else you are actually carrying accounting for > the default size (minus the actually allocated size). Should be > subtracting off the recently added default size. > > New patch below (based on 2.6.6-rc3-bk). Couple known issues are the > possible mq_bytes caluclation overflow (not yet fixed in this patch), > and setuid issue on signal side. All other known issues have been > addressed.
The setuid issue on signal side is pretty harmless though. It only affects the users which are setuid() capable, and which do so with signal pending.
I see no other way to fix it than cache the user struct in "struct sigqueue". Thats not good news for performance (not sure how bad it would matter).
Again, thanks! - 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/
| |