Re: [PATCH v3 1/5]IPC: Added New system call do_mq_timedreceive2() for non-destructive peek on posix mqueue
Andrei Vagin <[email protected]> Mon, 13 Apr 2026 17:52:52 -0700
| Newsgroups | dev.linux.lists.criu |
|---|---|
| Message-ID | <CANaxB-xAD48tyLuJ_cOchJCE8aifjF5kusSZ-jt3HP3k5uvt4g@mail.gmail.com> |
On Mon, Apr 13, 2026 at 5:04=E2=80=AFPM Mathura <[email protected]= > wrote: > > Hi, Andrei > Thanks for review, > > I have taken two lock, This is simple thought I had- > > In first attempt, I want to have a look into queue to see, is even > data exist to peek (because I preferred to allocate memory after > confirmation) then I taken second lock after allocating temporary > buffer to ensure, does even at this point already seen msg exist, > intact and till we finish to copy to user_space, we are not going to > allow other system call to consume same msg or alter anything. > If I do not take a second lock, What if someone else in between tries > to consume the same msg ? > > One more approach- pre-allocate memory before confirming data, then > take one lock during copy to temporary buffer only. I think this is what you need to do. > > How ensure atomicity and existence of data during the period of copy > if we do not take lock ? I don't suggest copying data without holding a lock... > > Because copy_msg(msg_ptr, k_msg_buffer, k_m_ts) is doing a temporary > copy to the kernel buffer only while holding a second lock, the final > copy to use_space is lock free. > > Please leave more insight on this. > > Mathura, > > On Mon, 13 Apr 2026 at 22:04, Andrei Vagin <[email protected]> wrote: > > > > Hi Mathura_Kumar, > > > > First of all, please read > > https://www.kernel.org/doc/html/v4.10/process/submitting-patches.html. > > > > You need to write a detailed commit message for each patch. > > > > Before sending a patch, it is recommended to run checkpatch: > > $ ./scripts/checkpatch.pl `git format-patch HEAD~1` > > WARNING: Missing commit description - Add an appropriate one > > > > WARNING: line length of 107 exceeds 100 columns > > #38: FILE: include/linux/compat.h:805: > > +asmlinkage long compat_sys_mq_timedreceive2(mqd_t mqdes, struct > > compat_mq_timedreceive2_args __user *uargs, > > > > WARNING: line length of 128 exceeds 100 columns > > #39: FILE: include/linux/compat.h:806: > > + unsigned int flags, unsigned long index, > > > > WARNING: line length of 131 exceeds 100 columns > > #40: FILE: include/linux/compat.h:807: > > + struct old_timespec32 __user *abs_timeout); > > > > WARNING: Use #include <linux/compat.h> instead of <asm/compat.h> > > #104: FILE: include/uapi/linux/mqueue.h:22: > > +#include <asm/compat.h> > > > > WARNING: line length of 122 exceeds 100 columns > > #306: FILE: ipc/mqueue.c:1636: > > + struct compat_mq_timedreceive2_args __user *uargs) > > > > total: 0 errors, 6 warnings, 408 lines checked > > > > > > On Wed, Apr 8, 2026 at 2:53=E2=80=AFAM Mathura_Kumar <academic1mathura@= gmail.com> wrote: > > > > > > Signed-off-by: Mathura_Kumar <[email protected]> > > > --- > > > include/linux/compat.h | 6 +- > > > include/linux/syscalls.h | 6 + > > > include/uapi/asm-generic/unistd.h | 7 +- > > > include/uapi/linux/mqueue.h | 14 ++- > > > ipc/mqueue.c | 186 ++++++++++++++++++++++++++++= -- > > > ipc/msg.c | 2 +- > > > ipc/msgutil.c | 48 ++++---- > > > ipc/util.h | 3 +- > > > kernel/sys_ni.c | 1 + > > > 9 files changed, 231 insertions(+), 42 deletions(-) > > > > > > diff --git a/include/linux/compat.h b/include/linux/compat.h > > > index 56cebaff0c91..9f5ca26e76d8 100644 > > > --- a/include/linux/compat.h > > > +++ b/include/linux/compat.h > > > @@ -22,6 +22,7 @@ > > > #include <asm/compat.h> > > > #include <asm/siginfo.h> > > > #include <asm/signal.h> > > > +#include <linux/mqueue.h> > > > > > > #ifdef CONFIG_ARCH_HAS_SYSCALL_WRAPPER > > > /* > > > @@ -801,8 +802,9 @@ asmlinkage long compat_sys_pwritev64v2(unsigned l= ong fd, > > > const struct iovec __user *vec, > > > unsigned long vlen, loff_t pos, rwf_t flags); > > > #endif > > > - > > > - > > > +asmlinkage long compat_sys_mq_timedreceive2(mqd_t mqdes, struct comp= at_mq_timedreceive2_args __user *uargs, > > > + = unsigned int flags, unsigned long index, > > > + = struct old_timespec32 __user *abs_timeout); > > > /* > > > * Deprecated system calls which are still defined in > > > * include/uapi/asm-generic/unistd.h and wanted by >=3D 1 arch > > > diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h > > > index 02bd6ddb6278..993e570c90ab 100644 > > > --- a/include/linux/syscalls.h > > > +++ b/include/linux/syscalls.h > > > @@ -79,6 +79,7 @@ struct mnt_id_req; > > > struct ns_id_req; > > > struct xattr_args; > > > struct file_attr; > > > +struct mq_timedreceive2_args; > > > > > > #include <linux/types.h> > > > #include <linux/aio_abi.h> > > > @@ -93,6 +94,7 @@ struct file_attr; > > > #include <linux/key.h> > > > #include <linux/personality.h> > > > #include <trace/syscall.h> > > > +#include <linux/mqueue.h> > > > > > > #ifdef CONFIG_ARCH_HAS_SYSCALL_WRAPPER > > > /* > > > @@ -746,6 +748,10 @@ asmlinkage long sys_mq_timedsend_time32(mqd_t mq= des, > > > const char __user *u_msg_ptr, > > > unsigned int msg_len, unsigned int msg_prio, > > > const struct old_timespec32 __user *u_abs_tim= eout); > > > +asmlinkage long > > > +sys_mq_timedreceive2(mqd_t mqdes, struct mq_timedreceive2_args __use= r *uargs, > > > + unsigned int flags, unsigned long index, > > > + struct __kernel_timespec __user *abs_timeout); > > > asmlinkage long sys_msgget(key_t key, int msgflg); > > > asmlinkage long sys_old_msgctl(int msqid, int cmd, struct msqid_ds _= _user *buf); > > > asmlinkage long sys_msgctl(int msqid, int cmd, struct msqid_ds __use= r *buf); > > > diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-gen= eric/unistd.h > > > index a627acc8fb5f..200ee7fde5c4 100644 > > > --- a/include/uapi/asm-generic/unistd.h > > > +++ b/include/uapi/asm-generic/unistd.h > > > @@ -863,9 +863,12 @@ __SYSCALL(__NR_listns, sys_listns) > > > #define __NR_rseq_slice_yield 471 > > > __SYSCALL(__NR_rseq_slice_yield, sys_rseq_slice_yield) > > > > > > -#undef __NR_syscalls > > > -#define __NR_syscalls 472 > > > +#define __NR_mq_timedreceive2 472 > > > +__SC_COMP(__NR_mq_timedreceive2, sys_mq_timedreceive2, > > > + compat_sys_mq_timedreceive2) > > > > > > +#undef __NR_syscalls > > > +#define __NR_syscalls 473 > > > /* > > > * 32 bit systems traditionally used different > > > * syscalls for off_t and loff_t arguments, while > > > diff --git a/include/uapi/linux/mqueue.h b/include/uapi/linux/mqueue.= h > > > index b516b66840ad..7cdced63f5d2 100644 > > > --- a/include/uapi/linux/mqueue.h > > > +++ b/include/uapi/linux/mqueue.h > > > @@ -18,8 +18,8 @@ > > > > > > #ifndef _LINUX_MQUEUE_H > > > #define _LINUX_MQUEUE_H > > > - > > > #include <linux/types.h> > > > +#include <asm/compat.h> > > > > > > #define MQ_PRIO_MAX 32768 > > > /* per-uid limit of kernel memory used by mqueue, in bytes */ > > > @@ -33,6 +33,18 @@ struct mq_attr { > > > __kernel_long_t __reserved[4]; /* ignored for input, zeroed = for output */ > > > }; > > > > > > +struct mq_timedreceive2_args { > > > + size_t msg_len; > > > + unsigned int *msg_prio; > > > + char *msg_ptr; > > > +}; > > > + > > > +struct compat_mq_timedreceive2_args { > > > + compat_size_t msg_len; > > > + compat_uptr_t msg_prio; > > > + compat_uptr_t msg_ptr; > > > +}; > > > + > > > /* > > > * SIGEV_THREAD implementation: > > > * SIGEV_THREAD must be implemented in user space. If SIGEV_THREAD i= s passed > > > diff --git a/ipc/mqueue.c b/ipc/mqueue.c > > > index 4798b375972b..78dc414967a2 100644 > > > --- a/ipc/mqueue.c > > > +++ b/ipc/mqueue.c > > > @@ -53,6 +53,7 @@ struct mqueue_fs_context { > > > > > > #define SEND 0 > > > #define RECV 1 > > > +#define MQ_PEEK 2 > > > > > > #define STATE_NONE 0 > > > #define STATE_READY 1 > > > @@ -1230,6 +1231,115 @@ static int do_mq_timedreceive(mqd_t mqdes, ch= ar __user *u_msg_ptr, > > > return ret; > > > } > > > > > > +static struct msg_msg *mq_peek_index(struct mqueue_inode_info *info,= int index) > > > +{ > > > + struct rb_node *node; > > > + struct posix_msg_tree_node *leaf; > > > + struct msg_msg *msg; > > > + > > > + int count =3D 0; > > > + > > > + /* Start from highest priority */ > > > + node =3D rb_last(&info->msg_tree); > > > + while (node) { > > > + leaf =3D rb_entry(node, struct posix_msg_tree_node, r= b_node); > > > + list_for_each_entry(msg, &leaf->msg_list, m_list) { > > > + if (count =3D=3D index) > > > + return msg; > > > + count++; > > > + } > > > + > > > + node =3D rb_prev(node); > > > + } > > > + > > > + return NULL; > > > +} > > > + > > > +static int do_mq_timedreceive2(mqd_t mqdes, struct mq_timedreceive2_= args *args, > > > + unsigned int flags, unsigned long inde= x, > > > + struct timespec64 *ts) > > > +{ > > > + ssize_t ret; > > > + struct msg_msg *msg_ptr, *k_msg_buffer; > > > + long k_m_type; > > > + size_t k_m_ts; > > > + struct inode *inode; > > > + struct mqueue_inode_info *info; > > > + > > > + if (!(flags & MQ_PEEK)) { > > > + return do_mq_timedreceive(mqdes, args->msg_ptr, args-= >msg_len, > > > + args->msg_prio, ts); > > > + } > > > + audit_mq_sendrecv(mqdes, args->msg_len, 0, ts); > > > + CLASS(fd, f)(mqdes); > > > + if (fd_empty(f)) > > > + return -EBADF; > > > + > > > + inode =3D file_inode(fd_file(f)); > > > + if (unlikely(fd_file(f)->f_op !=3D &mqueue_file_operations)) > > > + return -EBADF; > > > + info =3D MQUEUE_I(inode); > > > + audit_file(fd_file(f)); > > > + > > > + if (unlikely(!(fd_file(f)->f_mode & FMODE_READ))) > > > + return -EBADF; > > > + > > > + if (unlikely(args->msg_len < info->attr.mq_msgsize)) > > > + return -EMSGSIZE; > > > + if (index >=3D (unsigned long)info->attr.mq_maxmsg) > > > + return -ENOENT; > > > + > > > + spin_lock(&info->lock); > > > + if (info->attr.mq_curmsgs =3D=3D 0) { > > > + spin_unlock(&info->lock); > > > + return -EAGAIN; > > > > should it be ENOENT? > > > > > + } > > > + msg_ptr =3D mq_peek_index(info, index); > > > + if (!msg_ptr) { > > > + spin_unlock(&info->lock); > > > + return -ENOENT; > > > + } > > > + k_m_type =3D msg_ptr->m_type; > > > + k_m_ts =3D msg_ptr->m_ts; > > > + spin_unlock(&info->lock); > > > + > > > + k_msg_buffer =3D alloc_msg(k_m_ts); > > > + if (!k_msg_buffer) > > > + return -ENOMEM; > > > + > > > + /* > > > + * Two spin locks are necessary here. We are avoiding atomic = memory > > > + * allocation and premature allocation before confirming that > > > + * a message actually exists to peek. > > > + */ > > > > MSG_COPY doesn't require to lock the spinlock twice, so why can't we > > do the same thing here? > > > > > + spin_lock(&info->lock); > > > + msg_ptr =3D mq_peek_index(info, index); > > > + if (!msg_ptr || msg_ptr->m_type !=3D k_m_type || > > > + msg_ptr->m_ts !=3D k_m_ts) { > > > + spin_unlock(&info->lock); > > > + free_msg(k_msg_buffer); > > > + return -EAGAIN; > > > + } > > > + if (IS_ERR(copy_msg(msg_ptr, k_msg_buffer, k_m_ts))) { > > > + spin_unlock(&info->lock); > > > + free_msg(k_msg_buffer); > > > + return -EINVAL; > > > > you probably need to return the code returned by copy_msg... > > > > > + } > > > + spin_unlock(&info->lock); > > > + > > > + ret =3D k_msg_buffer->m_ts; > > > + if (args->msg_prio && put_user(k_m_type, args->msg_prio)) { > > > + free_msg(k_msg_buffer); > > > + return -EFAULT; > > > + } > > > + if (store_msg(args->msg_ptr, k_msg_buffer, k_m_ts)) { > > > + free_msg(k_msg_buffer); > > > + return -EFAULT; > > > + } > > > + free_msg(k_msg_buffer); > > > + return ret; > > > +} > > > + > > > SYSCALL_DEFINE5(mq_timedsend, mqd_t, mqdes, const char __user *, u_m= sg_ptr, > > > size_t, msg_len, unsigned int, msg_prio, > > > const struct __kernel_timespec __user *, u_abs_timeou= t) > > > @@ -1258,6 +1368,27 @@ SYSCALL_DEFINE5(mq_timedreceive, mqd_t, mqdes,= char __user *, u_msg_ptr, > > > return do_mq_timedreceive(mqdes, u_msg_ptr, msg_len, u_msg_pr= io, p); > > > } > > > > > > +SYSCALL_DEFINE5(mq_timedreceive2, mqd_t, mqdes, > > > + struct mq_timedreceive2_args __user *, uargs, unsigne= d int, > > > + flags, const unsigned long, index, > > > + const struct __kernel_timespec __user *, u_abs_timeou= t) > > > +{ > > > + struct mq_timedreceive2_args args; > > > + struct timespec64 ts, *p =3D NULL; > > > + > > > + if (copy_from_user(&args, uargs, sizeof(args))) > > > + return -EFAULT; > > > + > > > + if (u_abs_timeout) { > > > + int res =3D prepare_timeout(u_abs_timeout, &ts); > > > + > > > + if (res) > > > + return res; > > > + p =3D &ts; > > > + } > > > + return do_mq_timedreceive2(mqdes, &args, flags, index, p); > > > +} > > > + > > > /* > > > * Notes: the case when user wants us to deregister (with NULL as po= inter) > > > * and he isn't currently owner of notification, will be silently di= scarded. > > > @@ -1449,6 +1580,17 @@ SYSCALL_DEFINE3(mq_getsetattr, mqd_t, mqdes, > > > return 0; > > > } > > > > > > +#ifdef CONFIG_COMPAT_32BIT_TIME > > > +static int compat_prepare_timeout(const struct old_timespec32 __user= *p, > > > + struc= t timespec64 *ts) > > > +{ > > > + if (get_old_timespec32(ts, p)) > > > + return -EFAULT; > > > + if (!timespec64_valid(ts)) > > > + return -EINVAL; > > > + return 0; > > > +} > > > + > > > #ifdef CONFIG_COMPAT > > > > > > struct compat_mq_attr { > > > @@ -1490,6 +1632,22 @@ static inline int put_compat_mq_attr(const str= uct mq_attr *attr, > > > return 0; > > > } > > > > > > +static inline int get_compat_mq_args(struct mq_timedreceive2_args *a= rgs, > > > + = struct compat_mq_timedreceive2_args __user *uargs) > > > +{ > > > + struct compat_mq_timedreceive2_args v; > > > + > > > + if (copy_from_user(&v, uargs, sizeof(v))) > > > + return -EFAULT; > > > + > > > + memset(args, 0, sizeof(*args)); > > > + args->msg_len =3D (size_t)v.msg_len; > > > + args->msg_prio =3D (unsigned int *)compat_ptr(v.msg_prio); > > > + args->msg_ptr =3D (char *)compat_ptr(v.msg_ptr); > > > + > > > + return 0; > > > +} > > > + > > > COMPAT_SYSCALL_DEFINE4(mq_open, const char __user *, u_name, > > > int, oflag, compat_mode_t, mode, > > > struct compat_mq_attr __user *, u_attr) > > > @@ -1541,19 +1699,30 @@ COMPAT_SYSCALL_DEFINE3(mq_getsetattr, mqd_t, = mqdes, > > > return -EFAULT; > > > return 0; > > > } > > > -#endif > > > > > > -#ifdef CONFIG_COMPAT_32BIT_TIME > > > -static int compat_prepare_timeout(const struct old_timespec32 __user= *p, > > > - struct timespec64 *ts) > > > +COMPAT_SYSCALL_DEFINE5(mq_timedreceive2, mqd_t, mqdes, > > > + struct compat_mq_timedreceive2_args __user *, = uargs, > > > + unsigned int, flags, const unsigned long, inde= x, > > > + const struct old_timespec32 __user *, u_abs_ti= meout) > > > { > > > - if (get_old_timespec32(ts, p)) > > > + struct mq_timedreceive2_args args; > > > + struct timespec64 ts, *p =3D NULL; > > > + > > > + if (get_compat_mq_args(&args, uargs)) > > > return -EFAULT; > > > - if (!timespec64_valid(ts)) > > > - return -EINVAL; > > > - return 0; > > > + > > > + if (u_abs_timeout) { > > > + int res =3D compat_prepare_timeout(u_abs_timeout, &ts= ); > > > + > > > + if (res) > > > + return res; > > > + p =3D &ts; > > > + } > > > + return do_mq_timedreceive2(mqdes, &args, flags, index, p); > > > } > > > > > > +#endif > > > + > > > SYSCALL_DEFINE5(mq_timedsend_time32, mqd_t, mqdes, > > > const char __user *, u_msg_ptr, > > > unsigned int, msg_len, unsigned int, msg_prio, > > > @@ -1583,6 +1752,7 @@ SYSCALL_DEFINE5(mq_timedreceive_time32, mqd_t, = mqdes, > > > } > > > return do_mq_timedreceive(mqdes, u_msg_ptr, msg_len, u_msg_pr= io, p); > > > } > > > + > > > #endif > > > > > > static const struct inode_operations mqueue_dir_inode_operations =3D= { > > > diff --git a/ipc/msg.c b/ipc/msg.c > > > index 62996b97f0ac..6392b11dd7f7 100644 > > > --- a/ipc/msg.c > > > +++ b/ipc/msg.c > > > @@ -1156,7 +1156,7 @@ static long do_msgrcv(int msqid, void __user *b= uf, size_t bufsz, long msgtyp, in > > > * not update queue parameters. > > > */ > > > if (msgflg & MSG_COPY) { > > > - msg =3D copy_msg(msg, copy); > > > + msg =3D copy_msg(msg, copy, msg->m_ts= ); > > > goto out_unlock0; > > > } > > > > > > diff --git a/ipc/msgutil.c b/ipc/msgutil.c > > > index e28f0cecb2ec..c5536ce47cc2 100644 > > > --- a/ipc/msgutil.c > > > +++ b/ipc/msgutil.c > > > @@ -51,7 +51,7 @@ static int __init init_msg_buckets(void) > > > } > > > subsys_initcall(init_msg_buckets); > > > > > > -static struct msg_msg *alloc_msg(size_t len) > > > +struct msg_msg *alloc_msg(size_t len) > > > { > > > struct msg_msg *msg; > > > struct msg_msgseg **pseg; > > > @@ -122,39 +122,33 @@ struct msg_msg *load_msg(const void __user *src= , size_t len) > > > free_msg(msg); > > > return ERR_PTR(err); > > > } > > > -#ifdef CONFIG_CHECKPOINT_RESTORE > > > -struct msg_msg *copy_msg(struct msg_msg *src, struct msg_msg *dst) > > > + > > > +struct msg_msg *copy_msg(struct msg_msg *src, struct msg_msg *dst, s= ize_t len) > > > { > > > - struct msg_msgseg *dst_pseg, *src_pseg; > > > - size_t len =3D src->m_ts; > > > - size_t alen; > > > + struct msg_msgseg *src_seg, *dst_seg; > > > + size_t remaining, chunk; > > > > > > - if (src->m_ts > dst->m_ts) > > > + if (len > src->m_ts) > > > return ERR_PTR(-EINVAL); > > > - > > > - alen =3D min(len, DATALEN_MSG); > > > - memcpy(dst + 1, src + 1, alen); > > > - > > > - for (dst_pseg =3D dst->next, src_pseg =3D src->next; > > > - src_pseg !=3D NULL; > > > - dst_pseg =3D dst_pseg->next, src_pseg =3D src_pseg->next= ) { > > > - > > > - len -=3D alen; > > > - alen =3D min(len, DATALEN_SEG); > > > - memcpy(dst_pseg + 1, src_pseg + 1, alen); > > > + chunk =3D min(len, DATALEN_MSG); > > > + memcpy(dst + 1, src + 1, chunk); > > > + remaining =3D len - chunk; > > > + src_seg =3D src->next; > > > + dst_seg =3D dst->next; > > > + while (remaining > 0 && src_seg && dst_seg) { > > > + chunk =3D min(remaining, DATALEN_SEG); > > > + memcpy(dst_seg + 1, src_seg + 1, chunk); > > > + remaining -=3D chunk; > > > + src_seg =3D src_seg->next; > > > + dst_seg =3D dst_seg->next; > > > } > > > - > > > + if (remaining !=3D 0) > > > + return ERR_PTR(-EINVAL); > > > dst->m_type =3D src->m_type; > > > - dst->m_ts =3D src->m_ts; > > > - > > > + dst->m_ts =3D src->m_ts; > > > return dst; > > > } > > > -#else > > > -struct msg_msg *copy_msg(struct msg_msg *src, struct msg_msg *dst) > > > -{ > > > - return ERR_PTR(-ENOSYS); > > > -} > > > -#endif > > > + > > > int store_msg(void __user *dest, struct msg_msg *msg, size_t len) > > > { > > > size_t alen; > > > diff --git a/ipc/util.h b/ipc/util.h > > > index a55d6cebe6d3..374abeee79b3 100644 > > > --- a/ipc/util.h > > > +++ b/ipc/util.h > > > @@ -197,8 +197,9 @@ int ipc_parse_version(int *cmd); > > > > > > extern void free_msg(struct msg_msg *msg); > > > extern struct msg_msg *load_msg(const void __user *src, size_t len); > > > -extern struct msg_msg *copy_msg(struct msg_msg *src, struct msg_msg = *dst); > > > +extern struct msg_msg *copy_msg(struct msg_msg *src, struct msg_msg = *dst, size_t len); > > > extern int store_msg(void __user *dest, struct msg_msg *msg, size_t = len); > > > +extern struct msg_msg *alloc_msg(size_t len); > > > > > > static inline int ipc_checkid(struct kern_ipc_perm *ipcp, int id) > > > { > > > diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c > > > index add3032da16f..658d6b8274b3 100644 > > > --- a/kernel/sys_ni.c > > > +++ b/kernel/sys_ni.c > > > @@ -391,6 +391,7 @@ COND_SYSCALL(setuid16); > > > /* restartable sequence */ > > > COND_SYSCALL(rseq); > > > COND_SYSCALL(rseq_slice_yield); > > > +COND_SYSCALL(mq_timedreceive2); > > > > > > COND_SYSCALL(uretprobe); > > > COND_SYSCALL(uprobe); > > > -- > > > 2.43.0 > > >