linux-kernel - Re: [PATCH 2/1] ipc,mqueue: Delete bogus overflow check

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:	Sat, 11 Jul 2015 03:03:00 +0100
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Davidlohr Bueso <dave@...olabs.net>
Cc:	Marcus Gelderie <redmnic@...il.com>, mtk.manpages@...il.com,
	Doug Ledford <dledford@...hat.com>,
	lkml <linux-kernel@...r.kernel.org>,
	David Howells <dhowells@...hat.com>,
	John Duffy <jb_duffy@...nternet.com>,
	Arto Bendiken <arto@...diken.net>,
	Linux API <linux-api@...r.kernel.org>,
	akpm@...ux-foundation.org
Subject: Re: [PATCH 2/1] ipc,mqueue: Delete bogus overflow check

On Fri, Jul 10, 2015 at 05:48:11PM -0700, Davidlohr Bueso wrote:
> Mathematically, returning -EOVERFLOW in mq_attr_ok()
> cannot occur under this condition:
> 
>        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);
>        total_size = attr->mq_maxmsg * attr->mq_msgsize;
>        if (total_size + mq_treesize < total_size)
> 	       return -EOVERFLOW;

A proof would be nice.  More detailed one than "cannot occur", that is.

	Condition in question is basically mq_treesize < 0 or
total_size + mq_treesize (in natural numbers) > 2^BITS_PER_LONG.
Now, the maximal values of ->mq_maxmsg and ->mq_msgsize are 2^16 and
2^24 resp. and we are guaranteed that their product is below 2^BITS_PER_LONG.
For mq_treesize we are guaranteed that it's below 2^31.  Now, on a 64bit
box that would suffice to avoid overflow - the product is at most 2^40 and
its sum with mq_treesize can't wrap around.

For 32bit system, though...  Suppose attr->mq_maxmsg == 65535 and
attr->mq_msgsize == 65537.  Their product *is* below 2^BITS_PER_LONG - it's
exactly 1 less than that.  _Any_ non-zero value for mq_tresize (and it
will be non-zero in the above) will lead to wraparound.

Looks like a counterexample to your assertion above...
--
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/