linux-kernel - Re: [Patch 3/4] ipc/mqueue: strengthen checks on mqueue creation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 01 May 2012 16:01:53 -0400
From:	KOSAKI Motohiro <kosaki.motohiro@...il.com>
To:	Doug Ledford <dledford@...hat.com>
CC:	linux-kernel@...r.kernel.org, akpm@...ux-foundation.org,
	sfr@...b.auug.org.au, kosaki.motohiro@...il.com
Subject: Re: [Patch 3/4] ipc/mqueue: strengthen checks on mqueue creation

(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@...hat.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.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/