On Thu, Dec 21, 2017 at 1:34 PM, Jos Collin <jcollin@xxxxxxxxxx> wrote:
>
>
> On Thursday 21 December 2017 07:24 AM, scan-admin@xxxxxxxxxxxx wrote:
>>
>>
>> Hi,
>>
>> Please find the latest report on new defect(s) introduced to ceph found
>> with Coverity Scan.
>>
>> 3 new defect(s) introduced to ceph found with Coverity Scan.
>> 11 defect(s), reported by Coverity Scan earlier, were marked fixed in the
>> recent build analyzed by Coverity Scan.
>>
>> New defect(s) Reported-by: Coverity Scan
>> Showing 3 of 3 defect(s)
>>
>>
>> ** CID 1426746: Null pointer dereferences (REVERSE_INULL)
>> /home/brad/working/src/ceph/src/mds/MDBalancer.cc: 1270 in
>> MDBalancer::dump_loads(ceph::Formatter *)()
>>
>>
>>
>> ________________________________________________________________________________________________________
>> *** CID 1426746: Null pointer dereferences (REVERSE_INULL)
>> /home/brad/working/src/ceph/src/mds/MDBalancer.cc: 1270 in
>> MDBalancer::dump_loads(ceph::Formatter *)()
>> 1264
>> 1265 f->open_array_section("dirfrags");
>> 1266 while (!dfs.empty()) {
>> 1267 CDir *dir = dfs.front();
>> 1268 dfs.pop_front();
>> 1269
>>>>>
>>>>> CID 1426746: Null pointer dereferences (REVERSE_INULL)
>>>>> Null-checking "f" suggests that it may be null, but it has already
>>>>> been dereferenced on all paths leading to the check.
>>
>> 1270 if (f) {
This check for f being null is redundant since if it were null we
would have segfaulted on line 1263 or 1265.
I'd suggest it can be removed. We could put an assert put at the start
of the function if necessary/desirable to check the Formatter
argument being passed in but I haven't looked into it that deeply and
I'm not sure that's necessary. I'd also suggest taking a look at
similar functions that take a "Formatter" argument and see how they
handle the possibility of the Formatter being null.
>> 1271 f->open_object_section("dir");
>> 1272 dir->dump_load(f, now, decayrate);
>> 1273 f->close_section();
>> 1274 }
>> 1275
>>
>> ** CID 1426747: Null pointer dereferences (FORWARD_NULL)
>> /home/brad/working/src/ceph/src/mds/MDBalancer.cc: 1290 in
>> MDBalancer::dump_loads(ceph::Formatter *)()
>
>
> There is a possibility that `f` might be NULL. But it is not NULL.
>
> Please see:
> https://github.com/ceph/ceph/blob/f33ab7e03a13e18c8c883284033d511f1b43df12/src/mds/MDSDaemon.cc#L135
>
>>
>>
>>
>> ________________________________________________________________________________________________________
>> *** CID 1426747: Null pointer dereferences (FORWARD_NULL)
>> /home/brad/working/src/ceph/src/mds/MDBalancer.cc: 1290 in
>> MDBalancer::dump_loads(ceph::Formatter *)()
>> 1284 if (subdir->pop_nested.meta_load() < .001)
>> 1285 continue;
>> 1286 dfs.push_back(subdir);
>> 1287 }
>> 1288 }
>> 1289 }
>>>>>
>>>>> CID 1426747: Null pointer dereferences (FORWARD_NULL)
>>>>> Passing null pointer "f" to "close_section", which dereferences
>>>>> it. (The dereference happens because this is a virtual function call.)
>>
>> 1290 f->close_section(); // dirfrags array
>> 1291
>> 1292 f->open_object_section("mds_load");
>> 1293 {
>> 1294
>> 1295 auto dump_mds_load = [this, f, now](mds_load_t& load) {
>>
>> ** CID 1426748: Uninitialized members (UNINIT_CTOR)
>> /home/brad/working/src/ceph/src/osd/ECBackend.h: 511 in
>> ECBackend::Op::Op()()
>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Cheers,
Brad
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
