On 12/17/2014 11:29 AM, Niels de Vos wrote:
On Wed, Dec 17, 2014 at 10:24:46AM +0100, Xavier Hernandez wrote:
I think the root cause of this particular problem is a pattern like this:

     GF_ASSERT(type <= x);

     array[type] = y

I think there are several places where this pattern is used. GF_ASSERT() is
there to detect bugs, however it does not stop the execution, it simply logs
the problem. So if there's really a bug and 'type' > x, we will do an
incorrect access into the array. Additionally, if 'type' is obtained from a
tainted pointer, coverity considers this a potential security issue.

This is also applicable to NULL pointer checks and some others.

Wouldn't it be better to make GF_ASSERT() to really stop the program ?

Or, maybe we should fix these kind of GF_ASSERT() usages and call
GF_ASSERT_AND_GOTO_WITH_ERROR() instead?

That would be a good option also.


I do not know how Coverity runs checks, but when DEBUG is defined,
GF_ASSERT() will call assert() and execution should be aborted. Maybe
Coverity can be configured to hit assert() instead of ignoring it?

If this can be done, coverity won't report these issues, however the underlying problem will be there in release compilations.

If a bug really exists that makes the assert to fail, and the argument that doesn't satisfy the condition is used anyway, there could be a possibility in some places that this leads to data corruption, which would be very bad.

Xavi
_______________________________________________
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel

Reply via email to