Set IPC status code only for RECEIVE

diff --git a/kernel/ipc.h b/kernel/ipc.h
index 743825413c582cdf906032e8d68c6dd2b13510c7..3007a70320b871d47408c470170ad255c89bc1c1 100644 (file)
--- a/kernel/ipc.h
+++ b/kernel/ipc.h
@@ -17,8 +17,28 @@
 /* IPC status code macros. */
 #define IPC_STATUS_REG         bx
 #define IPC_STATUS_GET(p)      ((p)->p_reg.IPC_STATUS_REG)
-#define IPC_STATUS_SET(p, m)   ((p)->p_reg.IPC_STATUS_REG = m)
-#define IPC_STATUS_CLEAR(p)    IPC_STATUS_SET(p, 0)
-#define IPC_STATUS_ADD(p, m)   ((p)->p_reg.IPC_STATUS_REG |= m)
+#define IPC_STATUS_CLEAR(p)    ((p)->p_reg.IPC_STATUS_REG = 0)
+
+/*
+ * XXX: the following check is used to set the status code only on RECEIVE.
+ * SENDREC is not currently atomic for user processes. A process can return
+ * from SENDREC in a different context than the original when a Posix signal
+ * handler gets executed. For this reason, it is not safe to manipulate
+ * the context (i.e. registers) when a process is blocked on a SENDREC.
+ * Unfortunately, avoiding setting the status code for SENDREC doesn't solve
+ * the problem entirely because in rare situations it is still necessary to
+ * override retreg dynamically (and possibly in a different context).
+ * A possible reliable solution is to improve our Posix signal handling
+ * implementation and guarantee SENDREC atomicity w.r.t. the process context.
+ */
+#define IPC_STATUS_ADD(p, m)   do { \
+        if(!((p)->p_misc_flags & MF_REPLY_PEND)) { \
+            (p)->p_reg.IPC_STATUS_REG |= (m); \
+        } \
+    } while(0)
+#define IPC_STATUS_ADD_CALL(p, call) \
+    IPC_STATUS_ADD(p, IPC_STATUS_CALL_TO(call))
+#define IPC_STATUS_ADD_FLAGS(p, flags) \
+    IPC_STATUS_ADD(p, IPC_STATUS_FLAGS(flags))
 
 #endif /* IPC_H */

diff --git a/kernel/proc.c b/kernel/proc.c
index 0053e27744768a60a9ea64f4de14e718612ddfe1..d7df9a8e9b4432d262e49b0f762c3769287730a9 100644 (file)
--- a/kernel/proc.c
+++ b/kernel/proc.c
@@ -339,8 +339,10 @@ PRIVATE int do_sync_ipc(struct proc * caller_ptr, /* who made the call */
                break;                          /* done, or SEND failed */
        /* fall through for SENDREC */
   case RECEIVE:                        
-       if (call_nr == RECEIVE)
+       if (call_nr == RECEIVE) {
                caller_ptr->p_misc_flags &= ~MF_REPLY_PEND;
+               IPC_STATUS_CLEAR(caller_ptr);  /* clear IPC status code */
+       }
        result = mini_receive(caller_ptr, src_dst_e, m_ptr, 0);
        break;
   case NOTIFY:
@@ -397,9 +399,6 @@ PUBLIC int do_ipc(reg_t r1, reg_t r2, reg_t r3)
                caller_ptr->p_name, caller_ptr->p_endpoint);
   }
 
