Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Do not merge]fs/mqueue/mq_check.c: group level error check for mq operations #6570

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 50 additions & 11 deletions os/fs/mqueue/mq_close.c
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,13 @@
#include <mqueue.h>
#include <assert.h>
#include <debug.h>
#include <stdio.h>
#include <errno.h>

#include <tinyara/kmalloc.h>
#include <tinyara/sched.h>
#include <tinyara/mqueue.h>
#include <tinyara/arch.h>

#include "inode/inode.h"
#include "mqueue/mqueue.h"
Expand All @@ -72,6 +75,52 @@
* Public Functions
****************************************************************************/

/************************************************************************
* Name: mq_desc_in_grouplist
*
* Description:
* This function checks if a message queue descriptor is present in the
* calling task's group list of mq des.
*
* Parameters:
* mqdes - Message queue descriptor
*
* Return Value:
* OK - if mqdes is present in the calling task group list of mqdes
* EBADF - if mqdes is not present in the calling task group of mqdes
*
************************************************************************/

int mq_desc_in_grouplist(mqd_t mqdes)
{
int ret = -EBADF;
mqd_t mqdes_ptr;

/* If we are in irq hanlder, then we must NOT perform this check.
* Because in irq handler, we will not be able to find which task or
* task's group opened the current mqdes */
if (up_interrupt_context()) {
return OK;
}

FAR struct task_group_s *group = sched_self()->group;

DEBUGASSERT(mqdes != NULL && group != NULL);

sched_lock();
mqdes_ptr = (mqd_t)sq_peek(&group->tg_msgdesq);
while (mqdes_ptr) {
if (mqdes_ptr == mqdes) {
ret = OK;
break;
}
mqdes_ptr = (mqd_t)sq_next(mqdes_ptr);
}
sched_unlock();

return ret;
}

/****************************************************************************
* Name: mq_close_group
*
Expand All @@ -96,7 +145,6 @@ int mq_close_group(mqd_t mqdes, FAR struct task_group_s *group)
int ret = OK;
FAR struct mqueue_inode_s *msgq;
FAR struct inode *inode;
mqd_t mqdes_ptr;

DEBUGASSERT(mqdes != NULL && group != NULL);

Expand All @@ -105,17 +153,8 @@ int mq_close_group(mqd_t mqdes, FAR struct task_group_s *group)
if (mqdes) {
sched_lock();

/* Check that mqdes is in one's group */
mqdes_ptr = (mqd_t)sq_peek(&group->tg_msgdesq);
while (mqdes_ptr) {
if (mqdes_ptr == mqdes) {
break;
}
mqdes_ptr = (mqd_t)sq_next(mqdes_ptr);
}

