Re: [patch] ipc updates

Andi Kleen (ak@muc.de)
07 Nov 1999 18:16:39 +0100


manfreds@colorfullife.com (manfreds) writes:

> my ipc patch is finished, the important changes are:
>
> * sysctl added for all important limits (msg and sem)
> * most implementation limits got removed or vastly increased
> * new ipc helper functions: ipc/sem.c is now 2 lines shorter although new
> features were added :-)
>
> * fixes the DoS attack in ipc/msg found by Scott Maxwell
> * fixes a stupid bug in sem_exit() (AFAICS, it could only be triggered
> by oops'es)
> * compatibility fix: IPC_RM, IPC_STAT, ... failed if nsems<0, but they
> must ignore nsems.
>
> Could you please test it?

As far as I can see it still has the resource race (resources are charged
after sleeping without rechecking, so there is a window where the user is
able to overcommit). Here is for reference my 2.2 patch that fixes the
problem and Scott Maxwell's DoS.

--- linux-2.2.13/include/linux/msg.h.NOMSGFIX Fri Nov 5 10:15:02 1999
+++ linux-2.2.13/include/linux/msg.h Fri Nov 5 12:58:34 1999
@@ -49,6 +49,7 @@
#define MSGMNI 128 /* <= 1K */ /* max # of msg queue identifiers */
#define MSGMAX 4056 /* <= 4056 */ /* max size of message (bytes) */
#define MSGMNB 16384 /* ? */ /* default max size of a message queue */
+#define MSGQNUM 1024 /* <=65535 */ /* Max messages in flight / queue */

/* unused */
#define MSGPOOL (MSGMNI*MSGMNB/1024) /* size in kilobytes of message pool */
--- linux-2.2.13/ipc/msg.c.NOMSGFIX Fri Nov 5 10:02:21 1999
+++ linux-2.2.13/ipc/msg.c Fri Nov 5 12:48:33 1999
@@ -50,6 +50,7 @@
struct ipc_perm *ipcp;
struct msg *msgh;
long mtype;
+ int err;

if (msgsz > MSGMAX || (long) msgsz < 0 || msqid < 0)
return -EINVAL;
@@ -70,34 +71,38 @@
if (ipcperms(ipcp, S_IWUGO))
return -EACCES;

- if (msgsz + msq->msg_cbytes > msq->msg_qbytes) {
- if (msgsz + msq->msg_cbytes > msq->msg_qbytes) {
- /* still no space in queue */
- if (msgflg & IPC_NOWAIT)
- return -EAGAIN;
- if (signal_pending(current))
- return -EINTR;
- interruptible_sleep_on (&msq->wwait);
- goto slept;
- }
+ if (msgsz + msq->msg_cbytes > msq->msg_qbytes || msq->msg_qnum>= MSGQNUM) {
+ /* still no space in queue */
+ if (msgflg & IPC_NOWAIT)
+ return -EAGAIN;
+ if (signal_pending(current))
+ return -EINTR;
+ interruptible_sleep_on (&msq->wwait);
+ goto slept;
}
-
- /* allocate message header and text space*/
+
+ /* Charge first to avoid races */
+ msgbytes += msgsz;
+ msghdrs++;
+ msq->msg_qnum++;
+ msq->msg_cbytes += msgsz;
+
+ /* allocate message header and text space */
+ err = -ENOMEM;
msgh = (struct msg *) kmalloc (sizeof(*msgh) + msgsz, GFP_KERNEL);
- if (!msgh)
- return -ENOMEM;
+ if (!msgh)
+ goto uncharge;
msgh->msg_spot = (char *) (msgh + 1);

- if (copy_from_user(msgh->msg_spot, msgp->mtext, msgsz))
- {
- kfree(msgh);
- return -EFAULT;
+ err = -EFAULT;
+ if (copy_from_user(msgh->msg_spot, msgp->mtext, msgsz)) {
+ goto uncharge;
}

+ err = -EIDRM;
if (msgque[id] == IPC_UNUSED || msgque[id] == IPC_NOID
|| msq->msg_perm.seq != (unsigned int) msqid / MSGMNI) {
- kfree(msgh);
- return -EIDRM;
+ goto uncharge;
}

msgh->msg_next = NULL;
@@ -111,14 +116,19 @@
msq->msg_last->msg_next = msgh;
msq->msg_last = msgh;
}
- msq->msg_cbytes += msgsz;
- msgbytes += msgsz;
- msghdrs++;
- msq->msg_qnum++;
msq->msg_lspid = current->pid;
msq->msg_stime = CURRENT_TIME;
wake_up (&msq->rwait);
return 0;
+
+uncharge:
+ msgbytes -= msgsz;
+ msghdrs--;
+ msq->msg_qnum--;
+ msq->msg_cbytes -= msgsz;
+ if (msgh)
+ kfree(msgh);
+ return err;
}

static int real_msgrcv (int msqid, struct msgbuf *msgp, size_t msgsz, long msgtyp, int msgflg)

-Andi

-- 
This is like TV. I don't like TV.

- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/