lkml.org 
[lkml]   [2012]   [May]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [Patch 3/4] ipc/mqueue: strengthen checks on mqueue creation
    On 05/01/2012 04:01 PM, KOSAKI Motohiro wrote:
    > (5/1/12 1:50 PM), Doug Ledford wrote:
    >> We already check the mq attr struct if it's passed in, but now that the
    >> admin can set system wide defaults separate from maximums, it's actually
    >> possible to set the defaults to something that would overflow. So,
    >> if there is no attr struct passed in to the open call, check the default
    >> values.
    >>
    >> While we are at it, simplify mq_attr_ok() by making it return 0 or an
    >> error condition, so that way if we add more tests to it later, we have
    >> the option of what error should be returned instead of the calling
    >> location having to pick a possibly inaccurate error code.
    >>
    >> Signed-off-by: Doug Ledford<dledford@redhat.com>
    >> ---
    >> ipc/mqueue.c | 27 ++++++++++++++++++---------
    >> 1 files changed, 18 insertions(+), 9 deletions(-)
    >>
    >> diff --git a/ipc/mqueue.c b/ipc/mqueue.c
    >> index 4b2892e..6089f73 100644
    >> --- a/ipc/mqueue.c
    >> +++ b/ipc/mqueue.c
    >> @@ -673,27 +673,27 @@ static int mq_attr_ok(struct ipc_namespace *ipc_ns, struct mq_attr *attr)
    >> int mq_treesize;
    >>
    >> if (attr->mq_maxmsg<= 0 || attr->mq_msgsize<= 0)
    >> - return 0;
    >> + return -EINVAL;
    >> if (capable(CAP_SYS_RESOURCE)) {
    >> if (attr->mq_maxmsg> HARD_MSGMAX ||
    >> attr->mq_msgsize> HARD_MSGSIZEMAX)
    >> - return 0;
    >> + return -EINVAL;
    >> } else {
    >> if (attr->mq_maxmsg> ipc_ns->mq_msg_max ||
    >> attr->mq_msgsize> ipc_ns->mq_msgsize_max)
    >> - return 0;
    >> + return -EINVAL;
    >> }
    >> /* check for overflow */
    >> if (attr->mq_msgsize> ULONG_MAX/attr->mq_maxmsg)
    >> - return 0;
    >> + return -ENOMEM;
    >> mq_treesize = attr->mq_maxmsg * sizeof(struct msg_msg) +
    >> min_t(unsigned int, attr->mq_maxmsg, MQ_PRIO_MAX) *
    >> sizeof(struct posix_msg_tree_node);
    >> if ((unsigned long)(attr->mq_maxmsg * attr->mq_msgsize +
    >> mq_treesize)<
    >> (unsigned long)(attr->mq_maxmsg * attr->mq_msgsize))
    >> - return 0;
    >> - return 1;
    >> + return -ENOMEM;
    >> + return 0;
    >
    > But ENOMEM is more inaccurate. It almostly is used for kmalloc failure.

    I chose ENOMEM for that particular error because above there we have
    checked the passed in arguments to make sure that they don't violate our
    allowances for max message or max message size. If we violate either of
    those items, we return EINVAL. In this case, neither of the values is
    invalid, it's just that together they make an overly large allocation.
    I would see that as more helpful to a programmer than EINVAL when the
    values are within the maximums allowed. At least with ENOMEM the
    programmer knows they have to reduce their combined message size and
    message count in order to get things working.


    --
    Doug Ledford <dledford@redhat.com>
    GPG KeyID: 0E572FDD
    http://people.redhat.com/dledford


    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2012-05-01 22:41    [W:0.034 / U:36.720 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site