linux-kernel - Re: [Patch 2/4] ipc/mqueue: correct mq_attr

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]

Message-ID: <4FA03B99.2040306@redhat.com>
Date:	Tue, 01 May 2012 15:38:01 -0400
From:	Doug Ledford <dledford@...hat.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
CC:	linux-kernel@...r.kernel.org, sfr@...b.auug.org.au,
	Manfred Spraul <manfred@...orfullife.com>
Subject: Re: [Patch 2/4] ipc/mqueue: correct mq_attr_ok test

On 05/01/2012 03:34 PM, Andrew Morton wrote:
> On Tue,  1 May 2012 13:50:53 -0400
> Doug Ledford <dledford@...hat.com> wrote:
> 
>> While working on the other parts of the mqueue stuff, I noticed that
>> the calculation for overflow in mq_attr_ok didn't actually match
>> reality (this is especially true since my last patch which changed
>> how we account memory slightly).
> 
> Please cc Manfred on mqueue things?  He still watches ;)
> 
>> In particular, we used to test for overflow using:
>>   msgs * msgsize + msgs * sizeof(struct msg_msg *)
>>
>> That was never really correct because each message we allocate via
>> load_msg() is actually a struct msg_msg followed by the data for
>> the message (and if struct msg_msg + data exceeds PAGE_SIZE we end
>> up allocating struct msg_msgseg structs too, but accounting for them
>> would get really tedious, so let's ignore those...they're only a
>> pointer in size anyway).  This patch updates the calculation to be
>> more accurate in regards to maximum possible memory consumption by the
>> mqueue.
>>
>> ...
>>
>> --- a/ipc/mqueue.c
>> +++ b/ipc/mqueue.c
>>
>> ...
>>
>> @@ -684,8 +686,11 @@ static int mq_attr_ok(struct ipc_namespace *ipc_ns, struct mq_attr *attr)
>>  	/* check for overflow */
>>  	if (attr->mq_msgsize > ULONG_MAX/attr->mq_maxmsg)
>>  		return 0;
>> -	if ((unsigned long)(attr->mq_maxmsg * (attr->mq_msgsize
>> -	    + sizeof (struct msg_msg *))) <
>> +	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;
> 
> That's a bit of a mouthful.  Does this look OK?
> 
> --- a/ipc/mqueue.c~ipc-mqueue-correct-mq_attr_ok-test-fix
> +++ a/ipc/mqueue.c
> @@ -672,7 +672,8 @@ static void remove_notification(struct m
>  static int mq_attr_ok(struct ipc_namespace *ipc_ns, struct mq_attr *attr)
>  {
>  	int mq_treesize;
> -
> +	unsigned long total_size;
> +	
>  	if (attr->mq_maxmsg <= 0 || attr->mq_msgsize <= 0)
>  		return 0;
>  	if (capable(CAP_SYS_RESOURCE)) {
> @@ -690,9 +691,8 @@ static int mq_attr_ok(struct ipc_namespa
>  	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))
> +	total_size = attr->mq_maxmsg * attr->mq_msgsize;
> +	if (total_size + mq_treesize < total_size)
>  		return 0;
>  	return 1;
>  }

Sure, looks fine to me and should preserve the wrap around test behavior.


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



Download attachment "signature.asc" of type "application/pgp-signature" (901 bytes)