/* If there is no mqdes in one's group, skip to desclose and inode release. */
if (mqdes_ptr != NULL) {
if (mq_desc_in_grouplist(mqdes) == OK) {

/* Find the message queue associated with the message descriptor */

Expand Down
1 change: 1 addition & 0 deletions os/fs/mqueue/mq_open.c
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ mqd_t mq_open(FAR const char *mq_name, int oflags, ...)
errcode = ENOMEM;
goto errout_with_inode;
}
inode->i_crefs++;
} else {
/* The mqueue does not exists. Were we asked to create it? */

Expand Down
5 changes: 5 additions & 0 deletions os/include/mqueue.h
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,11 @@ int mq_setattr(mqd_t mqdes, FAR const struct mq_attr *mq_stat, FAR struct mq_att
* @since TizenRT v1.0
*/
int mq_getattr(mqd_t mqdes, FAR struct mq_attr *mq_stat);
/**
* @brief check mq descriptor in calling task's group's list of mq descriptors
* @details @b #include <mqueue.h> \n
*/
int mq_desc_in_grouplist(mqd_t mqdes);

#undef EXTERN
#ifdef __cplusplus
Expand Down
6 changes: 6 additions & 0 deletions os/kernel/mqueue/mq_getattr.c
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
#include <tinyara/config.h>

#include <mqueue.h>
#include <errno.h>
#include <tinyara/mqueue.h>

/************************************************************************
Expand Down Expand Up @@ -105,6 +106,11 @@ int mq_getattr(mqd_t mqdes, struct mq_attr *mq_stat)
{
int ret = ERROR;

if (mq_desc_in_grouplist(mqdes) != OK) {
set_errno(EBADF);
return ERROR;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to set the errno before we return.

}

if (mqdes && mq_stat) {
/* Return the attributes */

Expand Down
5 changes: 5 additions & 0 deletions os/kernel/mqueue/mq_notify.c
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,11 @@ int mq_notify(mqd_t mqdes, const struct sigevent *notification)
return ERROR;
}

if (mq_desc_in_grouplist(mqdes) != OK) {
set_errno(EBADF);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should use the return value instead of hard coding here.

return ERROR;
}

/* Get a pointer to the message queue */

msgq = mqdes->msgq;
Expand Down
7 changes: 7 additions & 0 deletions os/kernel/mqueue/mq_receive.c
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@
* message queue.
* EINTR The call was interrupted by a signal handler.
* EINVAL Invalid 'msg' or 'mqdes'
* EBADF Mqdes is not present in calling task group's mq list.
*
* Assumptions:
*
Expand All @@ -145,6 +146,12 @@ ssize_t mq_receive(mqd_t mqdes, FAR char *msg, size_t msglen, FAR int *prio)
/* mq_receive() is a cancellation point */
(void)enter_cancellation_point();

if (mq_desc_in_grouplist(mqdes) != OK) {
leave_cancellation_point();
set_errno(EBADF);
return ERROR;
}

if (mq_verifyreceive(mqdes, msg, msglen) != OK) {
leave_cancellation_point();
return ERROR;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before ERROR return, we should set proper errno.

 * Return Value:
 *   One success, the length of the selected message in bytes is returned.
 *   On failure, -1 (ERROR) is returned and the errno is set appropriately:
 *
 *   EAGAIN   The queue was empty, and the O_NONBLOCK flag was set
 *            for the message queue description referred to by 'mqdes'.
 *   EPERM    Message queue opened not opened for reading.
 *   EMSGSIZE 'msglen' was less than the maxmsgsize attribute of the
 *            message queue.
 *   EINTR    The call was interrupted by a signal handler.
 *   EINVAL   Invalid 'msg' or 'mqdes'

Expand Down
7 changes: 7 additions & 0 deletions os/kernel/mqueue/mq_send.c
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@
* EAGAIN The queue was empty, and the O_NONBLOCK flag was set for the
* message queue description referred to by mqdes.
* EINVAL Either msg or mqdes is NULL or the value of prio is invalid.
* EBADF Mqdes is not present in calling task group's mq list.
* EPERM Message queue opened not opened for writing.
* EMSGSIZE 'msglen' was greater than the maxmsgsize attribute of the
* message queue.
Expand All @@ -149,6 +150,12 @@ int mq_send(mqd_t mqdes, FAR const char *msg, size_t msglen, int prio)
/* mq_send() is a cancellation point */
(void)enter_cancellation_point();

if (mq_desc_in_grouplist(mqdes) != OK) {
leave_cancellation_point();
set_errno(EBADF);
return ERROR;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check needs to be done in all mq apis. Please check timedsend and timed receive.

if (mq_verifysend(mqdes, msg, msglen, prio) != OK) {
leave_cancellation_point();
return ERROR;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as previous

Expand Down
6 changes: 6 additions & 0 deletions os/kernel/mqueue/mq_setattr.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@

#include <fcntl.h> /* O_NONBLOCK */
#include <mqueue.h>
#include <errno.h>

#include <tinyara/mqueue.h>

Expand Down Expand Up @@ -114,6 +115,11 @@ int mq_setattr(mqd_t mqdes, const struct mq_attr *mq_stat, struct mq_attr *oldst
{
int ret = ERROR;

if (mq_desc_in_grouplist(mqdes) != OK) {
set_errno(EBADF);
return ERROR;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set errno is missnig

}

if (mqdes && mq_stat) {
/* Return the attributes if so requested */

Expand Down
7 changes: 7 additions & 0 deletions os/kernel/mqueue/mq_timedreceive.c
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ static void mq_rcvtimeout(int argc, uint32_t pid)
* message queue.
* EINTR The call was interrupted by a signal handler.
* EINVAL Invalid 'msg' or 'mqdes' or 'abstime'
* EBADF Mqdes is not present in calling task group's mq list.
* ETIMEDOUT The call timed out before a message could be transferred.
* ENOMEM The system lacks sufficient memory resources for watchdog.
*
Expand All @@ -209,6 +210,12 @@ ssize_t mq_timedreceive(mqd_t mqdes, FAR char *msg, size_t msglen, FAR int *prio
/* mq_timedreceive() is not a cancellation point */
(void)enter_cancellation_point();

if (mq_desc_in_grouplist(mqdes) != OK) {
leave_cancellation_point();
set_errno(EBADF);
return ERROR;
}

/* Verify the input parameters and, in case of an error, set
* errno appropriately.
*/
Expand Down
7 changes: 7 additions & 0 deletions os/kernel/mqueue/mq_timedsend.c
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ static void mq_sndtimeout(int argc, uint32_t pid)
* EAGAIN The queue was empty, and the O_NONBLOCK flag was set for the
* message queue description referred to by mqdes.
* EINVAL Either msg or mqdes is NULL or the value of prio is invalid.
* EBADF Mqdes is not present in calling task group's mq list.
* EPERM Message queue opened not opened for writing.
* EMSGSIZE 'msglen' was greater than the maxmsgsize attribute of the
* message queue.
Expand All @@ -212,6 +213,12 @@ int mq_timedsend(mqd_t mqdes, FAR const char *msg, size_t msglen, int prio, FAR
/* mq_timedsend() is a cancellation point */
(void)enter_cancellation_point();

if (mq_desc_in_grouplist(mqdes) != OK) {
leave_cancellation_point();
set_errno(EBADF);
return ERROR;
}

/* Verify the input parameters -- setting errno appropriately
* on any failures to verify.
*/
Expand Down