From owner-p4-projects@FreeBSD.ORG Wed Nov 15 20:48:54 2006 Return-Path: X-Original-To: p4-projects@freebsd.org Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id CC01916A47C; Wed, 15 Nov 2006 20:48:54 +0000 (UTC) X-Original-To: perforce@freebsd.org Delivered-To: perforce@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id A6D1816A40F for ; Wed, 15 Nov 2006 20:48:54 +0000 (UTC) (envelope-from jkim@freebsd.org) Received: from repoman.freebsd.org (repoman.freebsd.org [216.136.204.115]) by mx1.FreeBSD.org (Postfix) with ESMTP id 3AD6643E9E for ; Wed, 15 Nov 2006 20:46:56 +0000 (GMT) (envelope-from jkim@freebsd.org) Received: from repoman.freebsd.org (localhost [127.0.0.1]) by repoman.freebsd.org (8.13.6/8.13.6) with ESMTP id kAFKkkwM066955 for ; Wed, 15 Nov 2006 20:46:46 GMT (envelope-from jkim@freebsd.org) Received: (from perforce@localhost) by repoman.freebsd.org (8.13.6/8.13.4/Submit) id kAFKkkad066952 for perforce@freebsd.org; Wed, 15 Nov 2006 20:46:46 GMT (envelope-from jkim@freebsd.org) Date: Wed, 15 Nov 2006 20:46:46 GMT Message-Id: <200611152046.kAFKkkad066952@repoman.freebsd.org> X-Authentication-Warning: repoman.freebsd.org: perforce set sender to jkim@freebsd.org using -f From: Jung-uk Kim To: Perforce Change Reviews Cc: Subject: PERFORCE change 110058 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Nov 2006 20:48:55 -0000 http://perforce.freebsd.org/chv.cgi?CH=110058 Change 110058 by jkim@jkim_hammer on 2006/11/15 20:46:21 Redo 109706 and 109712. Copyin/copyout for message type is separated from kern_msgsnd() and kern_msgrcv() and it is done from its rwapper. It is still not optimal but better than the previous implementation. Fix printf formats for debugging while I am here. Add msgsz boundary check as Linux kernel does but MSGMAX check is not done because of implementation difference. This fixes LTP test case msgrcv03. Note: LTP does not need user intervention any more! Affected files ... .. //depot/projects/linuxolator/src/sys/compat/freebsd32/freebsd32_misc.c#6 edit .. //depot/projects/linuxolator/src/sys/compat/linux/linux_ipc.c#4 edit .. //depot/projects/linuxolator/src/sys/kern/sysv_msg.c#6 edit .. //depot/projects/linuxolator/src/sys/sys/syscallsubr.h#2 edit Differences ... ==== //depot/projects/linuxolator/src/sys/compat/freebsd32/freebsd32_misc.c#6 (text+ko) ==== @@ -91,10 +91,6 @@ #include #include -/* XXX This should not be here. */ -int do_msgsnd(struct thread *, struct msgsnd_args *, size_t); -int do_msgrcv(struct thread *, struct msgrcv_args *, size_t); - CTASSERT(sizeof(struct timeval32) == 8); CTASSERT(sizeof(struct timespec32) == 8); CTASSERT(sizeof(struct statfs32) == 256); @@ -1360,34 +1356,41 @@ int freebsd32_msgsnd(struct thread *td, struct freebsd32_msgsnd_args *uap) { - struct msgsnd_args ap; + const void *msgp; + long mtype; + int32_t mtype32; + int error; if (!SYSCALL_MODULE_PRESENT(msgsnd)) return (nosys(td, (struct nosys_args *)uap)); - ap.msqid = uap->msqid; - ap.msgp = PTRIN(uap->msgp); - ap.msgsz = uap->msgsz; - ap.msgflg = uap->msgflg; - - return (do_msgsnd(td, &ap, sizeof(int32_t))); + msgp = PTRIN(uap->msgp); + if ((error = copyin(msgp, &mtype32, sizeof(mtype32))) != 0) + return (error); + mtype = mtype32; + return (kern_msgsnd(td, uap->msqid, + (const char *)msgp + sizeof(mtype32), + uap->msgsz, uap->msgflg, mtype)); } int freebsd32_msgrcv(struct thread *td, struct freebsd32_msgrcv_args *uap) { - struct msgrcv_args ap; + void *msgp; + long mtype; + int32_t mtype32; + int error; if (!SYSCALL_MODULE_PRESENT(msgrcv)) return (nosys(td, (struct nosys_args *)uap)); - ap.msqid = uap->msqid; - ap.msgp = PTRIN(uap->msgp); - ap.msgsz = uap->msgsz; - ap.msgtyp = uap->msgtyp; - ap.msgflg = uap->msgflg; - - return (do_msgrcv(td, &ap, sizeof(int32_t))); + msgp = PTRIN(uap->msgp); + if ((error = kern_msgrcv(td, uap->msqid, + (char *)msgp + sizeof(mtype32), uap->msgsz, + uap->msgtyp, uap->msgflg, &mtype)) != 0) + return (error); + mtype32 = (int32_t)mtype; + return (copyout(&mtype32, msgp, sizeof(mtype32))); } int ==== //depot/projects/linuxolator/src/sys/compat/linux/linux_ipc.c#4 (text+ko) ==== @@ -83,10 +83,6 @@ l_ulong swap_successes; }; -/* XXX This should not be here. */ -int do_msgsnd(struct thread *, struct msgsnd_args *, size_t); -int do_msgrcv(struct thread *, struct msgrcv_args *, size_t); - static void bsd_to_linux_shminfo( struct shminfo *bpp, struct l_shminfo *lpp) { @@ -579,39 +575,39 @@ int linux_msgsnd(struct thread *td, struct linux_msgsnd_args *args) { - struct msgsnd_args /* { - int msqid; - void *msgp; - size_t msgsz; - int msgflg; - } */ bsd_args; + const void *msgp; + long mtype; + l_long lmtype; + int error; - bsd_args.msqid = args->msqid; - bsd_args.msgp = PTRIN(args->msgp); - bsd_args.msgsz = args->msgsz; - bsd_args.msgflg = args->msgflg; - - return (do_msgsnd(td, &bsd_args, sizeof(l_long))); + if ((l_long)args->msgsz < 0) + return (EINVAL); + msgp = PTRIN(args->msgp); + if ((error = copyin(msgp, &lmtype, sizeof(lmtype))) != 0) + return (error); + mtype = (long)lmtype; + return (kern_msgsnd(td, args->msqid, + (const char *)msgp + sizeof(lmtype), + args->msgsz, args->msgflg, mtype)); } int linux_msgrcv(struct thread *td, struct linux_msgrcv_args *args) { - struct msgrcv_args /* { - int msqid; - void *msgp; - size_t msgsz; - long msgtyp; - int msgflg; - } */ bsd_args; + void *msgp; + long mtype; + l_long lmtype; + int error; - bsd_args.msqid = args->msqid; - bsd_args.msgp = PTRIN(args->msgp); - bsd_args.msgsz = args->msgsz; - bsd_args.msgtyp = args->msgtyp; - bsd_args.msgflg = args->msgflg; - - return (do_msgrcv(td, &bsd_args, sizeof(l_long))); + if ((l_long)args->msgsz < 0) + return (EINVAL); + msgp = PTRIN(args->msgp); + if ((error = kern_msgrcv(td, args->msqid, + (char *)msgp + sizeof(lmtype), args->msgsz, + args->msgtyp, args->msgflg, &mtype)) != 0) + return (error); + lmtype = (l_long)mtype; + return (copyout(&lmtype, msgp, sizeof(lmtype))); } int ==== //depot/projects/linuxolator/src/sys/kern/sysv_msg.c#6 (text+ko) ==== @@ -396,7 +396,7 @@ struct msqid_ds msqbuf; int error; - DPRINTF(("call to msgctl(%d, %d, 0x%x)\n", msqid, cmd, uap->buf)); + DPRINTF(("call to msgctl(%d, %d, %p)\n", msqid, cmd, uap->buf)); if (cmd == IPC_SET && (error = copyin(uap->buf, &msqbuf, sizeof(msqbuf))) != 0) return (error); @@ -662,11 +662,6 @@ return (error); } -union msgtyp { - long mtype; - int32_t mtype32; -}; - #ifndef _SYS_SYSPROTO_H_ struct msgsnd_args { int msqid; @@ -676,50 +671,40 @@ }; #endif -/* XXX This should not be here. */ -int do_msgsnd(struct thread *, struct msgsnd_args *, size_t); - int -do_msgsnd(td, uap, mtsz) +kern_msgsnd(td, msqid, msgp, msgsz, msgflg, mtype) struct thread *td; - register struct msgsnd_args *uap; - size_t mtsz; + int msqid; + const void *msgp; /* XXX msgp is actually mtext. */ + size_t msgsz; + int msgflg; + long mtype; { - union msgtyp msgt; - int msqid = uap->msqid; - const void *user_msgp = uap->msgp; - size_t msgsz = uap->msgsz; - int msgflg = uap->msgflg; - int segs_needed, error = 0; + int msqix, segs_needed, error = 0; register struct msqid_kernel *msqkptr; register struct msg *msghdr; short next; - DPRINTF(("call to msgsnd(%d, 0x%x, %d, %d)\n", msqid, user_msgp, msgsz, - msgflg)); if (!jail_sysvipc_allowed && jailed(td->td_ucred)) return (ENOSYS); - if (mtsz != sizeof(msgt.mtype) && mtsz != sizeof(msgt.mtype32)) - return (EINVAL); - mtx_lock(&msq_mtx); - msqid = IPCID_TO_IX(msqid); + msqix = IPCID_TO_IX(msqid); - if (msqid < 0 || msqid >= msginfo.msgmni) { - DPRINTF(("msqid (%d) out of range (0<=msqid<%d)\n", msqid, + if (msqix < 0 || msqix >= msginfo.msgmni) { + DPRINTF(("msqid (%d) out of range (0<=msqid<%d)\n", msqix, msginfo.msgmni)); error = EINVAL; goto done2; } - msqkptr = &msqids[msqid]; + msqkptr = &msqids[msqix]; if (msqkptr->u.msg_qbytes == 0) { DPRINTF(("no such message queue id\n")); error = EINVAL; goto done2; } - if (msqkptr->u.msg_perm.seq != IPCID_TO_SEQ(uap->msqid)) { + if (msqkptr->u.msg_perm.seq != IPCID_TO_SEQ(msqid)) { DPRINTF(("wrong sequence number\n")); error = EINVAL; goto done2; @@ -737,8 +722,8 @@ #endif segs_needed = (msgsz + msginfo.msgssz - 1) / msginfo.msgssz; - DPRINTF(("msgsz=%d, msgssz=%d, segs_needed=%d\n", msgsz, msginfo.msgssz, - segs_needed)); + DPRINTF(("msgsz=%zu, msgssz=%d, segs_needed=%d\n", msgsz, + msginfo.msgssz, segs_needed)); for (;;) { int need_more_resources = 0; @@ -849,6 +834,7 @@ free_msghdrs = msghdr->msg_next; msghdr->msg_spot = -1; msghdr->msg_ts = msgsz; + msghdr->msg_type = mtype; #ifdef MAC /* * XXXMAC: Should the mac_check_sysv_msgmsq check follow here @@ -881,23 +867,6 @@ } /* - * Copy in the message type - */ - - mtx_unlock(&msq_mtx); - if ((error = copyin(user_msgp, &msgt, mtsz)) != 0) { - mtx_lock(&msq_mtx); - DPRINTF(("error %d copying the message type\n", error)); - msg_freehdr(msghdr); - msqkptr->u.msg_perm.mode &= ~MSG_LOCKED; - wakeup(msqkptr); - goto done2; - } - msghdr->msg_type = (mtsz == sizeof(long)) ? msgt.mtype : msgt.mtype32; - mtx_lock(&msq_mtx); - user_msgp = (const char *)user_msgp + mtsz; - - /* * Validate the message type */ @@ -905,7 +874,7 @@ msg_freehdr(msghdr); msqkptr->u.msg_perm.mode &= ~MSG_LOCKED; wakeup(msqkptr); - DPRINTF(("mtype (%d) < 1\n", msghdr->msg_type)); + DPRINTF(("mtype (%ld) < 1\n", msghdr->msg_type)); error = EINVAL; goto done2; } @@ -926,7 +895,7 @@ if (next >= msginfo.msgseg) panic("next out of range #2"); mtx_unlock(&msq_mtx); - if ((error = copyin(user_msgp, &msgpool[next * msginfo.msgssz], + if ((error = copyin(msgp, &msgpool[next * msginfo.msgssz], tlen)) != 0) { mtx_lock(&msq_mtx); DPRINTF(("error %d copying in message segment\n", @@ -938,7 +907,7 @@ } mtx_lock(&msq_mtx); msgsz -= tlen; - user_msgp = (const char *)user_msgp + tlen; + msgp = (const char *)msgp + tlen; next = msgmaps[next].next; } if (next != -1) @@ -1013,7 +982,19 @@ struct thread *td; register struct msgsnd_args *uap; { - return (do_msgsnd(td, uap, sizeof(long))); + int error; + long mtype; + + DPRINTF(("call to msgsnd(%d, %p, %zu, %d)\n", uap->msqid, uap->msgp, + uap->msgsz, uap->msgflg)); + + if ((error = copyin(uap->msgp, &mtype, sizeof(mtype))) != 0) { + DPRINTF(("error %d copying the message type\n", error)); + return (error); + } + return (kern_msgsnd(td, uap->msqid, + (const char *)uap->msgp + sizeof(mtype), + uap->msgsz, uap->msgflg, mtype)); } #ifndef _SYS_SYSPROTO_H_ @@ -1026,52 +1007,41 @@ }; #endif -/* XXX This should not be here. */ -int do_msgrcv(struct thread *, struct msgrcv_args *, size_t); - int -do_msgrcv(td, uap, mtsz) +kern_msgrcv(td, msqid, msgp, msgsz, msgtyp, msgflg, mtype) struct thread *td; - register struct msgrcv_args *uap; - size_t mtsz; + int msqid; + void *msgp; /* XXX msgp is actually mtext. */ + size_t msgsz; + long msgtyp; + int msgflg; + long *mtype; { - union msgtyp msgt; - int msqid = uap->msqid; - void *user_msgp = uap->msgp; - size_t msgsz = uap->msgsz; - long msgtyp = uap->msgtyp; - int msgflg = uap->msgflg; size_t len; register struct msqid_kernel *msqkptr; register struct msg *msghdr; - int error = 0; + int msqix, error = 0; short next; - DPRINTF(("call to msgrcv(%d, 0x%x, %d, %ld, %d)\n", msqid, user_msgp, - msgsz, msgtyp, msgflg)); - if (!jail_sysvipc_allowed && jailed(td->td_ucred)) return (ENOSYS); - if (mtsz != sizeof(msgt.mtype) && mtsz != sizeof(msgt.mtype32)) - return (EINVAL); + msqix = IPCID_TO_IX(msqid); - msqid = IPCID_TO_IX(msqid); - - if (msqid < 0 || msqid >= msginfo.msgmni) { - DPRINTF(("msqid (%d) out of range (0<=msqid<%d)\n", msqid, + if (msqix < 0 || msqix >= msginfo.msgmni) { + DPRINTF(("msqid (%d) out of range (0<=msqid<%d)\n", msqix, msginfo.msgmni)); return (EINVAL); } - msqkptr = &msqids[msqid]; + msqkptr = &msqids[msqix]; mtx_lock(&msq_mtx); if (msqkptr->u.msg_qbytes == 0) { DPRINTF(("no such message queue id\n")); error = EINVAL; goto done2; } - if (msqkptr->u.msg_perm.seq != IPCID_TO_SEQ(uap->msqid)) { + if (msqkptr->u.msg_perm.seq != IPCID_TO_SEQ(msqid)) { DPRINTF(("wrong sequence number\n")); error = EINVAL; goto done2; @@ -1096,7 +1066,7 @@ if (msgsz < msghdr->msg_ts && (msgflg & MSG_NOERROR) == 0) { DPRINTF(("first message on the queue " - "is too big (want %d, got %d)\n", + "is too big (want %zu, got %d)\n", msgsz, msghdr->msg_ts)); error = E2BIG; goto done2; @@ -1134,14 +1104,14 @@ if (msgtyp == msghdr->msg_type || msghdr->msg_type <= -msgtyp) { - DPRINTF(("found message type %d, " - "requested %d\n", + DPRINTF(("found message type %ld, " + "requested %ld\n", msghdr->msg_type, msgtyp)); if (msgsz < msghdr->msg_ts && (msgflg & MSG_NOERROR) == 0) { DPRINTF(("requested message " "on the queue is too big " - "(want %d, got %d)\n", + "(want %zu, got %hu)\n", msgsz, msghdr->msg_ts)); error = E2BIG; goto done2; @@ -1191,7 +1161,7 @@ */ if ((msgflg & IPC_NOWAIT) != 0) { - DPRINTF(("no appropriate message found (msgtyp=%d)\n", + DPRINTF(("no appropriate message found (msgtyp=%ld)\n", msgtyp)); /* The SVID says to return ENOMSG. */ error = ENOMSG; @@ -1218,7 +1188,7 @@ */ if (msqkptr->u.msg_qbytes == 0 || - msqkptr->u.msg_perm.seq != IPCID_TO_SEQ(uap->msqid)) { + msqkptr->u.msg_perm.seq != IPCID_TO_SEQ(msqid)) { DPRINTF(("msqid deleted\n")); error = EIDRM; goto done2; @@ -1242,31 +1212,13 @@ * (since msgsz is never increased). */ - DPRINTF(("found a message, msgsz=%d, msg_ts=%d\n", msgsz, + DPRINTF(("found a message, msgsz=%zu, msg_ts=%hu\n", msgsz, msghdr->msg_ts)); if (msgsz > msghdr->msg_ts) msgsz = msghdr->msg_ts; + *mtype = msghdr->msg_type; /* - * Return the type to the user. - */ - - mtx_unlock(&msq_mtx); - if (mtsz == sizeof(long)) - msgt.mtype = msghdr->msg_type; - else - msgt.mtype32 = msghdr->msg_type; - error = copyout(&msgt, user_msgp, mtsz); - mtx_lock(&msq_mtx); - if (error != 0) { - DPRINTF(("error (%d) copying out message type\n", error)); - msg_freehdr(msghdr); - wakeup(msqkptr); - goto done2; - } - user_msgp = (char *)user_msgp + mtsz; - - /* * Return the segments to the user */ @@ -1283,8 +1235,7 @@ if (next >= msginfo.msgseg) panic("next out of range #3"); mtx_unlock(&msq_mtx); - error = copyout(&msgpool[next * msginfo.msgssz], - user_msgp, tlen); + error = copyout(&msgpool[next * msginfo.msgssz], msgp, tlen); mtx_lock(&msq_mtx); if (error != 0) { DPRINTF(("error (%d) copying out message segment\n", @@ -1293,7 +1244,7 @@ wakeup(msqkptr); goto done2; } - user_msgp = (char *)user_msgp + tlen; + msgp = (char *)msgp + tlen; next = msgmaps[next].next; } @@ -1317,7 +1268,19 @@ struct thread *td; register struct msgrcv_args *uap; { - return (do_msgrcv(td, uap, sizeof(long))); + int error; + long mtype; + + DPRINTF(("call to msgrcv(%d, %p, %zu, %ld, %d)\n", uap->msqid, + uap->msgp, uap->msgsz, uap->msgtyp, uap->msgflg)); + + if ((error = kern_msgrcv(td, uap->msqid, + (char *)uap->msgp + sizeof(mtype), uap->msgsz, + uap->msgtyp, uap->msgflg, &mtype)) != 0) + return (error); + if ((error = copyout(&mtype, uap->msgp, sizeof(mtype))) != 0) + DPRINTF(("error %d copying the message type\n", error)); + return (error); } static int ==== //depot/projects/linuxolator/src/sys/sys/syscallsubr.h#2 (text+ko) ==== @@ -114,6 +114,8 @@ int kern_mknod(struct thread *td, char *path, enum uio_seg pathseg, int mode, int dev); int kern_msgctl(struct thread *, int, int, struct msqid_ds *); +int kern_msgsnd(struct thread *, int, const void *, size_t, int, long); +int kern_msgrcv(struct thread *, int, void *, size_t, long, int, long *); int kern_nanosleep(struct thread *td, struct timespec *rqt, struct timespec *rmt); int kern_open(struct thread *td, char *path, enum uio_seg pathseg,