-  /* Clear IPC status code. */
-  IPC_STATUS_CLEAR(caller_ptr);
-
   /* Now check if the call is known and try to perform the request. The only
    * system calls that exist in MINIX are sending and receiving messages.
    *   - SENDREC: combines SEND and RECEIVE in a single system call
@@ -540,8 +539,7 @@ const int flags;
                        return EFAULT;
        } else {
                dst_ptr->p_delivermsg = *m_ptr;
-               IPC_STATUS_ADD(dst_ptr,
-                               IPC_STATUS_FLAGS(IPC_FLG_MSG_FROM_KERNEL));
+               IPC_STATUS_ADD_FLAGS(dst_ptr, IPC_FLG_MSG_FROM_KERNEL);
        }
 
        dst_ptr->p_delivermsg.m_source = caller_ptr->p_endpoint;
@@ -549,7 +547,7 @@ const int flags;
 
        call = (caller_ptr->p_misc_flags & MF_REPLY_PEND ? SENDREC
                : (flags & NON_BLOCKING ? SENDNB : SEND));
-       IPC_STATUS_ADD(dst_ptr, IPC_STATUS_CALL_TO(call));
+       IPC_STATUS_ADD_CALL(dst_ptr, call);
        RTS_UNSET(dst_ptr, RTS_RECEIVING);
   } else {
        if(flags & NON_BLOCKING) {
@@ -665,7 +663,7 @@ const int flags;
            caller_ptr->p_delivermsg.m_source = hisep;
            caller_ptr->p_misc_flags |= MF_DELIVERMSG;
 
-           IPC_STATUS_ADD(caller_ptr, IPC_STATUS_CALL_TO(NOTIFY));
+           IPC_STATUS_ADD_CALL(caller_ptr, NOTIFY);
 
            return(OK);
         }
@@ -680,7 +678,7 @@ const int flags;
                r= try_async(caller_ptr);
 
        if (r == OK) {
-               IPC_STATUS_ADD(caller_ptr, IPC_STATUS_CALL_TO(SENDA));
+               IPC_STATUS_ADD_CALL(caller_ptr, SENDA);
                return OK;      /* Got a message */
        }
     }
@@ -701,15 +699,14 @@ const int flags;
            RTS_UNSET(*xpp, RTS_SENDING);
 
            call = ((*xpp)->p_misc_flags & MF_REPLY_PEND ? SENDREC : SEND);
-           IPC_STATUS_ADD(caller_ptr, IPC_STATUS_CALL_TO(call));
+           IPC_STATUS_ADD_CALL(caller_ptr, call);
 
            /*
             * if the message is originaly from the kernel on behalf of this
             * process, we must send the status flags accordingly
             */
            if ((*xpp)->p_misc_flags & MF_SENDING_FROM_KERNEL) {
-               IPC_STATUS_ADD(caller_ptr,
-                               IPC_STATUS_FLAGS(IPC_FLG_MSG_FROM_KERNEL));
+               IPC_STATUS_ADD_FLAGS(caller_ptr, IPC_FLG_MSG_FROM_KERNEL);
                /* we can clean the flag now, not need anymore */
                (*xpp)->p_misc_flags &= ~MF_SENDING_FROM_KERNEL;
            }
@@ -775,7 +772,7 @@ PUBLIC int mini_notify(
       dst_ptr->p_delivermsg.m_source = caller_ptr->p_endpoint;
       dst_ptr->p_misc_flags |= MF_DELIVERMSG;
 
-      IPC_STATUS_ADD(dst_ptr, IPC_STATUS_CALL_TO(NOTIFY));
+      IPC_STATUS_ADD_CALL(dst_ptr, NOTIFY);
       RTS_UNSET(dst_ptr, RTS_RECEIVING);
 
       return(OK);
@@ -957,8 +954,7 @@ PRIVATE int mini_senda(struct proc *caller_ptr, asynmsg_t *table, size_t size)
                        else {
                                dst_ptr->p_delivermsg.m_source = caller_ptr->p_endpoint;
                                dst_ptr->p_misc_flags |= MF_DELIVERMSG;
-                               IPC_STATUS_ADD(dst_ptr,
-                                       IPC_STATUS_CALL_TO(SENDA));
+                               IPC_STATUS_ADD_CALL(dst_ptr, SENDA);
                                RTS_UNSET(dst_ptr, RTS_RECEIVING);
                                tabent.result = OK;
                        }