On 12/17/2014 12:46 PM, Krishnan Parthasarathi wrote:
----- Original Message -----
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.
This issue is because we are reading contents of a file into a fixed
sized buffer. Coverity considers external sources like I/O as taint by
default. My original question was if there are ways by which we can
programmatically let Coverity know that the data is _not_ tainted. This could
be a Coverity primitive, like explained here
https://communities.coverity.com/thread/2303,
or refactoring our code. AFAICS, this pattern is not the same as boundary
checks.
(NB. buffer in question is fixed length array and fgets (I/O) was performed
with sizeof (buffer) as the limit.)
This problem derives to a boundary check as a side effect.
fgets() is safe (at least to avoid buffer overruns), what could not be
safe is the data it reads. The problem here is that gluster allocates
buffers with a header for memory accounting. The sequence is this:
1. fgets() gets data into a buffer. The buffer pointer is marked as tainted.
2. A buffer is allocated to store a copy of the string, so the resulting
pointer is also marked as tainted.
3. The string is assigned to an array of strings. This seems to mark the
array of strings as tainted (maybe this shouldn't happen).
4. The array of strings is redimensioned. This generates an access to
the memory area just before the pointer to read memory block header
info. Since this access has been made using a tainted pointer, the data
read is also marked as tainted. Basically this means that 'type' is
considered unsafe.
5. 'type' is used to access an array. Since 'type' is considered unsafe,
the access itself could be unsafe.
Anyway I agree that this isn't a bug. It seems more a limitation of the
tracking of what is really tainted and what not in coverity scan.
Xavi
_______________________________________________
Gluster-devel mailing list
Gluster-devel@gluster.org
http://supercolony.gluster.org/mailman/listinfo/gluster-devel
