[Gluster-devel] Help needed with Coverity - How to remove tainted_data_argument?

[Xavier Hernandez]

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