linux-kernel - Re: [PATCH 1/2] ipc/mq: Do not vaild queue attributes default/ceiling value If the user pass attr as NULL

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 for Android: free password hash cracker in your pocket

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

Message-ID: <520D3388.4090007@redhat.com>
Date:	Thu, 15 Aug 2013 16:01:12 -0400
From:	Doug Ledford <dledford@...hat.com>
To:	Sasikantha Babu <sasikanth.v19@...il.com>
CC:	Al Viro <viro@...iv.linux.org.uk>,
	Jeff Layton <jlayton@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Vladimir Davydov <vdavydov@...allels.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] ipc/mq: Do not vaild queue attributes default/ceiling
 value If the user pass attr as NULL

On 08/12/2013 11:09 AM, Sasikantha Babu wrote:
> Kernel should not validate queue attributes default/ceiling value while
> creating a mqueue, if user pass attr as NULL. Otherwise In worst case
> If the validation fails then sys_mq_open returns -EINVAL/-EOVERFLOW
> which will make user clueless about reason for the failure.
> 
> Signed-off-by: Sasikantha Babu <sasikanth.v19@...il.com>
> ---
>  ipc/mqueue.c |    3 ---
>  1 files changed, 0 insertions(+), 3 deletions(-)
> 
> diff --git a/ipc/mqueue.c b/ipc/mqueue.c
> index ae1996d..04ece80 100644
> --- a/ipc/mqueue.c
> +++ b/ipc/mqueue.c
> @@ -748,9 +748,6 @@ static struct file *do_create(struct ipc_namespace *ipc_ns, struct inode *dir,
>  					 ipc_ns->mq_msg_default);
>  		def_attr.mq_msgsize = min(ipc_ns->mq_msgsize_max,
>  					  ipc_ns->mq_msgsize_default);
> -		ret = mq_attr_ok(ipc_ns, &def_attr);
> -		if (ret)
> -			return ERR_PTR(ret);
>  	}
>  
>  	mode &= ~current_umask();
> 

Nak.  The only two instances where mq_attr_ok() will reject the default
values are if either of the size or num are <= 0 or if the possible
total size of the struct is large enough to overflow the memory
accounting.  Since we can't allow the memory accounting to overflow no
matter what the defaults are or what the user requests, we can't skip
that check.  And we don't allow 0 element or 0 size queues, so we can't
skip that check.  If the user tweaks the default knobs in the kernel
such that the default values result in an impossible allocation, that's
the user's fault.  You don't allow the kernel to create a known broken
queue just because someone twiddled the default values to something invalid.

There are two possible different fixes:

1) Change this code to recognize that the default values resulted in an
error and print out a kernel message for the user to find

or

2) Change the memory accounting such that we don't check accounting
overflow on allocation but instead check it on msg queue and allow
people to create really large queues that they can fill up until they
hit the memory allocation overflow and then start rejecting queueing any
new messages to the queue until some of the old messages have been
removed and space freed up.  If that were done, then the overflow checks
in mq_attr_ok could go away.  But this would add some (although not a
lot) of overhead on the msg queue path.
--
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/