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

[Al Viro]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/