cc: some libc folks
On Mon, Oct 12, 2020 at 6:45 AM Jessica Clarke <jrtc27@xxxxxxxxxx> wrote:
>
> POSIX specifies that the first field of the supplied msgp, namely mtype,
> is a long, not a __kernel_long_t, and it's a user-defined struct due to
> the variable-length mtext field so we can't even bend the spec and make
> it a __kernel_long_t even if we wanted to. Thus we must use the compat
> syscalls on x32 to avoid buffer overreads and overflows in msgsnd and
> msgrcv respectively.
This is a mess.
include/uapi/linux/msg.h has:
/* message buffer for msgsnd and msgrcv calls */
struct msgbuf {
__kernel_long_t mtype; /* type of message */
char mtext[1]; /* message text */
};
Your test has:
struct msg_long {
long mtype;
char mtext[8];
};
struct msg_long_ext {
struct msg_long msg_long;
char mext[4];
};
and I'm unclear as to exactly what you're trying to do there with the
"mext" part.
POSIX says:
The application shall ensure that the argument msgp points to a user-
defined buffer that contains first a field of type long specifying the
type of the message, and then a data portion that holds the data bytes
of the message. The structure below is an example of what this user-de‐
fined buffer might look like:
struct mymsg {
long mtype; /* Message type. */
char mtext[1]; /* Message text. */
}
NTP has this delightful piece of code:
44 typedef union {
45 struct msgbuf msgp;
46 struct {
47 long mtype;
48 int code;
49 struct timeval tv;
50 } msgb;
51 } MsgBuf;
bluefish has:
struct small_msgbuf {
long mtype;
char mtext[MSQ_QUEUE_SMALL_SIZE];
} small_msgp;
My laptop has nothing at all in /dev/mqueue.
So I don't really know what the right thing to do is. Certainly if
we're going to apply this patch, we should also fix the header. I
almost think we should *delete* struct msgbuf from the headers, since
it's all kinds of busted, but that will break the NTP build. Ideally
we would go back in time and remove it from the headers.
Libc people, any insight? We can probably fix the bug without
annoying anyone given how lightly x32 is used and how lightly POSIX
message queues are used.
--Andy
>
> Due to erroneously including the first 4 bytes of mtext in the mtype
> this would previously also cause non-zero msgtyp arguments for msgrcv to
> search for the wrong messages, and if sharing message queues between x32
> and non-x32 (i386 or x86_64) processes this would previously cause mtext
> to "move" and, depending on the direction and ABI combination, lose the
> first 4 bytes.
>
> Signed-off-by: Jessica Clarke <jrtc27@xxxxxxxxxx>
> ---
>
> I have verified that the test at the end of [1] now gives the correct
> result on x32 ("PAYL" not "PAY" as I erroneously claimed it should be in
> the above email) and that both i386 and amd64 give the same output with
> that test as before.
>
> [1] <1156938F-A9A3-4EE9-B059-2294A0B9FBFE@xxxxxxxxxx>
>
> Changes since v1:
> * Uses the same syscall numbers for x32 as amd64 and the current x32
> rather than (further) breaking ABI by allocating new ones from the
> legacy x32 range
>
> arch/x86/entry/syscalls/syscall_64.tbl | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
> index f30d6ae9a..f462123f3 100644
> --- a/arch/x86/entry/syscalls/syscall_64.tbl
> +++ b/arch/x86/entry/syscalls/syscall_64.tbl
> @@ -77,8 +77,10 @@
> 66 common semctl sys_semctl
> 67 common shmdt sys_shmdt
> 68 common msgget sys_msgget
> -69 common msgsnd sys_msgsnd
> -70 common msgrcv sys_msgrcv
> +69 64 msgsnd sys_msgsnd
> +69 x32 msgsnd compat_sys_msgsnd
> +70 64 msgrcv sys_msgrcv
> +70 x32 msgrcv compat_sys_msgrcv
> 71 common msgctl sys_msgctl
> 72 common fcntl sys_fcntl
> 73 common flock sys_flock
> --
> 2.28.0
>
