Hi list,
It seems that revision 1.22 of sys/kern/sysv_msg.c broke the message queues
implementation. The most visible effect of this is msgget(2) always returning
0 on success. You will find below a program which gives the following evidence:
- msgget(IPC_PRIVATE, ...) always returns 0 (see ipc_private()).
- msgget(ftok(...) ...) always returns 0 (see ftok_creat(); note that the
function depends on the existence of the file /tmp/msgQ).
- When a message queue is created and its identifier is retrieved in the same
way as ipcs(1) (that is, using sysctl(3)), the use of the said identifier as the
msgid argument to msgrcv(2) results in a failure (non existing message queue).
As mentionned, I adapted code from ipcs(1) to get the message queue identifiers.
Please forgive the roughness of the code.
My understanding of the code in sys/kern/sysv_msg.c leads me to the following
explanation:
Revision 1.22 of sysv_msg.c introduces a new structure: ``struct que''. There
is one ``struct que'' for each message queue and all of the ``struct que'' are
gathered in a TAILQ. The function that allocates and initializes a
`struct que'' is que_create(key_t, struct ucred, int). Allocation uses
malloc(..., M_ZERO) and the field ``que_id'' of the struct que is not
initialized later on. Therefore, all que_id fields of all struct que have
value 0.
Now, functions that operate on existing message queues, given an identifier, use
que_lookup(int id) to get a pointer to the corresponding struct que. que_lookup
tests que->que_id == id. In case ``id'' is what msgget(2) returned to the
calling
application (id == 0, then), this selects the first message queue in the list.
>From the user's point of view, message queues work until she/he tries to use
several of them and notices that all her/his messages go through the same queue.
On the other hand, ipcs(1) uses IXSEQ_TO_IPCID(ix, perm), which is defined in
sys/sys/ipc.h, to compute the identifier. ``ix'' is the index of the message
queue in the array of descriptors sysctl(3) returns for
mib = {CTL_KERN, KERN_SYSVIPC_INFO, KERN_SYSVIPC_MSG_INFO} and ``perm'' is used
for its ``seq'' field which is the value of the global variable ``sequence''
(defined in sysv_msg.c) which is incremented by one each time a new message
queue is created. From IXSEQ_TO_IPCID()'s point of view, the identifier is
never 0 (sequence starts at 1, see msginit()), which explains the third example
I gave at the beginning.
I'd happily have attached a patch but I believe the case requires more insight
than I have on the matter: calculating identifier with IXSEQ_TO_IPCID() requires
knowledge on the value of ix, which does not seem that straightforward using
TAILQ_INSERT_TAIL().
While here, two remarks on related code:
- ``sequence'' is pre-incremented on line 406 of sysv_msg.c and its 16 lower
bits are assigned to ``[..].msg_perm.seq''. Instead of letting ``sequence''
growing arbitrarily, wouldn't it make more sense to do the following:
sequence = (sequence + 1) & 0x7fff;
que->msqid_ds.msg_perm.seq = sequence;
The chance that the current behaviour eventually causes a signed integer
overflow (which behaviour is undefined) is nearly nil, but it does not hurt to
have correct code.
- The comment starting on line 103 of sys/msg.h says that msginit checks things
about ``msginfo.msgssz'', which is wrong nowadays.
I hope someone will have time to look into this.
Alexis
#include <sys/types.h>
#include <sys/param.h>
#include <sys/sysctl.h>
#define _KERNEL
#include <sys/ipc.h>
#include <sys/msg.h>
#undef _KERNEL
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
/* Not declared in sys/ipc.h and sys/msg.h since they were included with _KERNEL
defined. */
key_t ftok(const char *, int);
int msgget(key_t, int);
int msgctl(int, int, struct msqid_ds *);
int msgrcv(int, void *, size_t, long, int);
/* -1: no queue, -2: more than one queue, -3: error */
int get_msgQId(void) {
int ret = -1;
size_t len;
int mib[3] = {CTL_KERN, KERN_SYSVIPC_INFO, KERN_SYSVIPC_MSG_INFO};
int i;
struct msg_sysctl_info *m;
if(sysctl(mib, 3, NULL, &len, NULL, 0) == -1) {
printf("sysctl(3) failed. %s\n", strerror(errno));
return -3;
}
m = (struct msg_sysctl_info *)malloc(len);
if(m == NULL) {
printf("malloc(3) failed. %s\n", strerror(errno));
return -3;
}
if(sysctl(mib, 3, m, &len, NULL, 0) == -1) {
printf("sysctl(3) failed. %s\n", strerror(errno));
free(m);
return -3;
}
for(i = 0; i < m->msginfo.msgmni; i++) {
struct msqid_ds *ptr = &m->msgids[i];
if(ptr->msg_qbytes != 0) {
if(ret == -1)
ret = IXSEQ_TO_IPCID(i, ptr->msg_perm);
else
return -2;
}
}
return ret;
}
void msgrcv_fail(void) {
int id, ret, msgQ;
struct mymsg m;
switch(get_msgQId()) {
case -2:
fputs("There is already more than one message queue,
stopping.\n", stdout);
return;
case -3:
return;
case -1:
break;
default:
fputs("There is already one message queue, stopping.\n",
stdout);
return;
}
msgQ = msgget(IPC_PRIVATE, IPC_R | IPC_W);
if(msgQ == -1) {
printf("msgget(2) failed for msgQ. %s\n", strerror(errno));
return;
} else if(msgQ != 0) {
printf("msgget(2) returned 0x%x > 0\n", msgQ);
}
id = get_msgQId();
if(get_msgQId() < 0) {
fputs("Something went wrong.\n", stdout);
if(msgctl(msgQ, IPC_RMID, NULL) == -1)
printf("msgctl(2) failed for msgQ. %s\n",
strerror(errno));
return;
}
ret = msgrcv(id, &m, sizeof m, 0, IPC_NOWAIT);
if(ret == -1)
printf("msgrcv(2) failed. %s\n", strerror(errno));
if(msgctl(msgQ, IPC_RMID, NULL) == -1)
printf("msgctl(2) failed for msgQ. %s\n", strerror(errno));
}
void msgQ_list(void) {
size_t len;
int mib[3] = {CTL_KERN, KERN_SYSVIPC_INFO, KERN_SYSVIPC_MSG_INFO};
int i;
struct msg_sysctl_info *m;
if(sysctl(mib, 3, NULL, &len, NULL, 0) == -1) {
printf("sysctl(3) failed. %s\n", strerror(errno));
return;
}
m = (struct msg_sysctl_info *)malloc(len);
if(m == NULL) {
printf("malloc(3) failed. %s\n", strerror(errno));
return;
}
if(sysctl(mib, 3, m, &len, NULL, 0) == -1) {
printf("sysctl(3) failed. %s\n", strerror(errno));
free(m);
return;
}
fputs("Active messages queues:", stdout);
for(i = 0; i < m->msginfo.msgmni; i++) {
struct msqid_ds *ptr = &m->msgids[i];
if(ptr->msg_qbytes != 0)
printf(" 0x%x", IXSEQ_TO_IPCID(i, ptr->msg_perm));
}
fputs("\n", stdout);
free(m);
}
void ipc_private(void) {
int msgQ, msgQ1;
fputs("ipc_private()\n", stdout);
msgQ = msgget(IPC_PRIVATE, IPC_R | IPC_W);
if(msgQ == -1)
printf("msgget(2) failed for msgQ. %s\n", strerror(errno));
printf("msgget(2) returned %i for msgQ.\n", msgQ);
msgQ_list();
msgQ1 = msgget(IPC_PRIVATE, IPC_R | IPC_W);
if(msgQ1 == -1)
printf("msgget(2) failed for msgQ1. %s\n", strerror(errno));
printf("msgget(2) returned %i for msgQ1.\n", msgQ1);
msgQ_list();
if(msgQ != -1)
if(msgctl(msgQ, IPC_RMID, NULL) == -1)
printf("msgctl(2) failed for msgQ. %s\n",
strerror(errno));
if(msgQ != -1)
if(msgctl(msgQ1, IPC_RMID, NULL) == -1)
printf("msgctl(2) failed for msgQ1. %s\n",
strerror(errno));
}
void ftok_creat(void) {
int msgQ;
key_t key;
const char *path = "/tmp/msgQ";
fputs("ftok_creat()\n", stdout);
key = ftok(path, 'a');
if(key == (key_t)-1) {
fputs("ftok(3) failed.\n", stdout);
return;
}
msgQ = msgget(key, IPC_R | IPC_W | IPC_CREAT);
if(msgQ == -1) {
printf("msgget(2) failed for msgQ. %s\n", strerror(errno));
return;
}
printf("msgget(2) returned %i for msgQ.\n", msgQ);
msgQ_list();
if(msgctl(msgQ, IPC_RMID, NULL) == -1)
printf("msgctl(2) failed for msgQ. %s\n", strerror(errno));
}
int main(void) {
msgQ_list();
fputs("Starting.\n", stdout);
ipc_private();
ftok_creat();
msgrcv_fail();
fputs("Finished.\n", stdout);
msgQ_list();
return 0;
}
